Skip to content

Hold startup until etcd has stabilized cluster version #14095

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
Merged
merged 1 commit into from
May 13, 2017
Merged

Hold startup until etcd has stabilized cluster version #14095

merged 1 commit into from
May 13, 2017

Conversation

smarterclayton
Copy link
Contributor

Uncomment gate for version detection.

Fixes #14063 @ncdc re: the code I uncommented

@smarterclayton smarterclayton mentioned this pull request May 8, 2017
Closed
enj
enj suggested changes May 8, 2017
View reviewed changes
Copy link
Contributor

@enj enj left a comment

Choose a reason for hiding this comment

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

@smarterclayton you know it is very strange to start a go routine and then immediately block on it...

pkg/cmd/server/start/start_master.go Outdated
//if len(kc.Options.APIServerArguments) > 0 && len(kc.Options.APIServerArguments["storage-backend"]) > 0 && kc.Options.APIServerArguments["storage-backend"][0] == "etcd2" {
if _, err := etcd.GetAndTestEtcdClient(oc.Options.EtcdClientInfo); err != nil {
return err
if len(kc.Options.APIServerArguments) > 0 && len(kc.Options.APIServerArguments["storage-backend"]) > 0 && kc.Options.APIServerArguments["storage-backend"][0] == "etcd2" {
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @sttts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed and reversed this condition (etcd2 is the default).

@smarterclayton
Copy link
Contributor Author

Sometimes, but cleanup code is tricky to get right.

@smarterclayton
Copy link
Contributor Author

[test]

@smarterclayton
Hold startup until etcd has stabilized cluster version
27704a7 
Uncomment gate for version detection.
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 27704a7

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1328/) (Base Commit: 2d26eae)

@smarterclayton
Copy link
Contributor Author

Any other comments?

@sttts
Copy link
Contributor

sttts commented May 11, 2017

lgtm

@enj enj added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 11, 2017
@enj
Copy link
Contributor

enj commented May 11, 2017

Approved as this is causing integration failures.

[merge]

@enj enj mentioned this pull request May 11, 2017
Closed
@smarterclayton
Copy link
Contributor Author

[merge] install failure

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 27704a7

@openshift-bot
Copy link
Contributor

openshift-bot commented May 13, 2017 •
edited
Loading

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/624/) (Base Commit: e1a81e8) (Image: devenv-rhel7_6228)

@openshift-bot openshift-bot merged commit a2e505b into openshift:master May 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enj enj enj requested changes

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.
4 participants
@smarterclayton @openshift-bot @sttts @enj