Use static labels in pods

api/v1/helpers.go

@@ -337,6 +337,16 @@ func (v *VerticaDB) RequiresTransientSubcluster() bool {
 		v.Spec.TemporarySubclusterRouting.Template.Size > 0
 }
 
+// GetTransientSubclusterName returns the name of the transient subcluster, if
+// it should exist. The bool output parameter will be false if no transient is
+// used.
+func (v *VerticaDB) GetTransientSubclusterName() (string, bool) {
+	if !v.RequiresTransientSubcluster() {
+		return "", false
+	}
+	return v.Spec.TemporarySubclusterRouting.Template.Name, true
+}
+
 // IsOnlineUpgradeInProgress returns true if an online upgrade is in progress
 func (v *VerticaDB) IsOnlineUpgradeInProgress() bool {
 	return v.IsStatusConditionTrue(OnlineUpgradeInProgress)

pkg/builder/builder.go

@@ -99,7 +99,7 @@ func BuildExtSvc(nm types.NamespacedName, vdb *vapi.VerticaDB, sc *vapi.Subclust
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        nm.Name,
 			Namespace:   nm.Namespace,
-			Labels:      MakeLabelsForSvcObject(vdb, sc, "external"),
+			Labels:      MakeLabelsForSvcObject(vdb, sc, vmeta.SvcTypeExternal),
 			Annotations: MakeAnnotationsForSubclusterService(vdb, sc),
 		},
 		Spec: corev1.ServiceSpec{
@@ -121,7 +121,7 @@ func BuildHlSvc(nm types.NamespacedName, vdb *vapi.VerticaDB) *corev1.Service {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        nm.Name,
 			Namespace:   nm.Namespace,
-			Labels:      MakeLabelsForSvcObject(vdb, nil, "headless"),
+			Labels:      MakeLabelsForSvcObject(vdb, nil, vmeta.SvcTypeHeadless),
 			Annotations: MakeAnnotationsForObject(vdb),
 		},
 		Spec: corev1.ServiceSpec{
@@ -1312,7 +1312,7 @@ func BuildPod(vdb *vapi.VerticaDB, sc *vapi.Subcluster, podIndex int32) *corev1.
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        nm.Name,
 			Namespace:   nm.Namespace,
-			Labels:      MakeLabelsForSandboxPodObject(vdb, sc),
+			Labels:      MakeLabelsForPodObject(vdb, sc),
 			Annotations: MakeAnnotationsForObject(vdb),
 		},
 		Spec: buildPodSpec(vdb, sc),

pkg/builder/labels_annotations.go

@@ -16,8 +16,6 @@
 package builder
 
 import (
-	"strconv"
-
 	vapi "github.com/vertica/vertica-kubernetes/api/v1"
 	vmeta "github.com/vertica/vertica-kubernetes/pkg/meta"
 	"github.com/vertica/vertica-kubernetes/pkg/opcfg"
@@ -26,14 +24,8 @@ import (
 // MakeSubclusterLabels returns the labels added for the subcluster
 func MakeSubclusterLabels(sc *vapi.Subcluster) map[string]string {
 	m := map[string]string{
-		vmeta.SubclusterNameLabel:      sc.Name,
-		vmeta.SubclusterTypeLabel:      sc.GetType(),
-		vmeta.SubclusterTransientLabel: strconv.FormatBool(sc.IsTransient()),
-	}
-	// Transient subclusters never have the service name label set.  At various
-	// parts of the upgrade, it will accept traffic from all of the subclusters.
-	if !sc.IsTransient() {
-		m[vmeta.SubclusterSvcNameLabel] = sc.GetServiceName()
+		vmeta.SubclusterNameLabel: sc.Name,
+		vmeta.SubclusterTypeLabel: sc.GetType(),
 	}
 	return m
 }
@@ -52,8 +44,15 @@ func MakeOperatorLabels(vdb *vapi.VerticaDB) map[string]string {
 func MakeCommonLabels(vdb *vapi.VerticaDB, sc *vapi.Subcluster, forPod bool) map[string]string {
 	labels := MakeOperatorLabels(vdb)
 	// This can be overridden through 'labels' in the CR.
-	labels[vmeta.NameLabel] = "vertica"
-	if !forPod {
+	labels[vmeta.NameLabel] = vmeta.NameValue
+	if forPod {
+		labels[vmeta.SubclusterSelectorLabel] = sc.GetStatefulSetName(vdb)
+		// Transient subclusters never have the service name label set.  At various
+		// parts of the upgrade, it will accept traffic from all of the subclusters.
+		if !sc.IsTransient() {
+			labels[vmeta.SubclusterSvcNameLabel] = sc.GetServiceName()
+		}
+	} else {
 		// Apply a label to indicate a version of the operator that created the
 		// object.  This is separate from MakeOperatorLabels as we don't want to
 		// set this for pods in the template.  We set the operator version in
@@ -62,8 +61,10 @@ func MakeCommonLabels(vdb *vapi.VerticaDB, sc *vapi.Subcluster, forPod bool) map
 		labels[vmeta.OperatorVersionLabel] = vmeta.CurOperatorVersion
 	}
 
-	// Remaining labels are for objects that are subcluster specific
-	if sc == nil {
+	// Labels for pods or for non-subcluster specific objects can stop here.
+	// Pods don't have much of the subcluster labels because these can change
+	// (e.g. subcluster rename) and so are hard to maintain in the pods.
+	if sc == nil || forPod {
 		return labels
 	}
 
@@ -91,17 +92,6 @@ func MakeLabelsForPodObject(vdb *vapi.VerticaDB, sc *vapi.Subcluster) map[string
 	return makeLabelsForObject(vdb, sc, true)
 }
 
-// MakeLabelsForSandboxPodObject constructs the labels that are common for all pods plus
-// the sandbox name label. It is for testing purposes.
-func MakeLabelsForSandboxPodObject(vdb *vapi.VerticaDB, sc *vapi.Subcluster) map[string]string {
-	labels := makeLabelsForObject(vdb, sc, true)
-	sandbox := vdb.GetSubclusterSandboxName(sc.Name)
-	if sandbox != vapi.MainCluster {
-		labels[vmeta.SandboxNameLabel] = sandbox
-	}
-	return labels
-}
-
 // MakeLabelsForStsObject constructs the labels that are common for all statefulsets.
 func MakeLabelsForStsObject(vdb *vapi.VerticaDB, sc *vapi.Subcluster) map[string]string {
 	labels := makeLabelsForObject(vdb, sc, false)
@@ -172,8 +162,8 @@ func MakeSvcSelectorLabelsForServiceNameRouting(vdb *vapi.VerticaDB, sc *vapi.Su
 // we want a service object to pick the pods based on the subcluster name.
 func MakeSvcSelectorLabelsForSubclusterNameRouting(vdb *vapi.VerticaDB, sc *vapi.Subcluster) map[string]string {
 	m := MakeBaseSvcSelectorLabels(vdb)
-	// Routing is done using the subcluster name rather than the service name.
-	m[vmeta.SubclusterNameLabel] = sc.Name
+	// Routing is done using the subcluster's sts name rather than the service name.
+	m[vmeta.SubclusterSelectorLabel] = sc.GetStatefulSetName(vdb)
 	m[vmeta.ClientRoutingLabel] = vmeta.ClientRoutingVal
 
 	return m
@@ -182,7 +172,10 @@ func MakeSvcSelectorLabelsForSubclusterNameRouting(vdb *vapi.VerticaDB, sc *vapi
 // MakeStsSelectorLabels will create the selector labels for use within a StatefulSet
 func MakeStsSelectorLabels(vdb *vapi.VerticaDB, sc *vapi.Subcluster) map[string]string {
 	m := MakeBaseSvcSelectorLabels(vdb)
-	m[vmeta.SubclusterNameLabel] = sc.Name
+	// Set a special selector to pick only the pods for this statefulset. It's
+	// derived from the statefulset name as that stays constant and is unique in
+	// a namespace.
+	m[vmeta.SubclusterSelectorLabel] = sc.GetStatefulSetName(vdb)
 	return m
 }
 

pkg/controllers/vdb/dbremovesubcluster_reconciler.go

@@ -157,17 +157,17 @@ func (d *DBRemoveSubclusterReconciler) resetDefaultSubcluster(ctx context.Contex
 	_, ok := scMap[defSc]
 	if !ok {
 		scFinder := iter.MakeSubclusterFinder(d.VRec.Client, d.Vdb)
-		// We use the FindServices() API to get subclusters that already exist.
+		// We use the FindStatefulSets() API to get subclusters that already exist.
 		// We can only change the default subcluster to one of those.
-		svcs, err := scFinder.FindServices(ctx, iter.FindInVdb, vapi.MainCluster)
+		stss, err := scFinder.FindStatefulSets(ctx, iter.FindInVdb, vapi.MainCluster)
 		if err != nil {
 			return err
 		}
 		// If we don't find a service object we don't fail.  The attempt to
 		// remove the default subcluster that we do later will fail.  That
 		// provides a better error message than anything we do here.
-		if len(svcs.Items) > 0 {
-			return d.changeDefaultSubcluster(ctx, svcs.Items[0].Labels[vmeta.SubclusterNameLabel])
+		if len(stss.Items) > 0 {
+			return d.changeDefaultSubcluster(ctx, stss.Items[0].Labels[vmeta.SubclusterNameLabel])
 		}
 	}
 	return nil

pkg/controllers/vdb/drainnode_reconciler_test.go

@@ -31,12 +31,14 @@ var _ = Describe("drainnode_reconcile", func() {
 	ctx := context.Background()
 
 	It("should query sessions if pod is pending delete", func() {
+		const origSize = 2
 		vdb := vapi.MakeVDB()
 		vdb.Spec.Subclusters = []vapi.Subcluster{
-			{Name: "sc1", Size: 2},
+			{Name: "sc1", Size: origSize},
 		}
 		test.CreatePods(ctx, k8sClient, vdb, test.AllPodsRunning)
-		defer test.DeletePods(ctx, k8sClient, vdb)
+		// Restore original size prior to deletion to ensure all pods are cleaned up
+		defer func() { vdb.Spec.Subclusters[0].Size = origSize; test.DeletePods(ctx, k8sClient, vdb) }()
 		vdb.Spec.Subclusters[0].Size-- // Reduce size to make one pod pending delete
 		test.CreateVDB(ctx, k8sClient, vdb)
 		defer test.DeleteVDB(ctx, k8sClient, vdb)
@@ -50,12 +52,14 @@ var _ = Describe("drainnode_reconcile", func() {
 	})
 
 	It("should not query sessions if no pod is pending delete", func() {
+		const origSize = 2
 		vdb := vapi.MakeVDB()
 		vdb.Spec.Subclusters = []vapi.Subcluster{
-			{Name: "sc1", Size: 2},
+			{Name: "sc1", Size: origSize},
 		}
 		test.CreatePods(ctx, k8sClient, vdb, test.AllPodsRunning)
-		defer test.DeletePods(ctx, k8sClient, vdb)
+		// Restore original size prior to deletion to ensure all pods are cleaned up
+		defer func() { vdb.Spec.Subclusters[0].Size = origSize; test.DeletePods(ctx, k8sClient, vdb) }()
 		test.CreateVDB(ctx, k8sClient, vdb)
 		defer test.DeleteVDB(ctx, k8sClient, vdb)
 
@@ -68,12 +72,14 @@ var _ = Describe("drainnode_reconcile", func() {
 	})
 
 	It("should requeue if one pending delete pod has active connections", func() {
+		const origSize = 2
 		vdb := vapi.MakeVDB()
 		vdb.Spec.Subclusters = []vapi.Subcluster{
-			{Name: "sc1", Size: 2},
+			{Name: "sc1", Size: origSize},
 		}
 		test.CreatePods(ctx, k8sClient, vdb, test.AllPodsRunning)
-		defer test.DeletePods(ctx, k8sClient, vdb)
+		// Restore original size prior to deletion to ensure all pods are cleaned up
+		defer func() { vdb.Spec.Subclusters[0].Size = origSize; test.DeletePods(ctx, k8sClient, vdb) }()
 		vdb.Spec.Subclusters[0].Size-- // Reduce size to make one pod pending delete
 		test.CreateVDB(ctx, k8sClient, vdb)
 		defer test.DeleteVDB(ctx, k8sClient, vdb)

pkg/controllers/vdb/obj_reconciler_test.go

@@ -191,12 +191,12 @@ var _ = Describe("obj_reconcile", func() {
 			vdb.Spec.Labels["vertica.com/second-label"] = "r2"
 			vdb.Spec.Annotations["gitRef"] = "1234abc"
 
-			verifyLabelsAnnotations := func(objectMeta *metav1.ObjectMeta, isScSpecific bool) {
+			verifyLabelsAnnotations := func(objectMeta *metav1.ObjectMeta, isSts bool) {
 				Expect(objectMeta.Labels["my-label"]).Should(Equal("r1"))
 				Expect(objectMeta.Labels["vertica.com/second-label"]).Should(Equal("r2"))
 				Expect(objectMeta.Annotations["gitRef"]).Should(Equal("1234abc"))
 				Expect(objectMeta.Labels["vertica.com/database"]).Should(Equal(vdb.Spec.DBName))
-				if isScSpecific {
+				if isSts {
 					Expect(objectMeta.Labels[vmeta.SubclusterNameLabel]).Should(Equal(vdb.Spec.Subclusters[0].Name))
 				}
 			}
@@ -206,13 +206,13 @@ var _ = Describe("obj_reconcile", func() {
 
 			svc := &corev1.Service{}
 			Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, &vdb.Spec.Subclusters[0]), svc)).Should(Succeed())
-			verifyLabelsAnnotations(&svc.ObjectMeta, true /* subcluster specific */)
+			verifyLabelsAnnotations(&svc.ObjectMeta, false /* not a sts */)
 			Expect(k8sClient.Get(ctx, names.GenHlSvcName(vdb), svc)).Should(Succeed())
 			verifyLabelsAnnotations(&svc.ObjectMeta, false /* not subcluster specific */)
 
 			sts := &appsv1.StatefulSet{}
 			Expect(k8sClient.Get(ctx, names.GenStsName(vdb, &vdb.Spec.Subclusters[0]), sts)).Should(Succeed())
-			verifyLabelsAnnotations(&sts.ObjectMeta, true /* subcluster specific */)
+			verifyLabelsAnnotations(&sts.ObjectMeta, true /* is a sts */)
 		})
 
 		It("should update version in svc objects", func() {

pkg/controllers/vdb/onlineupgrade_reconciler.go

@@ -18,7 +18,6 @@ package vdb
 import (
 	"context"
 	"fmt"
-	"strconv"
 	"strings"
 
 	"github.com/go-logr/logr"
@@ -349,9 +348,8 @@ func (o *OnlineUpgradeReconciler) iterateSubclusterType(ctx context.Context, scT
 
 	for i := range stss.Items {
 		sts := &stss.Items[i]
-		if matches, err := o.isMatchingSubclusterType(sts, scType); err != nil {
-			return ctrl.Result{}, err
-		} else if !matches {
+		matches := o.isMatchingSubclusterType(sts, scType)
+		if !matches {
 			continue
 		}
 
@@ -426,12 +424,17 @@ func (o *OnlineUpgradeReconciler) processSecondary(ctx context.Context, sts *app
 
 // isMatchingSubclusterType will return true if the subcluster type matches the
 // input string.  Always returns false for the transient subcluster.
-func (o *OnlineUpgradeReconciler) isMatchingSubclusterType(sts *appsv1.StatefulSet, scType string) (bool, error) {
-	isTransient, err := strconv.ParseBool(sts.Labels[vmeta.SubclusterTransientLabel])
-	if err != nil {
-		return false, fmt.Errorf("could not parse label %s: %w", vmeta.SubclusterTransientLabel, err)
+func (o *OnlineUpgradeReconciler) isMatchingSubclusterType(sts *appsv1.StatefulSet, scType string) bool {
+	stsScType := sts.Labels[vmeta.SubclusterTypeLabel]
+	if stsScType != scType {
+		return false
+	}
+
+	transientName, hasTransient := o.Vdb.GetTransientSubclusterName()
+	if !hasTransient {
+		return true
 	}
-	return sts.Labels[vmeta.SubclusterTypeLabel] == scType && !isTransient, nil
+	return sts.Labels[vmeta.SubclusterNameLabel] != transientName
 }
 
 // drainSubcluster will reroute traffic away from a subcluster and wait for it to be idle.

pkg/controllers/vdb/onlineupgrade_reconciler_test.go

@@ -152,13 +152,13 @@ var _ = Describe("onlineupgrade_reconcile", func() {
 		svc := &corev1.Service{}
 		Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, sc), svc)).Should(Succeed())
 		Expect(svc.Spec.Selector[vmeta.SubclusterSvcNameLabel]).Should(Equal(""))
-		Expect(svc.Spec.Selector[vmeta.SubclusterNameLabel]).Should(Equal(TransientScName))
+		Expect(svc.Spec.Selector[vmeta.SubclusterSelectorLabel]).Should(ContainSubstring(TransientScName))
 
 		// Route back to original subcluster
 		Expect(r.routeClientTraffic(ctx, ScName, false)).Should(Succeed())
 		Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, sc), svc)).Should(Succeed())
 		Expect(svc.Spec.Selector[vmeta.SubclusterSvcNameLabel]).Should(Equal(sc.GetServiceName()))
-		Expect(svc.Spec.Selector[vmeta.SubclusterNameLabel]).Should(Equal(""))
+		Expect(svc.Spec.Selector[vmeta.SubclusterSelectorLabel]).Should(Equal(""))
 	})
 
 	It("should not route client traffic to transient subcluster since it doesn't exist", func() {
@@ -191,7 +191,7 @@ var _ = Describe("onlineupgrade_reconcile", func() {
 		svc := &corev1.Service{}
 		Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, sc), svc)).Should(Succeed())
 		Expect(svc.Spec.Selector[vmeta.SubclusterSvcNameLabel]).Should(Equal(""))
-		Expect(svc.Spec.Selector[vmeta.SubclusterNameLabel]).Should(Equal(ScName))
+		Expect(svc.Spec.Selector[vmeta.SubclusterSelectorLabel]).Should(ContainSubstring(ScName))
 		Expect(svc.Spec.Selector[vmeta.ClientRoutingLabel]).Should(Equal(vmeta.ClientRoutingVal))
 	})
 
@@ -249,19 +249,17 @@ var _ = Describe("onlineupgrade_reconcile", func() {
 		// Route for primary subcluster
 		Expect(r.routeClientTraffic(ctx, PriScName, true)).Should(Succeed())
 		Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, &vdb.Spec.Subclusters[0]), svc)).Should(Succeed())
-		Expect(svc.Spec.Selector[vmeta.SubclusterTransientLabel]).Should(Equal(""))
 		Expect(svc.Spec.Selector[vmeta.SubclusterSvcNameLabel]).Should(Equal(""))
-		Expect(svc.Spec.Selector[vmeta.SubclusterNameLabel]).Should(Equal(SecScName))
+		Expect(svc.Spec.Selector[vmeta.SubclusterSelectorLabel]).Should(ContainSubstring(SecScName))
 		Expect(r.routeClientTraffic(ctx, PriScName, false)).Should(Succeed())
 		Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, &vdb.Spec.Subclusters[0]), svc)).Should(Succeed())
-		Expect(svc.Spec.Selector[vmeta.SubclusterTransientLabel]).Should(Equal(""))
 		Expect(svc.Spec.Selector[vmeta.SubclusterSvcNameLabel]).Should(Equal(vdb.Spec.Subclusters[0].GetServiceName()))
-		Expect(svc.Spec.Selector[vmeta.SubclusterNameLabel]).Should(Equal(""))
+		Expect(svc.Spec.Selector[vmeta.SubclusterSelectorLabel]).Should(Equal(""))
 
 		// Route for secondary subcluster
 		Expect(r.routeClientTraffic(ctx, SecScName, true)).Should(Succeed())
 		Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, &vdb.Spec.Subclusters[1]), svc)).Should(Succeed())
-		Expect(svc.Spec.Selector[vmeta.SubclusterNameLabel]).Should(Equal(PriScName))
+		Expect(svc.Spec.Selector[vmeta.SubclusterSelectorLabel]).Should(ContainSubstring(PriScName))
 		Expect(r.routeClientTraffic(ctx, SecScName, false)).Should(Succeed())
 		Expect(k8sClient.Get(ctx, names.GenExtSvcName(vdb, &vdb.Spec.Subclusters[1]), svc)).Should(Succeed())
 		Expect(svc.Spec.Selector[vmeta.SubclusterSvcNameLabel]).Should(Equal(SecScName))

pkg/controllers/vdb/podfacts.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"os"
 	"sort"
-	"strconv"
 	"strings"
 	"time"
 
@@ -377,7 +376,7 @@ func (p *PodFacts) collectPodByStsIndex(ctx context.Context, vdb *vapi.VerticaDB
 		pf.dnsName = fmt.Sprintf("%s.%s.%s", pod.Spec.Hostname, pod.Spec.Subdomain, pod.Namespace)
 		pf.podIP = pod.Status.PodIP
 		pf.creationTimestamp = pod.CreationTimestamp.Format(time.DateTime)
-		pf.isTransient, _ = strconv.ParseBool(pod.Labels[vmeta.SubclusterTransientLabel])
+		pf.isTransient = sc.IsTransient()
 		pf.isPendingDelete = podIndex >= sc.Size
 		// Let's just pick the first container image
 		pf.image, err = vk8s.GetServerImage(pod.Spec.Containers)

pkg/controllers/vdb/replicatedupgrade_reconciler_test.go

@@ -430,10 +430,12 @@ var _ = Describe("replicatedupgrade_reconciler", func() {
 			Ω(found).Should(BeTrue())
 			expSbTarget, found := expectedMapping[sc.Name]
 			Ω(found).Should(BeTrue())
+			targetSc, found := scMap[expSbTarget]
+			Ω(found).Should(BeTrue())
 			svcNm := names.GenExtSvcName(vdb, sc)
 			svc := v1.Service{}
 			Ω(k8sClient.Get(ctx, svcNm, &svc)).Should(Succeed(), "svc name is %v", svcNm)
-			Ω(svc.Spec.Selector).Should(HaveKeyWithValue(vmeta.SubclusterNameLabel, expSbTarget), "svc name is %v", svcNm)
+			Ω(svc.Spec.Selector).Should(HaveKeyWithValue(vmeta.SubclusterSelectorLabel, targetSc.GetStatefulSetName(vdb)), "svc name is %v", svcNm)
 			Ω(svc.Spec.Selector).ShouldNot(HaveKey(vmeta.SubclusterSvcNameLabel), "svc name is %v", svcNm)
 		}
 	})