More reliable system tests (#1086)

* Dynamically check for PVC expansion support in tests

Previously, it was the responsibility of whoever was running the system tests to decide if PVC expansion was supported or not.
In addition, the tests created a storageClass with the gce-pd Provider, in order to ensure there was a storageClass available that supported expansion. This Provider is deprecated, and in modern gke versions supports expansion anyway, removing the need for a bespoke class.

This change now dynamically queries the k8s API for the default storage class, and checks if it supports expansion or not. If it does not, the expansion test is skipped as before (e.g. in kind). If it does, the test proceeds, using this default class.
This also means the tests should now be able to be run in non-GKE environments with storage drivers that support PVC expansion.

* Parse RMQ version for clustering test

Author: coro

system_tests/system_test.go

@@ -15,8 +15,8 @@ import (
 	"crypto/x509"
 	"encoding/json"
 	"fmt"
+	"golang.org/x/mod/semver"
 	"io/ioutil"
-	"os"
 	"strconv"
 	"strings"
 	"time"
@@ -318,13 +318,15 @@ CONSOLE_LOG=new`
 
 		BeforeEach(func() {
 			// volume expansion is not supported in kinD which is use in github action
-			if os.Getenv("SUPPORT_VOLUME_EXPANSION") == "false" {
-				Skip("SUPPORT_VOLUME_EXPANSION is set to false; skipping volume expansion test")
+			if !volumeExpansionSupported(ctx, clientSet) {
+				Skip("default storageClass does not support volume expansion; skipping volume expansion test")
 			}
 
+			oldCapacity, _ := k8sresource.ParseQuantity("10Gi")
+
 			cluster = newRabbitmqCluster(namespace, "resize-rabbit")
 			cluster.Spec.Persistence = rabbitmqv1beta1.RabbitmqClusterPersistenceSpec{
-				StorageClassName: pointer.StringPtr(storageClassName),
+				Storage: &oldCapacity,
 			}
 			Expect(createRabbitmqCluster(ctx, rmqClusterClient, cluster)).To(Succeed())
 			waitForRabbitmqRunning(cluster)
@@ -393,8 +395,10 @@ CONSOLE_LOG=new`
 
 				// test https://github.com/rabbitmq/cluster-operator/issues/662 is fixed
 				By("clustering correctly")
-				if strings.Contains(cluster.Spec.Image, ":3.8.8") {
-					Skip(cluster.Spec.Image + " is known to not cluster consistently (fixed in v3.8.18)")
+				testRabbitmqVersion := "v" + runningRabbitmqVersion(cluster)
+				if semver.Compare(testRabbitmqVersion, "v3.8") >= 0 &&
+					semver.Compare(testRabbitmqVersion, "v3.8.18") < 0 {
+					Skip(testRabbitmqVersion + " is known to not cluster consistently (fixed in v3.8.18)")
 				}
 				rmqc, err := rabbithole.NewClient(fmt.Sprintf("http://%s:%s", hostname, port), username, password)
 				Expect(err).NotTo(HaveOccurred())

system_tests/system_tests_suite_test.go

@@ -11,17 +11,14 @@ package system_tests
 
 import (
 	"context"
-	"k8s.io/utils/pointer"
 	controllerruntime "sigs.k8s.io/controller-runtime"
 	"testing"
 
-	storagev1 "k8s.io/api/storage/v1"
 	"k8s.io/client-go/kubernetes"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	defaultscheme "k8s.io/client-go/kubernetes/scheme"
@@ -56,21 +53,7 @@ var _ = BeforeSuite(func() {
 
 	namespace = MustHaveEnv("NAMESPACE")
 
-	// Create or update the StorageClass used in persistence expansion test spec
-	storageClass := &storagev1.StorageClass{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: storageClassName,
-		},
-		Provisioner:          "kubernetes.io/gce-pd",
-		AllowVolumeExpansion: pointer.BoolPtr(true),
-	}
 	ctx := context.Background()
-	err = rmqClusterClient.Create(ctx, storageClass)
-	if apierrors.IsAlreadyExists(err) {
-		Expect(rmqClusterClient.Update(ctx, storageClass)).To(Succeed())
-	} else {
-		Expect(err).NotTo(HaveOccurred())
-	}
 
 	Eventually(func() int32 {
 		operatorDeployment, err := clientSet.AppsV1().Deployments(namespace).Get(ctx, "rabbitmq-cluster-operator", metav1.GetOptions{})

system_tests/utils.go

@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	storagev1 "k8s.io/api/storage/v1"
 	"log"
 	"net/http"
 	"os"
@@ -434,7 +435,6 @@ func newRabbitmqCluster(namespace, instanceName string) *rabbitmqv1beta1.Rabbitm
 }
 
 func overrideSecurityContextForOpenshift(cluster *rabbitmqv1beta1.RabbitmqCluster) {
-
 	cluster.Spec.Override = rabbitmqv1beta1.RabbitmqClusterOverrideSpec{
 		StatefulSet: &rabbitmqv1beta1.StatefulSet{
 			Spec: &rabbitmqv1beta1.StatefulSetSpec{
@@ -451,7 +451,6 @@ func overrideSecurityContextForOpenshift(cluster *rabbitmqv1beta1.RabbitmqCluste
 			},
 		},
 	}
-
 }
 
 //the updateFn can change properties of the RabbitmqCluster CR
@@ -645,7 +644,18 @@ func hasFeatureEnabled(cluster *rabbitmqv1beta1.RabbitmqCluster, featureFlagName
 	return false
 }
 
-// asserts an event with reason: "TLSError", occurs for the cluster in it's namespace
+func runningRabbitmqVersion(cluster *rabbitmqv1beta1.RabbitmqCluster) string {
+	output, err := kubectlExec(cluster.Namespace,
+		statefulSetPodName(cluster, 0),
+		"rabbitmq",
+		"rabbitmqctl",
+		"version",
+	)
+	Expect(err).NotTo(HaveOccurred())
+	return strings.TrimSpace(string(output))
+}
+
+// asserts an event with reason: "TLSError", occurs for the cluster in its namespace
 func assertTLSError(cluster *rabbitmqv1beta1.RabbitmqCluster) {
 	var err error
 
@@ -1069,3 +1079,31 @@ func pod(ctx context.Context, clientSet *kubernetes.Clientset, r *rabbitmqv1beta
 	}, 10).Should(Succeed())
 	return pod
 }
+
+func defaultStorageClass(ctx context.Context, clientSet *kubernetes.Clientset) *storagev1.StorageClass {
+	var storageClasses *storagev1.StorageClassList
+	defaultClassAnnotation := "storageclass.kubernetes.io/is-default-class"
+	var err error
+	storageClasses, err = clientSet.StorageV1().StorageClasses().List(ctx, metav1.ListOptions{})
+	Expect(err).NotTo(HaveOccurred())
+	Expect(storageClasses.Items).NotTo(BeEmpty(), "expected at least 1 storageClass, but found 0")
+	for _, storageClass := range storageClasses.Items {
+		defaultClassAnnotationValue, ok := storageClass.ObjectMeta.Annotations[defaultClassAnnotation]
+		if !ok {
+			// StorageClass is not the default
+			continue
+		}
+		isDefaultClass, err := strconv.ParseBool(defaultClassAnnotationValue)
+		if err == nil && isDefaultClass {
+			return &storageClass
+		}
+	}
+	return nil
+}
+
+func volumeExpansionSupported(ctx context.Context, clientSet *kubernetes.Clientset) bool {
+	clusterDefaultStorageClass := defaultStorageClass(ctx, clientSet)
+	Expect(clusterDefaultStorageClass).NotTo(BeNil(), "expected to find a default storageClass, but failed to find one")
+	return clusterDefaultStorageClass.AllowVolumeExpansion != nil &&
+		*clusterDefaultStorageClass.AllowVolumeExpansion == true
+}