Fix controller shutdown status patching with a fresh context for patches (#191)

Fixes: aws-controllers-k8s/community#2497, aws-controllers-k8s/community#2103

**Description of changes:**

During controller shutdown (SIGTERM), status updates were failing because `patchResourceStatus` was attempting to patch using the original cancelled context. This resulted in resources not having their final status properly updated in the resource manifest, leaving them in an inconsistent state.

**FIX**: Always use `context.WithoutCancel(ctx)` for all patch operations to prevent cancellation propagation while preserving context values. The 30s SIGTERM grace period serves as the effective timeout, eliminating the need for additional timeout logic that could interfere with normal Kubernetes client timeout/retry strategy.

**Changes:**

Created util functions:
```
- patchWithoutCancel()
- patchStatusWithoutCancel() 
```


**Operations updated to Use` context.WithoutCancel(ctx)`**
Main Reconciler:
```
- patchResourceMetadataAndSpec()
- patchResourceStatus()

```
Adoption Reconciler:
```
- patchMetadataAndSpec()
- patchStatus()

```
Field Export Reconciler:
```
- patchMetadataAndSpec()
- patchStatus()
- writeToConfigMap()
- writeToSecret()
```

eg logs: 
```
{"level":"debug","ts":"2025-06-05T09:52:16.723-0700","logger":"ackrt","msg":"< r.Sync","kind":"Policy","namespace":"default","name":"iiobalic90","account":"xx","role":"","region":"us-west-2","is_adopted":false,"generation":1,"error":"operation error IAM: GetPolicy, context canceled"}
{"level":"info","ts":"2025-06-05T09:52:16.723-0700","logger":"ackrt","msg":"created patch context with timeout","kind":"Policy","namespace":"default","name":"iiobalic90","account":"xx","role":"","region":"us-west-2","is_adopted":false,"generation":1,"timeout_seconds":10}
{"level":"debug","ts":"2025-06-05T09:52:16.723-0700","logger":"ackrt","msg":"> r.patchResourceStatus","kind":"Policy","namespace":"default","name":"iiobalic90","account":"xx","role":"","region":"us-west-2","is_adopted":false,"generation":1}
```

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: rushmash91

pkg/runtime/adoption_reconciler.go

@@ -575,11 +575,8 @@ func (r *adoptionReconciler) patchMetadataAndSpec(
 	// content returned from apiserver.
 	// Keep a copy of status field to reset the status of 'res' after patch call
 	resStatusCopy := res.DeepCopy().Status
-	err := r.kc.Patch(
-		ctx,
-		res,
-		client.MergeFrom(base),
-	)
+
+	err := patchWithoutCancel(ctx, r.kc, res, client.MergeFrom(base))
 	res.Status = resStatusCopy
 	return err
 }
@@ -593,11 +590,7 @@ func (r *adoptionReconciler) patchStatus(
 	res *ackv1alpha1.AdoptedResource,
 	base *ackv1alpha1.AdoptedResource,
 ) error {
-	return r.kc.Status().Patch(
-		ctx,
-		res,
-		client.MergeFrom(base),
-	)
+	return patchStatusWithoutCancel(ctx, r.kc, res, client.MergeFrom(base))
 }
 
 // NewAdoptionReconciler returns a new adoptionReconciler object

pkg/runtime/adoption_reconciler_test.go

@@ -88,8 +88,8 @@ func mockManager() *ackmocks.AWSResourceManager {
 
 func setupMockClientForAdoptedResource(kc *ctrlrtclientmock.Client, statusWriter *ctrlrtclientmock.SubResourceWriter, ctx context.Context, adoptedRes *ackv1alpha1.AdoptedResource) {
 	kc.On("Status").Return(statusWriter)
-	statusWriter.On("Patch", ctx, adoptedRes, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
-	kc.On("Patch", ctx, adoptedRes, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
+	statusWriter.On("Patch", withoutCancelContextMatcher, adoptedRes, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
+	kc.On("Patch", withoutCancelContextMatcher, adoptedRes, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
 }
 
 func setupMockAwsResource(
@@ -403,7 +403,7 @@ func assertAdoptedCondition(
 	adoptedRes *ackv1alpha1.AdoptedResource,
 ) {
 	kc.AssertCalled(t, "Status")
-	statusWriter.AssertCalled(t, "Patch", ctx, adoptedRes, mock.AnythingOfType("*client.mergeFromPatch"))
+	statusWriter.AssertCalled(t, "Patch", withoutCancelContextMatcher, adoptedRes, mock.AnythingOfType("*client.mergeFromPatch"))
 	// Only one kind of condition present
 	require.Equal(1, len(adoptedRes.Status.Conditions))
 	require.Equal(ackv1alpha1.ConditionTypeAdopted, adoptedRes.Status.Conditions[0].Type)
@@ -421,9 +421,9 @@ func assertAdoptedResourceManaged(
 	object *ackv1alpha1.AdoptedResource,
 ) {
 	if expectedManaged {
-		kc.AssertCalled(t, "Patch", ctx, object, mock.AnythingOfType("*client.mergeFromPatch"))
+		kc.AssertCalled(t, "Patch", withoutCancelContextMatcher, object, mock.AnythingOfType("*client.mergeFromPatch"))
 	} else {
-		kc.AssertNotCalled(t, "Patch", ctx, object, mock.AnythingOfType("*client.mergeFromPatch"))
+		kc.AssertNotCalled(t, "Patch", withoutCancelContextMatcher, object, mock.AnythingOfType("*client.mergeFromPatch"))
 	}
 }
 

pkg/runtime/field_export_reconciler.go

@@ -324,7 +324,7 @@ func (r *fieldExportReconciler) writeToConfigMap(
 	cm.Data[key] = sourceValue
 
 	ackrtlog.DebugFieldExport(r.log, desired, "patching target config map")
-	err = r.kc.Patch(ctx, cm, patch)
+	err = patchWithoutCancel(ctx, r.kc, cm, patch)
 	if err != nil {
 		return err
 	}
@@ -371,7 +371,7 @@ func (r *fieldExportReconciler) writeToSecret(
 	secret.Data[key] = []byte(sourceValue)
 
 	ackrtlog.DebugFieldExport(r.log, desired, "patching target secret")
-	err = r.kc.Patch(ctx, secret, patch)
+	err = patchWithoutCancel(ctx, r.kc, secret, patch)
 	if err != nil {
 		return err
 	}
@@ -528,11 +528,7 @@ func (r *fieldExportReconciler) patchStatus(
 	res *ackv1alpha1.FieldExport,
 	base *ackv1alpha1.FieldExport,
 ) error {
-	return r.kc.Status().Patch(
-		ctx,
-		res,
-		client.MergeFrom(base),
-	)
+	return patchStatusWithoutCancel(ctx, r.kc, res, client.MergeFrom(base))
 }
 
 // markManaged places the supplied resource under the management of ACK.
@@ -579,11 +575,8 @@ func (r *fieldExportReconciler) patchMetadataAndSpec(
 	// content returned from apiserver.
 	// Keep a copy of status field to reset the status of 'res' after patch call
 	resStatusCopy := res.DeepCopy().Status
-	err := r.kc.Patch(
-		ctx,
-		res,
-		client.MergeFrom(base),
-	)
+
+	err := patchWithoutCancel(ctx, r.kc, res, client.MergeFrom(base))
 	res.Status = resStatusCopy
 	return err
 }

pkg/runtime/field_export_reconciler_test.go

@@ -108,8 +108,8 @@ func mockResourceDescriptor() *mocks.AWSResourceDescriptor {
 
 func setupMockClientForFieldExport(kc *ctrlrtclientmock.Client, statusWriter *ctrlrtclientmock.SubResourceWriter, ctx context.Context, fieldExport *ackv1alpha1.FieldExport) {
 	kc.On("Status").Return(statusWriter)
-	statusWriter.On("Patch", ctx, mock.Anything, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
-	kc.On("Patch", ctx, mock.Anything, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
+	statusWriter.On("Patch", withoutCancelContextMatcher, mock.Anything, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
+	kc.On("Patch", withoutCancelContextMatcher, mock.Anything, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
 }
 
 func setupMockApiReaderForFieldExport(apiReader *ctrlrtclientmock.Reader, ctx context.Context, res *mocks.AWSResource) {
@@ -336,7 +336,7 @@ func TestSync_FailureInPatchConfigMap(t *testing.T) {
 	statusWriter := &ctrlrtclientmock.SubResourceWriter{}
 
 	//Mock behavior setup
-	kc.On("Patch", ctx, mock.AnythingOfType("*v1.ConfigMap"), mock.AnythingOfType("*client.mergeFromPatch")).Return(errors.New("patching denied"))
+	kc.On("Patch", withoutCancelContextMatcher, mock.AnythingOfType("*v1.ConfigMap"), mock.AnythingOfType("*client.mergeFromPatch")).Return(errors.New("patching denied"))
 
 	setupMockClientForFieldExport(kc, statusWriter, ctx, fieldExport)
 	setupMockApiReaderForFieldExport(apiReader, ctx, res)
@@ -557,9 +557,9 @@ func assertPatchedConfigMap(expected bool, t *testing.T, ctx context.Context, kc
 		return val == "test-book-name"
 	})
 	if expected {
-		kc.AssertCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+		kc.AssertCalled(t, "Patch", withoutCancelContextMatcher, dataMatcher, mock.Anything)
 	} else {
-		kc.AssertNotCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+		kc.AssertNotCalled(t, "Patch", withoutCancelContextMatcher, dataMatcher, mock.Anything)
 	}
 }
 
@@ -576,9 +576,9 @@ func assertPatchedSecret(expected bool, t *testing.T, ctx context.Context, kc *c
 		return bytes.Equal(val, []byte("test-book-name"))
 	})
 	if expected {
-		kc.AssertCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+		kc.AssertCalled(t, "Patch", withoutCancelContextMatcher, dataMatcher, mock.Anything)
 	} else {
-		kc.AssertNotCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+		kc.AssertNotCalled(t, "Patch", withoutCancelContextMatcher, dataMatcher, mock.Anything)
 	}
 }
 
@@ -594,9 +594,9 @@ func assertPatchedSecretWithKey(expected bool, t *testing.T, ctx context.Context
 		return bytes.Equal(val, []byte("test-book-name"))
 	})
 	if expected {
-		kc.AssertCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+			kc.AssertCalled(t, "Patch", withoutCancelContextMatcher, dataMatcher, mock.Anything)
 	} else {
-		kc.AssertNotCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+		kc.AssertNotCalled(t, "Patch", withoutCancelContextMatcher, dataMatcher, mock.Anything)
 	}
 }
 
@@ -614,7 +614,7 @@ func assertRecoverableCondition(
 	latest *ackv1alpha1.FieldExport,
 ) {
 	kc.AssertCalled(t, "Status")
-	statusWriter.AssertCalled(t, "Patch", ctx, mock.AnythingOfType("*v1alpha1.FieldExport"), mock.AnythingOfType("*client.mergeFromPatch"))
+	statusWriter.AssertCalled(t, "Patch", withoutCancelContextMatcher, mock.AnythingOfType("*v1alpha1.FieldExport"), mock.AnythingOfType("*client.mergeFromPatch"))
 	// Only one kind of condition present
 	require.Equal(1, len(latest.Status.Conditions))
 	require.Equal(ackv1alpha1.ConditionTypeRecoverable, latest.Status.Conditions[0].Type)
@@ -636,7 +636,7 @@ func assertTerminalCondition(
 	latest *ackv1alpha1.FieldExport,
 ) {
 	kc.AssertCalled(t, "Status")
-	statusWriter.AssertCalled(t, "Patch", ctx, mock.AnythingOfType("*v1alpha1.FieldExport"), mock.AnythingOfType("*client.mergeFromPatch"))
+	statusWriter.AssertCalled(t, "Patch", withoutCancelContextMatcher, mock.AnythingOfType("*v1alpha1.FieldExport"), mock.AnythingOfType("*client.mergeFromPatch"))
 	// Only one kind of condition present
 	require.Equal(1, len(latest.Status.Conditions))
 	require.Equal(ackv1alpha1.ConditionTypeTerminal, latest.Status.Conditions[0].Type)

pkg/runtime/reconciler.go

@@ -57,10 +57,6 @@ const (
 	// resource if the CARM cache is not synced yet, or if the roleARN is not
 	// available.
 	roleARNNotAvailableRequeueDelay = 15 * time.Second
-	// adoptOrCreate is an annotation field that decides whether to create the
-	// resource if it doesn't exist, or adopt the resource if it exists.
-	// value comes from getAdoptionPolicy
-	// adoptOrCreate = "adopt-or-create"
 )
 
 // reconciler describes a generic reconciler within ACK.
@@ -880,7 +876,9 @@ func (r *resourceReconciler) patchResourceMetadataAndSpec(
 	rlog.Enter("kc.Patch (metadata + spec)")
 	lorig := latestCleaned.DeepCopy()
 	patch := client.MergeFrom(desiredCleaned.RuntimeObject())
-	err = r.kc.Patch(ctx, latestCleaned.RuntimeObject(), patch)
+
+	err = patchWithoutCancel(ctx, r.kc, latestCleaned.RuntimeObject(), patch)
+
 	if err == nil {
 		if rlog.IsDebugEnabled() {
 			js := getPatchDocument(patch, lorig.RuntimeObject())
@@ -916,7 +914,9 @@ func (r *resourceReconciler) patchResourceStatus(
 	dobj := desired.DeepCopy().RuntimeObject()
 	lobj := latest.DeepCopy().RuntimeObject()
 	patch := client.MergeFrom(dobj)
-	err = r.kc.Status().Patch(ctx, lobj, patch)
+
+	err = patchStatusWithoutCancel(ctx, r.kc, lobj, patch)
+
 	if err == nil {
 		if rlog.IsDebugEnabled() {
 			js := getPatchDocument(patch, lobj)