Add generic pre upgrade hook for node

[mickenordin]

This patch is based on #11368 and adds the ability to run a hook job before drain of a node. My specific use-case is doing live migration of all VMs running in OpenStack in Kubernetes before draining a host, allowing OpenStack workloads to survive upgrades. I am sure other use-cases are available too :)

What type of PR is this?

/kind feature

What this PR does / why we need it:

Kubespray already exposes post_upgrade_hooks for running custom tasks per-host after the kubelet/containerd upgrade but before uncordon. This PR adds a symmetrical pre_upgrade_hooks for running custom tasks per-host before the drain happens.

Without a pre-drain hook there is no clean place to inject workload-level prep work that has to happen while the node is still hosting workloads, such as live-migrating OpenStack VMs off a compute node, gracefully draining stateful clients, scheduling-policy adjustments, or anything else that needs the node to still be schedulable.

The hook fires from roles/upgrade/pre-upgrade/tasks/main.yml right after needs_cordoning is set and right before the Node draining block, mirroring the existing post-hook shape:

- name: Run pre upgrade hooks before draining
  when:
    - needs_cordoning
  loop: "{{ pre_upgrade_hooks | default([]) }}"
  ansible.builtin.include_tasks: "{{ item }}"

Default is an empty list, so the change is a no-op for any user who does not set the variable.

Which issue(s) this PR fixes:

Fixes #13288

Special notes for your reviewer:

• Builds on the same mechanism as Add generic post upgrade hooks for node #11368 (post_upgrade_hooks): same variable shape, same include_tasks loop, just earlier in the per-host serial loop.
• Gated on needs_cordoning so the hook only fires for nodes that will actually be drained on this run, matching the gating of the Node draining block right below it.
• No new dependencies, no defaults change, no behavior change unless the user sets pre_upgrade_hooks.

Does this PR introduce a user-facing change?:
Yes

Add `pre_upgrade_hooks` variable to `upgrade-cluster.yml`, mirroring the existing `post_upgrade_hooks`. Hook tasks listed in this variable run per-host before the drain step during a cluster upgrade, providing a place to perform workload-aware preparation such as live-migrating OpenStack VMs off a compute node.

[k8s-ci-robot]

Hi @mickenordin. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mickenordin
Once this PR has been reviewed and has the lgtm label, please assign tico88612 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

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for running configurable “pre-upgrade hooks” prior to draining a node during the pre-upgrade stage.

Changes:

• Introduces a pre_upgrade_hooks loop that includes hook task files before the drain step when needs_cordoning is true.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yankay]

/ok-to-test

[tico88612]

#12408 has a similar design, but after discussing it with the other maintainers, we don't think adding hooks (in anywhere) a good approach. (as you can see we don't merge #12408)

FYI:
#12408 (comment)
#12408 (comment)

[mickenordin]

#12408 has a similar design, but after discussing it with the other maintainers, we don't think adding hooks (in anywhere) a good approach. (as you can see we don't merge #12408)

FYI: #12408 (comment) #12408 (comment)

I don't see any comments about why adding these features is a bad idea, only that you preferred to add some special code for a few specific use-cases?

Being able to do post-processing (already possible with a hook today) and pre-processing of generic tasks is extremely useful, as demonstrated by the multiple previous pr:s (that I had missed). How would I be able to achieve my use-case of live-migrating vm:s running in openstack during rolling upgrades with kubespray any other way, unless you want to add a special case code for that specific use case too? Not having that ability pretty much excludes anyone running any of the openstack kubernetes deployments for k8s out there from using kubespray to manage their cluster?

[tico88612]

I don't see any comments about why adding these features is a bad idea

The reasoning is already in those comments — let me put it more directly.

The real problem isn't this particular use case; it's that the "generic hooks" mechanism lacks a unified design. The prior art (post_upgrade_hooks) was merged as a three-line task; its existence doesn't mean we endorse that direction, and it doesn't create an obligation to keep adding more. Once we add another, the next person shows up with their own use case, asking if they can add one too, and our upgrade flow ends up tied down by a series of ad-hoc, uncoordinated hooks; after which, changing any internal implementation risks breaking someone's hook.

This hook has no tests, no documentation, and no defined failure semantics — if a hook fails, should the upgrade stop or continue? That means there's no functional guarantee of any kind, and I don't think a maintainer can take responsibility for an execution point without a contract.

Live-migrating OpenStack VMs off a compute node. Kubernetes drain only knows about Pods, so any libvirt VMs running under nova-compute are killed when nova-compute is evicted. A pre-drain hook lets us call openstack server migrate --live for every VM on the host before kubespray touches it, so OpenStack tenants survive cluster upgrades.

What's the actual relationship between openstack server migrate and Kubespray here? I run OpenStack alongside Kubespray and have never hit this. For me, they're separate systems. Is nova-compute colocated on your kubespray-managed nodes?

[mickenordin]

The real problem isn't this particular use case; it's that the "generic hooks" mechanism lacks a unified design. The prior art (post_upgrade_hooks) was merged as a three-line task; its existence doesn't mean we endorse that direction, and it doesn't create an obligation to keep adding more. Once we add another, the next person shows up with their own use case, asking if they can add one too, and our upgrade flow ends up tied down by a series of ad-hoc, uncoordinated hooks; after which, changing any internal implementation risks breaking someone's hook.

You are of course entitled to manage your project as you wish, I was looking for a solution to my problem, found that you had implemented half of what I needed already, and wanted to add the other part.

This hook has no tests, no documentation, and no defined failure semantics — if a hook fails, should the upgrade stop or continue? That means there's no functional guarantee of any kind, and I don't think a maintainer can take responsibility for an execution point without a contract.

This is definitely a valid concern. Given the number of PR's that you have had at this point, wanting to implement a complete hook system, rather than a single hook at one point in the upgrade process, maybe this is something to focus on?

Perhaps a system of a small number of well defined hooks, with a well documented contract and usage documentation is something that would be of value to this project. I am willing to put some effort in adding such a mechanism, if you are not happy with just implementing a single hook, along the line of the one you already have.

What's the actual relationship between openstack server migrate and Kubespray here? I run OpenStack alongside Kubespray and have never hit this. For me, they're separate systems. Is nova-compute colocated on your kubespray-managed nodes?

Yes, exactly. There are several projects that package OpenStack as Kubernetes manifests with the compute happening on the worker nodes of the kubernetes cluster.

[mickenordin]

I take it you won't be merging this?

That is fine of course, I can soft-fork and maintain a patch set outside of the repo too, but I would prefer if it was not necessary 😅

[mickenordin]

I have now added a system of four hooks:

• pre_cordon_hooks
• pre_upgrade_hooks
• post_upgrade_hooks
• post_uncordon_hooks

With documentation and a failure contract. If you don't like it, please close the PR as WONTFIX.

[tico88612]

Yes, exactly. There are several projects that package OpenStack as Kubernetes manifests with the compute happening on the worker nodes of the kubernetes cluster.

I think it's reasonable. However, as for the specifics of how to implement or add it, you'll need to get the other approvers on board.

At least for now, I'm not looking to turn down this proposal. If we can have systematic hooks, I think we can leverage advantages that other K8s distributions don't have.

cc @VannTen