Test:KO-481 remove global storage validation by rpandey-stack · Pull Request #494 · aerospike/aerospike-kubernetes-operator

rpandey-stack · 2026-04-02T05:47:10Z

Added TCs for enhancement/KO-481-remove-global-storage-validation and for enhancement/KO-480-K8-Objects-name-validation

Modified .PHONY: env-test with --keep-going to continue execution even after failure

…bhook_test.go

…rom cluster_webhook_test.go to rack_revision_webhook_test.go

test/envtests/cluster/cluster_webhook_utils.go

test/envtests/cluster/rack_revision_webhook_test.go

…paced name for aerospikeCluster

test/envtests/cluster/dynamic_rf_webhook_test.go

test/envtests/cluster/rack_revision_webhook_test.go

test/envtests/cluster/cluster_webhook_test.go

abhishekdwivedi3060 · 2026-04-06T10:19:30Z

Makefile

 env-test-cluster:  fmt vet setup-envtest ## Run tests.
-	export KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)"; cd $(shell pwd)/test/envtests/cluster && mkdir -p ../../test-results && go run github.com/onsi/ginkgo/v2/ginkgo -r --focus "$(FOCUS)" -coverprofile envcover.out -v -show-node-events -timeout=1h0m0s --junit-report=../../test-results/junit-envtests-cluster.xml -- . ${ARGS}
+	export KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)"; cd $(shell pwd)/test/envtests/cluster && mkdir -p ../../test-results && go run github.com/onsi/ginkgo/v2/ginkgo -r --focus "$(FOCUS)" -coverprofile envcover.out -timeout=1h0m0s --junit-report=../../test-results/junit-envtests-cluster.xml -- . ${ARGS}

 .PHONY: env-test-eviction # Run test/envtests/eviction
 env-test-eviction:  fmt vet setup-envtest ## Run tests.
-	export KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)"; cd $(shell pwd)/test/envtests/eviction && mkdir -p ../../test-results && go run github.com/onsi/ginkgo/v2/ginkgo -r --focus "$(FOCUS)" -coverprofile envcover.out -v -show-node-events -timeout=1h0m0s --junit-report=../../test-results/junit-envtests-eviction.xml -- . ${ARGS}	


These changes seem inconsistent like we didn't remove -show-node-events param here whereas removed in above 2

abhishekdwivedi3060 · 2026-04-06T13:36:47Z

test/envtests/cluster/cluster_webhook_utils.go

+func uniqueNamespacedName(suffix string) types.NamespacedName {
+	name := fmt.Sprintf("envtests-%s", suffix)
+
+	return test.GetNamespacedName(name, testutil.DefaultNamespace)
+}


What's the use of this func?
We are only adding a envtests- prefix for all, so uniqueness only depends on suffix passed.

abhishekdwivedi3060 · 2026-04-06T13:44:13Z

test/envtests/cluster/rack_enabled_cluster_webhook_test.go

+
+				BeforeEach(func() {
+					nsName = uniqueNamespacedName("rackcfg-neg")
+				})
+
+				AfterEach(func() {
+					deleteCluster(ctx, nsName)
+				})


Any reason for not moving these Before and After at the top level to keep it common for all?
We'll loose diff naming convention for each context but that fine IMO.

abhishekdwivedi3060 · 2026-04-06T14:31:45Z

test/envtests/cluster/storage_webhook_test.go

+					fullRack := getStorageSpecForDevice("/test/dev/xvdf")
+					policies := aero.Spec.Storage
+					aero.Spec.Storage = asdbv1.AerospikeStorageSpec{
+						BlockVolumePolicy:      policies.BlockVolumePolicy,
+						FileSystemVolumePolicy: policies.FileSystemVolumePolicy,
+						Volumes:                nil,
+					}


We can test with complete removal of spec storage

Suggested change

fullRack := getStorageSpecForDevice("/test/dev/xvdf")

policies := aero.Spec.Storage

aero.Spec.Storage = asdbv1.AerospikeStorageSpec{

BlockVolumePolicy: policies.BlockVolumePolicy,

FileSystemVolumePolicy: policies.FileSystemVolumePolicy,

Volumes: nil,

}

rackStorage := getStorageSpecForDevice("/test/dev/xvdf")

aero.Spec.Storage = asdbv1.AerospikeStorageSpec{}

abhishekdwivedi3060 · 2026-04-06T16:57:23Z

test/envtests/cluster/storage_webhook_test.go

