Skip to content

Commit

Permalink
Change replicas field type to int32 and add validation
Browse files Browse the repository at this point in the history
  • Loading branch information
sircthulhu committed Mar 20, 2024
1 parent 94c538d commit a8d9ae9
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 27 deletions.
3 changes: 2 additions & 1 deletion api/v1alpha1/etcdcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand Down
38 changes: 32 additions & 6 deletions api/v1alpha1/etcdcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
}
Expand All @@ -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
}

Expand All @@ -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
Expand Down
51 changes: 35 additions & 16 deletions api/v1alpha1/etcdcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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")))
})

Expand All @@ -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())
})
})

})
2 changes: 2 additions & 0 deletions config/crd/bases/etcd.aenix.io_etcdclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/etcdcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 += ","
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/etcdcluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit a8d9ae9

Please sign in to comment.