[occm] Add Openstack server hostId as k8s node label

[chess-knight]

Is this a BUG REPORT or FEATURE REQUEST?:

/kind feature

What happened:
This issue follows a discussion in kubernetes-sigs/cluster-api-provider-openstack#1605, as a request to automatically label nodes with underlying hostId information, so e.g. workload can be scheduled on different physical hosts. It can be used as a topology differentiator when all other topology labels are the same.

Anything else we need to know?:
Issue kubernetes/cloud-provider#67 is closed and potentially resolved by kubernetes/kubernetes#123223 so now based on comment kubernetes/cloud-provider#67 (comment), AdditionalLabels with hostId information can be added to InstanceMetadata:

cloud-provider-openstack/pkg/openstack/instancesv2.go

Lines 136 to 142 in dab0f06

	return &cloudprovider.InstanceMetadata{
	ProviderID: i.makeInstanceID(&server.Server),
	InstanceType: instanceType,
	NodeAddresses: addresses,
	Zone: server.AvailabilityZone,
	Region: i.region,
	}, nil

This information should be available in the server struct AFAIK, so there should not be too much work I think now.

One potential issue that I see is live migration, e.g. see #1801, where occm will have to update the label because hostId will change.

Environment:

• openstack-cloud-controller-manager(or other related binary) version:
• OpenStack version:
• Others:

[dulek]

This seems like a valid request from my point of view. @mdbooth, what do you think?

@chess-knight: Are you planning to contribute implementation?

[dulek]

@gryf, @stephenfin: This sounds like a low hanging fruit you can grab to get up to speed with CPO code.

[zetaab]

is hostid really available to normal user in openstack? I do not have access now to openstack, but if I remember correctly normal user cannot see that?

[chess-knight]

According to our research in SovereignCloudStack/issues#540, hostId should be available for all users. You can see e.g. in the nova code, that it is supported from API version 2.62 https://opendev.org/openstack/nova/commit/c2f7d6585818c04e626aa4b6c292e5c2660cb8b3. hostId is different from host. The host is available only to admins but hostId is a hash of (project_id + host) so it can be available to all.

[zetaab]

actually I can see both hostId and host_id which has same value. Perhaps that label could be added if it exists. This can be added in https://github.com/kubernetes/cloud-provider-openstack/blob/master/pkg/openstack/instancesv2.go#L136-L142 as AdditionalLabels https://github.com/kubernetes/cloud-provider/blob/d2f5e75a5fd6d31f75b1519fe20879b1ab5347b8/cloud.go#L300

[chess-knight]

actually I can see both hostId and host_id which has same value. Perhaps that label could be added if it exists. This can be added in https://github.com/kubernetes/cloud-provider-openstack/blob/master/pkg/openstack/instancesv2.go#L136-L142 as AdditionalLabels https://github.com/kubernetes/cloud-provider/blob/d2f5e75a5fd6d31f75b1519fe20879b1ab5347b8/cloud.go#L300

Yes, exactly as I originally wrote in the issue, thanks.
As @dulek said, it shouldn't be hard to implement this, but IMO discussion is needed here, on what should/will happen when live migration happens. When we initially set this custom label, it may not be correct later because the underlying host might change. So, should we consider this custom label only as "initial_host_id"? Or is there anything that we can do to reconcile it?

[zetaab]

we are not using live migrations at all, so difficult to say how it works.

[mdbooth]

IMHO the node controller should update node labels to reflect their current reality, i.e. a live migration will trigger node relabelling the next time the node is reconciled. Most things currently running on the Node are unlikely to act on it, but:

• Newly scheduled workloads can act on it
• Existing workloads at least have the possibility to notice that they're in the wrong place now
• We should document the known limitations and recommend an alternative (node reprovisioning)

We should also validate this decision with the cloud-provider folks in case there are caveats we're not aware of.

[mdbooth]

I'm very much in favour of adding a HostID label. I can confirm that HostID is unprivileged. It's an opaque value which can't be used to determine anything about the host including its name.