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.

Sign up for GitHub

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

Jump to bottom

deployer controller: ensure phase direction #8417

Merged
merged 1 commit into from
Apr 11, 2016
Merged

deployer controller: ensure phase direction #8417

merged 1 commit into from
Apr 11, 2016

Conversation

0xmichalis
Copy link
Contributor

Found out in #8403

Fixes #8403 in conjunction with #8418

@smarterclayton @ironcladlou @pweil- PTAL

@0xmichalis 0xmichalis closed this Apr 8, 2016
@0xmichalis 0xmichalis reopened this Apr 8, 2016
@0xmichalis
Copy link
Contributor Author

[test]

soltysh
soltysh reviewed Apr 8, 2016
View reviewed changes
pkg/deploy/util/util_test.go
for _, test := range tests {
got := CanTransitionPhase(test.current, test.next)
if got != test.expected {
t.Errorf("%s: expected %t, got %t", test.expected, got)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing test name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@smarterclayton
Copy link
Contributor

smarterclayton commented Apr 8, 2016 via email

@pweil- pweil- added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 8, 2016
@ironcladlou
Copy link
Contributor

What invalid transition was possible prior to this change?

@0xmichalis
Copy link
Contributor Author

What invalid transition was possible prior to this change?

See #8403 (comment)

@0xmichalis
Copy link
Contributor Author

Any other comments here?

@smarterclayton
Copy link
Contributor

smarterclayton commented Apr 9, 2016 via email

@0xmichalis
Copy link
Contributor Author

unit test fixed

@soltysh
Copy link
Contributor

soltysh commented Apr 11, 2016

LGTM

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to e69b1ff

@openshift-bot
Copy link
Contributor

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

@0xmichalis
Copy link
Contributor Author

extended test failure seems unrelated

• Failure [322.818 seconds]
Kubectl client
/data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/test/e2e/kubectl.go:1146
  Update Demo
  /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/test/e2e/kubectl.go:161
    should scale a replication controller [Conformance] [It]
    /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/test/e2e/kubectl.go:150

    Apr 11 06:58:15.658: Timed out after 300 seconds waiting for name=update-demo pods to reach valid state

    /data/src/github.com/openshift/origin/Godeps/_workspace/src/k8s.io/kubernetes/test/e2e/util.go:1469
------------------------------

@0xmichalis
Copy link
Contributor Author

[merge]

@openshift-bot
Copy link
Contributor

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

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to e69b1ff

@openshift-bot openshift-bot merged commit e7eaff8 into openshift:master Apr 11, 2016
@0xmichalis 0xmichalis deleted the deployment-fixes branch April 11, 2016 13:17
@smarterclayton
Copy link
Contributor

Did not appear to fix the problem:

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/14441/artifact/origin/artifacts/test-end-to-end-docker/logs/container-router-1-deploy-deployment.log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@0xmichalis @smarterclayton @ironcladlou @soltysh @openshift-bot @pweil-