[occm] Add OpenStack server host ID as node label

[stblatzheim]

What this PR does / why we need it:

Adds the OpenStack server hostId as the
topology.openstack.org/host-id Kubernetes node label when Nova
returns a non-empty value.

The label provides a project-scoped, opaque representation of the
underlying compute host. It can be used with topology spread
constraints and pod anti-affinity to distribute workloads across
different OpenStack compute hosts.

The label is applied during cloud node initialization. It is currently
not reconciled after a live migration, which is documented as a known
limitation.

This PR supersedes #2628 and incorporates the latest review feedback.
Credit for the original implementation goes to @chess-knight.

Which issue this PR fixes:

Fixes #2579

Special notes for reviewers:
@kayrus as requested a new PR with the requested changes - original PR was #2628

Release note:

[openstack-cloud-controller-manager] Nodes are now labeled with `topology.openstack.org/host-id` when Nova returns a non-empty server host ID, enabling topology-aware workload distribution across OpenStack compute hosts.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: stblatzheim / name: Steven Blatzheim (d650600)

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign stephenfin for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[kubernetes-prow]

Welcome @stblatzheim!

It looks like this is your first PR to kubernetes/cloud-provider-openstack 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/cloud-provider-openstack has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[kubernetes-prow]

Hi @stblatzheim. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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-sigs/prow repository.

[kayrus]

/ok-to-test

[winiciusallan]

while this looks like a desired feature, I saw a lot of discussions in the linked issue about the capacity of reconciling node labels to avoid drift in case, for example, the instance has been migrated.

doesn't the node controller handle label reconciling?

[stblatzheim]

@winiciusallan I think reconciling node labels is out of scope here.
As you said there was a lot of discussion regarding this and the main point was that it is not in the intend of kubernetes to have such live migrations. The normal way would be to drain the node and spawn a new node.

Also the affinitys for scheduling pods are done at pod creation time => Even a changed node label afterwards would just be taken into account for new pods.

Maybe we can raise a new issue for updating node label on reconcile afterwards, atm we don't even have node labels to work on ...

[kayrus]

As you said there was a lot of discussion regarding this and the main point was that it is not in the intend of kubernetes to have such live migrations. The normal way would be to drain the node and spawn a new node.

there could be system migration to drain the hypervisor for maintenance on the openstack level.

[stblatzheim]

@kayrus

You are right that an OpenStack operator may live-migrate instances while
draining a hypervisor for maintenance, independently of Kubernetes.

I checked the current cloud-node-controller implementation used here
(k8s.io/cloud-provider v0.36.0). AdditionalLabels are applied only during
the initial synchronization of a node carrying the external-cloud-provider
taint. The periodic node status update calls InstanceMetadata() again, but
currently applies only NodeAddresses.

Therefore, this PR provides the OpenStack-specific desired state and labels
newly initialized nodes, while backfilling existing nodes and updating the
label after a migration require a generic change in
kubernetes/kubernetes/staging/src/k8s.io/cloud-provider.

There is no strict runtime dependency between the two changes: the generic
controller can reconcile the labels returned in AdditionalLabels, including
creating a missing label. Before this provider starts returning the host ID,
that reconciliation is simply a no-op.

There have already been two upstream attempts addressing this:
kubernetes/kubernetes#130888 and kubernetes/kubernetes#131096. I propose to
create a focused follow-up based on #130888 which periodically adds or updates
provider-supplied AdditionalLabels.

This also matches the previous conclusion in #2579 that exposing HostID is an
immediate provider-side improvement, while continuous reconciliation is a
generic and somewhat independent cloud-node-controller concern.

Existing pods will still not be automatically rescheduled after a migration,
but updating the label makes the changed placement observable and ensures that
new pods are scheduled against the current host topology.

Would you be comfortable merging this provider-side prerequisite while the
generic reconciliation is tracked in a linked upstream PR, or would you prefer
the upstream PR to be opened first?

[kayrus]

@stblatzheim I don't mind merging this right now. Need to discuss this with other maintainers
/assign @zetaab @stephenfin @mandre