From 5ef1694c5d3a1b854b2d65fae2c351eefbb98244 Mon Sep 17 00:00:00 2001 From: Mario Fernandez Date: Tue, 8 Aug 2023 14:17:31 +0200 Subject: [PATCH 1/7] chore: replace deprecated wait.Poll --- test/e2e/framework/assertions.go | 52 +++++++++----------- test/e2e/monitoring_stack_controller_test.go | 41 ++++++--------- test/e2e/prometheus_operator_test.go | 3 -- test/e2e/thanos_querier_controller_test.go | 29 +++++++++-- 4 files changed, 63 insertions(+), 62 deletions(-) diff --git a/test/e2e/framework/assertions.go b/test/e2e/framework/assertions.go index b661035c..f632e68c 100644 --- a/test/e2e/framework/assertions.go +++ b/test/e2e/framework/assertions.go @@ -23,6 +23,8 @@ import ( "k8s.io/apimachinery/pkg/util/wait" ) +// default ForeverTestTimeout is 30, some test fails because they take more than 30s +// change to custom in order to let the test finish withouth errors const CustomForeverTestTimeout = 40 * time.Second type AssertOption struct { @@ -44,7 +46,7 @@ func WithPollInterval(d time.Duration) OptionFn { } } -// AssertResourceNeverExists asserts that a statefulset is never created for the duration of CustomForeverTestTimeout +// AssertResourceNeverExists asserts that a statefulset is never created for the duration of customForeverTestTimeout func (f *Framework) AssertResourceNeverExists(name, namespace string, resource client.Object, fns ...OptionFn) func(t *testing.T) { option := AssertOption{ PollInterval: 5 * time.Second, @@ -55,25 +57,23 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c } return func(t *testing.T) { - //nolint - if err := wait.Poll(option.PollInterval, option.WaitTimeout, func() (done bool, err error) { + if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (done bool, err error) { key := types.NamespacedName{ Name: name, Namespace: namespace, } - if err := f.K8sClient.Get(context.Background(), key, resource); errors.IsNotFound(err) { + if err := f.K8sClient.Get(ctx, key, resource); errors.IsNotFound(err) { return false, nil } return true, fmt.Errorf("resource %s/%s should not have been created", namespace, name) - //nolint - }); err != wait.ErrWaitTimeout { + }); wait.Interrupted(err) { t.Fatal(err) } } } -// AssertResourceEventuallyExists asserts that a resource is created duration a time period of CustomForeverTestTimeout +// AssertResourceEventuallyExists asserts that a resource is created duration a time period of customForeverTestTimeout func (f *Framework) AssertResourceEventuallyExists(name, namespace string, resource client.Object, fns ...OptionFn) func(t *testing.T) { option := AssertOption{ PollInterval: 5 * time.Second, @@ -84,8 +84,7 @@ func (f *Framework) AssertResourceEventuallyExists(name, namespace string, resou } return func(t *testing.T) { - //nolint - if err := wait.Poll(option.PollInterval, option.WaitTimeout, func() (done bool, err error) { + if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (done bool, err error) { key := types.NamespacedName{ Name: name, Namespace: namespace, @@ -94,8 +93,7 @@ func (f *Framework) AssertResourceEventuallyExists(name, namespace string, resou return true, nil } return false, nil - //nolint - }); err == wait.ErrWaitTimeout { + }); wait.Interrupted(err) { t.Fatal(fmt.Errorf("resource %s/%s was never created", namespace, name)) } } @@ -112,8 +110,7 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option } return func(t *testing.T) { key := types.NamespacedName{Name: name, Namespace: namespace} - //nolint - if err := wait.Poll(5*time.Second, option.WaitTimeout, func() (bool, error) { + if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, option.WaitTimeout, true, func(ctx context.Context) (bool, error) { pod := &appsv1.StatefulSet{} err := f.K8sClient.Get(context.Background(), key, pod) return err == nil && pod.Status.ReadyReplicas == *pod.Spec.Replicas, nil @@ -124,8 +121,7 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option } func (f *Framework) GetResourceWithRetry(t *testing.T, name, namespace string, obj client.Object) { - //nolint - err := wait.Poll(5*time.Second, CustomForeverTestTimeout, func() (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { key := types.NamespacedName{Name: name, Namespace: namespace} if err := f.K8sClient.Get(context.Background(), key, obj); errors.IsNotFound(err) { @@ -136,14 +132,12 @@ func (f *Framework) GetResourceWithRetry(t *testing.T, name, namespace string, o return true, nil }) - //nolint - if err == wait.ErrWaitTimeout { + if wait.Interrupted(err) { t.Fatal(fmt.Errorf("resource %s/%s was never created", namespace, name)) } } func assertPromQL(t *testing.T, metrics []byte, query string, expected map[string]float64) { - now := time.Now() points, err := prom.ParseTextData(metrics, now) if err != nil { @@ -199,7 +193,6 @@ func assertPromQL(t *testing.T, metrics []byte, query string, expected map[strin // GetOperatorPod gets the operator pod assuming the operator is deployed in // "operators" namespace. func (f *Framework) GetOperatorPod(t *testing.T) *v1.Pod { - // get the operator deployment operator := appsv1.Deployment{} f.AssertResourceEventuallyExists("observability-operator", "operators", &operator)(t) @@ -233,8 +226,7 @@ func (f *Framework) GetOperatorMetrics(t *testing.T) []byte { stopChan := make(chan struct{}) defer close(stopChan) - //nolint - if err := wait.Poll(5*time.Second, CustomForeverTestTimeout, func() (bool, error) { + if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { err := f.StartPortForward(pod.Name, pod.Namespace, "8080", stopChan) return err == nil, nil }); err != nil { @@ -272,10 +264,13 @@ func (f *Framework) GetStackWhenAvailable(t *testing.T, name, namespace string) Name: name, Namespace: namespace, } - //nolint - err := wait.Poll(5*time.Second, CustomForeverTestTimeout, func() (bool, error) { + var lastErr error + + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + lastErr = nil err := f.K8sClient.Get(context.Background(), key, &ms) if err != nil { + lastErr = err return false, nil } availableC := getConditionByType(ms.Status.Conditions, v1alpha1.AvailableCondition) @@ -285,9 +280,8 @@ func (f *Framework) GetStackWhenAvailable(t *testing.T, name, namespace string) return false, nil }) - //nolint - if err == wait.ErrWaitTimeout { - t.Fatal(fmt.Errorf("resource %s/%s was not available", namespace, name)) + if wait.Interrupted(err) { + t.Fatal(fmt.Errorf("MonitoringStack %s/%s was not available - err: %w | %v", namespace, name, lastErr, ms.Status.Conditions)) } return ms } @@ -298,16 +292,14 @@ func (f *Framework) AssertAlertmanagerAbsent(t *testing.T, name, namespace strin Name: name, Namespace: namespace, } - //nolint - err := wait.Poll(5*time.Second, CustomForeverTestTimeout, func() (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { err := f.K8sClient.Get(context.Background(), key, &am) if errors.IsNotFound(err) { return true, nil } return false, nil }) - //nolint - if err == wait.ErrWaitTimeout { + if wait.Interrupted(err) { t.Fatal(fmt.Errorf("alertmanager %s/%s is present when expected to be absent", namespace, name)) } } diff --git a/test/e2e/monitoring_stack_controller_test.go b/test/e2e/monitoring_stack_controller_test.go index 51a15375..5f242318 100644 --- a/test/e2e/monitoring_stack_controller_test.go +++ b/test/e2e/monitoring_stack_controller_test.go @@ -49,7 +49,10 @@ func assertCRDExists(t *testing.T, crds ...string) { } func TestMonitoringStackController(t *testing.T) { - stack.AddToScheme(scheme.Scheme) + err := stack.AddToScheme(scheme.Scheme) + if err != nil { + return + } assertCRDExists(t, "prometheuses.monitoring.rhobs", "alertmanagers.monitoring.rhobs", @@ -148,8 +151,7 @@ func nilResrouceSelectorPropagatesToPrometheus(t *testing.T) { assert.NilError(t, err, "failed to patch monitoring stack with nil resource selector") prometheus := monv1.Prometheus{} - //nolint - err = wait.Poll(5*time.Second, framework.CustomForeverTestTimeout, func() (bool, error) { + err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { if err := f.K8sClient.Get(context.Background(), types.NamespacedName{Name: updatedMS.Name, Namespace: updatedMS.Namespace}, &prometheus); errors.IsNotFound(err) { return false, nil } @@ -160,8 +162,7 @@ func nilResrouceSelectorPropagatesToPrometheus(t *testing.T) { return true, nil }) - //nolint - if err == wait.ErrWaitTimeout { + if wait.Interrupted(err) { t.Fatal(fmt.Errorf("nil ResourceSelector did not propagate to Prometheus object")) } } @@ -298,8 +299,7 @@ func reconcileRevertsManualChanges(t *testing.T) { err = f.K8sClient.Update(context.Background(), modified) assert.NilError(t, err, "failed to update a prometheus") - //nolint - err = wait.Poll(5*time.Second, time.Minute, func() (bool, error) { + err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { reconciled := monv1.Prometheus{} key := types.NamespacedName{Name: ms.Name, Namespace: ms.Namespace} @@ -415,8 +415,7 @@ func assertPrometheusScrapesItself(t *testing.T) { stopChan := make(chan struct{}) defer close(stopChan) - //nolint - if err := wait.Poll(5*time.Second, 2*time.Minute, func() (bool, error) { + if err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { err = f.StartServicePortForward("self-scrape-prometheus", e2eTestNamespace, "9090", stopChan) return err == nil, nil }); err != nil { @@ -428,8 +427,7 @@ func assertPrometheusScrapesItself(t *testing.T) { "prometheus_build_info": 2, // scrapes from both endpoints "alertmanager_build_info": 2, } - //nolint - if err := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { + if err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { correct := 0 for query, value := range expectedResults { result, err := promClient.Query(query) @@ -534,16 +532,14 @@ func assertAlertmanagerReceivesAlerts(t *testing.T) { stopChan := make(chan struct{}) defer close(stopChan) - //nolint - if err := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { + if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { err := f.StartServicePortForward("alerting-alertmanager", e2eTestNamespace, "9093", stopChan) return err == nil, nil }); err != nil { t.Fatal(err) } - //nolint - if err := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { + if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { alerts, err := getAlertmanagerAlerts() if err != nil { return false, nil @@ -590,8 +586,7 @@ func prometheusScaleDown(t *testing.T) { ms.Spec.PrometheusConfig.Replicas = &numOfRep err = f.K8sClient.Update(context.Background(), ms) assert.NilError(t, err, "failed to update a monitoring stack") - //nolint - err = wait.Poll(5*time.Second, framework.CustomForeverTestTimeout, func() (bool, error) { + err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { if err := f.K8sClient.Get(context.Background(), key, &prom); errors.IsNotFound(err) { return false, nil } @@ -602,8 +597,7 @@ func prometheusScaleDown(t *testing.T) { return true, nil }) - //nolint - if err == wait.ErrWaitTimeout { + if wait.Interrupted(err) { t.Fatal(fmt.Errorf("Prometheus was not scaled down")) } } @@ -827,8 +821,7 @@ func newMonitoringStack(t *testing.T, name string, mods ...stackModifier) *stack } func waitForStackDeletion(name string) error { - //nolint - return wait.Poll(5*time.Second, framework.CustomForeverTestTimeout, func() (bool, error) { + return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { key := types.NamespacedName{Name: name, Namespace: e2eTestNamespace} var ms stack.MonitoringStack err := f.K8sClient.Get(context.Background(), key, &ms) @@ -865,8 +858,7 @@ func namespaceSelectorTest(t *testing.T) { stopChan := make(chan struct{}) defer close(stopChan) - //nolint - if pollErr := wait.Poll(15*time.Second, 5*time.Minute, func() (bool, error) { + if pollErr := wait.PollUntilContextTimeout(context.Background(), 15*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { err := f.StartServicePortForward(ms.Name+"-prometheus", e2eTestNamespace, "9090", stopChan) return err == nil, nil }); pollErr != nil { @@ -874,8 +866,7 @@ func namespaceSelectorTest(t *testing.T) { } promClient := framework.NewPrometheusClient("http://localhost:9090") - //nolint - if pollErr := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { + if pollErr := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { query := `version{pod="prometheus-example-app",namespace=~"test-ns-.*"}` result, err := promClient.Query(query) if err != nil { diff --git a/test/e2e/prometheus_operator_test.go b/test/e2e/prometheus_operator_test.go index 3a989bea..2745d825 100644 --- a/test/e2e/prometheus_operator_test.go +++ b/test/e2e/prometheus_operator_test.go @@ -53,17 +53,14 @@ func TestPrometheusOperatorForNonOwnedResources(t *testing.T) { name: "Operator should not reconcile resources which it does not own", scenario: func(t *testing.T) { t.Run("Prometheus never exists", func(t *testing.T) { - t.Parallel() f.AssertResourceNeverExists(prometheusStsName, e2eTestNamespace, &appsv1.StatefulSet{}, framework.WithTimeout(15*time.Second))(t) }) t.Run("Alertmanager never exists", func(t *testing.T) { - t.Parallel() f.AssertResourceNeverExists(alertmanagerStsName, e2eTestNamespace, &appsv1.StatefulSet{}, framework.WithTimeout(15*time.Second))(t) }) t.Run("Thanos Ruler never exists", func(t *testing.T) { - t.Parallel() f.AssertResourceNeverExists(thanosRulerStsName, e2eTestNamespace, &appsv1.StatefulSet{}, framework.WithTimeout(15*time.Second))(t) }) diff --git a/test/e2e/thanos_querier_controller_test.go b/test/e2e/thanos_querier_controller_test.go index 2e45166d..dae291b9 100644 --- a/test/e2e/thanos_querier_controller_test.go +++ b/test/e2e/thanos_querier_controller_test.go @@ -75,11 +75,15 @@ func singleStackWithSidecar(t *testing.T) { // Assert prometheus instance can be queried stopChan := make(chan struct{}) defer close(stopChan) +<<<<<<< Updated upstream //nolint if err := wait.Poll(5*time.Second, 2*time.Minute, func() (bool, error) { +======= + if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { +>>>>>>> Stashed changes err = f.StartServicePortForward(name, e2eTestNamespace, "9090", stopChan) return err == nil, nil - }); err != nil { + }); wait.Interrupted(err) { t.Fatal(err) } @@ -87,8 +91,12 @@ func singleStackWithSidecar(t *testing.T) { expectedResults := map[string]int{ "prometheus_build_info": 2, // must return from both prometheus pods } +<<<<<<< Updated upstream //nolint if err := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { +======= + if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { +>>>>>>> Stashed changes correct := 0 for query, value := range expectedResults { result, err := promClient.Query(query) @@ -113,7 +121,7 @@ func singleStackWithSidecar(t *testing.T) { } return correct == len(expectedResults), nil - }); err != nil { + }); wait.Interrupted(err) { t.Fatal(err) } } @@ -139,9 +147,14 @@ func newThanosQuerier(t *testing.T, name string, selector map[string]string) *ms } func waitForThanosQuerierDeletion(tq *msov1.ThanosQuerier) error { +<<<<<<< Updated upstream //nolint return wait.Poll(5*time.Second, wait.ForeverTestTimeout, func() (bool, error) { err := f.K8sClient.Get(context.Background(), +======= + return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { + err = f.K8sClient.Get(context.Background(), +>>>>>>> Stashed changes types.NamespacedName{Name: tq.Name, Namespace: tq.Namespace}, tq) return errors.IsNotFound(err), nil @@ -149,10 +162,14 @@ func waitForThanosQuerierDeletion(tq *msov1.ThanosQuerier) error { } func waitForDeploymentDeletion(name string) error { +<<<<<<< Updated upstream //nolint return wait.Poll(5*time.Second, wait.ForeverTestTimeout, func() (bool, error) { +======= + return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { +>>>>>>> Stashed changes var dep appsv1.Deployment - err := f.K8sClient.Get(context.Background(), + err = f.K8sClient.Get(context.Background(), types.NamespacedName{Name: name, Namespace: e2eTestNamespace}, &dep) return errors.IsNotFound(err), nil @@ -160,10 +177,14 @@ func waitForDeploymentDeletion(name string) error { } func waitForServiceDeletion(name string) error { +<<<<<<< Updated upstream //nolint return wait.Poll(5*time.Second, wait.ForeverTestTimeout, func() (bool, error) { +======= + return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { +>>>>>>> Stashed changes var svc corev1.Service - err := f.K8sClient.Get(context.Background(), + err = f.K8sClient.Get(context.Background(), types.NamespacedName{Name: name, Namespace: e2eTestNamespace}, &svc) return errors.IsNotFound(err), nil From 05c1dbc045a27359538fba48ea0af90ef4689dbf Mon Sep 17 00:00:00 2001 From: Mario Fernandez Date: Tue, 8 Aug 2023 15:56:41 +0200 Subject: [PATCH 2/7] fix: minor fixes Signed-off-by: Mario Fernandez --- test/e2e/framework/assertions.go | 26 ++++++++++---------- test/e2e/monitoring_stack_controller_test.go | 20 ++++++--------- test/e2e/po_admission_webhook_test.go | 4 +-- test/e2e/thanos_querier_controller_test.go | 26 -------------------- 4 files changed, 23 insertions(+), 53 deletions(-) diff --git a/test/e2e/framework/assertions.go b/test/e2e/framework/assertions.go index f632e68c..8b4fa6fb 100644 --- a/test/e2e/framework/assertions.go +++ b/test/e2e/framework/assertions.go @@ -25,7 +25,7 @@ import ( // default ForeverTestTimeout is 30, some test fails because they take more than 30s // change to custom in order to let the test finish withouth errors -const CustomForeverTestTimeout = 40 * time.Second +const DefaultTestTimeout = 40 * time.Second type AssertOption struct { PollInterval time.Duration @@ -46,11 +46,11 @@ func WithPollInterval(d time.Duration) OptionFn { } } -// AssertResourceNeverExists asserts that a statefulset is never created for the duration of customForeverTestTimeout +// AssertResourceNeverExists asserts that a statefulset is never created for the duration of DefaultTestTimeout func (f *Framework) AssertResourceNeverExists(name, namespace string, resource client.Object, fns ...OptionFn) func(t *testing.T) { option := AssertOption{ PollInterval: 5 * time.Second, - WaitTimeout: CustomForeverTestTimeout, + WaitTimeout: DefaultTestTimeout, } for _, fn := range fns { fn(&option) @@ -62,12 +62,12 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c Name: name, Namespace: namespace, } - if err := f.K8sClient.Get(ctx, key, resource); errors.IsNotFound(err) { + if err := f.K8sClient.Get(context.Background(), key, resource); errors.IsNotFound(err) { return false, nil } return true, fmt.Errorf("resource %s/%s should not have been created", namespace, name) - }); wait.Interrupted(err) { + }); !wait.Interrupted(err) { t.Fatal(err) } } @@ -77,7 +77,7 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c func (f *Framework) AssertResourceEventuallyExists(name, namespace string, resource client.Object, fns ...OptionFn) func(t *testing.T) { option := AssertOption{ PollInterval: 5 * time.Second, - WaitTimeout: CustomForeverTestTimeout, + WaitTimeout: DefaultTestTimeout, } for _, fn := range fns { fn(&option) @@ -103,16 +103,16 @@ func (f *Framework) AssertResourceEventuallyExists(name, namespace string, resou func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...OptionFn) func(t *testing.T) { option := AssertOption{ PollInterval: 5 * time.Second, - WaitTimeout: CustomForeverTestTimeout, + WaitTimeout: DefaultTestTimeout, } for _, fn := range fns { fn(&option) } return func(t *testing.T) { key := types.NamespacedName{Name: name, Namespace: namespace} - if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, option.WaitTimeout, true, func(ctx context.Context) (bool, error) { + if err := wait.PollUntilContextTimeout(context.Background(), 5+time.Second, option.WaitTimeout, true, func(ctx context.Context) (done bool, err error) { pod := &appsv1.StatefulSet{} - err := f.K8sClient.Get(context.Background(), key, pod) + err = f.K8sClient.Get(context.Background(), key, pod) return err == nil && pod.Status.ReadyReplicas == *pod.Spec.Replicas, nil }); err != nil { t.Fatal(err) @@ -121,7 +121,7 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option } func (f *Framework) GetResourceWithRetry(t *testing.T, name, namespace string, obj client.Object) { - err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { key := types.NamespacedName{Name: name, Namespace: namespace} if err := f.K8sClient.Get(context.Background(), key, obj); errors.IsNotFound(err) { @@ -226,7 +226,7 @@ func (f *Framework) GetOperatorMetrics(t *testing.T) []byte { stopChan := make(chan struct{}) defer close(stopChan) - if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { err := f.StartPortForward(pod.Name, pod.Namespace, "8080", stopChan) return err == nil, nil }); err != nil { @@ -266,7 +266,7 @@ func (f *Framework) GetStackWhenAvailable(t *testing.T, name, namespace string) } var lastErr error - err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { lastErr = nil err := f.K8sClient.Get(context.Background(), key, &ms) if err != nil { @@ -292,7 +292,7 @@ func (f *Framework) AssertAlertmanagerAbsent(t *testing.T, name, namespace strin Name: name, Namespace: namespace, } - err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { err := f.K8sClient.Get(context.Background(), key, &am) if errors.IsNotFound(err) { return true, nil diff --git a/test/e2e/monitoring_stack_controller_test.go b/test/e2e/monitoring_stack_controller_test.go index 5f242318..7709ed84 100644 --- a/test/e2e/monitoring_stack_controller_test.go +++ b/test/e2e/monitoring_stack_controller_test.go @@ -50,9 +50,7 @@ func assertCRDExists(t *testing.T, crds ...string) { func TestMonitoringStackController(t *testing.T) { err := stack.AddToScheme(scheme.Scheme) - if err != nil { - return - } + assert.NilError(t, err, "adding stack to scheme failed") assertCRDExists(t, "prometheuses.monitoring.rhobs", "alertmanagers.monitoring.rhobs", @@ -151,7 +149,7 @@ func nilResrouceSelectorPropagatesToPrometheus(t *testing.T) { assert.NilError(t, err, "failed to patch monitoring stack with nil resource selector") prometheus := monv1.Prometheus{} - err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { if err := f.K8sClient.Get(context.Background(), types.NamespacedName{Name: updatedMS.Name, Namespace: updatedMS.Namespace}, &prometheus); errors.IsNotFound(err) { return false, nil } @@ -299,7 +297,7 @@ func reconcileRevertsManualChanges(t *testing.T) { err = f.K8sClient.Update(context.Background(), modified) assert.NilError(t, err, "failed to update a prometheus") - err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { reconciled := monv1.Prometheus{} key := types.NamespacedName{Name: ms.Name, Namespace: ms.Namespace} @@ -586,7 +584,7 @@ func prometheusScaleDown(t *testing.T) { ms.Spec.PrometheusConfig.Replicas = &numOfRep err = f.K8sClient.Update(context.Background(), ms) assert.NilError(t, err, "failed to update a monitoring stack") - err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + err = wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { if err := f.K8sClient.Get(context.Background(), key, &prom); errors.IsNotFound(err) { return false, nil } @@ -743,8 +741,6 @@ func getAlertmanagerAlerts() ([]alert, error) { } func newAlerts(t *testing.T) *monv1.PrometheusRule { - durationTenSec := monv1.Duration("10s") - durationOneSec := monv1.Duration("1s") rule := &monv1.PrometheusRule{ TypeMeta: metav1.TypeMeta{ APIVersion: monv1.SchemeGroupVersion.String(), @@ -758,17 +754,17 @@ func newAlerts(t *testing.T) *monv1.PrometheusRule { Groups: []monv1.RuleGroup{ { Name: "Test", - Interval: &durationTenSec, + Interval: ptr.To(monv1.Duration("10s")), Rules: []monv1.Rule{ { Alert: "AlwaysOn", Expr: intstr.FromString("vector(1)"), - For: &durationOneSec, + For: ptr.To(monv1.Duration("1s")), }, { Alert: "NeverOn", Expr: intstr.FromString("vector(1) == 0"), - For: &durationOneSec, + For: ptr.To(monv1.Duration("1s")), }, }, }, @@ -821,7 +817,7 @@ func newMonitoringStack(t *testing.T, name string, mods ...stackModifier) *stack } func waitForStackDeletion(name string) error { - return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.CustomForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { key := types.NamespacedName{Name: name, Namespace: e2eTestNamespace} var ms stack.MonitoringStack err := f.K8sClient.Get(context.Background(), key, &ms) diff --git a/test/e2e/po_admission_webhook_test.go b/test/e2e/po_admission_webhook_test.go index cf21aea8..d10d32b2 100644 --- a/test/e2e/po_admission_webhook_test.go +++ b/test/e2e/po_admission_webhook_test.go @@ -8,6 +8,7 @@ import ( "gotest.tools/v3/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" ) func TestPrometheusRuleWebhook(t *testing.T) { @@ -42,7 +43,6 @@ func invalidPrometheusRuleIsRejected(t *testing.T) { } func newSinglePrometheusRule(t *testing.T, name, expr string) *monv1.PrometheusRule { - durationFifteenMinutes := monv1.Duration("15m") rule := &monv1.PrometheusRule{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -54,7 +54,7 @@ func newSinglePrometheusRule(t *testing.T, name, expr string) *monv1.PrometheusR Rules: []monv1.Rule{{ Alert: "alert name", Expr: intstr.FromString(expr), - For: &durationFifteenMinutes, + For: ptr.To(monv1.Duration("15m")), }}, }}, }, diff --git a/test/e2e/thanos_querier_controller_test.go b/test/e2e/thanos_querier_controller_test.go index dae291b9..4b25bce8 100644 --- a/test/e2e/thanos_querier_controller_test.go +++ b/test/e2e/thanos_querier_controller_test.go @@ -75,12 +75,7 @@ func singleStackWithSidecar(t *testing.T) { // Assert prometheus instance can be queried stopChan := make(chan struct{}) defer close(stopChan) -<<<<<<< Updated upstream - //nolint - if err := wait.Poll(5*time.Second, 2*time.Minute, func() (bool, error) { -======= if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { ->>>>>>> Stashed changes err = f.StartServicePortForward(name, e2eTestNamespace, "9090", stopChan) return err == nil, nil }); wait.Interrupted(err) { @@ -91,12 +86,7 @@ func singleStackWithSidecar(t *testing.T) { expectedResults := map[string]int{ "prometheus_build_info": 2, // must return from both prometheus pods } -<<<<<<< Updated upstream - //nolint - if err := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { -======= if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { ->>>>>>> Stashed changes correct := 0 for query, value := range expectedResults { result, err := promClient.Query(query) @@ -147,14 +137,8 @@ func newThanosQuerier(t *testing.T, name string, selector map[string]string) *ms } func waitForThanosQuerierDeletion(tq *msov1.ThanosQuerier) error { -<<<<<<< Updated upstream - //nolint - return wait.Poll(5*time.Second, wait.ForeverTestTimeout, func() (bool, error) { - err := f.K8sClient.Get(context.Background(), -======= return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { err = f.K8sClient.Get(context.Background(), ->>>>>>> Stashed changes types.NamespacedName{Name: tq.Name, Namespace: tq.Namespace}, tq) return errors.IsNotFound(err), nil @@ -162,12 +146,7 @@ func waitForThanosQuerierDeletion(tq *msov1.ThanosQuerier) error { } func waitForDeploymentDeletion(name string) error { -<<<<<<< Updated upstream - //nolint - return wait.Poll(5*time.Second, wait.ForeverTestTimeout, func() (bool, error) { -======= return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { ->>>>>>> Stashed changes var dep appsv1.Deployment err = f.K8sClient.Get(context.Background(), types.NamespacedName{Name: name, Namespace: e2eTestNamespace}, @@ -177,12 +156,7 @@ func waitForDeploymentDeletion(name string) error { } func waitForServiceDeletion(name string) error { -<<<<<<< Updated upstream - //nolint - return wait.Poll(5*time.Second, wait.ForeverTestTimeout, func() (bool, error) { -======= return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { ->>>>>>> Stashed changes var svc corev1.Service err = f.K8sClient.Get(context.Background(), types.NamespacedName{Name: name, Namespace: e2eTestNamespace}, From feb83bd146520318bbfd3210da9117e6d8a9afb6 Mon Sep 17 00:00:00 2001 From: Mario Fernandez Date: Wed, 9 Aug 2023 12:20:49 +0200 Subject: [PATCH 3/7] fix: add nolint Signed-off-by: Mario Fernandez --- test/e2e/framework/assertions.go | 6 ++++-- test/e2e/monitoring_stack_controller_test.go | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/e2e/framework/assertions.go b/test/e2e/framework/assertions.go index 8b4fa6fb..2decb89a 100644 --- a/test/e2e/framework/assertions.go +++ b/test/e2e/framework/assertions.go @@ -57,7 +57,8 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c } return func(t *testing.T) { - if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (done bool, err error) { + //nolint + if err := wait.Poll(option.PollInterval, option.WaitTimeout, func() (done bool, err error) { key := types.NamespacedName{ Name: name, Namespace: namespace, @@ -110,7 +111,8 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option } return func(t *testing.T) { key := types.NamespacedName{Name: name, Namespace: namespace} - if err := wait.PollUntilContextTimeout(context.Background(), 5+time.Second, option.WaitTimeout, true, func(ctx context.Context) (done bool, err error) { + //nolint + if err := wait.Poll(5+time.Second, option.WaitTimeout, func() (done bool, err error) { pod := &appsv1.StatefulSet{} err = f.K8sClient.Get(context.Background(), key, pod) return err == nil && pod.Status.ReadyReplicas == *pod.Spec.Replicas, nil diff --git a/test/e2e/monitoring_stack_controller_test.go b/test/e2e/monitoring_stack_controller_test.go index 7709ed84..99d9b77f 100644 --- a/test/e2e/monitoring_stack_controller_test.go +++ b/test/e2e/monitoring_stack_controller_test.go @@ -854,7 +854,8 @@ func namespaceSelectorTest(t *testing.T) { stopChan := make(chan struct{}) defer close(stopChan) - if pollErr := wait.PollUntilContextTimeout(context.Background(), 15*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + //nolint + if pollErr := wait.Poll(15*time.Second, 5*time.Minute, func() (bool, error) { err := f.StartServicePortForward(ms.Name+"-prometheus", e2eTestNamespace, "9090", stopChan) return err == nil, nil }); pollErr != nil { From 831411a91838add2ab8aff8e6f098679d59da4b5 Mon Sep 17 00:00:00 2001 From: Mario Fernandez Date: Thu, 10 Aug 2023 10:07:56 +0200 Subject: [PATCH 4/7] fix: typo Signed-off-by: Mario Fernandez --- test/e2e/framework/assertions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/framework/assertions.go b/test/e2e/framework/assertions.go index 2decb89a..99c230b5 100644 --- a/test/e2e/framework/assertions.go +++ b/test/e2e/framework/assertions.go @@ -112,7 +112,7 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option return func(t *testing.T) { key := types.NamespacedName{Name: name, Namespace: namespace} //nolint - if err := wait.Poll(5+time.Second, option.WaitTimeout, func() (done bool, err error) { + if err := wait.Poll(5*time.Second, option.WaitTimeout, func() (done bool, err error) { pod := &appsv1.StatefulSet{} err = f.K8sClient.Get(context.Background(), key, pod) return err == nil && pod.Status.ReadyReplicas == *pod.Spec.Replicas, nil From 96a7497dd39fac736294e36f12da5bc9f2ede975 Mon Sep 17 00:00:00 2001 From: Mario Fernandez Date: Thu, 10 Aug 2023 10:36:56 +0200 Subject: [PATCH 5/7] fix: change named funcs to maintain coherence Signed-off-by: Mario Fernandez --- test/e2e/framework/assertions.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/framework/assertions.go b/test/e2e/framework/assertions.go index 99c230b5..03628a02 100644 --- a/test/e2e/framework/assertions.go +++ b/test/e2e/framework/assertions.go @@ -58,7 +58,7 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c return func(t *testing.T) { //nolint - if err := wait.Poll(option.PollInterval, option.WaitTimeout, func() (done bool, err error) { + if err := wait.Poll(option.PollInterval, option.WaitTimeout, func() (bool, error) { key := types.NamespacedName{ Name: name, Namespace: namespace, @@ -85,7 +85,7 @@ func (f *Framework) AssertResourceEventuallyExists(name, namespace string, resou } return func(t *testing.T) { - if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (done bool, err error) { + if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (bool, error) { key := types.NamespacedName{ Name: name, Namespace: namespace, @@ -112,9 +112,9 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option return func(t *testing.T) { key := types.NamespacedName{Name: name, Namespace: namespace} //nolint - if err := wait.Poll(5*time.Second, option.WaitTimeout, func() (done bool, err error) { + if err := wait.Poll(5*time.Second, option.WaitTimeout, func() (bool, error) { pod := &appsv1.StatefulSet{} - err = f.K8sClient.Get(context.Background(), key, pod) + err := f.K8sClient.Get(context.Background(), key, pod) return err == nil && pod.Status.ReadyReplicas == *pod.Spec.Replicas, nil }); err != nil { t.Fatal(err) From 235b51820239c0c665cf6d04cef77a9533eea87c Mon Sep 17 00:00:00 2001 From: Mario Fernandez Date: Thu, 10 Aug 2023 11:42:19 +0200 Subject: [PATCH 6/7] fix: add nolint Signed-off-by: Mario Fernandez --- test/e2e/framework/assertions.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/framework/assertions.go b/test/e2e/framework/assertions.go index 03628a02..d80dbf69 100644 --- a/test/e2e/framework/assertions.go +++ b/test/e2e/framework/assertions.go @@ -123,7 +123,8 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option } func (f *Framework) GetResourceWithRetry(t *testing.T, name, namespace string, obj client.Object) { - err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, DefaultTestTimeout, true, func(ctx context.Context) (bool, error) { + //nolint + err := wait.Poll(5*time.Second, DefaultTestTimeout, func() (bool, error) { key := types.NamespacedName{Name: name, Namespace: namespace} if err := f.K8sClient.Get(context.Background(), key, obj); errors.IsNotFound(err) { From ffdbd542f3911cb27c0b944fd31c18d60ff07aea Mon Sep 17 00:00:00 2001 From: Mario Fernandez Herrero Date: Thu, 10 Aug 2023 15:47:01 +0200 Subject: [PATCH 7/7] fix: update test/e2e/framework/assertions.go Co-authored-by: Daniel Mellado <1313475+danielmellado@users.noreply.github.com> --- test/e2e/framework/assertions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/framework/assertions.go b/test/e2e/framework/assertions.go index d80dbf69..f507c1a4 100644 --- a/test/e2e/framework/assertions.go +++ b/test/e2e/framework/assertions.go @@ -23,7 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" ) -// default ForeverTestTimeout is 30, some test fails because they take more than 30s +// default ForeverTestTimeout is 30, some test fail because they take more than 30s // change to custom in order to let the test finish withouth errors const DefaultTestTimeout = 40 * time.Second