Skip to content

Commit

Permalink
Add validation minimum and change type to *int32 for compatibility (#57)
Browse files Browse the repository at this point in the history
- `Replicas` field type changed to *int32 according to k8s api
guidelines
- remove unnecessary validation (everything is validated using OpenAPI
scheme)
- allow creation of cluster with 0 replicas

fixes #51
  • Loading branch information
sircthulhu authored Mar 21, 2024
1 parent dfd4c74 commit 2e25f81
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 33 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
9 changes: 1 addition & 8 deletions api/v1alpha1/etcdcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,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 @@ -67,11 +64,7 @@ func (r *EtcdCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, er
if old.(*EtcdCluster).Spec.Replicas != r.Spec.Replicas {
warnings = append(warnings, "cluster resize is not currently supported")
}

if len(warnings) > 0 {
return warnings, nil
}
return nil, nil
return warnings, nil
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
Expand Down
34 changes: 16 additions & 18 deletions api/v1alpha1/etcdcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
. "github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/utils/ptr"
)

var _ = Describe("EtcdCluster Webhook", func() {
Expand All @@ -28,40 +29,37 @@ 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.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"),
},
},
}
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 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
//
// })
//})
Context("When creating EtcdCluster under Validating Webhook", func() {
It("Should admit if all required fields are provided", func() {
etcdCluster := &EtcdCluster{
Spec: EtcdClusterSpec{
Replicas: ptr.To(int32(1)),
},
}
w, err := etcdCluster.ValidateCreate()
gomega.Expect(err).To(gomega.Succeed())
gomega.Expect(w).To(gomega.BeEmpty())
})
})

})
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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
5 changes: 2 additions & 3 deletions internal/controller/etcdcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (r *EtcdClusterReconciler) ensureClusterStateConfigMap(
// configmap does not exist, create with cluster state "new"
if errors.IsNotFound(err) {
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 @@ -319,7 +319,7 @@ func (r *EtcdClusterReconciler) ensureClusterStatefulSet(
},
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{
Expand Down Expand Up @@ -439,7 +439,6 @@ func (r *EtcdClusterReconciler) ensureClusterStatefulSet(
},
},
}
*statefulSet.Spec.Replicas = int32(cluster.Spec.Replicas)
if err := ctrl.SetControllerReference(cluster, statefulSet, r.Scheme); err != nil {
return fmt.Errorf("cannot set controller reference: %w", err)
}
Expand Down
8 changes: 5 additions & 3 deletions internal/controller/etcdcluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"),
},
Expand Down Expand Up @@ -142,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 2e25f81

Please sign in to comment.