From ffefaf756233a18a0525ceea25130219993381b8 Mon Sep 17 00:00:00 2001 From: Mike Kolesnik Date: Wed, 19 Nov 2025 15:16:18 +0200 Subject: [PATCH 1/4] E2E: Extract common function to await conditions This code is very repetitive and verbose, extracting it to a common function allows the tests to be much more concise and readable, without affecting functionality. The main benefit of this is making the tests easier to read and maintain, and avoid repeating code (and making mistakes). Signed-off-by: Mike Kolesnik --- tests/e2e/ambient/ambient_test.go | 14 ++--- tests/e2e/controlplane/control_plane_test.go | 22 ++------ .../controlplane/control_plane_update_test.go | 15 ++---- tests/e2e/dualstack/dualstack_test.go | 14 ++--- .../multicluster_externalcontrolplane_test.go | 34 +++---------- .../multicluster_multiprimary_test.go | 51 ++++--------------- .../multicluster_primaryremote_test.go | 39 +++----------- .../multi_control_plane_test.go | 20 ++------ tests/e2e/operator/operator_install_test.go | 3 +- tests/e2e/util/common/checks.go | 45 ++++++++++++++++ 10 files changed, 90 insertions(+), 167 deletions(-) create mode 100644 tests/e2e/util/common/checks.go diff --git a/tests/e2e/ambient/ambient_test.go b/tests/e2e/ambient/ambient_test.go index 07f97cc9f3..5ee0d0ec38 100644 --- a/tests/e2e/ambient/ambient_test.go +++ b/tests/e2e/ambient/ambient_test.go @@ -27,12 +27,10 @@ import ( . "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/cleaner" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common" - . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -119,22 +117,16 @@ profile: ambient`) }) It("updates the Istio CR status to Reconciled", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReconciled, metav1.ConditionTrue), "Istio is not Reconciled; unexpected Condition") - Success("Istio CR is Reconciled") + common.AwaitCondition(ctx, v1.IstioConditionReconciled, kube.Key(istioName), &v1.Istio{}, k, cl) }) It("updates the Istio CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready; unexpected Condition") - Success("Istio CR is Ready") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k, cl) }) It("deploys istiod", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k, cl) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") - Success("Istiod is deployed in the namespace and Running") }) It("uses the correct image", func(ctx SpecContext) { diff --git a/tests/e2e/controlplane/control_plane_test.go b/tests/e2e/controlplane/control_plane_test.go index 413052edd7..0bcbc55172 100644 --- a/tests/e2e/controlplane/control_plane_test.go +++ b/tests/e2e/controlplane/control_plane_test.go @@ -29,13 +29,11 @@ import ( . "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/cleaner" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common" - . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/istioctl" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "istio.io/istio/pkg/ptr" @@ -128,15 +126,11 @@ metadata: }) It("updates the status to Reconciled", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioCniName), &v1.IstioCNI{}). - Should(HaveConditionStatus(v1.IstioCNIConditionReconciled, metav1.ConditionTrue), "IstioCNI is not Reconciled; unexpected Condition") - Success("IstioCNI is Reconciled") + common.AwaitCondition(ctx, v1.IstioCNIConditionReconciled, kube.Key(istioCniName), &v1.IstioCNI{}, k, cl) }) It("updates the status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioCniName), &v1.IstioCNI{}). - Should(HaveConditionStatus(v1.IstioCNIConditionReady, metav1.ConditionTrue), "IstioCNI is not Ready; unexpected Condition") - Success("IstioCNI is Ready") + common.AwaitCondition(ctx, v1.IstioCNIConditionReady, kube.Key(istioCniName), &v1.IstioCNI{}, k, cl) }) It("doesn't continuously reconcile the IstioCNI CR", func() { @@ -152,22 +146,16 @@ metadata: }) It("updates the Istio CR status to Reconciled", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReconciled, metav1.ConditionTrue), "Istio is not Reconciled; unexpected Condition") - Success("Istio CR is Reconciled") + common.AwaitCondition(ctx, v1.IstioConditionReconciled, kube.Key(istioName), &v1.Istio{}, k, cl) }) It("updates the Istio CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready; unexpected Condition") - Success("Istio CR is Ready") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k, cl) }) It("deploys istiod", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k, cl) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") - Success("Istiod is deployed in the namespace and Running") }) It("uses the correct image", func(ctx SpecContext) { diff --git a/tests/e2e/controlplane/control_plane_update_test.go b/tests/e2e/controlplane/control_plane_update_test.go index 7c3818c86a..49bc790dcc 100644 --- a/tests/e2e/controlplane/control_plane_update_test.go +++ b/tests/e2e/controlplane/control_plane_update_test.go @@ -61,9 +61,7 @@ var _ = Describe("Control Plane updates", Label("control-plane", "slow"), Ordere Expect(k.CreateNamespace(istioCniNamespace)).To(Succeed(), "IstioCNI namespace failed to be created") common.CreateIstioCNI(k, istioversion.Base) - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioCniName), &v1.IstioCNI{}). - Should(HaveConditionStatus(v1.IstioCNIConditionReady, metav1.ConditionTrue), "IstioCNI is not Ready; unexpected Condition") - Success("IstioCNI is Ready") + common.AwaitCondition(ctx, v1.IstioCNIConditionReady, kube.Key(istioCniName), &v1.IstioCNI{}, k, cl) }) When(fmt.Sprintf("the Istio CR is created with RevisionBased updateStrategy for base version %s", istioversion.Base), func() { @@ -75,9 +73,7 @@ updateStrategy: }) It("deploys istiod and pod is Ready", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key("default"), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istiod is not Available; unexpected Condition") - Success("Istiod is deployed in the namespace and Running") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key("default"), &v1.Istio{}, k, cl) }) }) @@ -138,9 +134,7 @@ spec: }) It("IstioRevisionTag state change to inUse true", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key("default"), &v1.IstioRevisionTag{}). - Should(HaveConditionStatus(v1.IstioRevisionTagConditionInUse, metav1.ConditionTrue), "unexpected Condition; expected InUse true") - Success("IstioRevisionTag is in use by the sample pods") + common.AwaitCondition(ctx, v1.IstioRevisionTagConditionInUse, kube.Key("default"), &v1.IstioRevisionTag{}, k, cl) }) }) @@ -225,8 +219,7 @@ spec: Expect(cl.List(ctx, samplePods, client.InNamespace(sampleNamespace))).To(Succeed()) Expect(samplePods.Items).ToNot(BeEmpty(), "No pods found in sample namespace") for _, pod := range samplePods.Items { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}). - Should(HaveConditionStatus(corev1.PodReady, metav1.ConditionTrue), "Pod is not Ready") + common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k, cl) } Success("sample pods restarted and are ready") diff --git a/tests/e2e/dualstack/dualstack_test.go b/tests/e2e/dualstack/dualstack_test.go index 817b54d759..dd0c96fea1 100644 --- a/tests/e2e/dualstack/dualstack_test.go +++ b/tests/e2e/dualstack/dualstack_test.go @@ -27,13 +27,11 @@ import ( . "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/cleaner" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common" - . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/types" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -99,22 +97,16 @@ values: }) It("updates the Istio CR status to Reconciled", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReconciled, metav1.ConditionTrue), "Istio is not Reconciled; unexpected Condition") - Success("Istio CR is Reconciled") + common.AwaitCondition(ctx, v1.IstioConditionReconciled, kube.Key(istioName), &v1.Istio{}, k, cl) }) It("updates the Istio CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready; unexpected Condition") - Success("Istio CR is Ready") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k, cl) }) It("deploys istiod", func(ctx SpecContext) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k, cl) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") - Success("Istiod is deployed in the namespace and Running") }) It("uses the correct image", func(ctx SpecContext) { diff --git a/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go b/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go index bbadb18b45..becd0ef7d8 100644 --- a/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go +++ b/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go @@ -27,14 +27,12 @@ import ( "github.com/istio-ecosystem/sail-operator/pkg/version" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/cleaner" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common" - . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/istioctl" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -78,18 +76,12 @@ values: }) It("updates the default Istio CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready on Cluster #1; unexpected Condition") - Success("Istio CR is Ready on Cluster #1") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k1, clPrimary) }) It("deploys istiod", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available on Cluster #1; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) Expect(common.GetVersionFromIstiod()).To(Equal(v.Version), "Unexpected istiod version") - Success("Istiod is deployed in the namespace and Running on Exernal Cluster") }) }) @@ -99,11 +91,7 @@ values: }) It("updates Gateway status to Available", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key("istio-ingressgateway", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Gateway is not Ready on Cluster #1; unexpected Condition") - - Success("Gateway is created and available in both clusters") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-ingressgateway", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) }) }) @@ -265,11 +253,8 @@ spec: Expect(k1.CreateFromString(externalControlPlaneYAML)).To(Succeed(), "Istio Resource creation failed on Cluster #1") }) - It("updates both Istio CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key("external-istiod"), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready on Cluster #1; unexpected Condition") - Success("Istio CR is Ready on Cluster #1") + It("updates external Istio CR status to Ready", func(ctx SpecContext) { + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key("external-istiod"), &v1.Istio{}, k1, clPrimary) }) }) @@ -340,10 +325,7 @@ spec: }) It("updates remote Istio CR status to Ready on Cluster #2", func(ctx SpecContext) { - Eventually(common.GetObject, 10*time.Minute). - WithArguments(ctx, clRemote, kube.Key(externalIstioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready on Remote; unexpected Condition") - Success("Remote Istio CR is Ready on Cluster #2") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(externalIstioName), &v1.Istio{}, k2, clRemote, 10*time.Minute) }) }) @@ -363,9 +345,7 @@ spec: Expect(samplePodsCluster2.Items).ToNot(BeEmpty(), "No pods found in sample namespace") for _, pod := range samplePodsCluster2.Items { - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}). - Should(HaveConditionStatus(corev1.PodReady, metav1.ConditionTrue), "Pod is not Ready on Cluster #2; unexpected Condition") + common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k2, clRemote) } Success("Sample app is Running") }) diff --git a/tests/e2e/multicluster/multicluster_multiprimary_test.go b/tests/e2e/multicluster/multicluster_multiprimary_test.go index 2994fc3ab9..15238b02f3 100644 --- a/tests/e2e/multicluster/multicluster_multiprimary_test.go +++ b/tests/e2e/multicluster/multicluster_multiprimary_test.go @@ -28,13 +28,11 @@ import ( "github.com/istio-ecosystem/sail-operator/tests/e2e/util/certs" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/cleaner" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common" - . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/istioctl" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -91,41 +89,21 @@ values: }) It("updates both Istio CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready on Cluster #1; unexpected Condition") - Success("Istio CR is Ready on Cluster #1") - - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready on Cluster #2; unexpected Condition") - Success("Istio CR is Ready on Cluster #2") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k1, clPrimary) + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k2, clRemote) }) It("updates both IstioCNI CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key(istioCniName), &v1.IstioCNI{}). - Should(HaveConditionStatus(v1.IstioCNIConditionReady, metav1.ConditionTrue), "Istio CNI is not Ready on Cluster #1; unexpected Condition") - Success("Istio CNI CR is Ready on Cluster #1") - - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key(istioCniName), &v1.IstioCNI{}). - Should(HaveConditionStatus(v1.IstioCNIConditionReady, metav1.ConditionTrue), "Istio CNI is not Ready on Cluster #2; unexpected Condition") - Success("Istio CNI CR is Ready on Cluster #2") + common.AwaitCondition(ctx, v1.IstioCNIConditionReady, kube.Key(istioCniName), &v1.IstioCNI{}, k1, clPrimary) + common.AwaitCondition(ctx, v1.IstioCNIConditionReady, kube.Key(istioCniName), &v1.IstioCNI{}, k2, clRemote) }) It("deploys istiod", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available on Cluster #1; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") - Success("Istiod is deployed in the namespace and Running on Cluster #1") - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available on Cluster #2; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k2, clRemote) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") - Success("Istiod is deployed in the namespace and Running on Cluster #2") }) It("deploys istio-cni-node", func(ctx SpecContext) { @@ -160,13 +138,8 @@ values: }) It("updates both Gateway status to Available", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Gateway is not Ready on Cluster #1; unexpected Condition") - - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Gateway is not Ready on Cluster #2; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k2, clRemote) Success("Gateway is created and available in both clusters") }) }) @@ -230,9 +203,7 @@ values: Expect(samplePodsCluster1.Items).ToNot(BeEmpty(), "No pods found in sample namespace") for _, pod := range samplePodsCluster1.Items { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}). - Should(HaveConditionStatus(corev1.PodReady, metav1.ConditionTrue), "Pod is not Ready on Cluster #1; unexpected Condition") + common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k1, clPrimary) } samplePodsCluster2 := &corev1.PodList{} @@ -240,9 +211,7 @@ values: Expect(samplePodsCluster2.Items).ToNot(BeEmpty(), "No pods found in sample namespace") for _, pod := range samplePodsCluster2.Items { - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}). - Should(HaveConditionStatus(corev1.PodReady, metav1.ConditionTrue), "Pod is not Ready on Cluster #2; unexpected Condition") + common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k2, clRemote) } Success("Sample app is created in both clusters and Running") }) diff --git a/tests/e2e/multicluster/multicluster_primaryremote_test.go b/tests/e2e/multicluster/multicluster_primaryremote_test.go index f0f15a978b..d49f213456 100644 --- a/tests/e2e/multicluster/multicluster_primaryremote_test.go +++ b/tests/e2e/multicluster/multicluster_primaryremote_test.go @@ -29,13 +29,11 @@ import ( "github.com/istio-ecosystem/sail-operator/tests/e2e/util/certs" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/cleaner" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common" - . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/istioctl" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -101,25 +99,16 @@ values: }) It("updates Istio CR on Primary cluster status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready on Primary; unexpected Condition") - Success("Istio CR is Ready on Primary Cluster") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k1, clPrimary) }) It("updates IstioCNI CR on Primary cluster status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key(istioCniName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioCNIConditionReady, metav1.ConditionTrue), "IstioCNI is not Ready on Primary; unexpected Condition") - Success("IstioCNI CR is Ready on Primary Cluster") + common.AwaitCondition(ctx, v1.IstioCNIConditionReady, kube.Key(istioCniName), &v1.IstioCNI{}, k1, clPrimary) }) It("deploys istiod", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Istiod is not Available on Primary; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) Expect(common.GetVersionFromIstiod()).To(Equal(v.Version), "Unexpected istiod version") - Success("Istiod is deployed in the namespace and Running on Primary Cluster") }) It("deploys istio-cni-node", func(ctx SpecContext) { @@ -146,9 +135,7 @@ values: }) It("updates Gateway status to Available", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Gateway is not Ready on Primary; unexpected Condition") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) }) }) @@ -207,10 +194,7 @@ values: }) It("updates remote Istio CR status to Ready", func(ctx SpecContext) { - Eventually(common.GetObject, 10*time.Minute). - WithArguments(ctx, clRemote, kube.Key(istioName), &v1.Istio{}). - Should(HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), "Istio is not Ready on Remote; unexpected Condition") - Success("Istio CR is Ready on Remote Cluster") + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(istioName), &v1.Istio{}, k2, clRemote, 10*time.Minute) }) }) @@ -221,10 +205,7 @@ values: }) It("updates Gateway status to Available", func(ctx SpecContext) { - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Gateway is not Ready on Remote; unexpected Condition") - Success("Gateway is created and available in Remote cluster") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k2, clRemote) }) }) @@ -253,9 +234,7 @@ values: Expect(samplePodsPrimary.Items).ToNot(BeEmpty(), "No pods found in sample namespace") for _, pod := range samplePodsPrimary.Items { - Eventually(common.GetObject). - WithArguments(ctx, clPrimary, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}). - Should(HaveConditionStatus(corev1.PodReady, metav1.ConditionTrue), "Pod is not Ready on Primary; unexpected Condition") + common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k1, clPrimary) } samplePodsRemote := &corev1.PodList{} @@ -263,9 +242,7 @@ values: Expect(samplePodsRemote.Items).ToNot(BeEmpty(), "No pods found in sample namespace") for _, pod := range samplePodsRemote.Items { - Eventually(common.GetObject). - WithArguments(ctx, clRemote, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}). - Should(HaveConditionStatus(corev1.PodReady, metav1.ConditionTrue), "Pod is not Ready on Remote; unexpected Condition") + common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k2, clRemote) } Success("Sample app is created in both clusters and Running") }) diff --git a/tests/e2e/multicontrolplane/multi_control_plane_test.go b/tests/e2e/multicontrolplane/multi_control_plane_test.go index 18fb8d94cf..7e50bddb26 100644 --- a/tests/e2e/multicontrolplane/multi_control_plane_test.go +++ b/tests/e2e/multicontrolplane/multi_control_plane_test.go @@ -26,11 +26,9 @@ import ( . "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/cleaner" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/common" - . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var _ = Describe("Multi control plane deployment model", Label("smoke", "multicontrol-plane"), Ordered, func() { @@ -60,10 +58,7 @@ var _ = Describe("Multi control plane deployment model", Label("smoke", "multico It("Installs IstioCNI", func(ctx SpecContext) { common.CreateIstioCNI(k, latestVersion.Name) - - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(istioCniName), &v1.IstioCNI{}). - Should(HaveConditionStatus(v1.IstioCNIConditionReady, metav1.ConditionTrue), "IstioCNI is not Ready; unexpected Condition") - Success("IstioCNI is Ready") + common.AwaitCondition(ctx, v1.IstioCNIConditionReady, kube.Key(istioCniName), &v1.IstioCNI{}, k, cl) }) DescribeTable("Installs Istios", @@ -100,13 +95,8 @@ spec: Entry("Mesh 1", istioName1), Entry("Mesh 2", istioName2), func(ctx SpecContext, name string) { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(name), &v1.Istio{}). - Should( - And( - HaveConditionStatus(v1.IstioConditionReconciled, metav1.ConditionTrue), - HaveConditionStatus(v1.IstioConditionReady, metav1.ConditionTrue), - ), "Istio is not Reconciled and Ready; unexpected Condition") - Success(fmt.Sprintf("Istio %s ready", name)) + common.AwaitCondition(ctx, v1.IstioConditionReconciled, kube.Key(name), &v1.Istio{}, k, cl) + common.AwaitCondition(ctx, v1.IstioConditionReady, kube.Key(name), &v1.Istio{}, k, cl) }) DescribeTable("Deploys applications", @@ -134,10 +124,8 @@ spec: Entry("App 2b", appNamespace2b), func(ctx SpecContext, ns string) { for _, deployment := range []string{"sleep", "httpbin"} { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(deployment, ns), &appsv1.Deployment{}). - Should(HaveConditionStatus(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Error waiting for deployment to be available") + common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key(deployment, ns), &appsv1.Deployment{}, k, cl) } - Success(fmt.Sprintf("Applications in namespace %s ready", ns)) }) }) diff --git a/tests/e2e/operator/operator_install_test.go b/tests/e2e/operator/operator_install_test.go index 9899437c26..03f3000191 100644 --- a/tests/e2e/operator/operator_install_test.go +++ b/tests/e2e/operator/operator_install_test.go @@ -75,8 +75,7 @@ var _ = Describe("Operator", Label("smoke", "operator"), Ordered, func() { It("updates the CRDs status to Established", func(ctx SpecContext) { for _, crdName := range sailCRDs { - Eventually(common.GetObject).WithArguments(ctx, cl, kube.Key(crdName), &apiextensionsv1.CustomResourceDefinition{}). - Should(HaveConditionStatus(apiextensionsv1.Established, metav1.ConditionTrue), "Error getting Istio CRD") + common.AwaitCondition(ctx, apiextensionsv1.Established, kube.Key(crdName), &apiextensionsv1.CustomResourceDefinition{}, k, cl) } Success("CRDs are Established") }) diff --git a/tests/e2e/util/common/checks.go b/tests/e2e/util/common/checks.go new file mode 100644 index 0000000000..44bd9b546e --- /dev/null +++ b/tests/e2e/util/common/checks.go @@ -0,0 +1,45 @@ +//go:build e2e + +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package common + +import ( + "context" + "fmt" + "reflect" + + . "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo" + . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" + "github.com/istio-ecosystem/sail-operator/tests/e2e/util/kubectl" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// AwaitCondition to be True. A key and a pointer to the object struct must be supplied. Extra arguments to pass to `Eventually` can be optionally supplied. +func AwaitCondition[T ~string](ctx context.Context, condition T, key client.ObjectKey, obj client.Object, k kubectl.Kubectl, cl client.Client, args ...any) { + kind := reflect.TypeOf(obj).Elem().Name() + cluster := "cluster" + if k.ClusterName != "" { + cluster = k.ClusterName + } + + Eventually(GetObject, args...). + WithArguments(ctx, cl, key, obj). + Should(HaveConditionStatus(condition, metav1.ConditionTrue), + fmt.Sprintf("%s %q is not %s on %s; unexpected Condition", kind, key.Name, condition, cluster)) + Success(fmt.Sprintf("%s %q is %s on %s", kind, key.Name, condition, cluster)) +} From 1ec3bdd1f9002290824b771a213c27590c43cc7e Mon Sep 17 00:00:00 2001 From: Mike Kolesnik Date: Thu, 20 Nov 2025 09:48:13 +0200 Subject: [PATCH 2/4] E2E: Add specific AwaitDeployment function Most of the tests that await for the DeploymentAvailable condition do it on the control plane namespace for a Deployment. Extracting this specific case makes the tests even easier to read and understand. Signed-off-by: Mike Kolesnik --- tests/e2e/ambient/ambient_test.go | 2 +- tests/e2e/controlplane/control_plane_test.go | 2 +- tests/e2e/dualstack/dualstack_test.go | 2 +- .../multicluster_externalcontrolplane_test.go | 4 ++-- tests/e2e/multicluster/multicluster_multiprimary_test.go | 8 ++++---- tests/e2e/multicluster/multicluster_primaryremote_test.go | 6 +++--- tests/e2e/util/common/checks.go | 7 +++++++ 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/tests/e2e/ambient/ambient_test.go b/tests/e2e/ambient/ambient_test.go index 5ee0d0ec38..288533ce9f 100644 --- a/tests/e2e/ambient/ambient_test.go +++ b/tests/e2e/ambient/ambient_test.go @@ -125,7 +125,7 @@ profile: ambient`) }) It("deploys istiod", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k, cl) + common.AwaitDeployment(ctx, "istiod", k, cl) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") }) diff --git a/tests/e2e/controlplane/control_plane_test.go b/tests/e2e/controlplane/control_plane_test.go index 0bcbc55172..37f66d0187 100644 --- a/tests/e2e/controlplane/control_plane_test.go +++ b/tests/e2e/controlplane/control_plane_test.go @@ -154,7 +154,7 @@ metadata: }) It("deploys istiod", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k, cl) + common.AwaitDeployment(ctx, "istiod", k, cl) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") }) diff --git a/tests/e2e/dualstack/dualstack_test.go b/tests/e2e/dualstack/dualstack_test.go index dd0c96fea1..8428b14993 100644 --- a/tests/e2e/dualstack/dualstack_test.go +++ b/tests/e2e/dualstack/dualstack_test.go @@ -105,7 +105,7 @@ values: }) It("deploys istiod", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k, cl) + common.AwaitDeployment(ctx, "istiod", k, cl) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") }) diff --git a/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go b/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go index becd0ef7d8..a9c06f5035 100644 --- a/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go +++ b/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go @@ -80,7 +80,7 @@ values: }) It("deploys istiod", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) + common.AwaitDeployment(ctx, "istiod", k1, clPrimary) Expect(common.GetVersionFromIstiod()).To(Equal(v.Version), "Unexpected istiod version") }) }) @@ -91,7 +91,7 @@ values: }) It("updates Gateway status to Available", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-ingressgateway", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) + common.AwaitDeployment(ctx, "istio-ingressgateway", k1, clPrimary) }) }) diff --git a/tests/e2e/multicluster/multicluster_multiprimary_test.go b/tests/e2e/multicluster/multicluster_multiprimary_test.go index 15238b02f3..6fd0c852b4 100644 --- a/tests/e2e/multicluster/multicluster_multiprimary_test.go +++ b/tests/e2e/multicluster/multicluster_multiprimary_test.go @@ -99,10 +99,10 @@ values: }) It("deploys istiod", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) + common.AwaitDeployment(ctx, "istiod", k1, clPrimary) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k2, clRemote) + common.AwaitDeployment(ctx, "istiod", k2, clRemote) Expect(common.GetVersionFromIstiod()).To(Equal(version.Version), "Unexpected istiod version") }) @@ -138,8 +138,8 @@ values: }) It("updates both Gateway status to Available", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k2, clRemote) + common.AwaitDeployment(ctx, "istio-eastwestgateway", k1, clPrimary) + common.AwaitDeployment(ctx, "istio-eastwestgateway", k2, clRemote) Success("Gateway is created and available in both clusters") }) }) diff --git a/tests/e2e/multicluster/multicluster_primaryremote_test.go b/tests/e2e/multicluster/multicluster_primaryremote_test.go index d49f213456..0a516fba02 100644 --- a/tests/e2e/multicluster/multicluster_primaryremote_test.go +++ b/tests/e2e/multicluster/multicluster_primaryremote_test.go @@ -107,7 +107,7 @@ values: }) It("deploys istiod", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istiod", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) + common.AwaitDeployment(ctx, "istiod", k1, clPrimary) Expect(common.GetVersionFromIstiod()).To(Equal(v.Version), "Unexpected istiod version") }) @@ -135,7 +135,7 @@ values: }) It("updates Gateway status to Available", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k1, clPrimary) + common.AwaitDeployment(ctx, "istio-eastwestgateway", k1, clPrimary) }) }) @@ -205,7 +205,7 @@ values: }) It("updates Gateway status to Available", func(ctx SpecContext) { - common.AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key("istio-eastwestgateway", controlPlaneNamespace), &appsv1.Deployment{}, k2, clRemote) + common.AwaitDeployment(ctx, "istio-eastwestgateway", k2, clRemote) }) }) diff --git a/tests/e2e/util/common/checks.go b/tests/e2e/util/common/checks.go index 44bd9b546e..ec547c89cd 100644 --- a/tests/e2e/util/common/checks.go +++ b/tests/e2e/util/common/checks.go @@ -21,10 +21,12 @@ import ( "fmt" "reflect" + "github.com/istio-ecosystem/sail-operator/pkg/kube" . "github.com/istio-ecosystem/sail-operator/pkg/test/util/ginkgo" . "github.com/istio-ecosystem/sail-operator/tests/e2e/util/gomega" "github.com/istio-ecosystem/sail-operator/tests/e2e/util/kubectl" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -43,3 +45,8 @@ func AwaitCondition[T ~string](ctx context.Context, condition T, key client.Obje fmt.Sprintf("%s %q is not %s on %s; unexpected Condition", kind, key.Name, condition, cluster)) Success(fmt.Sprintf("%s %q is %s on %s", kind, key.Name, condition, cluster)) } + +// AwaitDeployment to reach the Available state. +func AwaitDeployment(ctx context.Context, name string, k kubectl.Kubectl, cl client.Client) { + AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key(name, controlPlaneNamespace), &appsv1.Deployment{}, k, cl) +} From 35092776b4da749ae72cbba14f82b18d7d00124c Mon Sep 17 00:00:00 2001 From: Mike Kolesnik Date: Thu, 20 Nov 2025 10:18:44 +0200 Subject: [PATCH 3/4] E2E: Add CheckSamplePodsReady function Reuse CheckPodsReady and add a specific function CheckSamplePodsReady since most of the calls are on the sample namespace. Both checks are now in the `checks.go` file for logical consistency. Signed-off-by: Mike Kolesnik --- tests/e2e/controlplane/control_plane_test.go | 2 +- .../controlplane/control_plane_update_test.go | 9 ++--- .../multicluster_externalcontrolplane_test.go | 9 ++--- .../multicluster_multiprimary_test.go | 19 ++--------- .../multicluster_primaryremote_test.go | 19 ++--------- tests/e2e/util/common/checks.go | 33 +++++++++++++++++++ tests/e2e/util/common/e2e_utils.go | 28 +--------------- 7 files changed, 43 insertions(+), 76 deletions(-) diff --git a/tests/e2e/controlplane/control_plane_test.go b/tests/e2e/controlplane/control_plane_test.go index 37f66d0187..0339222ef7 100644 --- a/tests/e2e/controlplane/control_plane_test.go +++ b/tests/e2e/controlplane/control_plane_test.go @@ -182,7 +182,7 @@ metadata: samplePods := &corev1.PodList{} It("updates the pods status to Running", func(ctx SpecContext) { - Eventually(common.CheckPodsReady).WithArguments(ctx, cl, sampleNamespace).Should(Succeed(), "Error checking status of sample pods") + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, cl).Should(Succeed(), "Error checking status of sample pods") Expect(cl.List(ctx, samplePods, client.InNamespace(sampleNamespace))).To(Succeed(), "Error getting the pods in sample namespace") Success("sample pods are ready") diff --git a/tests/e2e/controlplane/control_plane_update_test.go b/tests/e2e/controlplane/control_plane_update_test.go index 49bc790dcc..e5ab3b4c99 100644 --- a/tests/e2e/controlplane/control_plane_update_test.go +++ b/tests/e2e/controlplane/control_plane_update_test.go @@ -120,7 +120,7 @@ spec: Success("sample deployed") samplePods := &corev1.PodList{} - Eventually(common.CheckPodsReady).WithArguments(ctx, cl, sampleNamespace).Should(Succeed(), "Error checking status of sample pods") + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, cl).Should(Succeed(), "Error checking status of sample pods") Expect(cl.List(ctx, samplePods, client.InNamespace(sampleNamespace))).To(Succeed(), "Error getting the pods in sample namespace") Success("sample pods are ready") @@ -216,12 +216,7 @@ spec: cl.Delete(ctx, &pod) } - Expect(cl.List(ctx, samplePods, client.InNamespace(sampleNamespace))).To(Succeed()) - Expect(samplePods.Items).ToNot(BeEmpty(), "No pods found in sample namespace") - for _, pod := range samplePods.Items { - common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k, cl) - } - + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, cl).Should(Succeed(), "Error checking status of sample pods") Success("sample pods restarted and are ready") }) diff --git a/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go b/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go index a9c06f5035..b33bc3389e 100644 --- a/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go +++ b/tests/e2e/multicluster/multicluster_externalcontrolplane_test.go @@ -339,18 +339,13 @@ spec: Success("Sample app is deployed in Cluster #2") }) - samplePodsCluster2 := &corev1.PodList{} It("updates the pods status to Ready", func(ctx SpecContext) { - Expect(clRemote.List(ctx, samplePodsCluster2, client.InNamespace(sampleNamespace))).To(Succeed()) - Expect(samplePodsCluster2.Items).ToNot(BeEmpty(), "No pods found in sample namespace") - - for _, pod := range samplePodsCluster2.Items { - common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k2, clRemote) - } + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, clRemote).Should(Succeed(), "Error checking status of sample pods on Cluster #2") Success("Sample app is Running") }) It("has istio.io/rev annotation external-istiod", func(ctx SpecContext) { + samplePodsCluster2 := &corev1.PodList{} Expect(clRemote.List(ctx, samplePodsCluster2, client.InNamespace(sampleNamespace))).To(Succeed()) Expect(samplePodsCluster2.Items).ToNot(BeEmpty(), "No pods found in sample namespace") diff --git a/tests/e2e/multicluster/multicluster_multiprimary_test.go b/tests/e2e/multicluster/multicluster_multiprimary_test.go index 6fd0c852b4..b146c2319f 100644 --- a/tests/e2e/multicluster/multicluster_multiprimary_test.go +++ b/tests/e2e/multicluster/multicluster_multiprimary_test.go @@ -33,7 +33,6 @@ import ( . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("Multicluster deployment models", Label("multicluster", "multicluster-multiprimary"), Ordered, func() { @@ -197,22 +196,8 @@ values: }) It("updates the pods status to Ready", func(ctx SpecContext) { - samplePodsCluster1 := &corev1.PodList{} - - Expect(clPrimary.List(ctx, samplePodsCluster1, client.InNamespace(sampleNamespace))).To(Succeed()) - Expect(samplePodsCluster1.Items).ToNot(BeEmpty(), "No pods found in sample namespace") - - for _, pod := range samplePodsCluster1.Items { - common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k1, clPrimary) - } - - samplePodsCluster2 := &corev1.PodList{} - Expect(clRemote.List(ctx, samplePodsCluster2, client.InNamespace(sampleNamespace))).To(Succeed()) - Expect(samplePodsCluster2.Items).ToNot(BeEmpty(), "No pods found in sample namespace") - - for _, pod := range samplePodsCluster2.Items { - common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k2, clRemote) - } + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, clPrimary).Should(Succeed(), "Error checking status of sample pods on Cluster #1") + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, clRemote).Should(Succeed(), "Error checking status of sample pods on Cluster #2") Success("Sample app is created in both clusters and Running") }) diff --git a/tests/e2e/multicluster/multicluster_primaryremote_test.go b/tests/e2e/multicluster/multicluster_primaryremote_test.go index 0a516fba02..03d7e11dc9 100644 --- a/tests/e2e/multicluster/multicluster_primaryremote_test.go +++ b/tests/e2e/multicluster/multicluster_primaryremote_test.go @@ -34,7 +34,6 @@ import ( . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("Multicluster deployment models", Label("multicluster", "multicluster-primaryremote"), Ordered, func() { @@ -228,22 +227,8 @@ values: }) It("updates the pods status to Ready", func(ctx SpecContext) { - samplePodsPrimary := &corev1.PodList{} - - Expect(clPrimary.List(ctx, samplePodsPrimary, client.InNamespace(sampleNamespace))).To(Succeed()) - Expect(samplePodsPrimary.Items).ToNot(BeEmpty(), "No pods found in sample namespace") - - for _, pod := range samplePodsPrimary.Items { - common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k1, clPrimary) - } - - samplePodsRemote := &corev1.PodList{} - Expect(clRemote.List(ctx, samplePodsRemote, client.InNamespace(sampleNamespace))).To(Succeed()) - Expect(samplePodsRemote.Items).ToNot(BeEmpty(), "No pods found in sample namespace") - - for _, pod := range samplePodsRemote.Items { - common.AwaitCondition(ctx, corev1.PodReady, kube.Key(pod.Name, sampleNamespace), &corev1.Pod{}, k2, clRemote) - } + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, clPrimary).Should(Succeed(), "Error checking status of sample pods on Primary cluster") + Eventually(common.CheckSamplePodsReady).WithArguments(ctx, clRemote).Should(Succeed(), "Error checking status of sample pods on Remote cluster") Success("Sample app is created in both clusters and Running") }) diff --git a/tests/e2e/util/common/checks.go b/tests/e2e/util/common/checks.go index ec547c89cd..a0bb4d1c1f 100644 --- a/tests/e2e/util/common/checks.go +++ b/tests/e2e/util/common/checks.go @@ -27,6 +27,7 @@ import ( "github.com/istio-ecosystem/sail-operator/tests/e2e/util/kubectl" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -50,3 +51,35 @@ func AwaitCondition[T ~string](ctx context.Context, condition T, key client.Obje func AwaitDeployment(ctx context.Context, name string, k kubectl.Kubectl, cl client.Client) { AwaitCondition(ctx, appsv1.DeploymentAvailable, kube.Key(name, controlPlaneNamespace), &appsv1.Deployment{}, k, cl) } + +func isPodReady(pod *corev1.Pod) bool { + for _, cond := range pod.Status.Conditions { + if cond.Type == corev1.PodReady && cond.Status == corev1.ConditionTrue { + return true + } + } + return false +} + +// CheckPodsReady in the given namespace. +func CheckPodsReady(ctx context.Context, cl client.Client, namespace string) error { + podList := &corev1.PodList{} + if err := cl.List(ctx, podList, client.InNamespace(namespace)); err != nil { + return fmt.Errorf("Failed to list pods: %w", err) + } + if len(podList.Items) == 0 { + return fmt.Errorf("No pods found in namespace %q", namespace) + } + + for _, pod := range podList.Items { + if !isPodReady(&pod) { + return fmt.Errorf("Pod %q in namespace %q is not ready", pod.Name, namespace) + } + } + + return nil +} + +func CheckSamplePodsReady(ctx context.Context, cl client.Client) error { + return CheckPodsReady(ctx, cl, sampleNamespace) +} diff --git a/tests/e2e/util/common/e2e_utils.go b/tests/e2e/util/common/e2e_utils.go index a11743961b..641cf7e5c4 100644 --- a/tests/e2e/util/common/e2e_utils.go +++ b/tests/e2e/util/common/e2e_utils.go @@ -64,6 +64,7 @@ var ( istioName = env.Get("ISTIO_NAME", "default") istioCniName = env.Get("ISTIOCNI_NAME", "default") istioCniNamespace = env.Get("ISTIOCNI_NAMESPACE", "istio-cni") + sampleNamespace = env.Get("SAMPLE_NAMESPACE", "sample") ztunnelNamespace = env.Get("ZTUNNEL_NAMESPACE", "ztunnel") // version can have one of the following formats: @@ -340,33 +341,6 @@ func GetVersionFromIstiod() (*semver.Version, error) { return nil, fmt.Errorf("error getting version from istiod: version not found in output: %s", output) } -func isPodReady(pod *corev1.Pod) bool { - for _, cond := range pod.Status.Conditions { - if cond.Type == corev1.PodReady && cond.Status == corev1.ConditionTrue { - return true - } - } - return false -} - -func CheckPodsReady(ctx context.Context, cl client.Client, namespace string) error { - podList := &corev1.PodList{} - if err := cl.List(ctx, podList, client.InNamespace(namespace)); err != nil { - return fmt.Errorf("Failed to list pods: %w", err) - } - if len(podList.Items) == 0 { - return fmt.Errorf("No pods found in namespace %q", namespace) - } - - for _, pod := range podList.Items { - if !isPodReady(&pod) { - return fmt.Errorf("pod %q in namespace %q is not ready", pod.Name, namespace) - } - } - - return nil -} - // Resolve domain name and return ip address. // By default, return ipv4 address and if missing, return ipv6. func ResolveHostDomainToIP(hostDomain string) (string, error) { From 2cb5e6d524a643e1ab568f0b5e382e563054c354 Mon Sep 17 00:00:00 2001 From: Mike Kolesnik Date: Wed, 19 Nov 2025 15:41:31 +0200 Subject: [PATCH 4/4] E2E: Extract waiting for CNI DaemonSet to be ready This is repetitive and verbose, extracting this makes the tests easier to read and maintain. Signed-off-by: Mike Kolesnik --- .../multicluster_multiprimary_test.go | 19 ++----------------- .../multicluster_primaryremote_test.go | 9 +-------- tests/e2e/util/common/checks.go | 13 +++++++++++++ 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/tests/e2e/multicluster/multicluster_multiprimary_test.go b/tests/e2e/multicluster/multicluster_multiprimary_test.go index b146c2319f..9365ec2c6b 100644 --- a/tests/e2e/multicluster/multicluster_multiprimary_test.go +++ b/tests/e2e/multicluster/multicluster_multiprimary_test.go @@ -106,23 +106,8 @@ values: }) It("deploys istio-cni-node", func(ctx SpecContext) { - Eventually(func() bool { - daemonset := &appsv1.DaemonSet{} - if err := clPrimary.Get(ctx, kube.Key("istio-cni-node", istioCniNamespace), daemonset); err != nil { - return false - } - return daemonset.Status.NumberAvailable == daemonset.Status.CurrentNumberScheduled - }).Should(BeTrue(), "CNI DaemonSet Pods are not Available on Cluster #1") - Success("CNI DaemonSet is deployed in the namespace and Running on Cluster #1") - - Eventually(func() bool { - daemonset := &appsv1.DaemonSet{} - if err := clRemote.Get(ctx, kube.Key("istio-cni-node", istioCniNamespace), daemonset); err != nil { - return false - } - return daemonset.Status.NumberAvailable == daemonset.Status.CurrentNumberScheduled - }).Should(BeTrue(), "IstioCNI DaemonSet Pods are not Available on Cluster #2") - Success("IstioCNI DaemonSet is deployed in the namespace and Running on Cluster #2") + common.AwaitCniDaemonSet(ctx, k1, clPrimary) + common.AwaitCniDaemonSet(ctx, k2, clRemote) }) }) diff --git a/tests/e2e/multicluster/multicluster_primaryremote_test.go b/tests/e2e/multicluster/multicluster_primaryremote_test.go index 03d7e11dc9..cb97c784e7 100644 --- a/tests/e2e/multicluster/multicluster_primaryremote_test.go +++ b/tests/e2e/multicluster/multicluster_primaryremote_test.go @@ -111,14 +111,7 @@ values: }) It("deploys istio-cni-node", func(ctx SpecContext) { - Eventually(func() bool { - daemonset := &appsv1.DaemonSet{} - if err := clPrimary.Get(ctx, kube.Key("istio-cni-node", istioCniNamespace), daemonset); err != nil { - return false - } - return daemonset.Status.NumberAvailable == daemonset.Status.CurrentNumberScheduled - }).Should(BeTrue(), "IstioCNI DaemonSet Pods are not Available on Primary Cluster") - Success("IstioCNI DaemonSet is deployed in the namespace and Running on Primary Cluster") + common.AwaitCniDaemonSet(ctx, k1, clPrimary) }) }) diff --git a/tests/e2e/util/common/checks.go b/tests/e2e/util/common/checks.go index a0bb4d1c1f..a3df3ca9a7 100644 --- a/tests/e2e/util/common/checks.go +++ b/tests/e2e/util/common/checks.go @@ -83,3 +83,16 @@ func CheckPodsReady(ctx context.Context, cl client.Client, namespace string) err func CheckSamplePodsReady(ctx context.Context, cl client.Client) error { return CheckPodsReady(ctx, cl, sampleNamespace) } + +// AwaitCniDaemonSet to be deployed and reach the scheduled number of pods. +func AwaitCniDaemonSet(ctx context.Context, k kubectl.Kubectl, cl client.Client) { + key := kube.Key("istio-cni-node", istioCniNamespace) + Eventually(func() bool { + daemonset := &appsv1.DaemonSet{} + if err := cl.Get(ctx, key, daemonset); err != nil { + return false + } + return daemonset.Status.NumberAvailable == daemonset.Status.CurrentNumberScheduled + }).Should(BeTrue(), fmt.Sprintf("DaemonSet '%s' is not Available in the '%s' namespace on %s cluster", key.Name, key.Namespace, k.ClusterName)) + Success(fmt.Sprintf("DaemonSet '%s' is deployed and running in the '%s' namespace on %s cluster", key.Name, key.Namespace, k.ClusterName)) +}