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

Read etcd data dir from appropriate config file. #850

Merged
merged 1 commit into from Nov 9, 2015

Conversation

dgoodwin
Copy link
Contributor

@dgoodwin dgoodwin commented Nov 9, 2015

Rather than assuming the etcd data dir, we now read if from master-config.yaml
if using embedded etcd, otherwise from etcd.conf.

Doing so now required use of PyYAML to parse config file when gathering facts.

Fixed discrepancy with data_dir fact and openshift-enterprise deployment_type.

@@ -558,17 +597,17 @@ def set_deployment_facts_if_unset(facts):
service_type = 'atomic-openshift'
if deployment_type == 'origin':
service_type = 'origin'
elif deployment_type in ['enterprise', 'online']:
elif deployment_type in ['openshift-enterprise', 'enterprise', 'online']:
Copy link
Contributor

Choose a reason for hiding this comment

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

These conditionals are for 3.0 deployments, we want the atomic-openshift service type for openshift-enterprise.

Likewise for /etc/origin and /var/lib/origin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you clarify, these three lines should revert, and we leave it with "origin" dirs if openshift-enterprise or atomic-enterprise? Do I have that right?

Copy link
Contributor

Choose a reason for hiding this comment

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

That is correct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake, thanks! Updated.

@detiber
Copy link
Contributor

detiber commented Nov 9, 2015

Outside of the comment around the deployment_type facts, lgtm

Rather than assuming the etcd data dir, we now read if from master-config.yaml
if using embedded etcd, otherwise from etcd.conf.

Doing so now required use of PyYAML to parse config file when gathering facts.

Fixed discrepancy with data_dir fact and openshift-enterprise deployment_type.
@detiber
Copy link
Contributor

detiber commented Nov 9, 2015

👍

brenton added a commit that referenced this pull request Nov 9, 2015
Read etcd data dir from appropriate config file.
@brenton brenton merged commit f5d81c3 into openshift:master Nov 9, 2015
@brenton
Copy link
Contributor

brenton commented Nov 9, 2015

FWIW, I confirmed PyYAML is available on Atomic Host.

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.

None yet

3 participants