Deployment is racing to update replication controller

[smarterclayton]

https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/772/artifact/origin/artifacts/e2e/logs/container-4b63c64b42ce.log

seeing these compare and swap problems. A, we should be logging a better error (maybe we're not using etcderr.InterpretUpdateError), B, we should figure out what is it racing against, and C, we should retry on race in most cases.

[smarterclayton]

@ironcladlou @abhgupta this is beta1 blocker and need to find someone to fix it

[ironcladlou]

I'll take this.

[ironcladlou]

This is my theory: The DeploymentController updates the ReplicationController (deployment) to keep the deployment status in sync with the deployer Pod associated with the deployment. The deployer logic in the Pod does a GET for the deployment. By the time the deployer code tries to PUT the new replica count, the DeploymentController may have updated the status on the RC annotations (e.g. to move it from Pending->Running), hence the stale resourceVersion used by the deployer code.

cc @pmorie

[smarterclayton]

Ok, so the client needs a merge operation that retrieves, checks that someone else hasn't set a size, and converges. We talked about this upstream somewhere about having client merge util, but until then you can emulate it pretty easily.

----- Original Message -----

This is my theory: The DeploymentController updates the ReplicationController
(deployment) to keep the deployment status in sync with the deployer Pod
associated with the deployment. The deployer logic in the Pod does a GET for
the deployment. By the time the deployer code tries to PUT the new replica
count, the DeploymentController may have updated the status on the RC
annotations (e.g. to move it from Pending->Running), hence the stale
resourceVersion used by the deployer code.

---

Reply to this email directly or view it on GitHub:
#836 (comment)

[ironcladlou]

@pmorie @smarterclayton @pweil-

Some options here.

• Retry forever with backoff logic to prevent rapid spamming
• Retry up to a timeout or count, also with backoff
• Make the retry characteristics configurable as a new params for the recreate strategy

Thoughts?

[smarterclayton]

Right now, for beta1, even a few retries with no delay or back off would be better than what we have.

----- Original Message -----

@pmorie @smarterclayton @pweil-

Some options here.

• Retry forever with backoff logic to prevent rapid spamming
• Retry up to a timeout or count, also with backoff
• Make the retry characteristics configurable as a new params for the
  recreate strategy

Thoughts?

---

Reply to this email directly or view it on GitHub:
#836 (comment)

[pweil-]

I think a default retry count (maybe 3-5) and a wait between would be ok, with a check each time to see if the replication count has been updated.

This is actually pretty relevant to the changes I'd like to make to the auto-scaler proposal that mention the interactions with a deployer updating a replication count must be able to work in the context of other components updating replica counts. (https://github.com/pweil-/kubernetes/pull/2)

[smarterclayton]

In the context here you don't need a wait - in a "race" you need to reread, check what has changed, and either proceed or fail immediately. This is a write conflict, not necessarily a logic conflict.

----- Original Message -----

I think a default retry count (maybe 3-5) and a wait between would be ok,
with a check each time to see if the replication count has been updated.

This is actually pretty relevant to the changes I'd like to make to the
auto-scaler proposal that mention the interactions with a deployer updating
a replication count must be able to work in the context of other components
updating replica counts. (https://github.com/pweil-/kubernetes/pull/2)

---

Reply to this email directly or view it on GitHub:
#836 (comment)

[ironcladlou]

Resolved by #846