Merge pull request #1660 from zimnx/mz/recreate-apply-sts-1.11

[v1.11] Support update of StatefulSet selector

pkg/resourceapply/apps.go

@@ -2,9 +2,13 @@ package resourceapply
 
 import (
 	"context"
+	"fmt"
 
+	"github.com/scylladb/scylla-operator/pkg/pointer"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1"
 	appsv1listers "k8s.io/client-go/listers/apps/v1"
 	"k8s.io/client-go/tools/record"
@@ -17,7 +21,31 @@ func ApplyStatefulSetWithControl(
 	required *appsv1.StatefulSet,
 	options ApplyOptions,
 ) (*appsv1.StatefulSet, bool, error) {
-	return ApplyGeneric[*appsv1.StatefulSet](ctx, control, recorder, required, options)
+	return ApplyGenericWithHandlers[*appsv1.StatefulSet](
+		ctx,
+		control,
+		recorder,
+		required,
+		options,
+		nil,
+		func(required *appsv1.StatefulSet, existing *appsv1.StatefulSet) (string, *metav1.DeletionPropagation, error) {
+			if !equality.Semantic.DeepEqual(existing.Spec.Selector, required.Spec.Selector) {
+				existingPodLabels := existing.Spec.Template.Labels
+				requiredSelector, err := metav1.LabelSelectorAsSelector(required.Spec.Selector)
+				if err != nil {
+					return "", nil, fmt.Errorf("can't parse required StatefulSet selector: %w", err)
+				}
+
+				if !requiredSelector.Matches(labels.Set(existingPodLabels)) {
+					return "", nil, fmt.Errorf("required StatefulSet selector %q doesn't match existing Pod Labels set %v", requiredSelector, existingPodLabels)
+				}
+
+				return "spec.selector is immutable", pointer.Ptr(metav1.DeletePropagationOrphan), nil
+			}
+
+			return "", nil, nil
+		},
+	)
 }
 
 func ApplyStatefulSet(
@@ -56,11 +84,11 @@ func ApplyDaemonSetWithControl(
 		required,
 		options,
 		nil,
-		func(required *appsv1.DaemonSet, existing *appsv1.DaemonSet) string {
+		func(required *appsv1.DaemonSet, existing *appsv1.DaemonSet) (string, *metav1.DeletionPropagation, error) {
 			if !equality.Semantic.DeepEqual(existing.Spec.Selector, required.Spec.Selector) {
-				return "spec.selector is immutable"
+				return "spec.selector is immutable", nil, nil
 			}
-			return ""
+			return "", nil, nil
 		},
 	)
 }

pkg/resourceapply/apps_test.go

@@ -46,8 +46,17 @@ func TestApplyStatefulSet(t *testing.T) {
 			},
 			Spec: appsv1.StatefulSetSpec{
 				Replicas: pointer.Ptr(int32(3)),
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"foo": "bar",
+					},
+				},
 				Template: corev1.PodTemplateSpec{
-					ObjectMeta: metav1.ObjectMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
 					Spec: corev1.PodSpec{
 						Containers: []corev1.Container{
 							{
@@ -493,6 +502,93 @@ func TestApplyStatefulSet(t *testing.T) {
 			expectedErr:     nil,
 			expectedEvents:  []string{"Normal StatefulSetUpdated StatefulSet default/test updated"},
 		},
+		{
+			name: "deletes and creates the StatefulSet when selector is changed and still matches the old pods",
+			existing: []runtime.Object{
+				func() *appsv1.StatefulSet {
+					sts := newSts()
+					sts.Spec.Template.Labels = map[string]string{
+						"foo": "bar",
+						"bar": "foo",
+					}
+					sts.Spec.Selector = &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"foo": "bar",
+							"bar": "foo",
+						},
+					}
+					return sts
+				}(),
+			},
+			required: func() *appsv1.StatefulSet {
+				sts := newSts()
+				sts.Spec.Template.Labels = map[string]string{
+					"foo": "bar",
+					"bar": "foo",
+				}
+				sts.Spec.Selector = &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"foo": "bar",
+					},
+				}
+				return sts
+			}(),
+			expectedSts: func() *appsv1.StatefulSet {
+				sts := newSts()
+				sts.Spec.Template.Labels = map[string]string{
+					"foo": "bar",
+					"bar": "foo",
+				}
+				sts.Spec.Selector = &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"foo": "bar",
+					},
+				}
+				utilruntime.Must(SetHashAnnotation(sts))
+				return sts
+			}(),
+			expectedChanged: true,
+			expectedErr:     nil,
+			expectedEvents: []string{
+				"Normal StatefulSetDeleted StatefulSet default/test deleted",
+				"Normal StatefulSetCreated StatefulSet default/test created",
+			},
+		},
+		{
+			name: "apply fails when StatefulSet selector differs and existing Pod labels doesn't match new selector",
+			existing: []runtime.Object{
+				func() *appsv1.StatefulSet {
+					sts := newSts()
+					sts.Spec.Template.Labels = map[string]string{
+						"foo": "bar",
+					}
+					sts.Spec.Selector = &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"foo": "bar",
+						},
+					}
+					return sts
+				}(),
+			},
+			required: func() *appsv1.StatefulSet {
+				sts := newSts()
+				sts.Spec.Template.Labels = map[string]string{
+					"foo": "bar",
+					"bar": "foo",
+				}
+				sts.Spec.Selector = &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"foo": "bar",
+						"bar": "foo",
+					},
+				}
+				return sts
+			}(),
+			expectedSts:     nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf("can't get recreate reason: %w", fmt.Errorf(`required StatefulSet selector "bar=foo,foo=bar" doesn't match existing Pod Labels set map[foo:bar]`)),
+			expectedEvents:  nil,
+		},
 	}
 
 	for _, tc := range tt {

pkg/resourceapply/batch.go

@@ -5,6 +5,7 @@ import (
 
 	batchv1 "k8s.io/api/batch/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	batchv1client "k8s.io/client-go/kubernetes/typed/batch/v1"
 	batchv1listers "k8s.io/client-go/listers/batch/v1"
 	"k8s.io/client-go/tools/record"
@@ -24,23 +25,23 @@ func ApplyJobWithControl(
 		required,
 		options,
 		nil,
-		func(required *batchv1.Job, existing *batchv1.Job) string {
+		func(required *batchv1.Job, existing *batchv1.Job) (string, *metav1.DeletionPropagation, error) {
 			if !equality.Semantic.DeepEqual(existing.Spec.Completions, required.Spec.Completions) {
-				return "spec.completions is immutable"
+				return "spec.completions is immutable", nil, nil
 			}
 			if !equality.Semantic.DeepEqual(existing.Spec.Selector, required.Spec.Selector) {
-				return "spec.selector is immutable"
+				return "spec.selector is immutable", nil, nil
 			}
 			if !equality.Semantic.DeepEqual(existing.Spec.Template, required.Spec.Template) {
-				return "spec.template is immutable"
+				return "spec.template is immutable", nil, nil
 			}
 			if !equality.Semantic.DeepEqual(existing.Spec.CompletionMode, required.Spec.CompletionMode) {
-				return "spec.completionMode is immutable"
+				return "spec.completionMode is immutable", nil, nil
 			}
 			if !equality.Semantic.DeepEqual(existing.Spec.PodFailurePolicy, required.Spec.PodFailurePolicy) {
-				return "spec.podFailurePolicy is immutable"
+				return "spec.podFailurePolicy is immutable", nil, nil
 			}
-			return ""
+			return "", nil, nil
 		},
 	)
 }

pkg/resourceapply/helpers.go

@@ -244,7 +244,7 @@ func ApplyGenericWithHandlers[T kubeinterfaces.ObjectInterface](
 	required T,
 	options ApplyOptions,
 	projectFunc func(required *T, existing T),
-	getRecreateReasonFunc func(required T, existing T) string,
+	getRecreateReasonFunc func(required T, existing T) (string, *metav1.DeletionPropagation, error),
 ) (T, bool, error) {
 	gvk := resource.GetObjectGVKOrUnknown(required)
 
@@ -315,8 +315,12 @@ func ApplyGenericWithHandlers[T kubeinterfaces.ObjectInterface](
 	}
 
 	var recreateReason string
+	var propagationPolicy *metav1.DeletionPropagation
 	if getRecreateReasonFunc != nil {
-		recreateReason = getRecreateReasonFunc(requiredCopy, existing)
+		recreateReason, propagationPolicy, err = getRecreateReasonFunc(requiredCopy, existing)
+		if err != nil {
+			return *new(T), false, fmt.Errorf("can't get recreate reason: %w", err)
+		}
 	}
 	if len(recreateReason) > 0 {
 		klog.V(2).InfoS(
@@ -326,9 +330,12 @@ func ApplyGenericWithHandlers[T kubeinterfaces.ObjectInterface](
 			"Ref", naming.ObjRefWithUID(existing),
 		)
 
-		propagationPolicy := metav1.DeletePropagationBackground
+		if propagationPolicy == nil {
+			propagationPolicy = pointer.Ptr(metav1.DeletePropagationBackground)
+		}
+
 		err := control.Delete(ctx, existing.GetName(), metav1.DeleteOptions{
-			PropagationPolicy: &propagationPolicy,
+			PropagationPolicy: propagationPolicy,
 		})
 		ReportDeleteEvent(recorder, existing, err)
 		if err != nil {

pkg/resourceapply/rbac.go

@@ -5,6 +5,7 @@ import (
 
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	rbacv1client "k8s.io/client-go/kubernetes/typed/rbac/v1"
 	rbacv1listers "k8s.io/client-go/listers/rbac/v1"
 	"k8s.io/client-go/tools/record"
@@ -56,11 +57,11 @@ func ApplyRoleBindingWithControl(
 		required,
 		options,
 		nil,
-		func(required *rbacv1.RoleBinding, existing *rbacv1.RoleBinding) string {
+		func(required *rbacv1.RoleBinding, existing *rbacv1.RoleBinding) (string, *metav1.DeletionPropagation, error) {
 			if !equality.Semantic.DeepEqual(existing.RoleRef, (*required).RoleRef) {
-				return "roleRef is immutable"
+				return "roleRef is immutable", nil, nil
 			}
-			return ""
+			return "", nil, nil
 		},
 	)
 }
@@ -101,11 +102,11 @@ func ApplyClusterRoleBindingWithControl(
 		required,
 		options,
 		nil,
-		func(required *rbacv1.ClusterRoleBinding, existing *rbacv1.ClusterRoleBinding) string {
+		func(required *rbacv1.ClusterRoleBinding, existing *rbacv1.ClusterRoleBinding) (string, *metav1.DeletionPropagation, error) {
 			if !equality.Semantic.DeepEqual(existing.RoleRef, (*required).RoleRef) {
-				return "roleRef is immutable"
+				return "roleRef is immutable", nil, nil
 			}
-			return ""
+			return "", nil, nil
 		},
 	)
 }