Repository: scylladb/scylla-operator
Pull request #673 (2021-07-02 15:51:30)

Fix operator upgrade bugs
**Description of your changes:**
Creating the ScyllaCluster with scylla-operator v1.3 and updating the scylla-operator uncovered few corner cases like objects with invalid selectors or a nil segfault.

v1.3 and previous versions didn't set selectors correctly for some cases. Given the Three laws of controllers v1.4/master (that respects them) had to release those objects. That got us to a state where we couldn't update them because we don't own them and we can't adopt them because they don't match our selector.

`forceOwnership` flag allows to migrate from such state and we'll switch it back to `false` in v1.5 (https://github.com/scylladb/scylla-operator/issues/672).


Patch:
diff --git a/deploy/operator/00_clusterrole_def.yaml b/deploy/operator/00_clusterrole_def.yaml
index d7af22f50..26f5d88a8 100644
--- a/deploy/operator/00_clusterrole_def.yaml
+++ b/deploy/operator/00_clusterrole_def.yaml
@@ -66,6 +66,7 @@ rules:
   - watch
   - create
   - update
+  - patch
 - apiGroups:
   - ""
   resources:
diff --git a/examples/common/operator.yaml b/examples/common/operator.yaml
index 560e51355..b40f0e592 100644
--- a/examples/common/operator.yaml
+++ b/examples/common/operator.yaml
@@ -77,6 +77,7 @@ rules:
   - watch
   - create
   - update
+  - patch
 - apiGroups:
   - ""
   resources:
diff --git a/helm/scylla-operator/templates/clusterrole_def.yaml b/helm/scylla-operator/templates/clusterrole_def.yaml
index d7af22f50..26f5d88a8 100644
--- a/helm/scylla-operator/templates/clusterrole_def.yaml
+++ b/helm/scylla-operator/templates/clusterrole_def.yaml
@@ -66,6 +66,7 @@ rules:
   - watch
   - create
   - update
+  - patch
 - apiGroups:
   - ""
   resources:
diff --git a/pkg/controller/helpers/apps.go b/pkg/controller/helpers/apps.go
index 43d1a009b..ae4beb072 100644
--- a/pkg/controller/helpers/apps.go
+++ b/pkg/controller/helpers/apps.go
@@ -16,11 +16,6 @@ func IsStatefulSetRolledOut(sts *appsv1.StatefulSet) (bool, error) {
 		return false, fmt.Errorf("statefulset.spec.replicas can't be nil")
 	}
 
-	if sts.Spec.UpdateStrategy.RollingUpdate == nil {
-		// This should never happen, but better safe then sorry.
-		return false, fmt.Errorf("statefulset.spec.updatestrategy.rollingupdate can't be nil")
-	}
-
 	if sts.Status.ObservedGeneration == 0 || sts.Generation > sts.Status.ObservedGeneration {
 		return false, nil
 	}
@@ -29,7 +24,7 @@ func IsStatefulSetRolledOut(sts *appsv1.StatefulSet) (bool, error) {
 		return false, nil
 	}
 
-	if sts.Spec.UpdateStrategy.RollingUpdate.Partition != nil {
+	if sts.Spec.UpdateStrategy.RollingUpdate != nil && sts.Spec.UpdateStrategy.RollingUpdate.Partition != nil {
 		return sts.Status.UpdatedReplicas == (*sts.Spec.Replicas - *sts.Spec.UpdateStrategy.RollingUpdate.Partition), nil
 	} else {
 		return sts.Status.UpdateRevision == sts.Status.CurrentRevision, nil
diff --git a/pkg/controller/scyllacluster/resource/resource.go b/pkg/controller/scyllacluster/resource/resource.go
index 922cf5f2e..a21e742db 100644
--- a/pkg/controller/scyllacluster/resource/resource.go
+++ b/pkg/controller/scyllacluster/resource/resource.go
@@ -413,8 +413,10 @@ func StatefulSetForRack(r scyllav1.RackSpec, c *scyllav1.ScyllaCluster, existing
 	if existingSts != nil {
 		sts.ResourceVersion = existingSts.ResourceVersion
 		if sts.Spec.UpdateStrategy.Type == appsv1.RollingUpdateStatefulSetStrategyType &&
-			existingSts.Spec.UpdateStrategy.Type == appsv1.RollingUpdateStatefulSetStrategyType {
-			sts.Spec.UpdateStrategy.RollingUpdate.Partition = existingSts.Spec.UpdateStrategy.RollingUpdate.Partition
+			existingSts.Spec.UpdateStrategy.Type == appsv1.RollingUpdateStatefulSetStrategyType &&
+			existingSts.Spec.UpdateStrategy.RollingUpdate != nil &&
+			existingSts.Spec.UpdateStrategy.RollingUpdate.Partition != nil {
+			*sts.Spec.UpdateStrategy.RollingUpdate.Partition = *existingSts.Spec.UpdateStrategy.RollingUpdate.Partition
 		}
 	}
 
diff --git a/pkg/controller/scyllacluster/sync_agent_config.go b/pkg/controller/scyllacluster/sync_agent_config.go
index 34cdd238e..fd439e52d 100644
--- a/pkg/controller/scyllacluster/sync_agent_config.go
+++ b/pkg/controller/scyllacluster/sync_agent_config.go
@@ -65,7 +65,8 @@ func (scc *Controller) syncAgentToken(
 		return status, fmt.Errorf("can't make auth token secret: %w", err)
 	}
 
-	_, _, err = resourceapply.ApplySecret(ctx, scc.kubeClient.CoreV1(), scc.secretLister, scc.eventRecorder, secret)
+	// TODO: Remove forced ownership in v1.5 (#672)
+	_, _, err = resourceapply.ApplySecret(ctx, scc.kubeClient.CoreV1(), scc.secretLister, scc.eventRecorder, secret, true)
 	if err != nil {
 		return status, fmt.Errorf("can't apply secret %q: %w", naming.ObjRef(secret), err)
 	}
diff --git a/pkg/controller/scyllacluster/sync_pdb.go b/pkg/controller/scyllacluster/sync_pdb.go
index 34c344b9d..5eb6e2035 100644
--- a/pkg/controller/scyllacluster/sync_pdb.go
+++ b/pkg/controller/scyllacluster/sync_pdb.go
@@ -48,7 +48,8 @@ func (scc *Controller) syncPodDisruptionBudgets(
 		return status, fmt.Errorf("can't delete pdb(s): %w", err)
 	}
 
-	_, _, err = resourceapply.ApplyPodDisruptionBudget(ctx, scc.kubeClient.PolicyV1beta1(), scc.pdbLister, scc.eventRecorder, requiredPDB)
+	// TODO: Remove forced ownership in v1.5 (#672)
+	_, _, err = resourceapply.ApplyPodDisruptionBudget(ctx, scc.kubeClient.PolicyV1beta1(), scc.pdbLister, scc.eventRecorder, requiredPDB, true)
 	if err != nil {
 		return status, fmt.Errorf("can't apply pdb: %w", err)
 	}
diff --git a/pkg/controller/scyllacluster/sync_services.go b/pkg/controller/scyllacluster/sync_services.go
index e11491e32..f672c69c5 100644
--- a/pkg/controller/scyllacluster/sync_services.go
+++ b/pkg/controller/scyllacluster/sync_services.go
@@ -125,7 +125,7 @@ func (scc *Controller) syncServices(
 
 	// We need to first propagate ReplaceAddressFirstBoot from status for the new service.
 	for _, svc := range requiredServices {
-		_, _, err = resourceapply.ApplyService(ctx, scc.kubeClient.CoreV1(), scc.serviceLister, scc.eventRecorder, svc)
+		_, _, err = resourceapply.ApplyService(ctx, scc.kubeClient.CoreV1(), scc.serviceLister, scc.eventRecorder, svc, false)
 		if err != nil {
 			return status, err
 		}
diff --git a/pkg/controller/scyllacluster/sync_statefulsets.go b/pkg/controller/scyllacluster/sync_statefulsets.go
index 255c1e539..25d55c18d 100644
--- a/pkg/controller/scyllacluster/sync_statefulsets.go
+++ b/pkg/controller/scyllacluster/sync_statefulsets.go
@@ -353,7 +353,7 @@ func (scc *Controller) createMissingStatefulSets(
 		_, found := statefulSets[sts.Name]
 		if !found {
 			klog.V(2).InfoS("Creating missing StatefulSet", "StatefulSet", klog.KObj(sts))
-			_, changed, err := resourceapply.ApplyStatefulSet(ctx, scc.kubeClient.AppsV1(), scc.statefulSetLister, scc.eventRecorder, sts)
+			_, changed, err := resourceapply.ApplyStatefulSet(ctx, scc.kubeClient.AppsV1(), scc.statefulSetLister, scc.eventRecorder, sts, false)
 			if err != nil {
 				errs = append(errs, fmt.Errorf("can't create missing statefulset: %w", err))
 				continue
@@ -550,7 +550,7 @@ func (scc *Controller) syncStatefulSets(
 				required.Spec.Replicas = pointer.Int32Ptr(*existing.Spec.Replicas)
 				required.Spec.UpdateStrategy.RollingUpdate.Partition = pointer.Int32Ptr(*existing.Spec.Replicas)
 				// Use apply to also update the spec.template
-				_, _, err := resourceapply.ApplyStatefulSet(ctx, scc.kubeClient.AppsV1(), scc.statefulSetLister, scc.eventRecorder, required)
+				_, _, err := resourceapply.ApplyStatefulSet(ctx, scc.kubeClient.AppsV1(), scc.statefulSetLister, scc.eventRecorder, required, false)
 				if err != nil {
 					errs = append(errs, fmt.Errorf("can't apply statefulset to set partition: %w", err))
 				}
@@ -708,7 +708,7 @@ func (scc *Controller) syncStatefulSets(
 			}
 		}
 
-		sts, _, err := resourceapply.ApplyStatefulSet(ctx, scc.kubeClient.AppsV1(), scc.statefulSetLister, scc.eventRecorder, required)
+		sts, _, err := resourceapply.ApplyStatefulSet(ctx, scc.kubeClient.AppsV1(), scc.statefulSetLister, scc.eventRecorder, required, false)
 		if err != nil {
 			return status, fmt.Errorf("can't apply statefulset update: %w", err)
 		}
diff --git a/pkg/resourceapply/apps.go b/pkg/resourceapply/apps.go
index c1f820501..827783a47 100644
--- a/pkg/resourceapply/apps.go
+++ b/pkg/resourceapply/apps.go
@@ -15,12 +15,16 @@ import (
 	"k8s.io/klog/v2"
 )
 
+// ApplyStatefulSet will apply the StatefulSet to match the required object.
+// forceOwnership allows to apply objects without an ownerReference. Normally such objects
+// would be adopted but the old objects may not have correct labels that we need to fix in the new version.
 func ApplyStatefulSet(
 	ctx context.Context,
 	client appv1client.StatefulSetsGetter,
 	lister appv1listers.StatefulSetLister,
 	recorder record.EventRecorder,
 	required *appsv1.StatefulSet,
+	forceOwnership bool,
 ) (*appsv1.StatefulSet, bool, error) {
 	requiredControllerRef := metav1.GetControllerOfNoCopy(required)
 	if requiredControllerRef == nil {
@@ -50,10 +54,14 @@ func ApplyStatefulSet(
 
 	existingControllerRef := metav1.GetControllerOfNoCopy(existing)
 	if !equality.Semantic.DeepEqual(existingControllerRef, requiredControllerRef) {
-		// This is not the place to handle adoption.
-		err := fmt.Errorf("statefulset %q isn't controlled by us", naming.ObjRef(requiredCopy))
-		ReportUpdateEvent(recorder, requiredCopy, err)
-		return nil, false, err
+		if existingControllerRef == nil && forceOwnership {
+			klog.V(2).InfoS("Forcing apply to claim the StatefulSet", "StatefulSet", naming.ObjRef(requiredCopy))
+		} else {
+			// This is not the place to handle adoption.
+			err := fmt.Errorf("statefulset %q isn't controlled by us", naming.ObjRef(requiredCopy))
+			ReportUpdateEvent(recorder, requiredCopy, err)
+			return nil, false, err
+		}
 	}
 
 	existingHash := existing.Annotations[naming.ManagedHash]
diff --git a/pkg/resourceapply/core.go b/pkg/resourceapply/core.go
index 894398c90..44822f107 100644
--- a/pkg/resourceapply/core.go
+++ b/pkg/resourceapply/core.go
@@ -15,12 +15,16 @@ import (
 	"k8s.io/klog/v2"
 )
 
+// ApplyService will apply the Service to match the required object.
+// forceOwnership allows to apply objects without an ownerReference. Normally such objects
+// would be adopted but the old objects may not have correct labels that we need to fix in the new version.
 func ApplyService(
 	ctx context.Context,
 	client appv1client.ServicesGetter,
 	lister appv1listers.ServiceLister,
 	recorder record.EventRecorder,
 	required *corev1.Service,
+	forceOwnership bool,
 ) (*corev1.Service, bool, error) {
 	requiredControllerRef := metav1.GetControllerOfNoCopy(required)
 	if requiredControllerRef == nil {
@@ -50,10 +54,14 @@ func ApplyService(
 
 	existingControllerRef := metav1.GetControllerOfNoCopy(existing)
 	if !equality.Semantic.DeepEqual(existingControllerRef, requiredControllerRef) {
-		// This is not the place to handle adoption.
-		err := fmt.Errorf("service %q isn't controlled by us", naming.ObjRef(requiredCopy))
-		ReportUpdateEvent(recorder, requiredCopy, err)
-		return nil, false, err
+		if existingControllerRef == nil && forceOwnership {
+			klog.V(2).InfoS("Forcing apply to claim the Service", "Service", naming.ObjRef(requiredCopy))
+		} else {
+			// This is not the place to handle adoption.
+			err := fmt.Errorf("service %q isn't controlled by us", naming.ObjRef(requiredCopy))
+			ReportUpdateEvent(recorder, requiredCopy, err)
+			return nil, false, err
+		}
 	}
 
 	// If they are the same do nothing.
@@ -78,12 +86,16 @@ func ApplyService(
 	return actual, true, nil
 }
 
+// ApplySecret will apply the Secret to match the required object.
+// forceOwnership allows to apply objects without an ownerReference. Normally such objects
+// would be adopted but the old objects may not have correct labels that we need to fix in the new version.
 func ApplySecret(
 	ctx context.Context,
 	client appv1client.SecretsGetter,
 	lister appv1listers.SecretLister,
 	recorder record.EventRecorder,
 	required *corev1.Secret,
+	forceOwnership bool,
 ) (*corev1.Secret, bool, error) {
 	requiredControllerRef := metav1.GetControllerOfNoCopy(required)
 	if requiredControllerRef == nil {
@@ -113,10 +125,14 @@ func ApplySecret(
 
 	existingControllerRef := metav1.GetControllerOfNoCopy(existing)
 	if !equality.Semantic.DeepEqual(existingControllerRef, requiredControllerRef) {
-		// This is not the place to handle adoption.
-		err := fmt.Errorf("secret %q isn't controlled by us", naming.ObjRef(requiredCopy))
-		ReportUpdateEvent(recorder, requiredCopy, err)
-		return nil, false, err
+		if existingControllerRef == nil && forceOwnership {
+			klog.V(2).InfoS("Forcing apply to claim the Secret", "Secret", naming.ObjRef(requiredCopy))
+		} else {
+			// This is not the place to handle adoption.
+			err := fmt.Errorf("secret %q isn't controlled by us", naming.ObjRef(requiredCopy))
+			ReportUpdateEvent(recorder, requiredCopy, err)
+			return nil, false, err
+		}
 	}
 
 	// If they are the same do nothing.
diff --git a/pkg/resourceapply/policy.go b/pkg/resourceapply/policy.go
index 0927849f2..75c6e4d5d 100644
--- a/pkg/resourceapply/policy.go
+++ b/pkg/resourceapply/policy.go
@@ -17,14 +17,16 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// ApplyPodDisruptionBudget creates or updates PDB. If desired object is equal (based on calculated hash)
-// to existing one update is not called.
+// ApplyPodDisruptionBudget will apply the PodDisruptionBudget to match the required object.
+// forceOwnership allows to apply objects without an ownerReference. Normally such objects
+// would be adopted but the old objects may not have correct labels that we need to fix in the new version.
 func ApplyPodDisruptionBudget(
 	ctx context.Context,
 	client policyv1beta1client.PodDisruptionBudgetsGetter,
 	lister policyv1beta1listers.PodDisruptionBudgetLister,
 	recorder record.EventRecorder,
 	required *policyv1beta1.PodDisruptionBudget,
+	forceOwnership bool,
 ) (*policyv1beta1.PodDisruptionBudget, bool, error) {
 	requiredControllerRef := metav1.GetControllerOfNoCopy(required)
 	if requiredControllerRef == nil {
@@ -54,10 +56,14 @@ func ApplyPodDisruptionBudget(
 
 	existingControllerRef := metav1.GetControllerOfNoCopy(existing)
 	if !equality.Semantic.DeepEqual(existingControllerRef, requiredControllerRef) {
-		// This is not the place to handle adoption.
-		err := fmt.Errorf("poddisruptionbudget %q isn't controlled by us", naming.ObjRef(requiredCopy))
-		ReportUpdateEvent(recorder, requiredCopy, err)
-		return nil, false, err
+		if existingControllerRef == nil && forceOwnership {
+			klog.V(2).InfoS("Forcing apply to claim the PodDisruptionBudget", "PodDisruptionBudget", naming.ObjRef(requiredCopy))
+		} else {
+			// This is not the place to handle adoption.
+			err := fmt.Errorf("poddisruptionbudget %q isn't controlled by us", naming.ObjRef(requiredCopy))
+			ReportUpdateEvent(recorder, requiredCopy, err)
+			return nil, false, err
+		}
 	}
 
 	// If they are the same do nothing.


Test patch:
diff --git a/pkg/resourceapply/apps_test.go b/pkg/resourceapply/apps_test.go
index fb411459e..450dd4466 100644
--- a/pkg/resourceapply/apps_test.go
+++ b/pkg/resourceapply/apps_test.go
@@ -72,6 +72,7 @@ func TestApplyStatefulSet(t *testing.T) {
 		existing        []runtime.Object
 		cache           []runtime.Object // nil cache means autofill from the client
 		required        *appsv1.StatefulSet
+		forceOwnership  bool
 		expectedSts     *appsv1.StatefulSet
 		expectedChanged bool
 		expectedErr     error
@@ -256,6 +257,52 @@ func TestApplyStatefulSet(t *testing.T) {
 			expectedErr:     fmt.Errorf("can't update statefulset: %w", apierrors.NewNotFound(appsv1.Resource("statefulsets"), "test")),
 			expectedEvents:  []string{`Warning UpdateStatefulSetFailed Failed to update StatefulSet default/test: statefulsets.apps "test" not found`},
 		},
+		{
+			name: "update fails if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *appsv1.StatefulSet {
+					sts := newSts()
+					sts.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(sts))
+					return sts
+				}(),
+			},
+			required: func() *appsv1.StatefulSet {
+				sts := newSts()
+				sts.Spec.Template.Spec.Containers[0].Image += "-rc.0"
+				return sts
+			}(),
+			expectedSts:     nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`statefulset "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdateStatefulSetFailed Failed to update StatefulSet default/test: statefulset "default/test" isn't controlled by us`},
+		},
+		{
+			name: "forced update succeeds if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *appsv1.StatefulSet {
+					sts := newSts()
+					sts.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(sts))
+					return sts
+				}(),
+			},
+			required: func() *appsv1.StatefulSet {
+				sts := newSts()
+				sts.Spec.Template.Spec.Containers[0].Image += "-rc.0"
+				return sts
+			}(),
+			forceOwnership: true,
+			expectedSts: func() *appsv1.StatefulSet {
+				sts := newSts()
+				sts.Spec.Template.Spec.Containers[0].Image += "-rc.0"
+				utilruntime.Must(SetHashAnnotation(sts))
+				return sts
+			}(),
+			expectedChanged: true,
+			expectedErr:     nil,
+			expectedEvents:  []string{"Normal StatefulSetUpdated StatefulSet default/test updated"},
+		},
 		{
 			name: "update fails if the existing object is owned by someone else",
 			existing: []runtime.Object{
@@ -276,6 +323,27 @@ func TestApplyStatefulSet(t *testing.T) {
 			expectedErr:     fmt.Errorf(`statefulset "default/test" isn't controlled by us`),
 			expectedEvents:  []string{`Warning UpdateStatefulSetFailed Failed to update StatefulSet default/test: statefulset "default/test" isn't controlled by us`},
 		},
+		{
+			name: "forced update fails if the existing object is owned by someone else",
+			existing: []runtime.Object{
+				func() *appsv1.StatefulSet {
+					sts := newSts()
+					sts.OwnerReferences[0].UID = "42"
+					utilruntime.Must(SetHashAnnotation(sts))
+					return sts
+				}(),
+			},
+			required: func() *appsv1.StatefulSet {
+				sts := newSts()
+				sts.Spec.Template.Spec.Containers[0].Image += "-rc.0"
+				return sts
+			}(),
+			forceOwnership:  true,
+			expectedSts:     nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`statefulset "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdateStatefulSetFailed Failed to update StatefulSet default/test: statefulset "default/test" isn't controlled by us`},
+		},
 	}
 
 	for _, tc := range tt {
@@ -325,7 +393,7 @@ func TestApplyStatefulSet(t *testing.T) {
 						}
 					}
 
-					gotSts, gotChanged, gotErr := ApplyStatefulSet(ctx, client.AppsV1(), stsLister, recorder, tc.required)
+					gotSts, gotChanged, gotErr := ApplyStatefulSet(ctx, client.AppsV1(), stsLister, recorder, tc.required, tc.forceOwnership)
 					if !reflect.DeepEqual(gotErr, tc.expectedErr) {
 						t.Fatalf("expected %v, got %v", tc.expectedErr, gotErr)
 					}
diff --git a/pkg/resourceapply/core_test.go b/pkg/resourceapply/core_test.go
index a7c385cdb..b36f3f5bb 100644
--- a/pkg/resourceapply/core_test.go
+++ b/pkg/resourceapply/core_test.go
@@ -56,6 +56,7 @@ func TestApplyService(t *testing.T) {
 		existing        []runtime.Object
 		cache           []runtime.Object // nil cache means autofill from the client
 		required        *corev1.Service
+		forceOwnership  bool
 		expectedService *corev1.Service
 		expectedChanged bool
 		expectedErr     error
@@ -228,6 +229,52 @@ func TestApplyService(t *testing.T) {
 			expectedErr:     fmt.Errorf("can't update service: %w", apierrors.NewNotFound(corev1.Resource("services"), "test")),
 			expectedEvents:  []string{`Warning UpdateServiceFailed Failed to update Service default/test: services "test" not found`},
 		},
+		{
+			name: "update fails if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *corev1.Service {
+					svc := newService()
+					svc.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(svc))
+					return svc
+				}(),
+			},
+			required: func() *corev1.Service {
+				svc := newService()
+				svc.Labels["foo"] = "bar"
+				return svc
+			}(),
+			expectedService: nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`service "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdateServiceFailed Failed to update Service default/test: service "default/test" isn't controlled by us`},
+		},
+		{
+			name: "forced update succeeds if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *corev1.Service {
+					svc := newService()
+					svc.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(svc))
+					return svc
+				}(),
+			},
+			required: func() *corev1.Service {
+				svc := newService()
+				svc.Labels["foo"] = "bar"
+				return svc
+			}(),
+			forceOwnership: true,
+			expectedService: func() *corev1.Service {
+				svc := newService()
+				svc.Labels["foo"] = "bar"
+				utilruntime.Must(SetHashAnnotation(svc))
+				return svc
+			}(),
+			expectedChanged: true,
+			expectedErr:     nil,
+			expectedEvents:  []string{"Normal ServiceUpdated Service default/test updated"},
+		},
 		{
 			name: "update fails if the existing object is owned by someone else",
 			existing: []runtime.Object{
@@ -248,6 +295,27 @@ func TestApplyService(t *testing.T) {
 			expectedErr:     fmt.Errorf(`service "default/test" isn't controlled by us`),
 			expectedEvents:  []string{`Warning UpdateServiceFailed Failed to update Service default/test: service "default/test" isn't controlled by us`},
 		},
+		{
+			name: "forced update fails if the existing object is owned by someone else",
+			existing: []runtime.Object{
+				func() *corev1.Service {
+					svc := newService()
+					svc.OwnerReferences[0].UID = "42"
+					utilruntime.Must(SetHashAnnotation(svc))
+					return svc
+				}(),
+			},
+			required: func() *corev1.Service {
+				svc := newService()
+				svc.Labels["foo"] = "bar"
+				return svc
+			}(),
+			forceOwnership:  true,
+			expectedService: nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`service "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdateServiceFailed Failed to update Service default/test: service "default/test" isn't controlled by us`},
+		},
 	}
 
 	for _, tc := range tt {
@@ -297,7 +365,7 @@ func TestApplyService(t *testing.T) {
 						}
 					}
 
-					gotSts, gotChanged, gotErr := ApplyService(ctx, client.CoreV1(), svcLister, recorder, tc.required)
+					gotSts, gotChanged, gotErr := ApplyService(ctx, client.CoreV1(), svcLister, recorder, tc.required, tc.forceOwnership)
 					if !reflect.DeepEqual(gotErr, tc.expectedErr) {
 						t.Fatalf("expected %v, got %v", tc.expectedErr, gotErr)
 					}
@@ -381,6 +449,7 @@ func TestApplySecret(t *testing.T) {
 		existing        []runtime.Object
 		cache           []runtime.Object // nil cache means autofill from the client
 		required        *corev1.Secret
+		forceOwnership  bool
 		expectedSecret  *corev1.Secret
 		expectedChanged bool
 		expectedErr     error
@@ -545,6 +614,52 @@ func TestApplySecret(t *testing.T) {
 			expectedErr:     fmt.Errorf("can't update secret: %w", apierrors.NewNotFound(corev1.Resource("secrets"), "test")),
 			expectedEvents:  []string{`Warning UpdateSecretFailed Failed to update Secret default/test: secrets "test" not found`},
 		},
+		{
+			name: "update fails if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *corev1.Secret {
+					sts := newSecret()
+					sts.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(sts))
+					return sts
+				}(),
+			},
+			required: func() *corev1.Secret {
+				secret := newSecret()
+				secret.Labels["foo"] = "bar"
+				return secret
+			}(),
+			expectedSecret:  nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`secret "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdateSecretFailed Failed to update Secret default/test: secret "default/test" isn't controlled by us`},
+		},
+		{
+			name: "forced update succeeds if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *corev1.Secret {
+					sts := newSecret()
+					sts.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(sts))
+					return sts
+				}(),
+			},
+			required: func() *corev1.Secret {
+				secret := newSecret()
+				secret.Labels["foo"] = "bar"
+				return secret
+			}(),
+			forceOwnership: true,
+			expectedSecret: func() *corev1.Secret {
+				secret := newSecret()
+				secret.Labels["foo"] = "bar"
+				utilruntime.Must(SetHashAnnotation(secret))
+				return secret
+			}(),
+			expectedChanged: true,
+			expectedErr:     nil,
+			expectedEvents:  []string{"Normal SecretUpdated Secret default/test updated"},
+		},
 		{
 			name: "update fails if the existing object is owned by someone else",
 			existing: []runtime.Object{
@@ -565,6 +680,27 @@ func TestApplySecret(t *testing.T) {
 			expectedErr:     fmt.Errorf(`secret "default/test" isn't controlled by us`),
 			expectedEvents:  []string{`Warning UpdateSecretFailed Failed to update Secret default/test: secret "default/test" isn't controlled by us`},
 		},
+		{
+			name: "forced update fails if the existing object is owned by someone else",
+			existing: []runtime.Object{
+				func() *corev1.Secret {
+					secret := newSecret()
+					secret.OwnerReferences[0].UID = "42"
+					utilruntime.Must(SetHashAnnotation(secret))
+					return secret
+				}(),
+			},
+			required: func() *corev1.Secret {
+				secret := newSecret()
+				secret.Labels["foo"] = "bar"
+				return secret
+			}(),
+			forceOwnership:  true,
+			expectedSecret:  nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`secret "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdateSecretFailed Failed to update Secret default/test: secret "default/test" isn't controlled by us`},
+		},
 	}
 
 	for _, tc := range tt {
@@ -614,7 +750,7 @@ func TestApplySecret(t *testing.T) {
 						}
 					}
 
-					gotSts, gotChanged, gotErr := ApplySecret(ctx, client.CoreV1(), secretLister, recorder, tc.required)
+					gotSts, gotChanged, gotErr := ApplySecret(ctx, client.CoreV1(), secretLister, recorder, tc.required, tc.forceOwnership)
 					if !reflect.DeepEqual(gotErr, tc.expectedErr) {
 						t.Fatalf("expected %v, got %v", tc.expectedErr, gotErr)
 					}
diff --git a/pkg/resourceapply/policy_test.go b/pkg/resourceapply/policy_test.go
index 8b844a7d5..f8b8f7359 100644
--- a/pkg/resourceapply/policy_test.go
+++ b/pkg/resourceapply/policy_test.go
@@ -60,6 +60,7 @@ func TestApplyPodDisruptionBudget(t *testing.T) {
 		existing        []runtime.Object
 		cache           []runtime.Object // nil cache means autofill from the client
 		required        *policyv1beta1.PodDisruptionBudget
+		forceOwnership  bool
 		expectedPDB     *policyv1beta1.PodDisruptionBudget
 		expectedChanged bool
 		expectedErr     error
@@ -224,6 +225,52 @@ func TestApplyPodDisruptionBudget(t *testing.T) {
 			expectedErr:     fmt.Errorf("can't update pdb: %w", apierrors.NewNotFound(policyv1beta1.Resource("poddisruptionbudgets"), "test")),
 			expectedEvents:  []string{`Warning UpdatePodDisruptionBudgetFailed Failed to update PodDisruptionBudget default/test: poddisruptionbudgets.policy "test" not found`},
 		},
+		{
+			name: "update fails if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *policyv1beta1.PodDisruptionBudget {
+					pdb := newPDB()
+					pdb.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(pdb))
+					return pdb
+				}(),
+			},
+			required: func() *policyv1beta1.PodDisruptionBudget {
+				pdb := newPDB()
+				pdb.Labels["foo"] = "bar"
+				return pdb
+			}(),
+			expectedPDB:     nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`poddisruptionbudget "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdatePodDisruptionBudgetFailed Failed to update PodDisruptionBudget default/test: poddisruptionbudget "default/test" isn't controlled by us`},
+		},
+		{
+			name: "forced update succeeds if the existing object has no ownerRef",
+			existing: []runtime.Object{
+				func() *policyv1beta1.PodDisruptionBudget {
+					pdb := newPDB()
+					pdb.OwnerReferences = nil
+					utilruntime.Must(SetHashAnnotation(pdb))
+					return pdb
+				}(),
+			},
+			required: func() *policyv1beta1.PodDisruptionBudget {
+				pdb := newPDB()
+				pdb.Labels["foo"] = "bar"
+				return pdb
+			}(),
+			forceOwnership: true,
+			expectedPDB: func() *policyv1beta1.PodDisruptionBudget {
+				pdb := newPDB()
+				pdb.Labels["foo"] = "bar"
+				utilruntime.Must(SetHashAnnotation(pdb))
+				return pdb
+			}(),
+			expectedChanged: true,
+			expectedErr:     nil,
+			expectedEvents:  []string{"Normal PodDisruptionBudgetUpdated PodDisruptionBudget default/test updated"},
+		},
 		{
 			name: "update fails if the existing object is owned by someone else",
 			existing: []runtime.Object{
@@ -244,6 +291,27 @@ func TestApplyPodDisruptionBudget(t *testing.T) {
 			expectedErr:     fmt.Errorf(`poddisruptionbudget "default/test" isn't controlled by us`),
 			expectedEvents:  []string{`Warning UpdatePodDisruptionBudgetFailed Failed to update PodDisruptionBudget default/test: poddisruptionbudget "default/test" isn't controlled by us`},
 		},
+		{
+			name: "forced update fails if the existing object is owned by someone else",
+			existing: []runtime.Object{
+				func() *policyv1beta1.PodDisruptionBudget {
+					pdb := newPDB()
+					pdb.OwnerReferences[0].UID = "42"
+					utilruntime.Must(SetHashAnnotation(pdb))
+					return pdb
+				}(),
+			},
+			required: func() *policyv1beta1.PodDisruptionBudget {
+				pdb := newPDB()
+				pdb.Labels["foo"] = "bar"
+				return pdb
+			}(),
+			forceOwnership:  true,
+			expectedPDB:     nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf(`poddisruptionbudget "default/test" isn't controlled by us`),
+			expectedEvents:  []string{`Warning UpdatePodDisruptionBudgetFailed Failed to update PodDisruptionBudget default/test: poddisruptionbudget "default/test" isn't controlled by us`},
+		},
 	}
 
 	for _, tc := range tt {
@@ -293,7 +361,7 @@ func TestApplyPodDisruptionBudget(t *testing.T) {
 						}
 					}
 
-					gotSts, gotChanged, gotErr := ApplyPodDisruptionBudget(ctx, client.PolicyV1beta1(), pdbLister, recorder, tc.required)
+					gotSts, gotChanged, gotErr := ApplyPodDisruptionBudget(ctx, client.PolicyV1beta1(), pdbLister, recorder, tc.required, tc.forceOwnership)
 					if !reflect.DeepEqual(gotErr, tc.expectedErr) {
 						t.Fatalf("expected %v, got %v", tc.expectedErr, gotErr)
 					}