fix(service update): Retry an update in case of a conflict. #240
Conversation
knative-prow-robot
added
size/M
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: navidshaikh, rhuss The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/assign sixolet |
| if err != nil { | ||
| return err | ||
| var retries = 0 | ||
| for true { |
| fmt.Fprintf(out, "Service '%s' successfully replaced in namespace '%s'.\n", service.Name, namespace) | ||
| return nil | ||
| // This line will be never reached, but go insists on having it | ||
| return fmt.Errorf("can not update service '%s' in namespace '%s'", service.Name, namespace) |
There was a problem hiding this comment.
common convention in go in such cases is to use panic("Unreachable")
| if err != nil { | ||
| return err | ||
| } | ||
| service = service.DeepCopy() |
There was a problem hiding this comment.
Not introduced in this PR, but happy to remove if safe. it was just my general understanding to copy objects before working on them to not compromise shared caches and other data structures which hold a reference.
Is related to knative#224 and should fix one flake.
rhuss
force-pushed
the
pr/force-update-fix-224
branch
from
d7e4133 to
7232f9a
Compare
|
The following is the coverage report on pkg/.
|
// @cppforlife |
|
/lgtm |
|
/retest |
- Update Dockerfile to use rhel8/go-toolset:1.13.4 builder image - Update 'version' label in Dockerfile as 'v0.11.0' (earlier it was without 'v')
AKA "display a banner when tests failed for a release, instead of just silently aborting the release" Bonus: factor out common code in tests.
Is related to #224 and should fix one flake.