Skip to content

Set OpenStack VM hostname to the entry in Nova #7971

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
vrutkovs merged 2 commits into from
Apr 20, 2018
Merged

Set OpenStack VM hostname to the entry in Nova #7971

vrutkovs merged 2 commits into from
Apr 20, 2018

Conversation

@tomassedovic
Copy link
Contributor

@tomassedovic tomassedovic commented Apr 16, 2018

OpenStack appends a domain suffix (.novalocal by default) to the
hostnames of the VMs it creates. This clashes with the new control plane
install tasks that look at the hostname rather than the
openshift_hostname variable.

This makes the OpenStack playbooks function again with the new control
plane/bootstrap installation.

@tomassedovic tomassedovic requested a review from tzumainn April 16, 2018 10:27
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 16, 2018
@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 16, 2018

@tzumainn this one is quite important to let us deploy origin on the recent openshift-ansible checkouts. Would appreciate a review.

@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 16, 2018

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 16, 2018
@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 16, 2018

Hm, so I've just tested a deployment without setting this code and it seems to work without it, too. I was probably running to some other issues beforehand.

I'll keep it open for now but let's not merge it just yet.

@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 16, 2018

Yeah this is not necessary at all. Closing.

@tomassedovic tomassedovic deleted the fix-hostname branch April 16, 2018 16:17
@tomassedovic tomassedovic restored the fix-hostname branch April 17, 2018 11:46
@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 17, 2018

Okay, so it is actually needed but much later in the game. The deployment succeeds (once #7993 is place), but building an STI fails with:

$ oc logs -f bc/cakephp-mysql-example
error streaming logs from build pod: test/cakephp-mysql-example-1-build container: git-clone, Get https://app-node-0.openshift.example.com.novalocal:10250/containerLogs/test/cakephp-mysql-example-1-build/git-clone: dial tcp: lookup app-node-0.openshift.example.com.novalocal on 192.168.99.7:53: no such host

Obviously we'll want to investigate why the openshift_hostname value isn't honoured in the build pods, but in the meantime let's just set the hostnames to a value OpenShift expects.

@tomassedovic tomassedovic reopened this Apr 17, 2018
@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 17, 2018

Hm, although this sets the hostname to openshift_hostname which isn't exactly right. That can be an IP address.

And we should probably make it optional.

@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 17, 2018

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2018
@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 18, 2018

/retest

@bogdando
Copy link
Contributor

bogdando commented Apr 18, 2018

Please do not restore the hostnames management. I strongly believe we want this to keep behaving, like AWS provider does, which is unrelated hostnames of VMs just work.

bogdando
bogdando reviewed Apr 18, 2018
View reviewed changes
roles/openshift_openstack/tasks/node-configuration.yml
@@ -1,4 +1,11 @@
---
# NOTE(shadower): we need to do this because some of the install tasks seem to
# ignore openshift_hostname and rely on the actual system's hostname
Copy link
Contributor

@bogdando bogdando Apr 18, 2018

Choose a reason for hiding this comment

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

I'd like to clarify this and open a bug for openshift-ansible. AFAIK, that's not the case for AWS provider installations at the very least.

@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 18, 2018

Note that you can opt out of this hostname configuration.

Also, AWS does rely on the hostnames being correct. The only difference is that the default values just work on AWS but they don't on OpenStack. We should get there with Designate, but that's still in the future.

We could do this instead: add each actual hostname to the DNS record as well (or maybe instead of the Nova names). So that in addition to master-0.example.com the DNS would also (or maybe only) have master-0.example.com.novalocal

This comes with its own problems. The node names wouldn't match the desired openshift domain and we would have to document that for any DNS deployment, both records need to be set. Since the actual Nova suffix is not predictable, this would get reasonably complicated.

I think the hostname fixup is a nicer change, but I'm not terribly opposed to the DNS fix.

I absolutely want to investigate the why there are cases where openshift_hostname is not honoured, but that will likely take more time than we have for the 3.10 release. If you submit a fix, I'll be happy to give it a try though.

@bogdando
Copy link
Contributor

bogdando commented Apr 18, 2018

Also, AWS does rely on the hostnames being correct.

Here is some background for AWS and VMs hostnames #5883 (comment)

@sosiouxme PTAL/WDYT? :)

@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 18, 2018

Yes that comment explains what I meant. AWS sets things up for openshift-ansible in the way that OpenStack currently doesn't.

Which isn't to say that we shouldn't fix openshift_hostname.

@tomassedovic
Set OpenStack VM hostname to the entry in Nova
86a0cc2 
OpenStack appends a domain suffix (`.novalocal` by default) to the
hostnames of the VMs it creates. This clashes with the new control plane
install tasks that look at the hostname rather than the
`openshift_hostname` variable.

This makes the OpenStack playbooks function again with the new control
plane/bootstrap installation.
@tomassedovic
Use inventory_hostname not openshift_hostname
5c83e46 
The latter can be an IP address. But `inventory_hostname` is always
the same as the name in Nova.
@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 18, 2018

Rebased.

@tzumainn
Copy link
Contributor

tzumainn commented Apr 19, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 19, 2018
@sdodson
Copy link
Member

sdodson commented Apr 19, 2018

/retest

@tomassedovic
Copy link
Contributor Author

tomassedovic commented Apr 19, 2018

/test gcp-upgrade

1 similar comment
@sdodson
Copy link
Member

sdodson commented Apr 19, 2018

/test gcp-upgrade

@vrutkovs
Copy link
Contributor

vrutkovs commented Apr 20, 2018

/test gcp

@vrutkovs vrutkovs merged commit e6daa0c into openshift:master Apr 20, 2018
@tomassedovic tomassedovic deleted the fix-hostname branch April 20, 2018 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tzumainn tzumainn Awaiting requested review from tzumainn

1 more reviewer

@bogdando bogdando bogdando left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@tzumainn tzumainn

Labels

lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tomassedovic @bogdando @tzumainn @sdodson @vrutkovs @openshift-ci-robot