diff --git a/api/v1alpha1/etcdcluster_types.go b/api/v1alpha1/etcdcluster_types.go index b61352a5..affc7d84 100644 --- a/api/v1alpha1/etcdcluster_types.go +++ b/api/v1alpha1/etcdcluster_types.go @@ -31,7 +31,8 @@ type EtcdClusterSpec struct { // Replicas is the count of etcd instances in cluster. // +optional // +kubebuilder:default:=3 - Replicas uint `json:"replicas,omitempty"` + // +kubebuilder:validation:Minimum:=0 + 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 233531a9..91f8a9c8 100644 --- a/api/v1alpha1/etcdcluster_webhook.go +++ b/api/v1alpha1/etcdcluster_webhook.go @@ -17,8 +17,11 @@ 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" @@ -42,9 +45,6 @@ var _ webhook.Defaulter = &EtcdCluster{} // Default implements webhook.Defaulter so a webhook will be registered for the type func (r *EtcdCluster) Default() { etcdclusterlog.Info("default", "name", r.Name) - if r.Spec.Replicas == 0 { - r.Spec.Replicas = 3 - } if r.Spec.Storage.Size.IsZero() { r.Spec.Storage.Size = resource.MustParse("4Gi") } @@ -57,6 +57,20 @@ 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) + } + return nil, nil } @@ -68,10 +82,22 @@ func (r *EtcdCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, er warnings = append(warnings, "cluster resize is not currently supported") } - if len(warnings) > 0 { - return warnings, nil + 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", + )) } - return nil, nil + + var err *apierrors.StatusError + if len(allErrors) > 0 { + err = apierrors.NewInvalid( + schema.GroupKind{Group: GroupVersion.Group, Kind: "EtcdCluster"}, + r.Name, allErrors) + } + return warnings, err } // 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 b6a88ccd..07efa657 100644 --- a/api/v1alpha1/etcdcluster_webhook_test.go +++ b/api/v1alpha1/etcdcluster_webhook_test.go @@ -19,7 +19,9 @@ 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" ) var _ = Describe("EtcdCluster Webhook", func() { @@ -28,7 +30,7 @@ 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(uint(3))) + 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.Storage.Size).To(gomega.Equal(resource.MustParse("4Gi"))) }) @@ -43,25 +45,42 @@ var _ = Describe("EtcdCluster Webhook", func() { }, } etcdCluster.Default() - gomega.Expect(etcdCluster.Spec.Replicas).To(gomega.Equal(uint(5))) + gomega.Expect(etcdCluster.Spec.Replicas).To(gomega.Equal(int32(5))) gomega.Expect(etcdCluster.Spec.Storage.Size).To(gomega.Equal(resource.MustParse("10Gi"))) }) }) - // Not yet applicable as currently all fields are optional. + 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", + })) + } + }) - //Context("When creating EtcdCluster under Validating Webhook", func() { - // It("Should deny if a required field is empty", func() { - // - // // TODO(user): Add your logic here - // - // }) - // - // It("Should admit if all required fields are provided", func() { - // - // // TODO(user): Add your logic here - // - // }) - //}) + It("Should admit if all required fields are provided", func() { + etcdCluster := &EtcdCluster{ + Spec: EtcdClusterSpec{ + Replicas: 1, + }, + } + w, err := etcdCluster.ValidateCreate() + gomega.Expect(err).To(gomega.Succeed()) + gomega.Expect(w).To(gomega.BeEmpty()) + }) + }) }) diff --git a/config/crd/bases/etcd.aenix.io_etcdclusters.yaml b/config/crd/bases/etcd.aenix.io_etcdclusters.yaml index 0f0a39ad..07cfb934 100644 --- a/config/crd/bases/etcd.aenix.io_etcdclusters.yaml +++ b/config/crd/bases/etcd.aenix.io_etcdclusters.yaml @@ -42,6 +42,8 @@ spec: replicas: default: 3 description: Replicas is the count of etcd instances in cluster. + format: int32 + minimum: 0 type: integer storage: properties: diff --git a/internal/controller/etcdcluster_controller.go b/internal/controller/etcdcluster_controller.go index bda0fa6a..fdf25533 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 := uint(0); i < cluster.Spec.Replicas; i++ { + for i := int32(0); i < cluster.Spec.Replicas; i++ { if i > 0 { initialCluster += "," } @@ -441,7 +441,7 @@ func (r *EtcdClusterReconciler) ensureClusterStatefulSet( }, }, } - *statefulSet.Spec.Replicas = int32(cluster.Spec.Replicas) + *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 70aa8234..77ff24bf 100644 --- a/internal/controller/etcdcluster_controller_test.go +++ b/internal/controller/etcdcluster_controller_test.go @@ -144,8 +144,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 = int32(etcdcluster.Spec.Replicas) - sts.Status.Replicas = int32(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{