diff --git a/api/v1alpha1/etcdcluster_types.go b/api/v1alpha1/etcdcluster_types.go index affc7d84..f7d35577 100644 --- a/api/v1alpha1/etcdcluster_types.go +++ b/api/v1alpha1/etcdcluster_types.go @@ -32,7 +32,7 @@ type EtcdClusterSpec struct { // +optional // +kubebuilder:default:=3 // +kubebuilder:validation:Minimum:=0 - Replicas int32 `json:"replicas,omitempty"` + Replicas *int32 `json:"replicas,omitempty"` Storage Storage `json:"storage,omitempty"` } diff --git a/api/v1alpha1/etcdcluster_webhook.go b/api/v1alpha1/etcdcluster_webhook.go index 91f8a9c8..efe4a792 100644 --- a/api/v1alpha1/etcdcluster_webhook.go +++ b/api/v1alpha1/etcdcluster_webhook.go @@ -17,11 +17,8 @@ limitations under the License. package v1alpha1 import ( - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -57,19 +54,13 @@ var _ webhook.Validator = &EtcdCluster{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type func (r *EtcdCluster) ValidateCreate() (admission.Warnings, error) { etcdclusterlog.Info("validate create", "name", r.Name) - var allErrors field.ErrorList - if r.Spec.Replicas < 0 { - allErrors = append(allErrors, field.Invalid( - field.NewPath("spec", "replicas"), - r.Spec.Replicas, - "cluster replicas cannot be less than zero", - )) - } - if len(allErrors) > 0 { - return nil, apierrors.NewInvalid( - schema.GroupKind{Group: GroupVersion.Group, Kind: "EtcdCluster"}, - r.Name, allErrors) - } + //var allErrors field.ErrorList + //// write validation here + //if len(allErrors) > 0 { + // return nil, apierrors.NewInvalid( + // schema.GroupKind{Group: GroupVersion.Group, Kind: "EtcdCluster"}, + // r.Name, allErrors) + //} return nil, nil } @@ -82,22 +73,16 @@ func (r *EtcdCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, er warnings = append(warnings, "cluster resize is not currently supported") } - var allErrors field.ErrorList - if r.Spec.Replicas < 0 { - allErrors = append(allErrors, field.Invalid( - field.NewPath("spec", "replicas"), - r.Spec.Replicas, - "cluster replicas cannot be less than zero", - )) - } - - var err *apierrors.StatusError - if len(allErrors) > 0 { - err = apierrors.NewInvalid( - schema.GroupKind{Group: GroupVersion.Group, Kind: "EtcdCluster"}, - r.Name, allErrors) - } - return warnings, err + //var allErrors field.ErrorList + //// write validation here + //var err *apierrors.StatusError + //if len(allErrors) > 0 { + // err = apierrors.NewInvalid( + // schema.GroupKind{Group: GroupVersion.Group, Kind: "EtcdCluster"}, + // r.Name, allErrors) + // return warnings, err + //} + return warnings, nil } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type diff --git a/api/v1alpha1/etcdcluster_webhook_test.go b/api/v1alpha1/etcdcluster_webhook_test.go index 07efa657..87d5a3a3 100644 --- a/api/v1alpha1/etcdcluster_webhook_test.go +++ b/api/v1alpha1/etcdcluster_webhook_test.go @@ -19,9 +19,8 @@ package v1alpha1 import ( . "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) var _ = Describe("EtcdCluster Webhook", func() { @@ -30,14 +29,14 @@ var _ = Describe("EtcdCluster Webhook", func() { It("Should fill in the default value if a required field is empty", func() { etcdCluster := &EtcdCluster{} etcdCluster.Default() - gomega.Expect(etcdCluster.Spec.Replicas).To(gomega.Equal(int32(0)), "User should have an opportunity to create cluster with 0 replicas") + gomega.Expect(etcdCluster.Spec.Replicas).To(gomega.BeNil(), "User should have an opportunity to create cluster with 0 replicas") gomega.Expect(etcdCluster.Spec.Storage.Size).To(gomega.Equal(resource.MustParse("4Gi"))) }) It("Should not override fields with default values if not empty", func() { etcdCluster := &EtcdCluster{ Spec: EtcdClusterSpec{ - Replicas: 5, + Replicas: ptr.To(int32(5)), Storage: Storage{ StorageClass: "local-path", Size: resource.MustParse("10Gi"), @@ -45,36 +44,36 @@ var _ = Describe("EtcdCluster Webhook", func() { }, } etcdCluster.Default() - gomega.Expect(etcdCluster.Spec.Replicas).To(gomega.Equal(int32(5))) + gomega.Expect(*etcdCluster.Spec.Replicas).To(gomega.Equal(int32(5))) gomega.Expect(etcdCluster.Spec.Storage.Size).To(gomega.Equal(resource.MustParse("10Gi"))) }) }) Context("When creating EtcdCluster under Validating Webhook", func() { - It("Should deny if replicas is negative", func() { - etcdCluster := &EtcdCluster{ - Spec: EtcdClusterSpec{ - Replicas: -1, - }, - } - _, err := etcdCluster.ValidateCreate() - var statusErr *apierrors.StatusError - - if gomega.Expect(err).To(gomega.BeAssignableToTypeOf(statusErr)) { - statusErr = err.(*apierrors.StatusError) - gomega.Expect(statusErr.ErrStatus.Reason).To(gomega.Equal(metav1.StatusReasonInvalid)) - gomega.Expect(statusErr.ErrStatus.Details.Causes).To(gomega.ContainElement(metav1.StatusCause{ - Type: metav1.CauseTypeFieldValueInvalid, - Message: "Invalid value: -1: cluster replicas cannot be less than zero", - Field: "spec.replicas", - })) - } - }) + //It("Should deny if replicas is negative", func() { + // etcdCluster := &EtcdCluster{ + // Spec: EtcdClusterSpec{ + // Replicas: ptr.To(int32(-1)), + // }, + // } + // _, err := etcdCluster.ValidateCreate() + // var statusErr *apierrors.StatusError + // + // if gomega.Expect(err).To(gomega.BeAssignableToTypeOf(statusErr)) { + // statusErr = err.(*apierrors.StatusError) + // gomega.Expect(statusErr.ErrStatus.Reason).To(gomega.Equal(metav1.StatusReasonInvalid)) + // gomega.Expect(statusErr.ErrStatus.Details.Causes).To(gomega.ContainElement(metav1.StatusCause{ + // Type: metav1.CauseTypeFieldValueInvalid, + // Message: "Invalid value: -1: cluster replicas cannot be less than zero", + // Field: "spec.replicas", + // })) + // } + //}) It("Should admit if all required fields are provided", func() { etcdCluster := &EtcdCluster{ Spec: EtcdClusterSpec{ - Replicas: 1, + Replicas: ptr.To(int32(1)), }, } w, err := etcdCluster.ValidateCreate() diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e19f556d..e8f87f2b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -87,6 +87,11 @@ func (in *EtcdClusterList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EtcdClusterSpec) DeepCopyInto(out *EtcdClusterSpec) { *out = *in + if in.Replicas != nil { + in, out := &in.Replicas, &out.Replicas + *out = new(int32) + **out = **in + } in.Storage.DeepCopyInto(&out.Storage) } diff --git a/internal/controller/etcdcluster_controller.go b/internal/controller/etcdcluster_controller.go index fdf25533..a61cb953 100644 --- a/internal/controller/etcdcluster_controller.go +++ b/internal/controller/etcdcluster_controller.go @@ -302,7 +302,7 @@ func (r *EtcdClusterReconciler) ensureClusterStatefulSet( notFound = true // prepare initial cluster members initialCluster := "" - for i := int32(0); i < cluster.Spec.Replicas; i++ { + for i := int32(0); i < *cluster.Spec.Replicas; i++ { if i > 0 { initialCluster += "," } @@ -318,8 +318,7 @@ func (r *EtcdClusterReconciler) ensureClusterStatefulSet( Name: cluster.Name, }, Spec: appsv1.StatefulSetSpec{ - // initialize static fields that cannot be changed across updates. - Replicas: new(int32), + Replicas: cluster.Spec.Replicas, ServiceName: cluster.Name, PodManagementPolicy: appsv1.ParallelPodManagement, Selector: &metav1.LabelSelector{ @@ -441,7 +440,6 @@ func (r *EtcdClusterReconciler) ensureClusterStatefulSet( }, }, } - *statefulSet.Spec.Replicas = cluster.Spec.Replicas if err := ctrl.SetControllerReference(cluster, statefulSet, r.Scheme); err != nil { return fmt.Errorf("cannot set controller reference: %w", err) } diff --git a/internal/controller/etcdcluster_controller_test.go b/internal/controller/etcdcluster_controller_test.go index 77ff24bf..63dda754 100644 --- a/internal/controller/etcdcluster_controller_test.go +++ b/internal/controller/etcdcluster_controller_test.go @@ -19,6 +19,8 @@ package controller import ( "context" + "k8s.io/utils/ptr" + appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" resource2 "k8s.io/apimachinery/pkg/api/resource" @@ -56,7 +58,7 @@ var _ = Describe("EtcdCluster Controller", func() { Namespace: "default", }, Spec: etcdaenixiov1alpha1.EtcdClusterSpec{ - Replicas: 3, + Replicas: ptr.To(int32(3)), Storage: etcdaenixiov1alpha1.Storage{ Size: resource2.MustParse("1Gi"), }, @@ -144,8 +146,8 @@ var _ = Describe("EtcdCluster Controller", func() { err = k8sClient.Get(ctx, typeNamespacedName, sts) Expect(err).NotTo(HaveOccurred(), "cluster statefulset should exist") // mark sts as ready - sts.Status.ReadyReplicas = etcdcluster.Spec.Replicas - sts.Status.Replicas = etcdcluster.Spec.Replicas + sts.Status.ReadyReplicas = *etcdcluster.Spec.Replicas + sts.Status.Replicas = *etcdcluster.Spec.Replicas Expect(k8sClient.Status().Update(ctx, sts)).To(Succeed()) // reconcile and check EtcdCluster status _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{