Add option to provide custom logic to determine whether an error mess…

…age (#82) indicates immutable fields have been modified. This allows detecting immutable field changes for StatefulSets, whose relevant error messages have different wording. Signed-off-by: Joe Kralicky <[email protected]> Co-authored-by: Peter Wilcsinszky <[email protected]>
cisco-open · Oct 18, 2021 · 367e967 · 367e967
1 parent 709df2b
commit 367e967
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 8 deletions.
diff --git a/pkg/reconciler/resource.go b/pkg/reconciler/resource.go
@@ -136,6 +136,8 @@ type ResourceReconcilerOption func(*ReconcilerOpts)
 
 type RecreateResourceCondition func(kind schema.GroupVersionKind, status metav1.Status) bool
 
+type ErrorMessageCondition func(string) bool
+
 // Recommended to use NewReconcilerWith + ResourceReconcilerOptions
 type ReconcilerOpts struct {
 	Log    logr.Logger
@@ -154,6 +156,20 @@ type ReconcilerOpts struct {
 	RecreatePropagationPolicy client.PropagationPolicy
 	// Check the update error message contains this substring before recreate. Default: "immutable"
 	RecreateErrorMessageSubstring *string
+	// Custom logic to decide if an error message indicates a resource should be recreated.
+	// Takes precedence over RecreateErrorMessageSubstring if set.
+	RecreateErrorMessageCondition ErrorMessageCondition
+}
+
+func MatchImmutableErrorMessages(errorMessage string) bool {
+	if strings.Contains(errorMessage, "immutable") {
+		return true
+	}
+	// StatefulSet is a special case because it has a different error message
+	if strings.Contains(errorMessage, "updates to statefulset spec for fields other than") {
+		return true
+	}
+	return false
 }
 
 // NewGenericReconciler returns GenericResourceReconciler
@@ -255,6 +271,13 @@ func WithRecreateErrorMessageSubstring(substring string) ResourceReconcilerOptio
 	}
 }
 
+// Recreate only if the error message contains the given substring
+func WithRecreateErrorMessageCondition(condition ErrorMessageCondition) ResourceReconcilerOption {
+	return func(o *ReconcilerOpts) {
+		o.RecreateErrorMessageCondition = condition
+	}
+}
+
 // Disable checking the error message before recreating resources
 func WithRecreateErrorMessageIgnored() ResourceReconcilerOption {
 	return func(o *ReconcilerOpts) {
@@ -281,6 +304,15 @@ func (r *GenericResourceReconciler) CreateResource(desired runtime.Object) error
 	return err
 }
 
+func (r *GenericResourceReconciler) shouldRecreate(sErr *apierrors.StatusError) bool {
+	// If a condition function is set, use it
+	if r.Options.RecreateErrorMessageCondition != nil {
+		return r.Options.RecreateErrorMessageCondition(sErr.ErrStatus.Message)
+	}
+	// Fall back to substring matching
+	return strings.Contains(sErr.ErrStatus.Message, utils.PointerToString(r.Options.RecreateErrorMessageSubstring))
+}
+
 // ReconcileResource reconciles various kubernetes types
 func (r *GenericResourceReconciler) ReconcileResource(desired runtime.Object, desiredState DesiredState) (*reconcile.Result, error) {
 	resourceDetails, gvk, err := r.resourceDetails(desired)
@@ -398,9 +430,7 @@ func (r *GenericResourceReconciler) ReconcileResource(desired runtime.Object, de
 		}
 		if err := r.Client.Update(context.TODO(), desired.(client.Object), updateOptions...); err != nil {
 			sErr, ok := err.(*apierrors.StatusError)
-			if ok && sErr.ErrStatus.Code == 422 && sErr.ErrStatus.Reason == metav1.StatusReasonInvalid &&
-				// Check the actual error message
-				strings.Contains(sErr.ErrStatus.Message, utils.PointerToString(r.Options.RecreateErrorMessageSubstring)) {
+			if ok && (sErr.ErrStatus.Code == 422 && sErr.ErrStatus.Reason == metav1.StatusReasonInvalid) && r.shouldRecreate(sErr) {
 				if r.Options.EnableRecreateWorkloadOnImmutableFieldChange {
 					if !r.Options.RecreateEnabledResourceCondition(gvk, sErr.ErrStatus) {
 						return nil, errors.WrapIfWithDetails(err, "resource type is not allowed to be recreated", resourceDetails...)

diff --git a/pkg/reconciler/resource_test.go b/pkg/reconciler/resource_test.go
@@ -22,6 +22,7 @@ import (
 	"github.com/banzaicloud/operator-tools/pkg/utils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -92,11 +93,11 @@ func TestNewReconcilerWithUnstructured(t *testing.T) {
 
 func TestRecreateObjectFailIfNotAllowed(t *testing.T) {
 	testData := []struct {
-		name string
-		desired runtime.Object
+		name       string
+		desired    runtime.Object
 		reconciler reconciler.ResourceReconciler
-		update func(object runtime.Object) runtime.Object
-		wantError func(error)
+		update     func(object runtime.Object) runtime.Object
+		wantError  func(error)
 		wantResult func(result *reconcile.Result)
 	}{
 		{
@@ -191,6 +192,57 @@ func TestRecreateObjectFailIfNotAllowed(t *testing.T) {
 				require.Equal(t, svc.Spec.ClusterIP, "10.0.0.32")
 			},
 		},
+		{
+			name: "recreate statefulset",
+			desired: &appsv1.StatefulSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: testNamespace,
+				},
+				Spec: appsv1.StatefulSetSpec{
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app": "test",
+						},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{
+								"app": "test",
+							},
+						},
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{
+								{
+									Name:  "test",
+									Image: "test",
+								},
+							},
+						},
+					},
+					ServiceName: "test",
+				},
+			},
+			reconciler: reconciler.NewReconcilerWith(k8sClient,
+				reconciler.WithEnableRecreateWorkload(),
+				reconciler.WithRecreateErrorMessageCondition(reconciler.MatchImmutableErrorMessages),
+				reconciler.WithRecreateImmediately(),
+			),
+			update: func(object runtime.Object) runtime.Object {
+				object.(*appsv1.StatefulSet).Spec.ServiceName = "test2"
+				return object
+			},
+			wantResult: func(result *reconcile.Result) {
+				require.Nil(t, result)
+				statefulSet := &appsv1.StatefulSet{}
+				err := k8sClient.Get(context.TODO(), types.NamespacedName{
+					Namespace: testNamespace,
+					Name:      "test",
+				}, statefulSet)
+				require.NoError(t, err)
+				require.Equal(t, statefulSet.Spec.ServiceName, "test2")
+			},
+		},
 	}
 
 	for _, tt := range testData {
@@ -213,4 +265,3 @@ func TestRecreateObjectFailIfNotAllowed(t *testing.T) {
 		})
 	}
 }
-