From 9c30fb51f8e73499713d076945ea47e3a8205c69 Mon Sep 17 00:00:00 2001 From: Francisco H Date: Mon, 16 Jun 2025 16:23:37 +0000 Subject: [PATCH 1/4] Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H Fix command error Signed-off-by: Francisco H --- tests/e2e/ambient/ambient_test.go | 2 +- tests/e2e/controlplane/control_plane_suite_test.go | 2 +- tests/e2e/dualstack/dualstack_test.go | 2 +- tests/e2e/multicluster/multicluster_suite_test.go | 4 ++-- .../multicontrolplane/multi_control_plane_test.go | 2 +- tests/e2e/operator/operator_install_test.go | 2 +- tests/e2e/setup/build-and-push-operator.sh | 12 ++++++++++++ 7 files changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/e2e/ambient/ambient_test.go b/tests/e2e/ambient/ambient_test.go index f10094206e..3365d72a58 100644 --- a/tests/e2e/ambient/ambient_test.go +++ b/tests/e2e/ambient/ambient_test.go @@ -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") } diff --git a/tests/e2e/controlplane/control_plane_suite_test.go b/tests/e2e/controlplane/control_plane_suite_test.go index b7aa35e522..fefc6fe1b4 100644 --- a/tests/e2e/controlplane/control_plane_suite_test.go +++ b/tests/e2e/controlplane/control_plane_suite_test.go @@ -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") } diff --git a/tests/e2e/dualstack/dualstack_test.go b/tests/e2e/dualstack/dualstack_test.go index 079899ad96..1b2124d8bc 100644 --- a/tests/e2e/dualstack/dualstack_test.go +++ b/tests/e2e/dualstack/dualstack_test.go @@ -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") } diff --git a/tests/e2e/multicluster/multicluster_suite_test.go b/tests/e2e/multicluster/multicluster_suite_test.go index 61d8d30cb0..5cf3c5540a 100644 --- a/tests/e2e/multicluster/multicluster_suite_test.go +++ b/tests/e2e/multicluster/multicluster_suite_test.go @@ -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). diff --git a/tests/e2e/multicontrolplane/multi_control_plane_test.go b/tests/e2e/multicontrolplane/multi_control_plane_test.go index 60c2f0222d..ad788e7758 100644 --- a/tests/e2e/multicontrolplane/multi_control_plane_test.go +++ b/tests/e2e/multicontrolplane/multi_control_plane_test.go @@ -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") } diff --git a/tests/e2e/operator/operator_install_test.go b/tests/e2e/operator/operator_install_test.go index e2f7e17416..16a40db9bc 100644 --- a/tests/e2e/operator/operator_install_test.go +++ b/tests/e2e/operator/operator_install_test.go @@ -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") } }) diff --git a/tests/e2e/setup/build-and-push-operator.sh b/tests/e2e/setup/build-and-push-operator.sh index c74f0223ea..bab325b605 100755 --- a/tests/e2e/setup/build-and-push-operator.sh +++ b/tests/e2e/setup/build-and-push-operator.sh @@ -81,3 +81,15 @@ if [ "${OCP}" == "true" ]; then fi build_and_push_operator_image + +# Workaround for OCP helm operator installation issues: +# We create the operator namespace before the tests run +# To avoid any cleanup issues, after we build annd push the image we check if the namespace exists and delete it if it does. +# The test logic already handle the namespace creation and deletion during the test run. +# TODO: remove hardcoded kubectl command and use the COMMAND variable instead. To be changed during OCP enable doc test. +if kubectl get namespace "${NAMESPACE}" &>/dev/null; then + echo "Deleting existing namespace: ${NAMESPACE}" + kubectl delete namespace "${NAMESPACE}" --wait=true --timeout=5m || true +else + echo "Namespace ${NAMESPACE} does not exist, skipping deletion." +fi From 20a08fbc29118dac1f3de81a38a7bd3be576cfad Mon Sep 17 00:00:00 2001 From: Francisco Herrera Date: Tue, 17 Jun 2025 15:33:23 +0200 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Mike Kolesnik Signed-off-by: Francisco H --- tests/e2e/setup/build-and-push-operator.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/e2e/setup/build-and-push-operator.sh b/tests/e2e/setup/build-and-push-operator.sh index bab325b605..c2942a1b71 100755 --- a/tests/e2e/setup/build-and-push-operator.sh +++ b/tests/e2e/setup/build-and-push-operator.sh @@ -84,8 +84,8 @@ build_and_push_operator_image # Workaround for OCP helm operator installation issues: # We create the operator namespace before the tests run -# To avoid any cleanup issues, after we build annd push the image we check if the namespace exists and delete it if it does. -# The test logic already handle the namespace creation and deletion during the test run. +# 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. # TODO: remove hardcoded kubectl command and use the COMMAND variable instead. To be changed during OCP enable doc test. if kubectl get namespace "${NAMESPACE}" &>/dev/null; then echo "Deleting existing namespace: ${NAMESPACE}" From 6bd0caef00f815416a50d817616bd72444b6fcb2 Mon Sep 17 00:00:00 2001 From: Francisco H Date: Tue, 17 Jun 2025 16:02:36 +0000 Subject: [PATCH 3/4] Improves from code review Signed-off-by: Francisco H --- tests/e2e/common-operator-integ-suite.sh | 24 ++++++++++++------- tests/e2e/setup/build-and-push-operator.sh | 19 ++++----------- tests/e2e/setup/config/role-bindings.yaml | 28 +++++++++++++++++++++- 3 files changed, 48 insertions(+), 23 deletions(-) diff --git a/tests/e2e/common-operator-integ-suite.sh b/tests/e2e/common-operator-integ-suite.sh index 784ebd04c0..5016332439 100755 --- a/tests/e2e/common-operator-integ-suite.sh +++ b/tests/e2e/common-operator-integ-suite.sh @@ -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. + 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 @@ -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 diff --git a/tests/e2e/setup/build-and-push-operator.sh b/tests/e2e/setup/build-and-push-operator.sh index c2942a1b71..98ed256b5a 100755 --- a/tests/e2e/setup/build-and-push-operator.sh +++ b/tests/e2e/setup/build-and-push-operator.sh @@ -37,9 +37,12 @@ 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 and avoid errors during test run. + 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 - @@ -80,16 +83,4 @@ if [ "${OCP}" == "true" ]; then get_internal_registry fi -build_and_push_operator_image - -# Workaround for OCP helm operator installation issues: -# We create the operator namespace before the tests run -# 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. -# TODO: remove hardcoded kubectl command and use the COMMAND variable instead. To be changed during OCP enable doc test. -if kubectl get namespace "${NAMESPACE}" &>/dev/null; then - echo "Deleting existing namespace: ${NAMESPACE}" - kubectl delete namespace "${NAMESPACE}" --wait=true --timeout=5m || true -else - echo "Namespace ${NAMESPACE} does not exist, skipping deletion." -fi +build_and_push_operator_image \ No newline at end of file diff --git a/tests/e2e/setup/config/role-bindings.yaml b/tests/e2e/setup/config/role-bindings.yaml index 1857c621f4..d628f09eee 100644 --- a/tests/e2e/setup/config/role-bindings.yaml +++ b/tests/e2e/setup/config/role-bindings.yaml @@ -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: @@ -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 \ No newline at end of file From 6f0c505088dcafec345842e2534edb39c9ccf498 Mon Sep 17 00:00:00 2001 From: Francisco H Date: Wed, 18 Jun 2025 07:57:04 +0000 Subject: [PATCH 4/4] Update comment in build and push script Signed-off-by: Francisco H --- tests/e2e/setup/build-and-push-operator.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/setup/build-and-push-operator.sh b/tests/e2e/setup/build-and-push-operator.sh index 98ed256b5a..237c5a2a5d 100755 --- a/tests/e2e/setup/build-and-push-operator.sh +++ b/tests/e2e/setup/build-and-push-operator.sh @@ -38,7 +38,8 @@ get_internal_registry() { URL=$(${COMMAND} get route default-route -n openshift-image-registry --template='{{ .spec.host }}') - # Create the istio-images namespace to store the operator image and avoid errors during test run. + # 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}"