MON-3934: Clean up and parallelize some e2e tests for faster runs

pkg/client/client.go

@@ -1488,25 +1488,6 @@ func (c *Client) CreateOrUpdateConfigMap(ctx context.Context, cm *v1.ConfigMap)
 	return err
 }
 
-func (c *Client) DeleteIfExists(ctx context.Context, nsName string) error {
-	nClient := c.kclient.CoreV1().Namespaces()
-	_, err := nClient.Get(ctx, nsName, metav1.GetOptions{})
-	if apierrors.IsNotFound(err) {
-		// Namespace already deleted
-		return nil
-	}
-
-	if err != nil {
-		return fmt.Errorf("retrieving Namespace object failed: %w", err)
-	}
-
-	err = nClient.Delete(ctx, nsName, metav1.DeleteOptions{})
-	if err != nil {
-		return fmt.Errorf("deleting ConfigMap object failed: %w", err)
-	}
-	return nil
-}
-
 func (c *Client) CreateIfNotExistConfigMap(ctx context.Context, cm *v1.ConfigMap) (*v1.ConfigMap, error) {
 	cClient := c.kclient.CoreV1().ConfigMaps(cm.GetNamespace())
 	res, err := cClient.Get(ctx, cm.GetName(), metav1.GetOptions{})

test/e2e/alert_relabel_config_test.go

@@ -40,6 +40,8 @@ const (
 )
 
 func TestAlertRelabelConfig(t *testing.T) {
+	// The test shouldn't be disruptive, safe to run in parallel with others.
+	t.Parallel()
 	initialRelabelConfig := prometheusRelabelConfig(t)
 
 	// By default, we drop prometheus_replica label + add openshift_io_alert_source = 2

test/e2e/alerting_rule_test.go

@@ -39,6 +39,8 @@ const (
 )
 
 func TestAlertingRule(t *testing.T) {
+	// The test shouldn't be disruptive, safe to run in parallel with others.
+	t.Parallel()
 	ctx := context.Background()
 	alertingRules := f.OpenShiftMonitoringClient.MonitoringV1().AlertingRules(f.Ns)
 
@@ -167,6 +169,7 @@ func prometheusRuleCount(t *testing.T) int {
 }
 
 func assertPrometheusRuleCount(t *testing.T, count int) {
+	t.Helper()
 	currentCount := prometheusRuleCount(t)
 	if currentCount != count {
 		t.Fatalf("Different generated PrometheusRule count (%d != %d)", currentCount, count)

test/e2e/alertmanager_helpers.go

@@ -183,7 +183,7 @@ func (wr *webhookReceiver) getAlertsByID(id string) ([]alert, error) {
 func (wr *webhookReceiver) tearDown(t *testing.T, f *framework.Framework) {
 	t.Helper()
 	err := framework.Poll(time.Second, 5*time.Minute, func() error {
-		return f.OperatorClient.DeleteIfExists(ctx, wr.namespace)
+		return f.DeleteNamespace(t, wr.namespace)
 	})
 
 	if err != nil {

test/e2e/alertmanager_test.go

@@ -431,7 +431,10 @@ func TestAlertmanagerDataReplication(t *testing.T) {
 			),
 		},
 	} {
-		t.Run(test.name, test.assertion)
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			test.assertion(t)
+		})
 	}
 
 	// Ensure that the silence has been preserved.
@@ -465,6 +468,8 @@ func TestAlertmanagerDataReplication(t *testing.T) {
 
 // The Alertmanager API should be protected by authentication/authorization.
 func TestAlertmanagerAPI(t *testing.T) {
+	// The test shouldn't be disruptive, safe to run in parallel with others.
+	t.Parallel()
 	err := framework.Poll(5*time.Second, 5*time.Minute, func() error {
 		body, err := f.AlertmanagerClient.GetAlertmanagerAlerts(
 			"filter", `alertname="Watchdog"`,
@@ -747,51 +752,6 @@ func TestAlertmanagerDisabling(t *testing.T) {
 	})
 }
 
-func TestAlertManagerHasAdditionalAlertRelabelConfigs(t *testing.T) {
-	const (
-		expectPlatformLabel      = "openshift_io_alert_source"
-		expectPlatformLabelValue = "platform"
-	)
-
-	type Alerts []struct {
-		Labels map[string]string `json:"labels"`
-	}
-
-	var alerts Alerts
-
-	err := framework.Poll(5*time.Second, time.Minute, func() error {
-		resp, err := f.AlertmanagerClient.Do("GET", "/api/v2/alerts", nil)
-		if err != nil {
-			return err
-		}
-		defer resp.Body.Close()
-
-		if resp.StatusCode != http.StatusOK {
-			return fmt.Errorf("expecting 200 status code, got %d (%q)", resp.StatusCode, resp.Body)
-		}
-
-		if err := json.NewDecoder(resp.Body).Decode(&alerts); err != nil {
-			return fmt.Errorf("error decoding alert response")
-		}
-
-		return nil
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	for _, alert := range alerts {
-		v, found := alert.Labels[expectPlatformLabel]
-		if !found {
-			t.Fatal("expected correct label to be present")
-		}
-
-		if v != expectPlatformLabelValue {
-			t.Fatalf("expected correct value for %s but got %s", expectPlatformLabel, v)
-		}
-	}
-}
-
 // TestAlertmanagerConfigPipeline ensures that the AlertManagerConfig CR's
 // created in a user namespace can be reconciled and have alerts sent to the
 // correct Alertmanager (depending on whether user-defined Alertmanager is
@@ -907,7 +867,7 @@ func testAlertmanagerConfigPipeline(t *testing.T, wr *webhookReceiver, am *monit
 		t.Fatal(err)
 	}
 
-	if err := createUWMTestNsIfNotExist(t, f); err != nil {
+	if err := createUWMTestNsIfNotExist(t, f, userWorkloadTestNs); err != nil {
 		t.Fatal(err)
 	}
 

test/e2e/alertmanager_user_workload_test.go

@@ -38,17 +38,7 @@ func TestUserWorkloadAlertmanager(t *testing.T) {
 	f.AssertStatefulSetExistsAndRollout("alertmanager-user-workload", f.UserWorkloadMonitoringNs)(t)
 	f.AssertServiceExists("alertmanager-user-workload", f.UserWorkloadMonitoringNs)(t)
 
-	for _, scenario := range []struct {
-		name string
-		f    func(*testing.T)
-	}{
-		{
-			name: "assert UWM alert access",
-			f:    assertUWMAlertsAccess,
-		},
-	} {
-		t.Run(scenario.name, scenario.f)
-	}
+	t.Run("assert UWM alert access", assertUWMAlertsAccess)
 }
 
 // assertUWMAlertsAccess ensures that a user can't access all alerts from the UWM alertmanager via the api.
@@ -79,17 +69,16 @@ func assertUWMAlertsAccess(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// The uwm alerts port (9095) is only exposed in-cluster, so we need to use
-	// port forwarding to access kube-rbac-proxy.
-	host, cleanUp, err := f.ForwardPort(t, f.UserWorkloadMonitoringNs, "alertmanager-user-workload", 9095)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer cleanUp()
-
-	client := framework.NewPrometheusClient(host, token)
-
 	err = framework.Poll(5*time.Second, time.Minute, func() error {
+		// The uwm alerts port (9095) is only exposed in-cluster, so we need to use
+		// port forwarding to access kube-rbac-proxy.
+		host, cleanUp, err := f.ForwardPort(t, f.UserWorkloadMonitoringNs, "alertmanager-user-workload", 9095)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer cleanUp()
+
+		client := framework.NewPrometheusClient(host, token)
 		resp, err := client.Do("GET", "/api/v2/alerts", nil)
 		if err != nil {
 			return err

test/e2e/config_test.go

@@ -292,7 +292,10 @@ func TestClusterMonitorPrometheusK8Config(t *testing.T) {
 			assertion: f.AssertPrometheusRuleExists(thanosRule, f.Ns),
 		},
 	} {
-		t.Run(tc.name, tc.assertion)
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			tc.assertion(t)
+		})
 	}
 }
 
@@ -344,7 +347,10 @@ func TestClusterMonitorAlertManagerConfig(t *testing.T) {
 			),
 		},
 	} {
-		t.Run(tc.name, tc.assertion)
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			tc.assertion(t)
+		})
 	}
 }
 
@@ -664,7 +670,10 @@ func TestUserWorkloadMonitorPrometheusK8Config(t *testing.T) {
 			assertion: assertQueryLogValueEquals(f.UserWorkloadMonitoringNs, crName, "/tmp/test.log"),
 		},
 	} {
-		t.Run(tc.name, tc.assertion)
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			tc.assertion(t)
+		})
 	}
 }
 
@@ -721,7 +730,10 @@ func TestUserWorkloadMonitorThanosRulerConfig(t *testing.T) {
 			),
 		},
 	} {
-		t.Run(tc.name, tc.assertion)
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			tc.assertion(t)
+		})
 	}
 }
 
