From ebfbd1b12c70d7307a15a228c331513eb7de52ec Mon Sep 17 00:00:00 2001 From: mehrdadbn9 Date: Wed, 5 Aug 2026 19:53:36 +0400 Subject: [PATCH] test: cover storage access mode defaulting The PVC access mode used to be copied straight out of the EtcdCluster spec, so leaving storageSpec.accessModes unset produced a volume claim template with accessModes: [""]. The API server rejects that, and every pod stays pending with: PersistentVolumeClaim "etcd-data-example-0" is invalid: spec.accessModes: Unsupported value: "" That was fixed in 3905389 by hardcoding ReadWriteOnce for the empty and ReadWriteOnce cases, but nothing covers it, so the defaulting can be dropped again without a test failing. The e2e cases always set the access mode explicitly, so they never exercise the empty path. Add a table-driven test over createOrPatchStatefulSet covering the access mode branches: unset defaults to ReadWriteOnce, an explicit ReadWriteOnce is kept, ReadWriteMany mounts the named PVC instead of generating a claim template, ReadWriteMany without a PVC name is rejected, and an unsupported mode is rejected. Reverting 3905389 locally turns the first case red with actual: [""], so it guards the original report. Refs #107 Signed-off-by: mehrdadbn9 --- internal/controller/utils_test.go | 113 ++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/internal/controller/utils_test.go b/internal/controller/utils_test.go index 67592316..6c9c1fc8 100644 --- a/internal/controller/utils_test.go +++ b/internal/controller/utils_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -701,6 +702,118 @@ func TestCreateOrPatchStatefulSetWithPodLabels(t *testing.T) { } } +func TestCreateOrPatchStatefulSetStorageAccessModes(t *testing.T) { + ctx := t.Context() + logger := log.FromContext(ctx) + + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + _ = ecv1alpha1.AddToScheme(scheme) + _ = appsv1.AddToScheme(scheme) + + tests := []struct { + name string + etcdClusterName string + storageSpec *ecv1alpha1.StorageSpec + expectedErr string + // expectedAccessModes is what the generated volume claim template must + // carry; empty means no volume claim template is expected at all. + expectedAccessModes []corev1.PersistentVolumeAccessMode + expectedClaimName string + }{ + { + name: "defaults to ReadWriteOnce when accessModes is not provided", + etcdClusterName: "test-etcd-storage-default", + storageSpec: &ecv1alpha1.StorageSpec{ + VolumeSizeRequest: resource.MustParse("100Mi"), + }, + expectedAccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + }, + { + name: "keeps ReadWriteOnce when it is set explicitly", + etcdClusterName: "test-etcd-storage-rwo", + storageSpec: &ecv1alpha1.StorageSpec{ + AccessModes: corev1.ReadWriteOnce, + VolumeSizeRequest: resource.MustParse("100Mi"), + }, + expectedAccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + }, + { + name: "ReadWriteMany mounts the provided PVC instead of a claim template", + etcdClusterName: "test-etcd-storage-rwx", + storageSpec: &ecv1alpha1.StorageSpec{ + AccessModes: corev1.ReadWriteMany, + PVCName: "shared-etcd-data", + VolumeSizeRequest: resource.MustParse("100Mi"), + }, + expectedClaimName: "shared-etcd-data", + }, + { + name: "ReadWriteMany without a PVC name is rejected", + etcdClusterName: "test-etcd-storage-rwx-no-pvc", + storageSpec: &ecv1alpha1.StorageSpec{ + AccessModes: corev1.ReadWriteMany, + VolumeSizeRequest: resource.MustParse("100Mi"), + }, + expectedErr: "PVCName must be set when AccessModes is ReadWriteMany", + }, + { + name: "unsupported access mode is rejected", + etcdClusterName: "test-etcd-storage-rox", + storageSpec: &ecv1alpha1.StorageSpec{ + AccessModes: corev1.ReadOnlyMany, + VolumeSizeRequest: resource.MustParse("100Mi"), + }, + expectedErr: "AccessMode ReadOnlyMany is not supported", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + ec := &ecv1alpha1.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.etcdClusterName, + Namespace: "default", + }, + Spec: ecv1alpha1.EtcdClusterSpec{ + Size: 1, + Version: "3.5.17", + StorageSpec: tt.storageSpec, + }, + } + + err := createOrPatchStatefulSet(ctx, logger, ec, fakeClient, 1, scheme) + if tt.expectedErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedErr) + return + } + require.NoError(t, err) + + sts := &appsv1.StatefulSet{} + err = fakeClient.Get(ctx, client.ObjectKey{Name: tt.etcdClusterName, Namespace: "default"}, sts) + require.NoError(t, err) + + if tt.expectedClaimName != "" { + assert.Empty(t, sts.Spec.VolumeClaimTemplates) + require.Len(t, sts.Spec.Template.Spec.Volumes, 1) + require.NotNil(t, sts.Spec.Template.Spec.Volumes[0].PersistentVolumeClaim) + assert.Equal(t, tt.expectedClaimName, sts.Spec.Template.Spec.Volumes[0].PersistentVolumeClaim.ClaimName) + return + } + + require.Len(t, sts.Spec.VolumeClaimTemplates, 1) + assert.Equal(t, tt.expectedAccessModes, sts.Spec.VolumeClaimTemplates[0].Spec.AccessModes) + // An empty access mode reaches the API server as accessModes: [""], + // which the PVC schema rejects and which leaves every pod pending. + assert.NotContains(t, sts.Spec.VolumeClaimTemplates[0].Spec.AccessModes, + corev1.PersistentVolumeAccessMode("")) + }) + } +} + func TestCreatingArgs(t *testing.T) { tests := []struct { testName string