-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Ensure the test deployment invariant is maintained #9839
Ensure the test deployment invariant is maintained #9839
Conversation
c71118b
to
71d80ca
Compare
if deployment.Spec.Replicas != 0 { | ||
switch nextStatus { | ||
case deployapi.DeploymentStatusComplete, deployapi.DeploymentStatusFailed: | ||
if config, err := deployutil.DecodeDeploymentConfig(deployment, c.codec); err == nil && config.Spec.Test { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is getting annoying. Can't we just add a test annotation in the deployment when it's created? Then you wouldn't need to deserialize at all... and it's all over the place in this controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update scale for deployment while also setting next phase.
71d80ca
to
74a5893
Compare
Comment addressed, [merge] |
Back to the first one
We may want to use the failure trap for this test. |
I think this is #9775 |
[test] |
Evaluated for origin test up to 74a5893 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6293/) |
[merge] and will switch to failure trap later |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6293/) (Image: devenv-rhel7_4594) |
Evaluated for origin merge up to 74a5893 |
Update scale for deployment while also setting next phase.
Fixes #9195 [test]