Skip to content

Commit

Permalink
Support update of StatefulSet selector
Browse files Browse the repository at this point in the history 
When Pods managed by existing StatefulSet matches new selector, StatefulSet
is orphan deleted and recreated.
  • Loading branch information
@zimnx
zimnx committed Dec 22, 2023
1 parent 1abde59 commit 32b1c43
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 22 deletions.
36 changes: 32 additions & 4 deletions pkg/resourceapply/apps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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(
Expand Down Expand Up @@ -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
},
)
}
Expand Down
98 changes: 97 additions & 1 deletion pkg/resourceapply/apps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 8 additions & 7 deletions pkg/resourceapply/batch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
},
)
}
Expand Down
15 changes: 11 additions & 4 deletions pkg/resourceapply/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down
13 changes: 7 additions & 6 deletions pkg/resourceapply/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
},
)
}
Expand Down Expand Up @@ -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
},
)
}
Expand Down

0 comments on commit 32b1c43

Please sign in to comment.