Merge pull request #724 from matysek/ols-1333

OLS-1333: Fix prometheus metrics test flakyness

Author: openshift-merge-bot[bot]

test/e2e/client.go

@@ -30,6 +30,7 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	olsv1alpha1 "github.com/openshift/lightspeed-operator/api/v1alpha1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 )
 
 const (
@@ -427,7 +428,13 @@ func (c *Client) CreateClusterRoleBinding(namespace, serviceAccount, clusterRole
 
 	err := c.Create(clusterRoleBinding)
 	if err != nil {
-		return nil, err
+		// ROSA clusters have more restricted permissions.
+		// Deleting ClusterRoleBindings is not allowed.
+		if k8serrors.IsAlreadyExists(err) {
+			logf.Log.Error(err, "ClusterRoleBindings for test already exists (happens for ROSA clusters)")
+		} else {
+			return nil, err
+		}
 	}
 
 	return func() {

test/e2e/metrics_test.go

@@ -13,7 +13,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 )
 
-var _ = Describe("Prometheus Metrics", Ordered, func() {
+// Try to retry test case multipletimes when failing.
+var _ = Describe("Prometheus Metrics", FlakeAttempts(5), Ordered, func() {
 	const metricsViewerSAName = "metrics-viewer-sa"
 	const clusterMonitoringViewClusterRole = "cluster-monitoring-view"
 	var cr *olsv1alpha1.OLSConfig
@@ -36,32 +37,37 @@ var _ = Describe("Prometheus Metrics", Ordered, func() {
 
 		By("create a service account")
 		cleanUp, err := client.CreateServiceAccount(OLSNameSpace, metricsViewerSAName)
-		Expect(err).NotTo(HaveOccurred())
 		cleanUpFuncs = append(cleanUpFuncs, cleanUp)
+		Expect(err).NotTo(HaveOccurred())
 
 		By("create a role binding for application metrics access")
 		cleanUp, err = client.CreateClusterRoleBinding(OLSNameSpace, metricsViewerSAName, clusterMonitoringViewClusterRole)
-		Expect(err).NotTo(HaveOccurred())
 		cleanUpFuncs = append(cleanUpFuncs, cleanUp)
+		Expect(err).NotTo(HaveOccurred())
 
 		By("fetch the service account token")
 		saToken, err = client.GetServiceAccountToken(OLSNameSpace, metricsViewerSAName)
 		Expect(err).NotTo(HaveOccurred())
 
-		// Get a Kubernetes rest config
+		By("fetch a Kubernetes rest config")
 		cfg, err := config.GetConfig()
 		Expect(err).NotTo(HaveOccurred())
 
 		openshiftRouteClient, err := routev1.NewForConfig(cfg)
 		Expect(err).NotTo(HaveOccurred())
 
-		prometheusClient, err = NewPrometheusClientFromRoute(
-			context.Background(),
-			openshiftRouteClient,
-			"openshift-monitoring", "thanos-querier",
-			saToken,
-		)
-		Expect(err).NotTo(HaveOccurred())
+		By("fetch a Prometheus route")
+		// Retry multiple times to fetch route
+		// mitigates networking issues
+		Eventually(func() error {
+			prometheusClient, err = NewPrometheusClientFromRoute(
+				context.Background(),
+				openshiftRouteClient,
+				"openshift-monitoring", "thanos-querier",
+				saToken,
+			)
+			return err
+		}, 10*time.Second, 2*time.Second).ShouldNot(HaveOccurred())
 
 		By("wait for application server deployment rollout")
 		deployment := &appsv1.Deployment{

test/e2e/prometheus_client.go

@@ -16,6 +16,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 // PrometheusClient provides access to the Prometheus, Thanos & Alertmanager API.
@@ -30,7 +31,7 @@ type PrometheusClient struct {
 
 // DefaultPollInterval is the default interval for polling Prometheus metrics.
 // Prometheus metrics are typically scraped every 30 seconds. This is enough for 3 scrapes.
-const DefaultPrometheusQueryTimeout = 95 * time.Second
+const DefaultPrometheusQueryTimeout = 120 * time.Second
 
 // NewPrometheusClientFromRoute creates and returns a new PrometheusClient from the given OpenShift route.
 func NewPrometheusClientFromRoute(
@@ -41,6 +42,8 @@ func NewPrometheusClientFromRoute(
 ) (*PrometheusClient, error) {
 	route, err := routeClient.Routes(namespace).Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
+
+		logf.Log.Error(err, "Error fetching Prometheus route")
 		return nil, err
 	}