Add integration tests, adapt docs

Author: shreyas-s-rao

docs/usage/validating-etcd-clusters.md

@@ -1,26 +1,36 @@
 # CRD Validations for etcd
 
 * The validations for the fields within the `etcd` resource are done via [kubebuilder markers for CRD validation](https://book.kubebuilder.io/reference/markers/crd-validation). 
-* The validations for clusters with kubernetes versions `>= 1.29` are written using a combination of [CEL expressions](https://kubernetes.io/docs/reference/using-api/cel/) via the `x-validation` tag which provides a straightforward syntax to write validation rules for the fields, and pattern matching with the use of the `validation` tag.
+* The validations for clusters with kubernetes versions `>= 1.29` are written using a combination of [CEL expressions](https://kubernetes.io/docs/reference/using-api/cel/) via the `XValidation` kubebuilder tag which provides a straightforward syntax to write validation rules for the fields, and pattern matching with the use of the `validation` kubebuilder tag.
 * The validations for clusters with kubernetes versions `< 1.29` will not contain validations via `CEL` expressions since this is GA for kubernetes version 1.29 or higher.
 * Upon any changes to the validation rules to the etcd resource, the `yaml` files for the same can be generated by running the `make generate` command.
 
 ## Validation rules:
 ### Type Validation rules:
 The validations for fields of types `Duration`(metav1.Duration) and `cron` expressions are done via `regex` matching. These use the `validation:Pattern` marker.(The checking for the `Quantity`(resource.Quantity) fields are done by default, hence, no explicit validation is needed for the fields of this type):
 * Duration fields: 
-`'^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$')`
+  ```
+  '^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$')
+  ```
 * Cron expression:
-`^(\*|[1-5]?[0-9]|[1-5]?[0-9]-[1-5]?[0-9]|(?:[1-9]|[1-4][0-9]|5[0-9])\/(?:[1-9]|[1-4][0-9]|5[0-9]|60)|\*\/(?:[1-9]|[1-4][0-9]|5[0-9]|60))\s+(\*|[0-9]|1[0-9]|2[0-3]|[0-9]-(?:[0-9]|1[0-9]|2[0-3])|1[0-9]-(?:1[0-9]|2[0-3])|2[0-3]-2[0-3]|(?:[1-9]|1[0-9]|2[0-3])\/(?:[1-9]|1[0-9]|2[0-4])|\*\/(?:[1-9]|1[0-9]|2[0-4]))\s+(\*|[1-9]|[12][0-9]|3[01]|[1-9]-(?:[1-9]|[12][0-9]|3[01])|[12][0-9]-(?:[12][0-9]|3[01])|3[01]-3[01]|(?:[1-9]|[12][0-9]|30)\/(?:[1-9]|[12][0-9]|3[01])|\*\/(?:[1-9]|[12][0-9]|3[01]))\s+(\*|[1-9]|1[0-2]|[1-9]-(?:[1-9]|1[0-2])|1[0-2]-1[0-2]|(?:[1-9]|1[0-2])\/(?:[1-9]|1[0-2])|\*\/(?:[1-9]|1[0-2]))\s+(\*|[1-7]|[1-6]-[1-7]|[1-6]\/[1-7]|\*\/[1-7])$`
+  ```
+  ^(\*|[1-5]?[0-9]|[1-5]?[0-9]-[1-5]?[0-9]|(?:[1-9]|[1-4][0-9]|5[0-9])\/(?:[1-9]|[1-4][0-9]|5[0-9]|60)|\*\/(?:[1-9]|[1-4][0-9]|5[0-9]|60))\s+(\*|[0-9]|1[0-9]|2[0-3]|[0-9]-(?:[0-9]|1[0-9]|2[0-3])|1[0-9]-(?:1[0-9]|2[0-3])|2[0-3]-2[0-3]|(?:[1-9]|1[0-9]|2[0-3])\/(?:[1-9]|1[0-9]|2[0-4])|\*\/(?:[1-9]|1[0-9]|2[0-4]))\s+(\*|[1-9]|[12][0-9]|3[01]|[1-9]-(?:[1-9]|[12][0-9]|3[01])|[12][0-9]-(?:[12][0-9]|3[01])|3[01]-3[01]|(?:[1-9]|[12][0-9]|30)\/(?:[1-9]|[12][0-9]|3[01])|\*\/(?:[1-9]|[12][0-9]|3[01]))\s+(\*|[1-9]|1[0-2]|[1-9]-(?:[1-9]|1[0-2])|1[0-2]-1[0-2]|(?:[1-9]|1[0-2])\/(?:[1-9]|1[0-2])|\*\/(?:[1-9]|1[0-2]))\s+(\*|[1-7]|[1-6]-[1-7]|[1-6]\/[1-7]|\*\/[1-7])$
+  ```
 
     * **NOTE**: The provided regex does not account for `special strings` such as `@yearly` or `@monthly`. Additionally, it fails to invalidate cases involving the `step operator (x/y)` and the `range operator (x-y)`, where the cron expression is considered valid even if `x > y`. Please ensure these values are validated before passing the expression.
 
 ### Update validations
 These validations are triggered when an update operation is done on the etcd resource.
-* Immutable fields: The fields `etcd.spec.StorageClass` , `etcd.spec.StorageCapacity` and `etcd.spec.VolumeClaimTemplate` are immutable. The immutability is enforced by the CEL expression : `self == oldSelf`.
+* Immutable fields: The fields `etcd.spec.StorageClass` , `etcd.spec.StorageCapacity` and `etcd.spec.VolumeClaimTemplate` are immutable. The immutability is enforced by the CEL expression:
+  ```
+  self == oldSelf
+  ```
 
-* The value set for the field `etcd.spec.replicas` can either be decreased to `0` or increased. This is enforced by the CEL expression:
-    `self==0 ? true : self < oldSelf ? false : true`
+* The value set for the field `etcd.spec.replicas` can either be decreased to `0` or increased, since etcd-druid does not yet support scaling down of etcd cluster size. While scaling up a hibernated etcd cluster from 0 replicas, the `etcd.status.clusterSize` field is checked to ensure that replicas can only be set to the previously recorded cluster size and not higher or lower. This is enforced by the CEL expression:
+  ```
+  self.spec.replicas == 0 || (oldSelf.spec.replicas == 0 ? (!has(self.status.clusterSize) || (self.spec.replicas == self.status.clusterSize)) : (self.spec.replicas >= oldSelf.spec.replicas))
+  ```
+  Hibernating the etcd cluster to 0 replicas is always allowed. If replicas field is changed from a non-zero value to another non-zero value, then the rule ensures that the replicas cannot be decreased, since down-scaling of etcd clusters is not currently supported. If the cluster is already hibernated and is attempted to be scaled up, then `etcd.status.clusterSize` field is checked to ensure that replicas can only be set to the previously recorded etcd cluster size. This is required to ensure that the scale-up logic is allowed to be executed correctly. If `etcd.status.clusterSize` is not set, then it is assumed that the etcd cluster has not been created yet and the replicas can be set to any value.
 
 ### Field validations
 - The fields which expect only a particular set of values are checked by using the kubebuilder marker: `+kubebuilder:validation:Enum=<value1>;<value2>`
@@ -30,9 +40,14 @@ These validations are triggered when an update operation is done on the etcd res
     * The `etcd.spec.sharedConfig.autoCompactionMode` can only be set as either `periodic` or `revision`.
 
 
-* The value of `etcd.spec.backup.garbageCollectionPeriod` must be greater than `etcd.spec.backup.deltaSnapshotPeriod`. This is enforced by the CEL expression
-`!(has(self.deltaSnapshotPeriod) && has(self.garbageCollectionPeriod)) || duration(self.deltaSnapshotPeriod).getSeconds() < duration(self.garbageCollectionPeriod).getSeconds()`. The first part of the expression ensures that both the fields are present and then compares the values of the garbageCollectionPeriod and deltaSnapshotPeriod fields, if not, skips the check.
+* The value of `etcd.spec.backup.garbageCollectionPeriod` must be greater than `etcd.spec.backup.deltaSnapshotPeriod`. This is enforced by the CEL expression:
+  ```
+  !(has(self.deltaSnapshotPeriod) && has(self.garbageCollectionPeriod)) || duration(self.deltaSnapshotPeriod).getSeconds() < duration(self.garbageCollectionPeriod).getSeconds()
+  ```
+  The first part of the expression ensures that both the fields are present and then compares the values of the garbageCollectionPeriod and deltaSnapshotPeriod fields, if not, skips the check.
 
-* The value of `etcd.spec.StorageCapacity` must be more than 3 times that of the `etcd.spec.etcd.quota` if backups are enabled. If not, the value must be greater than that of the `etcd.spec.etcd.quota` field. This is enforced by using the CEL expression: 
-`has(self.storageCapacity) && has(self.etcd.quota) ? (has(self.backup.store) ? quantity(self.storageCapacity).compareTo(quantity(self.etcd.quota).add(quantity(self.etcd.quota)).add(quantity(self.etcd.quota))) > 0 : quantity(self.storageCapacity).compareTo(quantity(self.etcd.quota)) > 0 ): true`
-The check for whether backups are enabled or not is done by checking if the field `etcd.spec.backup.store` exists.
+* The value of `etcd.spec.StorageCapacity` must be more than 3 times that of the `etcd.spec.etcd.quota` if backups are enabled. If not, the value must be greater than that of the `etcd.spec.etcd.quota` field. This is enforced by using the CEL expression:
+  ```
+  has(self.storageCapacity) && has(self.etcd.quota) ? (has(self.backup.store) ? quantity(self.storageCapacity).compareTo(quantity(self.etcd.quota).add(quantity(self.etcd.quota)).add(quantity(self.etcd.quota))) > 0 : quantity(self.storageCapacity).compareTo(quantity(self.etcd.quota)) > 0 ): true
+  ```
+  The check for whether backups are enabled or not is done by checking if the field `etcd.spec.backup.store` exists.

test/it/crdvalidation/etcd/specupdate_test.go

@@ -16,38 +16,38 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-// etcd.spec.storageClass is immutable
+// TestValidateUpdateSpecStorageClass tests the immutability of etcd.spec.storageClass
 func TestValidateUpdateSpecStorageClass(t *testing.T) {
 	skipCELTestsForOlderK8sVersions(t)
 	testNs, g := setupTestEnvironment(t)
 
 	tests := []struct {
 		name                    string
 		etcdName                string
-		initalStorageClassName  string
+		initialStorageClassName string
 		updatedStorageClassName string
 		expectErr               bool
 	}{
 		{
 			name:                    "Valid #1: Unchanged storageClass",
 			etcdName:                "etcd-valid-1",
-			initalStorageClassName:  "gardener.cloud-fast",
-			updatedStorageClassName: "gardener.cloud-fast",
+			initialStorageClassName: "storageClass1",
+			updatedStorageClassName: "storageClass1",
 			expectErr:               false,
 		},
 		{
 			name:                    "Invalid #1: Updated storageClass",
 			etcdName:                "etcd-invalid-1",
-			initalStorageClassName:  "gardener.cloud-fast",
-			updatedStorageClassName: "default",
+			initialStorageClassName: "storageClass1",
+			updatedStorageClassName: "storageClass2",
 			expectErr:               true,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			etcd := utils.EtcdBuilderWithoutDefaults(test.etcdName, testNs).WithReplicas(3).Build()
-			etcd.Spec.StorageClass = &test.initalStorageClassName
+			etcd.Spec.StorageClass = &test.initialStorageClassName
 
 			cl := itTestEnv.GetClient()
 			ctx := context.Background()
@@ -59,76 +59,152 @@ func TestValidateUpdateSpecStorageClass(t *testing.T) {
 	}
 }
 
-// checks the update on the etcd.spec.replicas field
+// TestValidateUpdateSpecReplicas tests the update on the etcd.spec.replicas field
 func TestValidateUpdateSpecReplicas(t *testing.T) {
 	skipCELTestsForOlderK8sVersions(t)
-	tests := []struct {
+	testCases := []struct {
 		name            string
 		etcdName        string
-		initialReplicas int
-		updatedReplicas int
+		initialReplicas int32
+		updatedReplicas int32
+		clusterSize     int32
 		expectErr       bool
 	}{
 		{
-			name:            "Valid updation of replicas #1",
-			etcdName:        "etcd-valid-inc",
+			// allow etcd cluster to be unhibernated
+			name:            "0 -> n, where n > 0 and clusterSize = n",
+			etcdName:        "etcd-valid-replicas-1",
+			initialReplicas: 0,
+			updatedReplicas: 3,
+			clusterSize:     3,
+			expectErr:       false,
+		},
+		{
+			// etcd cluster can be unhibernated only to the size of the cluster, to allow scale-up logic to run
+			name:            "0 -> n, where n > 0 and clusterSize > n",
+			etcdName:        "etcd-invalid-replicas-1",
+			initialReplicas: 0,
+			updatedReplicas: 3,
+			clusterSize:     5,
+			expectErr:       true,
+		},
+		{
+			// etcd cluster can be unhibernated only to the size of the cluster, because the cluster cannot be scale down
+			name:            "0 -> m, where m > 0 and clusterSize < m",
+			etcdName:        "etcd-invalid-replicas-2",
+			initialReplicas: 0,
+			updatedReplicas: 3,
+			clusterSize:     5,
+			expectErr:       true,
+		},
+		{
+			// allow hibernated etcd cluster to continue being hibernated
+			name:            "0 -> 0, where clusterSize = 0 (etcd cluster not started)",
+			etcdName:        "etcd-valid-replicas-2",
+			initialReplicas: 0,
+			updatedReplicas: 0,
+			clusterSize:     0,
+			expectErr:       false,
+		},
+		{
+			// allow hibernated etcd cluster to continue being hibernated
+			name:            "0 -> 0, where clusterSize != 0 (etcd cluster already started)",
+			etcdName:        "etcd-invalid-replicas-3",
+			initialReplicas: 0,
+			updatedReplicas: 0,
+			clusterSize:     1,
+			expectErr:       false,
+		},
+		{
+			// etcd replicas remains the same as cluster size
+			name:            "n -> n, where n > 0",
+			etcdName:        "etcd-valid-replicas-3",
 			initialReplicas: 3,
+			updatedReplicas: 3,
+			clusterSize:     3,
+			expectErr:       false,
+		},
+		{
+			// etcd replicas remains the same, but does not match cluster size
+			name:            "n -> n, where n > 0 and clusterSize != n",
+			etcdName:        "etcd-valid-replicas-4",
+			initialReplicas: 5,
 			updatedReplicas: 5,
+			clusterSize:     3,
 			expectErr:       false,
 		},
 		{
-			name:            "Valid updation of replicas #2",
-			etcdName:        "etcd-valid-zero",
+			// scale up etcd cluster
+			name:            "n -> m, where m > n > 0",
+			etcdName:        "etcd-valid-replicas-5",
+			initialReplicas: 3,
+			updatedReplicas: 5,
+			clusterSize:     3,
+			expectErr:       false,
+		},
+		{
+			// hibernate etcd cluster to 0 replicas
+			name:            "m -> 0, where m > 0",
+			etcdName:        "etcd-valid-replica-6",
 			initialReplicas: 3,
 			updatedReplicas: 0,
+			clusterSize:     3,
 			expectErr:       false,
 		},
 		{
-			name:            "Invalid updation of replicas #1",
-			etcdName:        "etcd-invalid-dec",
+			// etcd cluster cannot be scaled down
+			name:            "m -> n, where m > n > 0",
+			etcdName:        "etcd-invalid-replicas-4",
 			initialReplicas: 5,
 			updatedReplicas: 3,
+			clusterSize:     5,
 			expectErr:       true,
 		},
 	}
 
 	testNs, g := setupTestEnvironment(t)
+	t.Parallel()
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			etcd := utils.EtcdBuilderWithoutDefaults(test.etcdName, testNs).WithReplicas(int32(test.initialReplicas)).Build()
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			etcd := utils.EtcdBuilderWithoutDefaults(tc.etcdName, testNs).
+				WithReplicas(tc.initialReplicas).
+				Build()
 			cl := itTestEnv.GetClient()
 			ctx := context.Background()
 			g.Expect(cl.Create(ctx, etcd)).To(Succeed())
+			// update status subresource, since status is not created in the object Create() call
+			etcd.Status.ClusterSize = tc.clusterSize
+			g.Expect(cl.Status().Update(ctx, etcd)).To(Succeed())
 
-			etcd.Spec.Replicas = int32(test.updatedReplicas)
-			validateEtcdUpdation(t, g, etcd, test.expectErr, ctx, cl)
+			etcd.Spec.Replicas = tc.updatedReplicas
+			validateEtcdUpdation(t, g, etcd, tc.expectErr, ctx, cl)
 		})
 	}
 }
 
-// checks the immutablility of the etcd.spec.StorageCapacity field
+// TestValidateUpdateSpecStorageCapacity tests the immutablility of the etcd.spec.StorageCapacity field
 func TestValidateUpdateSpecStorageCapacity(t *testing.T) {
 	skipCELTestsForOlderK8sVersions(t)
 	testNs, g := setupTestEnvironment(t)
 	tests := []struct {
 		name                   string
 		etcdName               string
-		initalStorageCapacity  resource.Quantity
+		initialStorageCapacity resource.Quantity
 		updatedStorageCapacity resource.Quantity
 		expectErr              bool
 	}{
 		{
 			name:                   "Valid #1: Unchanged storageCapacity",
 			etcdName:               "etcd-valid-1-storagecap",
-			initalStorageCapacity:  resource.MustParse("25Gi"),
+			initialStorageCapacity: resource.MustParse("25Gi"),
 			updatedStorageCapacity: resource.MustParse("25Gi"),
 			expectErr:              false,
 		},
 		{
 			name:                   "Invalid #1: Updated storageCapacity",
 			etcdName:               "etcd-invalid-1-storagecap",
-			initalStorageCapacity:  resource.MustParse("15Gi"),
+			initialStorageCapacity: resource.MustParse("15Gi"),
 			updatedStorageCapacity: resource.MustParse("20Gi"),
 			expectErr:              true,
 		},
@@ -137,7 +213,7 @@ func TestValidateUpdateSpecStorageCapacity(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			etcd := utils.EtcdBuilderWithoutDefaults(test.etcdName, testNs).WithReplicas(3).Build()
-			etcd.Spec.StorageCapacity = &test.initalStorageCapacity
+			etcd.Spec.StorageCapacity = &test.initialStorageCapacity
 
 			cl := itTestEnv.GetClient()
 			ctx := context.Background()
@@ -149,28 +225,28 @@ func TestValidateUpdateSpecStorageCapacity(t *testing.T) {
 	}
 }
 
-// check the immutability of the etcd.spec.VolumeClaimTemplate field
+// TestValidateUpdateSpecVolumeClaimTemplate tests the immutability of the etcd.spec.VolumeClaimTemplate field
 func TestValidateUpdateSpecVolumeClaimTemplate(t *testing.T) {
 	skipCELTestsForOlderK8sVersions(t)
 	testNs, g := setupTestEnvironment(t)
 	tests := []struct {
 		name                string
 		etcdName            string
-		initalVolClaimTemp  string
+		initialVolClaimTemp string
 		updatedVolClaimTemp string
 		expectErr           bool
 	}{
 		{
 			name:                "Valid #1: Unchanged volumeClaimTemplate",
 			etcdName:            "etcd-valid-1-volclaim",
-			initalVolClaimTemp:  "main-etcd",
+			initialVolClaimTemp: "main-etcd",
 			updatedVolClaimTemp: "main-etcd",
 			expectErr:           false,
 		},
 		{
 			name:                "Invalid #1: Updated storageCapacity",
 			etcdName:            "etcd-invalid-1-volclaim",
-			initalVolClaimTemp:  "main-etcd",
+			initialVolClaimTemp: "main-etcd",
 			updatedVolClaimTemp: "new-vol-temp",
 			expectErr:           true,
 		},
@@ -179,7 +255,7 @@ func TestValidateUpdateSpecVolumeClaimTemplate(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			etcd := utils.EtcdBuilderWithoutDefaults(test.etcdName, testNs).WithReplicas(3).Build()
-			etcd.Spec.VolumeClaimTemplate = &test.initalVolClaimTemp
+			etcd.Spec.VolumeClaimTemplate = &test.initialVolClaimTemp
 
 			cl := itTestEnv.GetClient()
 			ctx := context.Background()