data/data/openstack: set correct hostnames for machines during bootstrap

[Fedosin]

According to https://kubernetes.io/docs/concepts/cluster-administration/cloud-providers/#node-name-4 to enable OpenStack cloud provider hostnames, node names & nova names must match.

This patch sets correct hostnames for masters and the bootstrap machine during the initial deployment.
To set hostnames for workers and masters that are added later we also need: openshift/machine-config-operator#964

Reverts: #1820

[tomassedovic]

/label platform/openstack

[tomassedovic]

Thanks @Fedosin!

This looks good to me, but it needs a proper commit title and message explaining why we're doing this.

The commit/PR title needs a date/data/openstack: prefix and the commit something like this maybe:

The Kubernetes cloud providers all operate under the assumption, that the name of the node matches
the name of the server (in OpenStack's case the name presented in Nova).

The cloud provider will not be working properly otherwise.

Also, would you mind doing the same for the bootstrap node? It's not a requirement, but I'd prefer if the names matched everywhere in that case.

[markmc]

Thanks @Fedosin!

This looks good to me, but it needs a proper commit title and message explaining why we're doing this.

Indeed - might also want to mention that this is a revert of #1820

The commit/PR title needs a date/data/openstack: prefix and the commit something like this maybe:

The Kubernetes cloud providers all operate under the assumption, that the name of the node matches
the name of the server (in OpenStack's case the name presented in Nova).
The cloud provider will not be working properly otherwise.

Some references maybe? Where exactly do we see this assumption?

And in the previous PR you said "The convention for the fully-qualified names is master-$index.$cluster_name.$domain_name". Is that not the case anymore? If it is, why not change the Nova instance name to match that convention?

[jichenjc]

we did see this error in our test ..thanks for fixing

[tomassedovic]

@markmc

Here's the reference (as well as this being true in practice for as long as I remember):

https://kubernetes.io/docs/concepts/cluster-administration/cloud-providers/#node-name-4

I misspoke by the way: the OpenStack cloud provider definitely requires the node name (which comes out of the hostname) matches the server name in Nova but I'm not sure whether that's the case for the other cloud providers.

That PR you mention was written by me and I think I'd made a mistake submitting it. The current Nova Server names are correct insofar as they match what AWS and GCP do as well:

installer/data/data/aws/master/main.tf

Line 111 in 8472747

"Name" = "${var.cluster_id}-master-${count.index}"

installer/data/data/gcp/master/main.tf

Line 29 in 8472747

name = "${var.cluster_id}-master-${count.index}"

installer/data/data/openstack/masters/main.tf

Line 52 in 8472747

name = "${var.cluster_id}-master-${count.index}"

So it is indeed the hostname that needs to change.

@Fedosin I've tested this patch and it does what we need!

Please prefix the commit title with data/data/openstack and add a description to the commit message that hostnames, node names & nova names must match and link this URL:

https://kubernetes.io/docs/concepts/cluster-administration/cloud-providers/#node-name-4

I'll be happy to merge it afterwards.

[Fedosin]

/test e2e-openstack

[tomassedovic]

/approve

Going to LGTM once the OpenStack test passes.

[tomassedovic]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Fedosin, tomassedovic

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• data/data/openstack/OWNERS [tomassedovic]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci-robot]

@Fedosin: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-scaleup-rhel7	cb550ef	link	/test e2e-aws-scaleup-rhel7

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[Fedosin]

/test e2e-aws