deploy: retry in conflicts in instantiate #14902
Conversation
|
@Kargakis seems this works :-) |
| @@ -102,6 +102,8 @@ func (r *REST) Create(ctx apirequest.Context, obj runtime.Object) (runtime.Objec | |||
| return nil, err | |||
| } | |||
|
|
|||
| // Do unconditional update here to guarantee it always succeed. | |||
| config.ResourceVersion = "" | |||
| updated, _, err := r.store.Update(ctx, config.Name, rest.DefaultUpdatedObjectInfo(config, kapi.Scheme)) | |||
There was a problem hiding this comment.
It seems like the entire method could be done in a RetryOnConflict block. The end user wouldn't know that you're retrying for them, but the conditional check in the code would re-run so that changes that affect the branching path in here would be reflected.
mfojtik
force-pushed
the
deploy-unconditional-update
branch
from
50c1aad
to
5a40c8c
Compare
mfojtik
changed the title
|
fixes: #13105 |
| @@ -51,58 +52,63 @@ func (r *REST) Create(ctx apirequest.Context, obj runtime.Object) (runtime.Objec | |||
| if !ok { | |||
| return nil, errors.NewInternalError(fmt.Errorf("wrong object passed for requesting a new rollout: %#v", obj)) | |||
| } | |||
| var updated runtime.Object | |||
There was a problem hiding this comment.
nit: rename to ret or some such? Sometimes this is a status object.
|
nit, lgtm otherwise. |
mfojtik
force-pushed
the
deploy-unconditional-update
branch
from
5a40c8c
to
486977b
Compare
|
[merge] |
|
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 13 |
|
Evaluated for origin merge up to 486977b |
|
[Test]ing while waiting on the merge queue |
|
re[test] |
|
Evaluated for origin test up to 486977b |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2764/) (Base Commit: 478cfe2) (PR Branch Commit: 486977b) |
|
green on head of master, no conflicts with the top of th queue. |
|
This is the last thing that changed instantiate behavior and now instantiate is consistently failing. |
|
(not completely, but consistently) https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended_conformance_gce/3828/ |
|
|
How is last version wrong? |
|
deploy instantiate is wired up to builconfiginstantiate somehow? |
No. cc: @bparees |
|
@smarterclayton yeah instantiate here has no relation to build instantiation. |
|
the instantiate failure is disturbing but it looks like the build (a build) ultimately did run successfully. we're deep into an investigation on why that test is failing (the ultimately failure reason was that the deployment failed) already, so one way or another we should get to the bottom of it. |
This will retry on conflicts for instantiate. Should cleanup a master log with failed instantiate calls and relax the generictrigger controller doing retries.