restore: Use warning when Create IsAlreadyExist and Get error #7004
Conversation
kaovilai
force-pushed
the
warnOnCreateAlreadyExistsGetError
branch
from
00fabac
to
4295eec
Compare
|
whoops changelog in wrong location |
kaovilai
force-pushed
the
warnOnCreateAlreadyExistsGetError
branch
from
4295eec
to
1178598
Compare
Codecov Report
@@ Coverage Diff @@
## main #7004 +/- ##
==========================================
- Coverage 61.05% 61.03% -0.02%
==========================================
Files 251 252 +1
Lines 26846 26899 +53
==========================================
+ Hits 16390 16418 +28
- Misses 9303 9319 +16
- Partials 1153 1162 +9
|
pkg/restore/restore.go
Outdated
| @@ -1534,8 +1534,8 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso | |||
| fromCluster, err = resourceClient.Get(name, metav1.GetOptions{}) | |||
| } | |||
| if err != nil && isAlreadyExistsError { | |||
| ctx.log.Errorf("Error retrieving in-cluster version of %s: %v", kube.NamespaceAndName(obj), err) | |||
| errs.Add(namespace, err) | |||
| ctx.log.Warnf("Unable to retrieve in-cluster version of %s: %v, object won't have restore labels", kube.NamespaceAndName(obj), err) | |||
There was a problem hiding this comment.
Another impact is ExistingPolicy will be skipped for this resource, is this expected?
After this change the behavior is not as described as in the comment in line1527-1530, please also update the comments.
Additionally, you should also log that the restore of the object is skipped, though we can't determine if the in cluster resource is same or not.
There was a problem hiding this comment.
@reasonerjt Existing resource policy is already skipped in the current code where we error out, but yes I think the warning should probably mention that explicitly here. We can't apply the existing resource policy if we can't Get the resource. Note, also, that when we can Get the resource, we don't error out if the patch fails due to immutable fields, etc., we warn, so I think that's ok here that we skip it, but the warning should mention it.
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
kaovilai
dismissed stale reviews from shubham-pampattiwar and sseago
via
54ecf94
kaovilai
force-pushed
the
warnOnCreateAlreadyExistsGetError
branch
from
1178598
to
54ecf94
Compare
|
will cherry-pick to 1.11 and 1.12 once merged |
…-tanzu#7004) Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…-tanzu#7004) Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…-tanzu#7004) Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…-tanzu#7004) Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…ror-release-1.11 release-1.11: restore: Use warning when Create IsAlreadyExist and Get error (#7004)
…ror-release-1.12 release-1.12: restore: Use warning when Create IsAlreadyExist and Get error (#7004)
Signed-off-by: Tiger Kaovilai tkaovila@redhat.com
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Fixes #6952
Please indicate you've done the following:
/kind changelog-not-requiredas a comment on this pull request.site/content/docs/main.