From d3a10bb37c19b65d2a39721efde14531e4cfad44 Mon Sep 17 00:00:00 2001 From: Radovan Zvoncek Date: Tue, 9 Jul 2024 14:32:56 +0300 Subject: [PATCH] Add Reaper storage config validation to k8ssandra cluster webhook --- .../v1alpha1/k8ssandracluster_webhook.go | 49 ++++++++++++++--- .../v1alpha1/k8ssandracluster_webhook_test.go | 55 +++++++++++++++++++ pkg/reaper/deployment.go | 25 +-------- 3 files changed, 98 insertions(+), 31 deletions(-) diff --git a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go index 4b30f3bf0..fff3c01a8 100644 --- a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go +++ b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go @@ -19,6 +19,7 @@ package v1alpha1 import ( "fmt" "github.com/Masterminds/semver/v3" + reaperapi "github.com/k8ssandra/k8ssandra-operator/apis/reaper/v1alpha1" "strings" "k8s.io/apimachinery/pkg/util/validation" @@ -33,13 +34,18 @@ import ( ) var ( - clientCache *clientcache.ClientCache - ErrNumTokens = fmt.Errorf("num_tokens value can't be changed") - ErrReaperKeyspace = fmt.Errorf("reaper keyspace can not be changed") - ErrNoStorageConfig = fmt.Errorf("storageConfig must be defined at cluster level or dc level") - ErrNoResourcesSet = fmt.Errorf("softPodAntiAffinity requires Resources to be set") - ErrClusterName = fmt.Errorf("cluster name can not be changed") - ErrNoStoragePrefix = fmt.Errorf("medusa storage prefix must be set when a medusaConfigurationRef is used") + clientCache *clientcache.ClientCache + ErrNumTokens = fmt.Errorf("num_tokens value can't be changed") + ErrReaperKeyspace = fmt.Errorf("reaper keyspace can not be changed") + ErrNoStorageConfig = fmt.Errorf("storageConfig must be defined at cluster level or dc level") + ErrNoResourcesSet = fmt.Errorf("softPodAntiAffinity requires Resources to be set") + ErrClusterName = fmt.Errorf("cluster name can not be changed") + ErrNoStoragePrefix = fmt.Errorf("medusa storage prefix must be set when a medusaConfigurationRef is used") + ErrNoReaperStorageConfig = fmt.Errorf("reaper StorageConfig not set") + ErrNoReaperStorageClassName = fmt.Errorf("reaper StorageConfig.StorageClassName not set") + ErrNoReaperAccessMode = fmt.Errorf("reaper StorageConfig.AccessModes not set") + ErrNoReaperResourceRequests = fmt.Errorf("reaper StorageConfig.Resources.Requests not set") + ErrNoReaperStorageRequest = fmt.Errorf("reaper StorageConfig.Resources.Requests.Storage not set") ) // log is for logging in this package. @@ -104,6 +110,10 @@ func (r *K8ssandraCluster) validateK8ssandraCluster() error { return err } + if err := r.ValidateReaper(); err != nil { + return err + } + if err := r.validateStatefulsetNameSize(); err != nil { return err } @@ -271,3 +281,28 @@ func (r *K8ssandraCluster) ValidateMedusa() error { return nil } + +func (r *K8ssandraCluster) ValidateReaper() error { + if r.Spec.Reaper == nil { + return nil + } + if r.Spec.Reaper.StorageType != reaperapi.StorageTypeMemory { + return nil + } + if r.Spec.Reaper.StorageConfig == nil { + return ErrNoReaperStorageConfig + } + if r.Spec.Reaper.StorageConfig.StorageClassName == nil { + return ErrNoReaperStorageClassName + } + if r.Spec.Reaper.StorageConfig.AccessModes == nil { + return ErrNoReaperAccessMode + } + if r.Spec.Reaper.StorageConfig.Resources.Requests == nil { + return ErrNoReaperResourceRequests + } + if r.Spec.Reaper.StorageConfig.Resources.Requests.Storage().IsZero() { + return ErrNoReaperStorageRequest + } + return nil +} diff --git a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go index d7e62821f..1c2b83ce7 100644 --- a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go +++ b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go @@ -20,6 +20,7 @@ import ( "context" "crypto/tls" "fmt" + "k8s.io/apimachinery/pkg/api/resource" "net" "path/filepath" "testing" @@ -53,6 +54,23 @@ var testEnv *envtest.Environment var ctx context.Context var cancel context.CancelFunc +var minimalInMemoryReaperStorageConfig = &corev1.PersistentVolumeClaimSpec{ + StorageClassName: func() *string { s := "test"; return &s }(), + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, +} + +var minimalInMemoryReaperConfig = &reaperapi.ReaperClusterTemplate{ + ReaperTemplate: reaperapi.ReaperTemplate{ + StorageType: reaperapi.StorageTypeMemory, + StorageConfig: minimalInMemoryReaperStorageConfig, + }, +} + func TestWebhook(t *testing.T) { required := require.New(t) ctx, cancel = context.WithCancel(context.TODO()) @@ -159,6 +177,7 @@ func TestWebhook(t *testing.T) { t.Run("MedusaPrefixMissing", testMedusaPrefixMissing) t.Run("InvalidDcName", testInvalidDcName) t.Run("MedusaConfigNonLocalNamespace", testMedusaNonLocalNamespace) + t.Run("ReaperStorage", testReaperStorage) } func testContextValidation(t *testing.T) { @@ -495,6 +514,42 @@ func testMedusaNonLocalNamespace(t *testing.T) { required.Contains(err.Error(), "Medusa config must be namespace local") } +func testReaperStorage(t *testing.T) { + required := require.New(t) + + reaperWithNoStorageConfig := createMinimalClusterObj("reaper-no-storage-config", "ns") + reaperWithNoStorageConfig.Spec.Reaper = &reaperapi.ReaperClusterTemplate{ + ReaperTemplate: reaperapi.ReaperTemplate{ + StorageType: reaperapi.StorageTypeMemory, + }, + } + err := reaperWithNoStorageConfig.validateK8ssandraCluster() + required.Error(err) + + reaperWithDefaultConfig := createClusterObjWithCassandraConfig("reaper-default-storage-config", "ns") + reaperWithDefaultConfig.Spec.Reaper = minimalInMemoryReaperConfig.DeepCopy() + err = reaperWithDefaultConfig.validateK8ssandraCluster() + required.NoError(err) + + reaperWithoutStorageClass := createClusterObjWithCassandraConfig("reaper-no-storage-class", "ns") + reaperWithoutStorageClass.Spec.Reaper = minimalInMemoryReaperConfig.DeepCopy() + reaperWithoutStorageClass.Spec.Reaper.StorageConfig.StorageClassName = nil + err = reaperWithoutStorageClass.validateK8ssandraCluster() + required.Error(err) + + reaperWithoutAccessMode := createClusterObjWithCassandraConfig("reaper-no-access-mode", "ns") + reaperWithoutAccessMode.Spec.Reaper = minimalInMemoryReaperConfig.DeepCopy() + reaperWithoutAccessMode.Spec.Reaper.StorageConfig.AccessModes = nil + err = reaperWithoutAccessMode.validateK8ssandraCluster() + required.Error(err) + + reaperWithoutStorageSize := createClusterObjWithCassandraConfig("reaper-no-storage-size", "ns") + reaperWithoutStorageSize.Spec.Reaper = minimalInMemoryReaperConfig.DeepCopy() + reaperWithoutStorageSize.Spec.Reaper.StorageConfig.Resources.Requests = corev1.ResourceList{} + err = reaperWithoutStorageSize.validateK8ssandraCluster() + required.Error(err) +} + // TestValidateUpdateNumTokens is a unit test for numTokens updates. func TestValidateUpdateNumTokens(t *testing.T) { type config struct { diff --git a/pkg/reaper/deployment.go b/pkg/reaper/deployment.go index 843c8305b..012934b74 100644 --- a/pkg/reaper/deployment.go +++ b/pkg/reaper/deployment.go @@ -325,30 +325,7 @@ func computeVolumeClaims(reaper *api.Reaper) []corev1.PersistentVolumeClaim { vcs := make([]corev1.PersistentVolumeClaim, 0) var volumeClaimsPec *corev1.PersistentVolumeClaimSpec - storageClass := api.DefaultStorageClass - accessModes := []corev1.PersistentVolumeAccessMode{api.DefaultAccessMode} - if reaper.Spec.StorageConfig != nil { - volumeClaimsPec = reaper.Spec.StorageConfig.DeepCopy() - if volumeClaimsPec.Resources.Requests.Storage() == nil { - volumeClaimsPec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse(api.DefaultStorageSize) - } - if volumeClaimsPec.StorageClassName == nil { - volumeClaimsPec.StorageClassName = &storageClass - } - if volumeClaimsPec.AccessModes == nil { - volumeClaimsPec.AccessModes = accessModes - } - } else { - volumeClaimsPec = &corev1.PersistentVolumeClaimSpec{ - StorageClassName: &storageClass, - AccessModes: accessModes, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse(api.DefaultStorageSize), - }, - }, - } - } + volumeClaimsPec = reaper.Spec.StorageConfig.DeepCopy() pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{