@@ -767,7 +779,10 @@ monitoringPlugin:
 			),
 		},
 	} {
-		t.Run(tc.name, tc.assertion)
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			tc.assertion(t)
+		})
 	}
 }
 

test/e2e/framework/helpers.go

@@ -2,6 +2,7 @@ package framework
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -15,6 +16,7 @@ import (
 	"gopkg.in/yaml.v2"
 	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 )
@@ -174,12 +176,12 @@ func (f *Framework) MustGetStatefulSet(t *testing.T, name, namespace string) *ap
 	return statefulSet
 }
 
-// MustGetPods return all pods from `namespace` within 5 minutes or fail
-func (f *Framework) MustGetPods(t *testing.T, namespace string) *v1.PodList {
+// MustListPods returns all pods matching labelSelector from `namespace` within 5 minutes or fail
+func (f *Framework) MustListPods(t *testing.T, namespace string, labelSelector string) *v1.PodList {
 	t.Helper()
 	var pods *v1.PodList
 	err := wait.Poll(time.Second, 5*time.Minute, func() (bool, error) {
-		pl, err := f.KubeClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{})
+		pl, err := f.KubeClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: labelSelector})
 		if err != nil {
 			return false, nil
 		}
@@ -212,3 +214,17 @@ func ensureCreatedByTestLabel(obj metav1.Object) {
 		labels[E2eTestLabelName] = E2eTestLabelValue
 	}
 }
+
+func (f *Framework) DeleteNamespace(t *testing.T, nsName string) error {
+	t.Helper()
+	nClient := f.KubeClient.CoreV1().Namespaces()
+	_, err := nClient.Get(ctx, nsName, metav1.GetOptions{})
+	if err != nil && !apierrors.IsNotFound(err) {
+		return fmt.Errorf("retrieving Namespace object failed: %w", err)
+	}
+
+	err = nClient.Delete(ctx, nsName, metav1.DeleteOptions{})
+	if err != nil {
+		return fmt.Errorf("deleting Namespace object failed: %w", err)
+	}
+}