Remove sandbox and subclusters from vdb at the same time

.github/actions/run-e2e-leg/action.yaml

@@ -158,6 +158,10 @@ runs:
           fi
         fi
 
+        if [[ "${{ inputs.leg-identifier }}" == "leg-4" ]]; then
+          export CONCURRENCY_VERTICADB=10
+        fi
+
         # Set BASE_VERTICA_IMG based on $VERTICA_IMG
         # Some tests in some suites do an upgrade, so set the base image to upgrade from
         if [[ "${{ inputs.need-base-vertica-image }}" == "true" ]]; then

api/v1/helpers.go

@@ -384,6 +384,24 @@ func (s *Subcluster) GetService(ctx context.Context, vdb *VerticaDB, c client.Cl
 	return
 }
 
+// IsZombie checks if a subcluster is zombie. A zombie subcluster
+// is one that is no longer in vdb spec, no longer part of a sandbox
+// but still has a sandbox label different from the main cluster on
+// its statefulset.
+// It can happen when you remove a subcluster from spec.subclusters
+// and spec.sandboxes at once
+func (s *Subcluster) IsZombie(vdb *VerticaDB) bool {
+	sbName := s.Annotations[vmeta.SandboxNameLabel]
+	if sbName == MainCluster {
+		return false
+	}
+	scInSandboxMap := vdb.GenSubclusterSandboxMap()
+	scInSandboxStatusMap := vdb.GenSubclusterSandboxStatusMap()
+	_, foundInSandbox := scInSandboxMap[s.Name]
+	_, foundInSandboxStatus := scInSandboxStatusMap[s.Name]
+	return !foundInSandbox && !foundInSandboxStatus
+}
+
 // FindSubclusterForServiceName will find any subclusters that match the given service name
 func (v *VerticaDB) FindSubclusterForServiceName(svcName string) (scs []*Subcluster, totalSize int32) {
 	totalSize = int32(0)

api/v1/verticadb_webhook.go

@@ -1605,7 +1605,15 @@ func (v *VerticaDB) checkImmutableSubclusterInSandbox(oldObj *VerticaDB, allErrs
 	newScMap := v.GenSubclusterMap()
 	path := field.NewPath("spec").Child("subclusters")
 
-	// check if the sizes of subclusters in a sandbox get changed
+	allErrs = v.checkSubclusterSizeInSandbox(allErrs, persistScsWithSbIndex, newScIndexMap, oldScMap,
+		newScMap, path)
+	allErrs = v.checkSandboxSubclustersRemoved(allErrs, oldObj, oldScIndexMap, oldScMap, newScMap, path)
+	return v.checkSandboxPrimary(allErrs, oldObj, oldScIndexMap, oldScMap, path)
+}
+
+// checkSubclusterSizeInSandbox checks if the sizes of subclusters in a sandbox get changed
+func (v *VerticaDB) checkSubclusterSizeInSandbox(allErrs field.ErrorList, persistScsWithSbIndex, newScIndexMap map[string]int,
+	oldScMap, newScMap map[string]*Subcluster, path *field.Path) field.ErrorList {
 	for sc, i := range persistScsWithSbIndex {
 		oldSc, hasOldSc := oldScMap[sc]
 		newSc, hasNewSc := newScMap[sc]
@@ -1625,31 +1633,40 @@ func (v *VerticaDB) checkImmutableSubclusterInSandbox(oldObj *VerticaDB, allErrs
 			continue
 		}
 	}
+	return allErrs
+}
 
-	// find subclusters that are sandboxed in old vdb but removed in new vdb
-	oldScInSandbox := oldObj.GenSubclusterSandboxMap()
+// checkSandboxSubclustersRemoved checks subclusters that are sandboxed and removed in new vdb
+func (v *VerticaDB) checkSandboxSubclustersRemoved(allErrs field.ErrorList, oldObj *VerticaDB, oldScIndexMap map[string]int,
+	oldScMap, newScMap map[string]*Subcluster, path *field.Path) field.ErrorList {
+	newScInSandbox := v.GenSubclusterSandboxMap()
 	removedScs := vutil.MapKeyDiff(oldScMap, newScMap)
 	for _, sc := range removedScs {
-		if _, ok := oldScInSandbox[sc]; ok {
-			i := oldScIndexMap[sc]
-			err := field.Invalid(path.Index(i),
-				oldObj.Spec.Subclusters[i],
-				fmt.Sprintf("Cannot remove subcluster %q when it is in a sandbox", sc))
-			allErrs = append(allErrs, err)
+		if _, ok := newScInSandbox[sc]; !ok {
 			continue
 		}
+		i := oldScIndexMap[sc]
+		err := field.Invalid(path.Index(i),
+			oldObj.Spec.Subclusters[i],
+			fmt.Sprintf("Cannot remove subcluster %q when it is in a sandbox", sc))
+		allErrs = append(allErrs, err)
 	}
+	return allErrs
+}
 
+// checkSandboxPrimary ensures a couple of things:
+//   - a sandbox primary subcluster cannot be moved to another sandbox
+//   - cannot remove the sandbox primary subcluster from a sandbox
+//
+// The sandbox primary subcluster must stay constant until the sandbox is
+// removed entirely.
+func (v *VerticaDB) checkSandboxPrimary(allErrs field.ErrorList, oldObj *VerticaDB, oldScIndexMap map[string]int,
+	oldScMap map[string]*Subcluster, path *field.Path) field.ErrorList {
+	oldScInSandbox := oldObj.GenSubclusterSandboxMap()
 	newScInSandbox := v.GenSubclusterSandboxMap()
 	oldSbIndexMap := oldObj.GenSandboxIndexMap()
 	oldSbMap := oldObj.GenSandboxMap()
 	newSbMap := v.GenSandboxMap()
-
-	// This loop ensures a couple of things:
-	// - a sandbox primary subcluster cannot be moved to another sandbox
-	// - cannot remove the sandbox primary subcluster from a sandbox
-	// The sandbox primary subcluster must stay constant until the sandbox is
-	// removed entirely.
 	for oldScName, oldSbName := range oldScInSandbox {
 		newSbName, newFound := newScInSandbox[oldScName]
 		sc := oldScMap[oldScName]
@@ -1681,7 +1698,6 @@ func (v *VerticaDB) checkImmutableSubclusterInSandbox(oldObj *VerticaDB, allErrs
 			allErrs = append(allErrs, err)
 		}
 	}
-
 	return allErrs
 }
 

api/v1/verticadb_webhook_test.go

@@ -1040,7 +1040,22 @@ var _ = Describe("verticadb_webhook", func() {
 		Ω(newVdb.validateImmutableFields(oldVdb)).Should(HaveLen(2))
 
 		// cannot remove a subcluster that is sandboxed
-		// remove sc3 which is in a sandbox of oldVdb
+		// remove sc3 which is in sandbox2
+		newVdb.Spec.Subclusters = []Subcluster{
+			{Name: "main", Size: 3, Type: PrimarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+			{Name: "sc1", Size: 3, Type: SecondarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+			{Name: "sc2", Size: 3, Type: SecondarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+			{Name: "sc4", Size: 3, Type: SecondarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+		}
+		newVdb.Spec.Sandboxes = []Sandbox{
+			{Name: "sandbox1", Image: mainClusterImageVer, Subclusters: []SubclusterName{{Name: "sc1"}}},
+			{Name: "sandbox2", Image: mainClusterImageVer, Subclusters: []SubclusterName{{Name: "sc2"}, {Name: "sc3"}}},
+		}
+		Ω(newVdb.validateImmutableFields(oldVdb)).Should(HaveLen(2))
+
+		// can remove a subcluster if it is removed
+		// from any sandbox at the same time
+		// remove sc3 which is also removed from sandbox2
 		newVdb.Spec.Subclusters = []Subcluster{
 			{Name: "main", Size: 3, Type: PrimarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
 			{Name: "sc1", Size: 3, Type: SecondarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
@@ -1051,7 +1066,20 @@ var _ = Describe("verticadb_webhook", func() {
 			{Name: "sandbox1", Image: mainClusterImageVer, Subclusters: []SubclusterName{{Name: "sc1"}}},
 			{Name: "sandbox2", Image: mainClusterImageVer, Subclusters: []SubclusterName{{Name: "sc2"}}},
 		}
-		Ω(newVdb.validateImmutableFields(oldVdb)).Should(HaveLen(1))
+		Ω(newVdb.validateImmutableFields(oldVdb)).Should(HaveLen(0))
+
+		// can remove a sandbox and all of its subclusters at the same time
+		// remove sandbox1 and sc1 at the same time
+		newVdb.Spec.Subclusters = []Subcluster{
+			{Name: "main", Size: 3, Type: PrimarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+			{Name: "sc2", Size: 3, Type: SecondarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+			{Name: "sc3", Size: 3, Type: SecondarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+			{Name: "sc4", Size: 3, Type: SecondarySubcluster, ServiceType: v1.ServiceTypeClusterIP},
+		}
+		newVdb.Spec.Sandboxes = []Sandbox{
+			{Name: "sandbox2", Image: mainClusterImageVer, Subclusters: []SubclusterName{{Name: "sc2"}}},
+		}
+		Ω(newVdb.validateImmutableFields(oldVdb)).Should(HaveLen(0))
 
 		// can remove an unsandboxed subcluster
 		// remove sc4 which is not in a sandbox of oldVdb
@@ -1066,6 +1094,7 @@ var _ = Describe("verticadb_webhook", func() {
 			{Name: "sandbox2", Image: mainClusterImageVer, Subclusters: []SubclusterName{{Name: "sc2"}, {Name: "sc3"}}},
 		}
 		Ω(newVdb.validateImmutableFields(oldVdb)).Should(HaveLen(0))
+
 	})
 
 	It("should validate sandboxes", func() {

pkg/controllers/vdb/obj_reconciler.go

@@ -203,8 +203,22 @@ func (o *ObjReconciler) checkSecretHasKeys(ctx context.Context, secretType, secr
 // checkForCreatedSubclusters handles reconciliation of subclusters that should exist
 func (o *ObjReconciler) checkForCreatedSubclusters(ctx context.Context) (ctrl.Result, error) {
 	processedExtSvc := map[string]bool{} // Keeps track of service names we have reconciled
-	for i := range o.Vdb.Spec.Subclusters {
-		sc := &o.Vdb.Spec.Subclusters[i]
+	subclusters := []vapi.Subcluster{}
+	subclusters = append(subclusters, o.Vdb.Spec.Subclusters...)
+	if o.PFacts.GetSandboxName() == vapi.MainCluster {
+		scs, err := o.getZombieSubclusters(ctx)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+		if len(scs) > 0 {
+			subclusters = append(subclusters, scs...)
+			// At least one zombie subcluster was found and will rejoin the main cluster,
+			// so we invalidate to collect the pod facts.
+			o.PFacts.Invalidate()
+		}
+	}
+	for i := range subclusters {
+		sc := &subclusters[i]
 		// Transient subclusters never have their own service objects.  They always
 		// reuse ones we have for other primary/secondary subclusters.
 		if !sc.IsTransient() {
@@ -554,3 +568,20 @@ func (o *ObjReconciler) checkIfReadyForStsUpdate(newStsSize int32, sts *appsv1.S
 
 	return ctrl.Result{}, nil
 }
+
+// getZombieSubclusters returns all the zombie subclusters
+func (o *ObjReconciler) getZombieSubclusters(ctx context.Context) ([]vapi.Subcluster, error) {
+	subclusters := []vapi.Subcluster{}
+	finder := iter.MakeSubclusterFinder(o.Rec.GetClient(), o.Vdb)
+	scs, err := finder.FindSubclusters(ctx, iter.FindNotInVdbAcrossSandboxes, o.PFacts.GetSandboxName())
+	if err != nil {
+		return subclusters, err
+	}
+	for i := range scs {
+		sc := scs[i]
+		if sc.IsZombie(o.Vdb) {
+			subclusters = append(subclusters, *sc)
+		}
+	}
+	return subclusters, nil
+}

pkg/iter/sc_finder.go

@@ -58,6 +58,8 @@ const (
 	// Find all subclusters, both in the vdb and not in the vdb, regardless
 	// of the sandbox they belong to.
 	FindAllAcrossSandboxes = FindAll | FindSkipSandboxFilter
+	// Find subclusters not in vdb regardless of the sandbox they belong to.
+	FindNotInVdbAcrossSandboxes = FindNotInVdb | FindSkipSandboxFilter
 )
 
 func MakeSubclusterFinder(cli client.Client, vdb *vapi.VerticaDB) SubclusterFinder {
@@ -142,6 +144,8 @@ func (m *SubclusterFinder) FindSubclusters(ctx context.Context, flags FindFlags,
 				Size: 0,
 				Annotations: map[string]string{
 					vmeta.StsNameOverrideAnnotation: missingSts.Items[i].ObjectMeta.Name,
+					// This will let the operator know if the subcluster is zombie
+					vmeta.SandboxNameLabel: missingSts.Items[i].Labels[vmeta.SandboxNameLabel],
 				},
 			})
 		}

pkg/iter/sc_finder_test.go

@@ -326,6 +326,29 @@ var _ = Describe("sc_finder", func() {
 		Expect(subclusters[0].Name).Should(ContainSubstring(scNames[1]))
 		Expect(subclusters[1].Name).Should(ContainSubstring(scNames[0]))
 	})
+
+	It("should find subclusters not in vdb regardless of sandbox", func() {
+		vdb := vapi.MakeVDB()
+		scNames := []string{"sc1", "sc2", "sc3", "sc4"}
+		scSizes := []int32{10, 5, 8, 6}
+		const sbName = "sand"
+		vdb.Spec.Subclusters = []vapi.Subcluster{
+			{Name: scNames[0], Size: scSizes[0]},
+			{Name: scNames[1], Size: scSizes[1]},
+			{Name: scNames[2], Size: scSizes[2]},
+			{Name: scNames[3], Size: scSizes[3]},
+		}
+		vdb.Status.Sandboxes = []vapi.SandboxStatus{
+			{Name: sbName, Subclusters: scNames[:2]},
+		}
+		test.CreatePods(ctx, k8sClient, vdb, test.AllPodsRunning)
+		defer test.DeletePods(ctx, k8sClient, vdb)
+
+		vdb.Spec.Subclusters = vdb.Spec.Subclusters[1:]
+		vdb.Spec.Subclusters = vdb.Spec.Subclusters[:2]
+		scNames = append(scNames[0:1], scNames[3])
+		verifySubclusters(ctx, vdb, scNames, []int32{0, 0}, sbName, FindNotInVdbAcrossSandboxes)
+	})
 })
 
 func verifySubclusters(ctx context.Context, vdb *vapi.VerticaDB, scNames []string,

tests/e2e-leg-10/sandbox-basic/71-wait-for-steady-state.yaml

@@ -0,0 +1,17 @@
+# (c) Copyright [2021-2024] Open Text.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: bash -c "../../../scripts/wait-for-verticadb-steady-state.sh -n verticadb-operator -t 360 $NAMESPACE"

tests/e2e-leg-10/sandbox-basic/75-assert.yaml

@@ -0,0 +1,36 @@
+# (c) Copyright [2021-2024] Open Text.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: vertica.com/v1
+kind: VerticaDB
+metadata:
+  name: v-sandbox
+spec:
+  subclusters:
+    - name: pri1
+      type: primary
+    - name: sec1
+      type: secondary
+    - name: sec3
+      type: secondary
+status:
+  subclusters:
+    - addedToDBCount: 3
+      upNodeCount: 3
+      name: pri1
+    - addedToDBCount: 3
+      upNodeCount: 3
+      name: sec1
+    - addedToDBCount: 1
+      upNodeCount: 1
+      name: sec3

tests/e2e-leg-10/sandbox-basic/75-terminate-sandbox2.yaml

@@ -0,0 +1,29 @@
+# (c) Copyright [2021-2024] Open Text.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: vertica.com/v1
+kind: VerticaDB
+metadata:
+  name: v-sandbox
+spec:
+  sandboxes: []
+  subclusters:
+    - name: pri1
+      type: primary
+      size: 3
+    - name: sec1
+      type: secondary
+      size: 3
+    - name: sec3
+      type: secondary
+      size: 1

tests/e2e-leg-10/sandbox-on-create/30-assert.yaml

@@ -0,0 +1,27 @@
+# (c) Copyright [2021-2024] Open Text.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: vertica.com/v1
+kind: VerticaDB
+metadata:
+  name: v-sandbox-on-create
+status:
+  sandboxes:
+  - name: sand1
+    subclusters:
+    - sec1
+  subclusters:
+    - addedToDBCount: 3
+      upNodeCount: 3
+    - addedToDBCount: 1
+      upNodeCount: 1