Fixes for Snapshot failures #580
Conversation
1. Fix snapshot crud failures:
Cause: update did not save the sync status condition
Fix: update() needed to return latest retrieved.
2. Fix endless reconciler loop:
Cause: ReadOne->onError() sets terminal condition to false using patchResource,
followed by error returned from Create() leads to
setting terminal condition to true using patchResource;
causing reconciler loop.
Fix: ensure patchResource is invoked once in the reconciler::sync()
3 Snapshots e2e tests fix - updated scenarios where create snapshot expects either Replication Group or Cache Cluster Id and fails if other is supplied.
kumargauravsharma
requested review from
echen-98,
jaypipes,
nmvk and
vijtrip2
| // updated in the resource | ||
| // Thus, patchResource() call should be made here | ||
| _ = r.patchResource(ctx, desired, latest) | ||
| } |
There was a problem hiding this comment.
I don't understand why this change was necessary. Why would ReadOne be returning err == NotFound && a non-nil latest variable?
There was a problem hiding this comment.
The earlier version of the code was having the err == NotFound && a non-nil latest scenario. The non-nil latest was the side effect of #574.
This PR fixes that.
With the changes in this PR; the resource would be patched when err != NotFound && a not nil latest and method would return. This scenario occurs when invalid input is given to SDK find.
And when err == NotFound, the Create() is tried first and if it fails with error then resource is patched and method returns.
In all cases only one patch resource call is made.
There was a problem hiding this comment.
gotcha, thanks for the explanation. This is partly why I was a little hesitant about #574
| // updated in the resource | ||
| // Thus, patchResource() call should be made here | ||
| _ = r.patchResource(ctx, desired, latest) | ||
| } |
There was a problem hiding this comment.
gotcha, thanks for the explanation. This is partly why I was a little hesitant about #574
5 participants
Fixes for Snapshot failures:
Testing done:
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.