Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent deployer pod creation conflicts #8588

Merged

Conversation

ironcladlou
Copy link
Contributor

Take advantage of a single-controller assumption by checking for the
existence of deployer pods before trying to create new ones. This allows
the controller to progress the phase of the deployment without incurring
a resource conflict during create under most circumstances- a behavior
which can result in poor interactions with the quota system which will
charge quota for the conflicting pod.

@ironcladlou
Copy link
Contributor Author

@0xmichalis
Copy link
Contributor

LGTM

@liggitt
Copy link
Contributor

liggitt commented Apr 21, 2016

Does this handle cases where the controller's pod cache is not populated yet (like on startup)? What about cases where the cache has not yet observed a deletion of the pod?

@ironcladlou
Copy link
Contributor Author

@liggitt

Does this handle cases where the controller's pod cache is not populated yet (like on startup)?
What about cases where the cache has not yet observed a deletion of the pod?

This controller doesn't use a pod cache, so everything is responsive to updates to the RC. No caching issues to worry about.

@smarterclayton
Copy link
Contributor

[test]

@smarterclayton
Copy link
Contributor

Can you create a soak extended test that proves this fixes the issue? We don't have to merge it, but I want to see code and a run that proves it's not regressing

@smarterclayton
Copy link
Contributor

[test]

Take advantage of a single-controller assumption by checking for the
existence of deployer pods before trying to create new ones. This allows
the controller to progress the phase of the deployment without incurring
a resource conflict during create under most circumstances- a behavior
which can result in poor interactions with the quota system which will
charge quota for the conflicting pod.
@ironcladlou ironcladlou force-pushed the prevent-deployer-pod-conflicts branch from 50a101e to 552aa83 Compare April 21, 2016 19:27
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 552aa83

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3185/)

@smarterclayton
Copy link
Contributor

Waiting for the rc1 fix to merge

@smarterclayton
Copy link
Contributor

Approved as per earlier decision today, [merge]

On Thu, Apr 21, 2016 at 6:10 PM, OpenShift Bot [email protected]
wrote:

continuous-integration/openshift-jenkins/test FAILURE (
https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3185/)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8588 (comment)

@0xmichalis
Copy link
Contributor

#7429 flake

[merge]

@ironcladlou
Copy link
Contributor Author

#7429 flake again. [merge]

@0xmichalis
Copy link
Contributor

It's not the same flake, I think our merges are blanks:(

@smarterclayton
Copy link
Contributor

[merge]

1 similar comment
@smarterclayton
Copy link
Contributor

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5664/) (Image: devenv-rhel7_4016)

@smarterclayton
Copy link
Contributor

Failed because of image import retry on a method I didn't wrap:

[merge]

On Fri, Apr 22, 2016 at 3:45 PM, OpenShift Bot [email protected]
wrote:

continuous-integration/openshift-jenkins/merge FAILURE (
https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5662/
)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8588 (comment)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 552aa83

@openshift-bot openshift-bot merged commit e349536 into openshift:master Apr 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants