Specify DHCP domain for hostname

[dims]

What this PR does / why we need it:

In 9a8c6db, we looked at the hostname
in the metadata service and used '.' as the delimiter to chop off the
dhcp_domain (specified in nova.conf). However administrators need to
better control the dhcp domain better as there may be a '.' in the host
name itself. So let's introduce a config option that we can use and
default it to what nova uses when dhcp_domain is not specified which is
"novalocal"

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

new dhcp-domain parameter to be used for figuring out the hostname of a node

[dims]

cc @voelzmo

[dims]

/test pull-kubernetes-integration

[voelzmo]

Hey @dims,

I'm not sure if this PR goes into the right direction. As proposed in #57765 (comment), #61774 (comment), and #61000 (comment) I think we should not use metadata.hostname, but rather metadata.UUID instead.

[dims]

@voelzmo this PR fixes the problem you mentioned about "what happens if the hostname has a dot". are there other problems/issues that i am not aware of?

[voelzmo]

@dims Here's a quick summary

• Before Openstack: register metadata.hostname as node name #58502, we had an issue with taking the VM name as nodename, because it can contain arbitrary characters, such as slashes, unicode, etc, which aren't conforming to DNS rfc. See OpenStack cloud provider should register with instance id instead of instance name #57765 for the discussion of this issue.
• In this issue, I suggested using one of two things other than the VM name: VM UUID or hostname
  • UUIDS are somewhat defined in format and are strings of numbers separated by dashes
  • hostname is generated by sanitizing the VM name and appending a local domain
• Then Openstack: register metadata.hostname as node name #58502 switched to using the hostname, where I asked about the local domain at the end
• This never worked, see Kubelet cannot register nodes since hostnames are used as node names on OpenStack #61774, because we were still using the VM name to find a VM given a nodename – which was no longer the case

  kubernetes/pkg/cloudprovider/providers/openstack/openstack.go

  Lines 419 to 421 in e3ac402

  	opts := servers.ListOpts{
  	Name: fmt.Sprintf("^%s$", regexp.QuoteMeta(mapNodeNameToServerName(name))),
  	}

• You fixed this quickly with dropping the local domain.

Here is the issue with this approach:

• This fix works, if and only if the VM name doesn't need sanitation. If we have a funny VM name such as in my original example:

E.g. some weird display name like lol/what?This is weird. Ümlauts rule! gets a reasonable hostname:

this *doesn't* work, as there's no way to compute the VM name from the sanitized hostname.

In conclusion, the current implementation is working under the exact same conditions as the original implementation which used VM name instead of hostname. It also doesn't work in the case when the VM name contains funny characters, as in my example.

Therefore, I'd suggest to go back to square one and take the UUID instead. This allows you for a much cleaner way to resolve a VM given a hostname (it just returns the same UUID string), and it allows people to assign arbitrary VM names they like, for one reason or another.
The drawback is: your nodenames contain UUIDs.

WDYT?

/cc @afritzler @BugRoger

[dims]

@voelzmo so the outstanding problem is that if hostname from metadata has been sanitized, then there is no way to find the vm. right? Can we please create a fresh issue? of course the history above is invaluable as well. thanks for that!

[BugRoger]

I’m ok with this commit. In our use case we have the VM names under control. ;)

I prefer a name over a UUID as the node’s name is intended for humans. And there’s yet another guid in Kubernetes anyway. But I see the problems with the transformation not being bijective.

Nova allows to store and query information in tags. It would be an option to store the sanitized name in a tag.

Another option would be to just error the registration for malformed names.

In any case, this whole naming “interface” is in my opinion pretty messy und hard to follow. Maybe a refactoring would be the right (and gargantuan) way to go.

My vote would be to throw an error.

[dims]

This commit ties up one loose end, so let's get it in. let's work on if uuid is a good idea on another issue/pr. @FengyunPan2 @NickrenREN does that sound good?

[FengyunPan2]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, FengyunPan2

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:

• pkg/cloudprovider/providers/openstack/OWNERS [FengyunPan2,dims]

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

[dims]

since we dont have another issue yet. leaving my notes here - the code responsible for sanitizing host names in nova is:
https://github.com/openstack/nova/blame/master/nova/utils.py#L543-L580

looks like it has not been touched for a few years. which is a good thing for us :)

[dims]

/test pull-kubernetes-e2e-kops-aws

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue (batch tested with PRs 61871, 61890, 61786). If you want to cherry-pick this change to another branch, please follow the instructions here.