From 9ef2e0bc0ccf4975a033fec44960e93e830d8ce9 Mon Sep 17 00:00:00 2001 From: Christoph Blecker Date: Sat, 14 Oct 2023 14:52:39 -0700 Subject: [PATCH] Add eventually timeout when we use contexts --- test/e2e/adminapi_cluster_getlogs.go | 2 +- test/e2e/adminapi_cluster_update.go | 2 +- test/e2e/adminapi_kubernetesobjects.go | 8 ++-- test/e2e/adminapi_redeployvm.go | 2 +- test/e2e/cluster.go | 17 ++++----- test/e2e/operator.go | 52 +++++++++++++------------- test/e2e/setup.go | 7 +++- 7 files changed, 46 insertions(+), 44 deletions(-) diff --git a/test/e2e/adminapi_cluster_getlogs.go b/test/e2e/adminapi_cluster_getlogs.go index 2f286ddda..422de1f4e 100644 --- a/test/e2e/adminapi_cluster_getlogs.go +++ b/test/e2e/adminapi_cluster_getlogs.go @@ -61,7 +61,7 @@ func testGetPodLogsOK(ctx context.Context, containerName, podName, namespace str g.Expect(err).NotTo(HaveOccurred()) g.Expect(pod.Status.Phase).To(Equal(corev1.PodSucceeded)) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("requesting logs via RP admin API") params := url.Values{ diff --git a/test/e2e/adminapi_cluster_update.go b/test/e2e/adminapi_cluster_update.go index 4534b79e3..eb32e5cfb 100644 --- a/test/e2e/adminapi_cluster_update.go +++ b/test/e2e/adminapi_cluster_update.go @@ -33,7 +33,7 @@ var _ = Describe("[Admin API] Cluster admin update action", func() { Eventually(func(g Gomega, ctx context.Context) { oc = adminGetCluster(g, ctx, clusterResourceID) g.Expect(oc.Properties.ProvisioningState).To(Equal(admin.ProvisioningStateSucceeded)) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) Expect(oc.Properties.LastAdminUpdateError).To(Equal("")) }) diff --git a/test/e2e/adminapi_kubernetesobjects.go b/test/e2e/adminapi_kubernetesobjects.go index f1ac8f588..d7760c0b9 100644 --- a/test/e2e/adminapi_kubernetesobjects.go +++ b/test/e2e/adminapi_kubernetesobjects.go @@ -83,7 +83,7 @@ var _ = Describe("[Admin API] Kubernetes objects action", func() { Eventually(func(g Gomega, ctx context.Context) { _, err := clients.Kubernetes.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{}) g.Expect(kerrors.IsNotFound(err)).To(BeTrue(), "expect Namespace to be deleted") - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }() testConfigMapCreateOrUpdateForbidden(ctx, "creating", objName, namespace) @@ -121,7 +121,7 @@ var _ = Describe("[Admin API] Kubernetes objects action", func() { Eventually(func(g Gomega, ctx context.Context) { _, err := clients.Kubernetes.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{}) g.Expect(kerrors.IsNotFound(err)).To(BeTrue(), "expect Namespace to be deleted") - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }() By("creating an object via Kubernetes API") @@ -304,7 +304,7 @@ func testConfigMapDeleteOK(ctx context.Context, objName, namespace string) { Eventually(func(g Gomega, ctx context.Context) { _, err = clients.Kubernetes.CoreV1().ConfigMaps(namespace).Get(ctx, objName, metav1.GetOptions{}) g.Expect(kerrors.IsNotFound(err)).To(BeTrue(), "expect ConfigMap to be deleted") - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) } func testConfigMapCreateOrUpdateForbidden(ctx context.Context, operation, objName, namespace string) { @@ -392,7 +392,7 @@ func testPodForceDeleteOK(ctx context.Context, objName, namespace string) { Eventually(func(g Gomega, ctx context.Context) { _, err = clients.Kubernetes.CoreV1().Pods(namespace).Get(ctx, objName, metav1.GetOptions{}) g.Expect(kerrors.IsNotFound(err)).To(BeTrue(), "expect Pod to be deleted") - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) } func mockSecret(name, namespace string) corev1.Secret { diff --git a/test/e2e/adminapi_redeployvm.go b/test/e2e/adminapi_redeployvm.go index 3fa000371..8775edd07 100644 --- a/test/e2e/adminapi_redeployvm.go +++ b/test/e2e/adminapi_redeployvm.go @@ -123,7 +123,7 @@ func getNodeUptime(g Gomega, ctx context.Context, node string) (time.Time, error g.Expect(err).NotTo(HaveOccurred()) g.Expect(p.Status.Phase).To(Equal(corev1.PodSucceeded)) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("getting logs") req := clients.Kubernetes.CoreV1().Pods(namespace).GetLogs(name, &corev1.PodLogOptions{}) diff --git a/test/e2e/cluster.go b/test/e2e/cluster.go index b07d9ed5e..d92dca499 100644 --- a/test/e2e/cluster.go +++ b/test/e2e/cluster.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "strings" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -43,7 +42,7 @@ var _ = Describe("Cluster", Serial, func() { By("verifying the namespace is ready") Eventually(func(ctx context.Context) error { return p.Verify(ctx) - }).WithContext(ctx).WithTimeout(5 * time.Minute).Should(BeNil()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(BeNil()) DeferCleanup(func(ctx context.Context) { By("deleting a test namespace") @@ -53,7 +52,7 @@ var _ = Describe("Cluster", Serial, func() { By("verifying the namespace is deleted") Eventually(func(ctx context.Context) error { return p.VerifyProjectIsDeleted(ctx) - }).WithContext(ctx).WithTimeout(5 * time.Minute).Should(BeNil()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(BeNil()) }) }) @@ -78,7 +77,7 @@ var _ = Describe("Cluster", Serial, func() { g.Expect(err).NotTo(HaveOccurred()) g.Expect(ready.StatefulSetIsReady(s)).To(BeTrue(), "expect stateful to be ready") GinkgoWriter.Println(s) - }).WithContext(ctx).WithTimeout(5 * time.Minute).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) // TODO: this test is marked as pending as it isn't working as expected @@ -169,7 +168,7 @@ var _ = Describe("Cluster", Serial, func() { g.Expect(nAclSubnets).To(ContainElement(strings.ToLower(subnet))) } - }).WithContext(ctx).WithTimeout(5 * time.Minute).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("creating stateful set") storageClass := "azurefile-csi" @@ -186,7 +185,7 @@ var _ = Describe("Cluster", Serial, func() { pvc, err := clients.Kubernetes.CoreV1().PersistentVolumeClaims(p.Name).Get(ctx, ssName, metav1.GetOptions{}) g.Expect(err).NotTo(HaveOccurred()) GinkgoWriter.Println(pvc) - }).WithContext(ctx).WithTimeout(5 * time.Minute).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("cleaning up the cluster subnets (removing service endpoints)") for _, s := range ocpSubnets { @@ -230,7 +229,7 @@ var _ = Describe("Cluster", Serial, func() { return false } return ready.ServiceIsReady(svc) - }).WithContext(ctx).Should(BeTrue()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(BeTrue()) By("verifying the internal load balancer service is ready") Eventually(func(ctx context.Context) bool { @@ -239,7 +238,7 @@ var _ = Describe("Cluster", Serial, func() { return false } return ready.ServiceIsReady(svc) - }).WithContext(ctx).Should(BeTrue()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(BeTrue()) }) // mainly we want to test the gateway/egress functionality - this request for the image will travel from @@ -257,7 +256,7 @@ var _ = Describe("Cluster", Serial, func() { g.Expect(err).NotTo(HaveOccurred()) g.Expect(ready.DeploymentIsReady(s)).To(BeTrue(), "expect stateful to be ready") - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) diff --git a/test/e2e/operator.go b/test/e2e/operator.go index 15d56651d..4c968b0dc 100644 --- a/test/e2e/operator.go +++ b/test/e2e/operator.go @@ -96,7 +96,7 @@ var _ = Describe("ARO Operator - Internet checking", func() { g.Expect(conditions.IsTrue(co.Status.Conditions, arov1alpha1.InternetReachableFromMaster)).To(BeTrue()) g.Expect(conditions.IsTrue(co.Status.Conditions, arov1alpha1.InternetReachableFromWorker)).To(BeTrue()) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) It("sets InternetReachableFromMaster and InternetReachableFromWorker to false when URL is not reachable", func(ctx context.Context) { @@ -119,7 +119,7 @@ var _ = Describe("ARO Operator - Internet checking", func() { g.Expect(conditions.IsFalse(co.Status.Conditions, arov1alpha1.InternetReachableFromMaster)).To(BeTrue()) g.Expect(conditions.IsFalse(co.Status.Conditions, arov1alpha1.InternetReachableFromWorker)).To(BeTrue()) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) @@ -133,7 +133,7 @@ var _ = Describe("ARO Operator - Geneva Logging", func() { } By("checking that mdsd DaemonSet is ready before the test") - Eventually(mdsdIsReady).WithContext(ctx).Should(Succeed()) + Eventually(mdsdIsReady).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) initial, err := updatedObjects(ctx, "openshift-azure-logging") Expect(err).NotTo(HaveOccurred()) @@ -143,7 +143,7 @@ var _ = Describe("ARO Operator - Geneva Logging", func() { Expect(err).NotTo(HaveOccurred()) By("checking that mdsd DaemonSet is ready") - Eventually(mdsdIsReady).WithContext(ctx).Should(Succeed()) + Eventually(mdsdIsReady).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("confirming that only one object was updated") final, err := updatedObjects(ctx, "openshift-azure-logging") @@ -166,7 +166,7 @@ var _ = Describe("ARO Operator - Cluster Monitoring ConfigMap", func() { } By("waiting for the ConfigMap to make sure it exists") - Eventually(configMapExists).WithContext(ctx).Should(Succeed()) + Eventually(configMapExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("unmarshalling the config from the ConfigMap data") var configData monitoring.Config @@ -191,14 +191,14 @@ var _ = Describe("ARO Operator - Cluster Monitoring ConfigMap", func() { } By("waiting for the ConfigMap to make sure it exists") - Eventually(configMapExists).WithContext(ctx).Should(Succeed()) + Eventually(configMapExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("deleting for the ConfigMap") err := clients.Kubernetes.CoreV1().ConfigMaps("openshift-monitoring").Delete(ctx, "cluster-monitoring-config", metav1.DeleteOptions{}) Expect(err).NotTo(HaveOccurred()) By("waiting for the ConfigMap to make sure it was restored") - Eventually(configMapExists).WithContext(ctx).Should(Succeed()) + Eventually(configMapExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) @@ -210,14 +210,14 @@ var _ = Describe("ARO Operator - RBAC", func() { } By("waiting for the ClusterRole to make sure it exists") - Eventually(clusterRoleExists).WithContext(ctx).Should(Succeed()) + Eventually(clusterRoleExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("deleting for the ClusterRole") err := clients.Kubernetes.RbacV1().ClusterRoles().Delete(ctx, "system:aro-sre", metav1.DeleteOptions{}) Expect(err).NotTo(HaveOccurred()) By("waiting for the ClusterRole to make sure it was restored") - Eventually(clusterRoleExists).WithContext(ctx).Should(Succeed()) + Eventually(clusterRoleExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) @@ -244,7 +244,7 @@ var _ = Describe("ARO Operator - MachineHealthCheck", func() { Expect(err).NotTo(HaveOccurred()) By("waiting for the machine health check to be restored") - Eventually(getMachineHealthCheck).WithContext(ctx).Should(Succeed()) + Eventually(getMachineHealthCheck).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) It("the alerting rule must recreated if deleted", func(ctx context.Context) { @@ -253,7 +253,7 @@ var _ = Describe("ARO Operator - MachineHealthCheck", func() { Expect(err).NotTo(HaveOccurred()) By("waiting for the machine health check remediation alert to be restored") - Eventually(getMachineHealthCheckRemediationAlertName).WithContext(ctx).Should(Succeed()) + Eventually(getMachineHealthCheckRemediationAlertName).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) @@ -271,7 +271,7 @@ var _ = Describe("ARO Operator - Conditions", func() { for _, condition := range arov1alpha1.ClusterChecksTypes() { g.Expect(conditions.IsTrue(co.Status.Conditions, condition)).To(BeTrue(), "Condition %s", condition) } - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) It("must have all the conditions on the cluster operator set to the expected values", func(ctx context.Context) { @@ -282,7 +282,7 @@ var _ = Describe("ARO Operator - Conditions", func() { g.Expect(cov1Helpers.IsStatusConditionTrue(co.Status.Conditions, configv1.OperatorAvailable)) g.Expect(cov1Helpers.IsStatusConditionFalse(co.Status.Conditions, configv1.OperatorProgressing)) g.Expect(cov1Helpers.IsStatusConditionFalse(co.Status.Conditions, configv1.OperatorDegraded)) - }).WithContext(ctx).WithTimeout(timeout).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).WithTimeout(timeout).Should(Succeed()) }) }) @@ -384,7 +384,7 @@ var _ = Describe("ARO Operator - Azure Subnet Reconciler", func() { co, err := clients.AROClusters.AroV1alpha1().Clusters().Get(ctx, "cluster", metav1.GetOptions{}) g.Expect(err).NotTo(HaveOccurred()) g.Expect(co.Annotations).To(Satisfy(subnetReconciliationAnnotationExists)) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) } }) }) @@ -409,7 +409,7 @@ var _ = Describe("ARO Operator - MUO Deployment", func() { g.Expect(err).NotTo(HaveOccurred()) g.Expect(string(b)).To(ContainSubstring(`X:boringcrypto,strictfipsruntime`)) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }, SpecTimeout(2*time.Minute)) It("must be restored if deleted", func(ctx context.Context) { @@ -430,13 +430,13 @@ var _ = Describe("ARO Operator - MUO Deployment", func() { } By("waiting for the MUO deployment to be ready") - Eventually(muoDeploymentExists).WithContext(ctx).Should(Succeed()) + Eventually(muoDeploymentExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("deleting the MUO deployment") Expect(deleteMUODeployment(ctx)).Should(Succeed()) By("waiting for the MUO deployment to be reconciled") - Eventually(muoDeploymentExists).WithContext(ctx).Should(Succeed()) + Eventually(muoDeploymentExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }, SpecTimeout(2*time.Minute)) }) @@ -508,7 +508,7 @@ var _ = Describe("ARO Operator - ImageConfig Reconciler", func() { Expect(err).NotTo(HaveOccurred()) By("waiting for the Image config to be reset") - Eventually(verifyLists(nil, nil)).WithContext(ctx).Should(Succeed()) + Eventually(verifyLists(nil, nil)).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) It("must set empty allow and block lists in Image config by default", func() { @@ -528,7 +528,7 @@ var _ = Describe("ARO Operator - ImageConfig Reconciler", func() { By("checking that Image config eventually has ARO service registries and the test registry in the allow list") expectedAllowlist := append(requiredRegistries, optionalRegistry) - Eventually(verifyLists(expectedAllowlist, nil)).WithContext(ctx).Should(Succeed()) + Eventually(verifyLists(expectedAllowlist, nil)).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) It("must remove ARO service registries from the block lists, but keep customer added registries", func(ctx context.Context) { @@ -539,7 +539,7 @@ var _ = Describe("ARO Operator - ImageConfig Reconciler", func() { By("checking that Image config eventually doesn't include ARO service registries") expectedBlocklist := []string{optionalRegistry} - Eventually(verifyLists(nil, expectedBlocklist)).WithContext(ctx).Should(Succeed()) + Eventually(verifyLists(nil, expectedBlocklist)).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) @@ -634,13 +634,13 @@ var _ = Describe("ARO Operator - Guardrails", func() { } By("waiting for the gatekeeper Controller Manager deployment to be ready") - Eventually(controllerManagerDeploymentExists).WithContext(ctx).Should(Succeed()) + Eventually(controllerManagerDeploymentExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("deleting the gatekeeper Controller Manager deployment") Expect(deleteControllerManagerDeployment(ctx)).Should(Succeed()) By("waiting for the gatekeeper Controller Manager deployment to be reconciled") - Eventually(controllerManagerDeploymentExists).WithContext(ctx).Should(Succeed()) + Eventually(controllerManagerDeploymentExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) It("Audit must be restored if deleted", func(ctx context.Context) { @@ -669,13 +669,13 @@ var _ = Describe("ARO Operator - Guardrails", func() { } By("waiting for the gatekeeper Audit deployment to be ready") - Eventually(auditDeploymentExists).WithContext(ctx).Should(Succeed()) + Eventually(auditDeploymentExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("deleting the gatekeeper Audit deployment") Expect(deleteAuditDeployment(ctx)).Should(Succeed()) By("waiting for the gatekeeper Audit deployment to be reconciled") - Eventually(auditDeploymentExists).WithContext(ctx).Should(Succeed()) + Eventually(auditDeploymentExists).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) @@ -700,13 +700,13 @@ var _ = Describe("ARO Operator - Cloud Provder Config ConfigMap", func() { var err error cm, err = clients.Kubernetes.CoreV1().ConfigMaps("openshift-config").Get(ctx, "cloud-provider-config", metav1.GetOptions{}) g.Expect(err).ToNot(HaveOccurred()) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) By("waiting for disableOutboundSNAT to be true") Eventually(func(g Gomega, ctx context.Context) { disableOutboundSNAT, err := cpcController.GetDisableOutboundSNAT(cm.Data["config"]) g.Expect(err).NotTo(HaveOccurred()) g.Expect(disableOutboundSNAT).To(BeTrue()) - }).WithContext(ctx).Should(Succeed()) + }).WithContext(ctx).WithTimeout(DefaultEventuallyTimeout).Should(Succeed()) }) }) diff --git a/test/e2e/setup.go b/test/e2e/setup.go index 570ca5536..000ea843f 100644 --- a/test/e2e/setup.go +++ b/test/e2e/setup.go @@ -48,7 +48,10 @@ import ( "github.com/Azure/ARO-RP/test/util/kubeadminkubeconfig" ) -var disallowedInFilenameRegex = regexp.MustCompile(`[<>:"/\\|?*\x00-\x1F]`) +var ( + disallowedInFilenameRegex = regexp.MustCompile(`[<>:"/\\|?*\x00-\x1F]`) + DefaultEventuallyTimeout = 5 * time.Minute +) type clientSet struct { Operations redhatopenshift20220904.OperationsClient @@ -428,7 +431,7 @@ func done(ctx context.Context) error { var _ = BeforeSuite(func() { log.Info("BeforeSuite") - SetDefaultEventuallyTimeout(5 * time.Minute) + SetDefaultEventuallyTimeout(DefaultEventuallyTimeout) SetDefaultEventuallyPollingInterval(10 * time.Second) if err := setup(context.Background()); err != nil {