Skip to content

Commit

Permalink
Fix #8132: managed fields failure during Restore
Browse files Browse the repository at this point in the history
This commit addresses issue #8132, where an error randomly
appears in the logs during the restore operation.

The error occurs due to a race condition when attempting
to patch managed fields on an object that has been modified
in the cluster. The error message indicates that the operation
cannot be fulfilled because the object has been modified,
suggesting that changes should be applied to the latest version.

To resolve this, a retry mechanism has been implemented in the restore
process when encountering this error, ensuring that managed fields
are properly restored without the error message appearing in the logs.

Signed-off-by: Michal Pryc <[email protected]>
  • Loading branch information
mpryc committed Aug 21, 2024
1 parent 86963bf commit 097a9f8
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 16 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8132-mpryc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Random race condition in the restore with managed fields
53 changes: 37 additions & 16 deletions pkg/restore/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"k8s.io/client-go/dynamic/dynamicinformer"
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/retry"
crclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/vmware-tanzu/velero/internal/credentials"
Expand Down Expand Up @@ -1669,24 +1670,44 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
}

// restore the managedFields
withoutManagedFields := createdObj.DeepCopy()
createdObj.SetManagedFields(obj.GetManagedFields())
patchBytes, err := generatePatch(withoutManagedFields, createdObj)
if err != nil {
ctx.log.Errorf("error generating patch for managed fields %s: %v", kube.NamespaceAndName(obj), err)
errs.Add(namespace, err)
return warnings, errs, itemExists
}
if patchBytes != nil {
if _, err = resourceClient.Patch(name, patchBytes); err != nil {
ctx.log.Errorf("error patch for managed fields %s: %v", kube.NamespaceAndName(obj), err)
if !apierrors.IsNotFound(err) {
errs.Add(namespace, err)
return warnings, errs, itemExists
firstPatchTry := true

err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
var withoutManagedFields *unstructured.Unstructured
var patchBytes []byte
var patchErr error

if !firstPatchTry {
createdObj, patchErr = resourceClient.Get(name, metav1.GetOptions{})
if patchErr != nil {
ctx.log.Errorf("error fetching latest object for %s: %v", kube.NamespaceAndName(obj), patchErr)
return patchErr

Check warning on line 1684 in pkg/restore/restore.go

View check run for this annotation

Codecov / codecov/patch

pkg/restore/restore.go#L1681-L1684

Added lines #L1681 - L1684 were not covered by tests
}
} else {
ctx.log.Infof("the managed fields for %s is patched", kube.NamespaceAndName(obj))
}

firstPatchTry = false
withoutManagedFields = createdObj.DeepCopy()
createdObj.SetManagedFields(obj.GetManagedFields())
patchBytes, patchErr = generatePatch(withoutManagedFields, createdObj)
if patchErr != nil {
ctx.log.Errorf("error generating patch for managed fields %s: %v", kube.NamespaceAndName(obj), patchErr)
return patchErr

Check warning on line 1694 in pkg/restore/restore.go

View check run for this annotation

Codecov / codecov/patch

pkg/restore/restore.go#L1693-L1694

Added lines #L1693 - L1694 were not covered by tests
}
if patchBytes != nil {
_, patchErr = resourceClient.Patch(name, patchBytes)
if patchErr != nil {
ctx.log.Errorf("error patching managed fields %s: %v", kube.NamespaceAndName(obj), patchErr)
return patchErr

Check warning on line 1700 in pkg/restore/restore.go

View check run for this annotation

Codecov / codecov/patch

pkg/restore/restore.go#L1697-L1700

Added lines #L1697 - L1700 were not covered by tests
}
ctx.log.Infof("managed fields for %s patched successfully", kube.NamespaceAndName(obj))

Check warning on line 1702 in pkg/restore/restore.go

View check run for this annotation

Codecov / codecov/patch

pkg/restore/restore.go#L1702

Added line #L1702 was not covered by tests
}

return nil
})

if err != nil && !apierrors.IsNotFound(err) {
errs.Add(namespace, err)
return warnings, errs, itemExists

Check warning on line 1710 in pkg/restore/restore.go

View check run for this annotation

Codecov / codecov/patch

pkg/restore/restore.go#L1709-L1710

Added lines #L1709 - L1710 were not covered by tests
}

if groupResource == kuberesource.Pods {
Expand Down

0 comments on commit 097a9f8

Please sign in to comment.