Skip to content

Commit 74560ca

Browse files
committed
fixup! Use Patch instead of Update for finalizer operations
Signed-off-by: Todd Short <[email protected]>
1 parent 385295c commit 74560ca

File tree

2 files changed

+5
-19
lines changed

2 files changed

+5
-19
lines changed

internal/catalogd/controllers/core/clustercatalog_controller_test.go

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -662,12 +662,6 @@ func TestCatalogdControllerReconcile(t *testing.T) {
662662
},
663663
},
664664
},
665-
// Note: When the controller patches the object to remove finalizers,
666-
// the fake client may update the object with a fresh copy from its storage,
667-
// which can overwrite in-memory status changes made before the patch.
668-
// In production, the Reconcile() method handles this by doing Status().Update()
669-
// separately. Since this test calls reconcile() directly, we only verify
670-
// the finalizer was removed - status updates are tested separately.
671665
expectedCatalog: &ocv1.ClusterCatalog{
672666
ObjectMeta: metav1.ObjectMeta{
673667
Name: "catalog",
@@ -682,21 +676,12 @@ func TestCatalogdControllerReconcile(t *testing.T) {
682676
},
683677
AvailabilityMode: ocv1.AvailabilityModeUnavailable,
684678
},
685-
// Status fields are not checked here because the fake client's Patch operation
686-
// may reset them. The status updates are properly handled in production via
687-
// the separate Status().Update() call in the Reconcile() method.
688679
Status: ocv1.ClusterCatalogStatus{
689-
URLs: &ocv1.ClusterCatalogURLs{Base: "URL"},
690-
LastUnpacked: nil,
691-
ResolvedSource: &ocv1.ResolvedCatalogSource{
692-
Type: ocv1.SourceTypeImage,
693-
Image: &ocv1.ResolvedImageSource{},
694-
},
695680
Conditions: []metav1.Condition{
696681
{
697682
Type: ocv1.TypeServing,
698-
Status: metav1.ConditionTrue,
699-
Reason: ocv1.ReasonAvailable,
683+
Status: metav1.ConditionFalse,
684+
Reason: ocv1.ReasonUserSpecifiedUnavailable,
700685
},
701686
{
702687
Type: ocv1.TypeProgressing,

internal/shared/util/finalizer/finalizer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ func EnsureFinalizers(ctx context.Context, owner string, c client.Client, obj cl
9393
return false, fmt.Errorf("updating finalizers: %w", err)
9494
}
9595

96-
// Update only the finalizers in the original object to reflect the change,
97-
// without copying other metadata that may have changed on the server.
96+
// Update the finalizers and resource version in the original object to reflect the change.
97+
// The resource version must be updated to avoid conflicts with subsequent operations.
9898
obj.SetFinalizers(objCopy.GetFinalizers())
99+
obj.SetResourceVersion(objCopy.GetResourceVersion())
99100

100101
return true, nil
101102
}

0 commit comments

Comments
 (0)