Merge pull request #1643 from zimnx/mz/rollback-statefulset-selector-…

…1.11 [v1.11] Rollback StatefulSet selector
scylladb · Dec 14, 2023 · f5e3ea2 · f5e3ea2
2 parents a6c60f6 + 6615971
commit f5e3ea2
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 41 deletions.
diff --git a/pkg/controller/scyllacluster/resource.go b/pkg/controller/scyllacluster/resource.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"path"
 	"sort"
+	"strconv"
 	"strings"
 
 	scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1"
@@ -214,14 +215,24 @@ func servicePorts(cluster *scyllav1.ScyllaCluster) []corev1.ServicePort {
 
 // StatefulSetForRack make a StatefulSet for the rack.
 // existingSts may be nil if it doesn't exist yet.
-func StatefulSetForRack(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster, existingSts *appsv1.StatefulSet, sidecarImage string) (*appsv1.StatefulSet, error) {
-	rackLabels, err := naming.RackLabels(r, c)
+func StatefulSetForRack(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster, existingSts *appsv1.StatefulSet, sidecarImage string, rackOrdinal int) (*appsv1.StatefulSet, error) {
+	rackLabels, err := naming.RackSelectorLabels(r, c)
 	if err != nil {
 		return nil, fmt.Errorf("can't get rack labels: %w", err)
 	}
-	matchLabels := helpers.ShallowCopyMap(rackLabels)
+	rackLabels[naming.RackOrdinalLabel] = strconv.Itoa(rackOrdinal)
 	rackLabels[naming.ScyllaVersionLabel] = c.Spec.Version
 
+	selectorLabels, err := naming.RackSelectorLabels(r, c)
+	if err != nil {
+		return nil, fmt.Errorf("can't get selector labels: %w", err)
+	}
+
+	pvcLabels, err := naming.RackSelectorLabels(r, c)
+	if err != nil {
+		return nil, fmt.Errorf("can't get PVC Template labels: %w", err)
+	}
+
 	placement := r.Placement
 	if placement == nil {
 		placement = &scyllav1.PlacementSpec{}
@@ -247,7 +258,7 @@ func StatefulSetForRack(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster, existing
 			// Use a common Headless Service for all StatefulSets
 			ServiceName: naming.HeadlessServiceNameForCluster(c),
 			Selector: &metav1.LabelSelector{
-				MatchLabels: matchLabels,
+				MatchLabels: selectorLabels,
 			},
 			PodManagementPolicy: appsv1.OrderedReadyPodManagement,
 			UpdateStrategy: appsv1.StatefulSetUpdateStrategy{
@@ -569,7 +580,7 @@ func StatefulSetForRack(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster, existing
 				{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:   naming.PVCTemplateName,
-						Labels: matchLabels,
+						Labels: pvcLabels,
 					},
 					Spec: corev1.PersistentVolumeClaimSpec{
 						AccessModes:      []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},

diff --git a/pkg/controller/scyllacluster/resource_test.go b/pkg/controller/scyllacluster/resource_test.go
@@ -579,29 +579,24 @@ func TestStatefulSetForRack(t *testing.T) {
 		}
 	}
 
-	newBasicStatefulSetLabels := func() map[string]string {
+	newBasicStatefulSetLabels := func(ordinal int) map[string]string {
 		return map[string]string{
 			"app":                          "scylla",
 			"app.kubernetes.io/managed-by": "scylla-operator",
 			"app.kubernetes.io/name":       "scylla",
 			"scylla/cluster":               "basic",
 			"scylla/datacenter":            "dc",
 			"scylla/rack":                  "rack",
-			"scylla/rack-ordinal":          "0",
+			"scylla/scylla-version":        "",
+			"scylla/rack-ordinal":          fmt.Sprintf("%d", ordinal),
 		}
 	}
 
-	newBasicStatefulSetLabelsWithVersion := func() map[string]string {
-		m := newBasicStatefulSetLabels()
-		m["scylla/scylla-version"] = ""
-		return m
-	}
-
 	newBasicStatefulSet := func() *appsv1.StatefulSet {
 		return &appsv1.StatefulSet{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:        "basic-dc-rack",
-				Labels:      newBasicStatefulSetLabelsWithVersion(),
+				Labels:      newBasicStatefulSetLabels(0),
 				Annotations: nil,
 				OwnerReferences: []metav1.OwnerReference{
 					{
@@ -617,11 +612,18 @@ func TestStatefulSetForRack(t *testing.T) {
 			Spec: appsv1.StatefulSetSpec{
 				Replicas: pointer.Ptr(int32(0)),
 				Selector: &metav1.LabelSelector{
-					MatchLabels: newBasicStatefulSetLabels(),
+					MatchLabels: map[string]string{
+						"app":                          "scylla",
+						"app.kubernetes.io/managed-by": "scylla-operator",
+						"app.kubernetes.io/name":       "scylla",
+						"scylla/cluster":               "basic",
+						"scylla/datacenter":            "dc",
+						"scylla/rack":                  "rack",
+					},
 				},
 				Template: corev1.PodTemplateSpec{
 					ObjectMeta: metav1.ObjectMeta{
-						Labels: newBasicStatefulSetLabelsWithVersion(),
+						Labels: newBasicStatefulSetLabels(0),
 						Annotations: map[string]string{
 							"prometheus.io/port":   "9180",
 							"prometheus.io/scrape": "true",
@@ -961,8 +963,15 @@ func TestStatefulSetForRack(t *testing.T) {
 				VolumeClaimTemplates: []corev1.PersistentVolumeClaim{
 					{
 						ObjectMeta: metav1.ObjectMeta{
-							Name:   "data",
-							Labels: newBasicStatefulSetLabels(),
+							Name: "data",
+							Labels: map[string]string{
+								"app":                          "scylla",
+								"app.kubernetes.io/managed-by": "scylla-operator",
+								"app.kubernetes.io/name":       "scylla",
+								"scylla/cluster":               "basic",
+								"scylla/datacenter":            "dc",
+								"scylla/rack":                  "rack",
+							},
 						},
 						Spec: corev1.PersistentVolumeClaimSpec{
 							AccessModes:      []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
@@ -1216,7 +1225,7 @@ func TestStatefulSetForRack(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			got, err := StatefulSetForRack(tc.rack, tc.scyllaCluster, tc.existingStatefulSet, "scylladb/scylla-operator:latest")
+			got, err := StatefulSetForRack(tc.rack, tc.scyllaCluster, tc.existingStatefulSet, "scylladb/scylla-operator:latest", 0)
 
 			if !reflect.DeepEqual(err, tc.expectedError) {
 				t.Fatalf("expected and actual errors differ: %s",

diff --git a/pkg/controller/scyllacluster/sync_statefulsets.go b/pkg/controller/scyllacluster/sync_statefulsets.go
@@ -49,9 +49,9 @@ func snapshotTag(prefix string, t time.Time) string {
 
 func (scc *Controller) makeRacks(sc *scyllav1.ScyllaCluster, statefulSets map[string]*appsv1.StatefulSet) ([]*appsv1.StatefulSet, error) {
 	sets := make([]*appsv1.StatefulSet, 0, len(sc.Spec.Datacenter.Racks))
-	for _, rack := range sc.Spec.Datacenter.Racks {
+	for i, rack := range sc.Spec.Datacenter.Racks {
 		oldSts := statefulSets[naming.StatefulSetNameForRack(rack, sc)]
-		sts, err := StatefulSetForRack(rack, sc, oldSts, scc.operatorImage)
+		sts, err := StatefulSetForRack(rack, sc, oldSts, scc.operatorImage, i)
 		if err != nil {
 			return nil, err
 		}

diff --git a/pkg/helpers/collections.go b/pkg/helpers/collections.go
@@ -18,11 +18,3 @@ func GetMapValues[M ~map[K]V, K comparable, V any](m M) []V {
 	}
 	return res
 }
-
-func ShallowCopyMap[M ~map[K]V, K comparable, V any](m M) M {
-	res := make(M, len(m))
-	for k, v := range m {
-		res[k] = v
-	}
-	return res
-}
diff --git a/pkg/naming/labels.go b/pkg/naming/labels.go
@@ -2,10 +2,8 @@ package naming
 
 import (
 	"fmt"
-	"strconv"
 
 	scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1"
-	"github.com/scylladb/scylla-operator/pkg/helpers/slices"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/labels"
 )
@@ -33,19 +31,13 @@ func DatacenterLabels(c *scyllav1.ScyllaCluster) map[string]string {
 	return mergeLabels(dcLabels, recLabels)
 }
 
-// RackLabels returns a map of label keys and values
+// RackSelectorLabels returns a map of label keys and values
 // for the given Rack.
-func RackLabels(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster) (map[string]string, error) {
+// Labels set cannot be changed.
+func RackSelectorLabels(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster) (map[string]string, error) {
 	recLabels := ScyllaLabels()
 	rackLabels := DatacenterLabels(c)
 	rackLabels[RackNameLabel] = r.Name
-	_, rackOrdinal, ok := slices.Find(c.Spec.Datacenter.Racks, func(rack scyllav1.RackSpec) bool {
-		return rack.Name == r.Name
-	})
-	if !ok {
-		return nil, fmt.Errorf("can't find ordinal of rack %q in ScyllaCluster %q", r.Name, ObjRef(c))
-	}
-	rackLabels[RackOrdinalLabel] = strconv.Itoa(rackOrdinal)
 
 	return mergeLabels(rackLabels, recLabels), nil
 }
@@ -60,7 +52,7 @@ func StatefulSetPodLabel(name string) map[string]string {
 
 // RackSelector returns a LabelSelector for the given rack.
 func RackSelector(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster) (labels.Selector, error) {
-	rackLabels, err := RackLabels(r, c)
+	rackLabels, err := RackSelectorLabels(r, c)
 	if err != nil {
 		return nil, fmt.Errorf("can't get rack labels: %w", err)
 	}