Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

fix: Clean up deprecated functions #326

Merged
merged 7 commits into from
Aug 10, 2023
Merged

fix: Clean up deprecated functions #326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ef1694
chore: replace deprecated wait.Poll
marioferh Aug 8, 2023
05c1dbc
fix: minor fixes
marioferh Aug 8, 2023
feb83bd
fix: add nolint
marioferh Aug 9, 2023
831411a
fix: typo
marioferh Aug 10, 2023
96a7497
fix: change named funcs to maintain coherence
marioferh Aug 10, 2023
235b518
fix: add nolint
marioferh Aug 10, 2023
ffdbd54
fix: update test/e2e/framework/assertions.go
marioferh Aug 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 24 additions & 29 deletions test/e2e/framework/assertions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
)

const CustomForeverTestTimeout = 40 * time.Second
// 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

type AssertOption struct {
PollInterval time.Duration
Expand All @@ -44,19 +46,19 @@ 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)
}

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) {
danielmellado marked this conversation as resolved.
Show resolved Hide resolved
key := types.NamespacedName{
Name: name,
Namespace: namespace,
Expand All @@ -66,26 +68,24 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c
}

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,
WaitTimeout: CustomForeverTestTimeout,
WaitTimeout: DefaultTestTimeout,
}
for _, fn := range fns {
fn(&option)
}

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) (bool, error) {
key := types.NamespacedName{
Name: name,
Namespace: namespace,
Expand All @@ -94,8 +94,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))
}
}
Expand All @@ -105,7 +104,7 @@ 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)
Expand All @@ -125,7 +124,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.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) {
Expand All @@ -136,14 +135,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 {
Expand Down Expand Up @@ -199,7 +196,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)
Expand Down Expand Up @@ -233,8 +229,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, DefaultTestTimeout, true, func(ctx context.Context) (bool, error) {
err := f.StartPortForward(pod.Name, pod.Namespace, "8080", stopChan)
return err == nil, nil
}); err != nil {
Expand Down Expand Up @@ -272,10 +267,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, DefaultTestTimeout, 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)
Expand All @@ -285,9 +283,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
}
Expand All @@ -298,16 +295,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, DefaultTestTimeout, 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))
}
}
Expand Down
44 changes: 16 additions & 28 deletions test/e2e/monitoring_stack_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func assertCRDExists(t *testing.T, crds ...string) {
}

func TestMonitoringStackController(t *testing.T) {
stack.AddToScheme(scheme.Scheme)
err := stack.AddToScheme(scheme.Scheme)
assert.NilError(t, err, "adding stack to scheme failed")
marioferh marked this conversation as resolved.
Show resolved Hide resolved
assertCRDExists(t,
"prometheuses.monitoring.rhobs",
"alertmanagers.monitoring.rhobs",
Expand Down Expand Up @@ -148,8 +149,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.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
}
Expand All @@ -160,8 +160,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"))
}
}
Expand Down Expand Up @@ -298,8 +297,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.DefaultTestTimeout, true, func(ctx context.Context) (bool, error) {
reconciled := monv1.Prometheus{}
key := types.NamespacedName{Name: ms.Name, Namespace: ms.Namespace}

Expand Down Expand Up @@ -415,8 +413,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 {
Expand All @@ -428,8 +425,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)
Expand Down Expand Up @@ -534,16 +530,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
Expand Down Expand Up @@ -590,8 +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")
//nolint
err = wait.Poll(5*time.Second, framework.CustomForeverTestTimeout, func() (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
}
Expand All @@ -602,8 +595,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"))
}
}
Expand Down Expand Up @@ -749,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(),
Expand All @@ -764,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")),
},
},
},
Expand Down Expand Up @@ -827,8 +817,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.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)
Expand Down Expand Up @@ -874,8 +863,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 {
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/po_admission_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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")),
}},
}},
},
Expand Down
3 changes: 0 additions & 3 deletions test/e2e/prometheus_operator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
Loading
Loading