+	Context("Update validation", func() {
+		Context("spec.storage", func() {
+			Context("negative", func() {
+				var nsName types.NamespacedName
+
+				BeforeEach(func() {
+					nsName = uniqueNamespacedName("updstor-neg")
+				})
+
+				AfterEach(func() {
+					deleteCluster(ctx, nsName)
+				})
+
+				It("rejects update that only mutates top-level spec.storage when there is no rackConfig", func() {
+					aero := testCluster.CreateDummyAerospikeCluster(nsName, 2)
+					Expect(envtests.K8sClient.Create(ctx, aero)).To(Succeed())
+
+					current, err := testCluster.GetCluster(envtests.K8sClient, ctx, nsName)
+					Expect(err).ToNot(HaveOccurred())
+
+					patchFirstPVStorageClassSpec(&current.Spec.Storage)
+
+					err = envtests.K8sClient.Update(ctx, current)
+					Expect(err).To(HaveOccurred())
+
+					// Webhook response validation
+					envtests.NewStatusErrorMatcher().
+						WithMessageSubstrings("\"vaerospikecluster.kb.io\"",
+							"rack storage config cannot be updated",
+							"cannot change volumes old").
+						Validate(err)
+				})


We have similar context in cluster_webhook_test.go with similar test-case. Check once

abhishekdwivedi3060 · 2026-04-07T11:48:00Z

test/envtests/cluster/rack_revision_webhook_test.go

+
+				// validateActualPodNames finds the longest pod name across all racks.
+				// Even if only one rack's pod name overflows, the whole UPDATE is rejected.
+				It("rejects UPDATE when only one rack's pod name exceeds the DNS label limit (multi-rack)", func() {


Suggested change

It("rejects UPDATE when only one rack's pod name exceeds the DNS label limit (multi-rack)", func() {

It("rejects UPDATE when only one rack's pod name exceeds the DNS label limit due to rack revision change(multi-rack)", func() {

abhishekdwivedi3060 · 2026-04-07T11:52:52Z

test/envtests/cluster/rack_revision_webhook_test.go

+				It("allows UPDATE without re-checking CREATE-only length bounds", func() {
+					// Create a valid cluster then update an unrelated field (size).
+					// The webhook must not surface any CREATE-only naming error on UPDATE.
+					aeroCluster := testCluster.CreateDummyAerospikeCluster(clusterNamespacedName, 2)
+					err := envtests.K8sClient.Create(ctx, aeroCluster)
+					Expect(err).ToNot(HaveOccurred())
+
+					current, err := testCluster.GetCluster(envtests.K8sClient, ctx, clusterNamespacedName)
+					Expect(err).ToNot(HaveOccurred())
+
+					// Change size — an otherwise valid update.
+					current.Spec.Size = 2
+					err = envtests.K8sClient.Update(ctx, current)
+					// The webhook must not surface any CREATE-only naming error on UPDATE.
+					if err != nil {
+						Expect(err).NotTo(MatchError(ContainSubstring("computed at max rack ID")),
+							"CREATE-only pod name length check must not be re-run on UPDATE")
+					}
+				})
+
+				// validateActualPodNames returns nil immediately when no user-defined
+				// racks are present (the controller manages a default rack internally).
+				It("allows UPDATE when cluster has no user-defined racks", func() {
+					aeroCluster := testCluster.CreateDummyAerospikeCluster(clusterNamespacedName, 2)
+					// No explicit RackConfig — controller will create a default rack.
+					err := envtests.K8sClient.Create(ctx, aeroCluster)
+					Expect(err).ToNot(HaveOccurred())
+
+					current, err := testCluster.GetCluster(envtests.K8sClient, ctx, clusterNamespacedName)
+					Expect(err).ToNot(HaveOccurred())
+
+					current.Spec.Size = 2
+					err = envtests.K8sClient.Update(ctx, current)
+					Expect(err).ToNot(HaveOccurred(),
+						"UPDATE with no user-defined racks must not fail on actual pod name check")
+				})


Didn't understand these test-cases. What are we exactly testing here? The size is still 2 in update

abhishekdwivedi3060 · 2026-04-07T11:58:21Z

test/envtests/cluster/rack_revision_webhook_test.go

+
+				It("allows UPDATE when one rack has zero pods under DistributeItems"+
+					"(validateActualPodNames skips rackSize 0)", func() {
+					cName := uniqueNamespacedName("actpod-zero-rack")


Suggested change

cName := uniqueNamespacedName("actpod-zero-rack")

cName = uniqueNamespacedName("actpod-zero-rack")

abhishekdwivedi3060 · 2026-04-07T11:58:32Z

test/envtests/cluster/rack_revision_webhook_test.go

+				})
+
+				It("allows UPDATE when max pod ordinal is greater than zero (validateActualPodNames uses rackSize-1)", func() {
+					cName := uniqueNamespacedName("actpod-ord-gt0")


Suggested change

cName := uniqueNamespacedName("actpod-ord-gt0")

cName = uniqueNamespacedName("actpod-ord-gt0")

abhishekdwivedi3060 · 2026-04-07T12:02:56Z

test/envtests/cluster/rack_revision_webhook_test.go

+					Expect(envtests.K8sClient.Update(ctx, current)).To(Succeed())
+				})
+
+				It("allows UPDATE when max pod ordinal is greater than zero (validateActualPodNames uses rackSize-1)", func() {


This seems redundant as this "allows UPDATE when max pod ordinal is greater than zero" is already getting checked in above positive test-cases.

abhishekdwivedi3060 · 2026-04-08T06:35:24Z

test/envtests/cluster/rack_enabled_cluster_webhook_test.go

+					aero.Spec.Storage = asdbv1.AerospikeStorageSpec{
+						BlockVolumePolicy:      policies.BlockVolumePolicy,
+						FileSystemVolumePolicy: policies.FileSystemVolumePolicy,
+						Volumes:                nil,
+					}


We can simply set it empty. Check wherver possible

abhishekdwivedi3060 · 2026-04-08T06:41:23Z

test/envtests/cluster/rack_enabled_cluster_webhook_test.go

+					Expect(envtests.K8sClient.Create(ctx, aero)).To(Succeed())
+				})
+
+				It("allows multiple racks each using distinct InputStorage paths consistent "+


How is this test-case different from above? We have diff storage paths for each rack in both the cases

…e console log lines

Added TCs for KO-481-remove-global-storage-validations

70da3eb

rpandey-stack requested review from abhishekdwivedi3060 and tanmayja as code owners April 2, 2026 05:47

rpandey-stack changed the base branch from master to enhancement/KO-481-remove-global-storage-validation April 2, 2026 05:48

rpandey-stack added the Work In Progress label Apr 2, 2026

Richa Pandey added 4 commits April 2, 2026 15:57

Changes done to reduce console logs during TC execution

b776f16

Modified .PHONY: env-test with --keep-going to continue execution even after failure

Moved TCs from removeGlobalStorage_webhook_test.go to respective *_we…

71e0625

…bhook_test.go

Moved TCs related to K8 object name validation having rack revision f…

14839fc

…rom cluster_webhook_test.go to rack_revision_webhook_test.go

Added more TCs for KO-480

d5ebd04

rpandey-stack removed the Work In Progress label Apr 3, 2026

tanmayja reviewed Apr 3, 2026

View reviewed changes

Richa Pandey added 2 commits April 6, 2026 09:58

Addressed review comments

af89808

Updated every test file to use uniqueNamespacedName() to create names…

2a34660

…paced name for aerospikeCluster

rpandey-stack requested a review from tanmayja April 6, 2026 06:23

tanmayja reviewed Apr 6, 2026

View reviewed changes

test/envtests/cluster/dynamic_rf_webhook_test.go Outdated Show resolved Hide resolved

test/envtests/cluster/rack_revision_webhook_test.go Outdated Show resolved Hide resolved

test/envtests/cluster/cluster_webhook_test.go Outdated Show resolved Hide resolved

Richa Pandey added 2 commits April 7, 2026 07:13

Addressed review comments

cc23c60

Replaced ns with cName cause deleteCluster is called only for cName

4635e16

rpandey-stack requested a review from tanmayja April 7, 2026 01:57

abhishekdwivedi3060 requested changes Apr 7, 2026

View reviewed changes

Moved Context() level BeforeEach and AfterEach block to root level

4abbf46

abhishekdwivedi3060 reviewed Apr 8, 2026

View reviewed changes

Richa Pandey added 3 commits April 8, 2026 14:58

Addressed review comments on storage_webhook_test.go

4a1a40d

Removed spec.storage configuration wherever not needed

62d0a45

Removed -show-node-events for make env-test-eviction command to reduc…

f3fbe7f

…e console log lines

	It("rejects UPDATE when only one rack's pod name exceeds the DNS label limit (multi-rack)", func() {
	It("rejects UPDATE when only one rack's pod name exceeds the DNS label limit due to rack revision change(multi-rack)", func() {

	cName := uniqueNamespacedName("actpod-zero-rack")
	cName = uniqueNamespacedName("actpod-zero-rack")

	cName := uniqueNamespacedName("actpod-ord-gt0")
	cName = uniqueNamespacedName("actpod-ord-gt0")

Conversation

rpandey-stack commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rpandey-stack commented Apr 2, 2026 •

edited

Loading