From eee16f8c6ce3d6d01dabb0a8db134e95d3879f20 Mon Sep 17 00:00:00 2001 From: Connor Rogers <23215294+coro@users.noreply.github.com> Date: Wed, 6 Jul 2022 16:43:51 +0100 Subject: [PATCH 1/2] 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. --- system_tests/system_test.go | 9 +++---- system_tests/system_tests_suite_test.go | 17 -------------- system_tests/utils.go | 31 +++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/system_tests/system_test.go b/system_tests/system_test.go index 35642e517..c1cf38590 100644 --- a/system_tests/system_test.go +++ b/system_tests/system_test.go @@ -16,7 +16,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "os" "strconv" "strings" "time" @@ -318,13 +317,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) diff --git a/system_tests/system_tests_suite_test.go b/system_tests/system_tests_suite_test.go index 8c3663e3c..058f7f7e3 100644 --- a/system_tests/system_tests_suite_test.go +++ b/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{}) diff --git a/system_tests/utils.go b/system_tests/utils.go index 909b9b1ed..90975664f 100644 --- a/system_tests/utils.go +++ b/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 @@ -1069,3 +1068,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 +} From 0208176828fd530ae7f89c0d6b27521da840258c Mon Sep 17 00:00:00 2001 From: Connor Rogers <23215294+coro@users.noreply.github.com> Date: Wed, 6 Jul 2022 18:12:48 +0100 Subject: [PATCH 2/2] Parse RMQ version for clustering test --- system_tests/system_test.go | 7 +++++-- system_tests/utils.go | 13 ++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/system_tests/system_test.go b/system_tests/system_test.go index c1cf38590..06e37ac06 100644 --- a/system_tests/system_test.go +++ b/system_tests/system_test.go @@ -15,6 +15,7 @@ import ( "crypto/x509" "encoding/json" "fmt" + "golang.org/x/mod/semver" "io/ioutil" "strconv" "strings" @@ -394,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()) diff --git a/system_tests/utils.go b/system_tests/utils.go index 90975664f..a03c77fc9 100644 --- a/system_tests/utils.go +++ b/system_tests/utils.go @@ -644,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