Skip to content

Fix helm issue during e2e test run operator installation #914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
istio-testing merged 4 commits into from
Jun 18, 2025
Merged

Fix helm issue during e2e test run operator installation #914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c30fb5
Fix helm issue during e2e test run operator installation
fjglira Jun 16, 2025
20a08fb
Apply suggestions from code review
fjglira Jun 17, 2025
6bd0cae
Improves from code review
fjglira Jun 17, 2025
6f0c505
Update comment in build and push script
fjglira Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/e2e/ambient/ambient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ var _ = Describe("Ambient configuration ", Label("smoke", "ambient"), Ordered, f
if skipDeploy {
Success("Skipping operator installation because it was deployed externally")
} else {
Eventually(common.InstallOperatorViaHelm).
Expect(common.InstallOperatorViaHelm()).
To(Succeed(), "Operator failed to be deployed")
}

Expand Down
24 changes: 16 additions & 8 deletions tests/e2e/common-operator-integ-suite.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,22 @@ export COMMAND OCP HUB IMAGE_BASE TAG NAMESPACE
if [ "${SKIP_BUILD}" == "false" ]; then
"${WD}/setup/build-and-push-operator.sh"

if [ "${OCP}" = "true" ]; then
# This is a workaround when pulling the image from internal registry
# To avoid errors of certificates meanwhile we are pulling the operator image from the internal registry
# We need to set image $HUB to a fixed known value after the push
# This value always will be equal to the svc url of the internal registry
HUB="image-registry.openshift-image-registry.svc:5000/istio-images"
echo "Using internal registry: ${HUB}"

# Workaround for OCP helm operator installation issues:
# To avoid any cleanup issues, after we build and push the image we check if the namespace exists and delete it if it does.
# The test logic already handles the namespace creation and deletion during the test run.
Copy link
Copy Markdown
Collaborator

@FilipB FilipB Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's done by tests, why it's needed also here?

Copy link
Copy Markdown
Contributor Author

@fjglira fjglira Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few weeks ago, we introduced a new cleanup method to ensure that all resources are cleaned up after each test run. This involves recording all resources before and after the test. because at the start of the first test, the sail operator namespace already exists from the setup, it's not being deleted at the end of the test, and this causes an error because the helm release still exists in the cluster and gives us a helm can't use any in-use name

if ${COMMAND} get ns "${NAMESPACE}" &>/dev/null; then
echo "Namespace ${NAMESPACE} already exists. Deleting it to avoid conflicts."
${COMMAND} delete ns "${NAMESPACE}"
fi
fi
# If OLM is enabled, deploy the operator using OLM
# We are skipping the deploy via OLM test on OCP because the workaround to avoid the certificate issue is not working.
# Jira ticket related to the limitation: https://issues.redhat.com/browse/OSSM-7993
Expand Down Expand Up @@ -182,14 +198,6 @@ if [ "${SKIP_BUILD}" == "false" ]; then
fi
fi

if [ "${OCP}" == "true" ]; then
# This is a workaround
# To avoid errors of certificates meanwhile we are pulling the operator image from the internal registry
# We need to set image $HUB to a fixed known value after the push
# This value always will be equal to the svc url of the internal registry
HUB="image-registry.openshift-image-registry.svc:5000/sail-operator"
fi

export SKIP_DEPLOY IP_FAMILY ISTIO_MANIFEST NAMESPACE CONTROL_PLANE_NS DEPLOYMENT_NAME MULTICLUSTER ARTIFACTS ISTIO_NAME COMMAND KUBECONFIG ISTIOCTL_PATH

# shellcheck disable=SC2086
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/controlplane/control_plane_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ var _ = BeforeSuite(func(ctx SpecContext) {
if skipDeploy {
Success("Skipping operator installation because it was deployed externally")
} else {
Eventually(common.InstallOperatorViaHelm).
Expect(common.InstallOperatorViaHelm()).
To(Succeed(), "Operator failed to be deployed")
}

Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/dualstack/dualstack_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var _ = Describe("DualStack configuration ", Label("dualstack"), Ordered, func()
if skipDeploy {
Success("Skipping operator installation because it was deployed externally")
} else {
Eventually(common.InstallOperatorViaHelm).
Expect(common.InstallOperatorViaHelm()).
To(Succeed(), "Operator failed to be deployed")
}

Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/multicluster/multicluster_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ var _ = BeforeSuite(func(ctx SpecContext) {
Expect(k1.CreateNamespace(namespace)).To(Succeed(), "Namespace failed to be created on Primary Cluster")
Expect(k2.CreateNamespace(namespace)).To(Succeed(), "Namespace failed to be created on Remote Cluster")

Eventually(common.InstallOperatorViaHelm).WithArguments("--kubeconfig", kubeconfig).
Expect(common.InstallOperatorViaHelm("--kubeconfig", kubeconfig)).
To(Succeed(), "Operator failed to be deployed in Primary Cluster")

Eventually(common.InstallOperatorViaHelm).WithArguments("--kubeconfig", kubeconfig2).
Expect(common.InstallOperatorViaHelm("--kubeconfig", kubeconfig2)).
To(Succeed(), "Operator failed to be deployed in Remote Cluster")

Eventually(common.GetObject).
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/multicontrolplane/multi_control_plane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe("Multi control plane deployment model", Label("smoke", "multico
if skipDeploy {
Success("Skipping operator installation because it was deployed externally")
} else {
Eventually(common.InstallOperatorViaHelm).
Expect(common.InstallOperatorViaHelm()).
To(Succeed(), "Operator failed to be deployed")
}

Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/operator/operator_install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var _ = Describe("Operator", Label("smoke", "operator"), Ordered, func() {
if skipDeploy {
Success("Skipping operator installation because it was deployed externally")
} else {
Eventually(common.InstallOperatorViaHelm).
Expect(common.InstallOperatorViaHelm()).
To(Succeed(), "Operator failed to be deployed")
}
})
Expand Down
8 changes: 6 additions & 2 deletions tests/e2e/setup/build-and-push-operator.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,13 @@ get_internal_registry() {
fi

URL=$(${COMMAND} get route default-route -n openshift-image-registry --template='{{ .spec.host }}')
export HUB="${URL}/${NAMESPACE}"

# Create the istio-images namespace to store the operator image
# This will ensure that no matter the operator namespace is deleted, the image will still be available
export HUB="${URL}/istio-images"
echo "Registry URL: ${HUB}"

${COMMAND} create namespace istio-images || true
${COMMAND} create namespace "${NAMESPACE}" || true
envsubst < "${WD}/config/role-bindings.yaml" | ${COMMAND} apply -f -

Expand Down Expand Up @@ -80,4 +84,4 @@ if [ "${OCP}" == "true" ]; then
get_internal_registry
fi

build_and_push_operator_image
build_and_push_operator_image
28 changes: 27 additions & 1 deletion tests/e2e/setup/config/role-bindings.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ items:
subjects:
- kind: Group
apiGroup: rbac.authorization.k8s.io
name: system:unauthenticated
name: system:authenticated
- apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
Expand All @@ -24,6 +24,32 @@ items:
kind: ClusterRole
name: system:image-builder
subjects:
- kind: Group
apiGroup: rbac.authorization.k8s.io
name: system:unauthenticated
- apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: image-puller
namespace: istio-images
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: system:image-puller
subjects:
- kind: Group
apiGroup: rbac.authorization.k8s.io
name: system:authenticated
- apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: image-pusher
namespace: istio-images
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: system:image-builder
subjects:
- kind: Group
apiGroup: rbac.authorization.k8s.io
name: system:unauthenticated