Sliding-window worker upgrades without a strategy plugin

[Payback159]

What type of PR is this?

/kind feature

What this PR does / why we need it:

upgrade-cluster.yml walks the worker nodes in serial: batches. That has two
costs:

• Every node in a batch waits for the slowest one. A node that drains slowly
  holds its peers idle even when the cluster has capacity to keep going.
• The batch is drained as a unit. With serial: 2 and a PodDisruptionBudget
  of minAvailable: 2, the first eviction is allowed, the second breaches the
  budget, and the budget cannot recover because the first node may not uncordon
  until the whole batch clears the drain task. Both nodes fail.

This adds an opt-in upgrade_strategy: graceful_rolling that replaces the batch
with a sliding window: every worker runs independently and a node that finishes
hands its slot to the next waiting node immediately. linear remains the
default and is unchanged.

$ ansible-playbook upgrade-cluster.yml -i inventory/mycluster/hosts.yaml \
    -e upgrade_strategy=graceful_rolling \
    -e upgrade_node_concurrency=3 --forks 10

Measured on six Ubuntu 24.04 VMs (three workers), window and serial both 2:

Situation	linear	graceful_rolling
Uniform nodes, nothing blocking	same	same
One node 150s slower than its peers	471s	369s
PodDisruptionBudget, minAvailable: 2	drain fails after 591s	394s

The first row is deliberate and documented: with uniform nodes the window admits
the next node when one finishes, which is when a batch would have rotated
anyway. The gain comes from variance, which is the common case in practice, and
from not draining in lockstep.

Which issue(s) this PR fixes:

Fixes #12929

Special notes for your reviewer:

This does not add a strategy plugin, despite the issue title. ansible-core
deprecated custom strategy plugins
with no replacement API, which is why #13080 was closed. It also turned out to
be unnecessary: the built-in host_pinned already is a sliding window. What
#13080's fork of free.StrategyModule.run() added on top was a window narrower
than the fork count and per-group ceilings, and both fit in two action plugins —
a supported extension point. This PR supersedes #13080.

Most of the diff is not the semaphore but working around three ansible-core
behaviours. They are worth knowing before reviewing:

1. run_once is ignored outside linear — free warns and runs the task on
   every host. This is why only the worker play is rolling (the control plane
   already runs serial: 1; the calico play never cordons a node), and why
   roles/kubernetes/kubeadm's cluster-scoped kube-proxy tasks were split out.
   Left alone they would kubectl replace one ConfigMap from several nodes at
   once and each node would delete every kube-proxy pod in the cluster.
2. any_errors_fatal and max_fail_percentage exist only in linear —
   free just warns. The worker play sets any_errors_fatal, so switching
   strategy would have dropped it silently. upgrade_abort_on_failure (default
   on) restores the intent: once a node fails, nodes not yet drained abort
   instead of starting, while nodes already inside the window finish rather than
   being left cordoned.
3. pause sets BYPASS_HOST_LOOP and free raises on it at dispatch,
   before any when: is evaluated — so guarding the prompts with a condition
   does not help. The two confirmation prompts moved into dynamically included
   files; the timed waits use wait_for, which runs per host.

Points worth deliberate attention:

• kubeadm_patch_kube_proxy defaults to off. The cluster-scoped kube-proxy
  tasks are now opt-in; cluster.yml and scale.yml ask for them explicitly.
  Anyone invoking kubernetes/kubeadm directly from the collection loses the
  patch unless they opt in. Deriving the flag from upgrade_strategy instead is
  unsafe — it is an inventory-level setting, so graceful_rolling in
  group_vars would silently disable the patch for ordinary installs, and
  Ansible exposes no magic variable for the running play's strategy.
  upgrade_cluster.yml now runs the patch once in its own linear play, which
  also stops the linear path repeating it per serial batch.
• The worker play's roles: became tasks: + import_role, because
  always: needs a block. Checked first that no role in that play reads a
  sibling role's defaults/; import_role is static, so tags propagate as
  before.
• Forks must exceed the concurrency, since a waiting host holds a worker.
  The window is clamped to forks - 1 with a warning rather than a hard failure.
  The fork count is read from CLIARGS because DEFAULT_FORKS has no cli:
  mapping and would miss -f entirely.
• The CI job uses mode: ha, not all-in-one. An all-in-one node is in both
  kube_control_plane and kube_node, so kube_node:!kube_control_plane is
  empty and the rolling play would be skipped without failing. ha gives one
  dedicated worker, which covers the plugins, the validation play, host_pinned
  and the pause path. It does not cover the window itself — that needs three
  workers, and is covered by the unit tests instead.
• roles/kubernetes/client/tasks/main.yml:70 fails ansible-lint on master
  already; this PR does not touch that file.

Testing: 46 unit tests (tests/unit/plugins/action/, wired into pre-commit)
cover the concurrency maths, per-group ceilings and lease lifecycle. Manually
verified on Vagrant for a 1.35.7 → 1.36.3 upgrade with a blocking PDB, for the
linear-vs-rolling comparison above, and separately for system_upgrade: true
where the download role runs inside the window and both workers reboot without
losing a slot. docs/developers/upgrade-slots.md records the design constraints
and what to re-verify on an ansible-core bump.

Does this PR introduce a user-facing change?:

Add an opt-in `upgrade_strategy: graceful_rolling` for `upgrade-cluster.yml`, which upgrades worker nodes in a sliding window instead of `serial:` batches. A node that finishes frees its slot immediately, so no node waits for a slower peer and nodes are never drained in lockstep, avoiding the PodDisruptionBudget deadlock that can fail a batch upgrade. Window size is set with `upgrade_node_concurrency` (integer or percentage, capped at `forks - 1`) and can be constrained per inventory group with `upgrade_per_group_concurrency`; it requires more Ansible forks than the configured concurrency. The default remains `linear` and is unchanged. Note that the cluster-scoped kube-proxy kubeconfig patch in the `kubernetes/kubeadm` role is now gated behind `kubeadm_patch_kube_proxy`, which defaults to false; `cluster.yml`, `scale.yml` and `upgrade-cluster.yml` enable it themselves, but playbooks calling that role directly need to set it.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

Hi @Payback159. 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.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

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.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Payback159 / name: Payback159 (e8fcf04)

[VannTen]

Ok, this is very, very interesting.

In fact, reading the docs, it looks outright fantastic (per group ceilings !!).

I'm on vacation for four weeks, so I'm not gonna be able to review this quickly, but I'll try to chip away at it a bit.

One meta question: I think I saw some stuff about the download role ? How compatible would the PR be with #12299 and #12937 (which basically optimize for delegation to the controller and download stuff at once, letting only in the specific role putting the binary/files in their definitive place) ?

[Payback159]

Thanks, and enjoy the vacation — there's no rush on this at all.
Short answer: compatible, and #12299 / #12937 actually make this PR smaller.

The only thing this PR needs from the download story is that nothing cluster-scoped runs inside the rolling window, because run_once is ignored outside the linear strategy — free/host_pinned warn and then run the task on every host.

After your refactors, the roles inside the window contain no download logic at all. Those are node-local copy/unarchive tasks, which is precisely what a per-node sliding window wants. So the refactors shrink the surface this feature has to worry about.

[Payback159]

I test-merged each against this branch and ran a real upgrade on 6 VMs (3 control-plane,
3 workers, v1.34.4 -> v1.35.3, upgrade_node_concurrency=2). All 6 nodes Ready on the target version afterwards, no worker ever observed NotReady.

#12299 needs no change here. roles/download/tasks/main.yml and download_run_once
survive, so the worker play's import_role: download and the system_upgrade guard both
still do real work. That run exercises more of this PR than the #12937 one does.

#12937 lets me delete two things (~20 lines):

1. import_role: name: download in the worker play becomes a silent no-op — roles/download
   is a container for the subroles now and has no tasks/main.yml, so Ansible resolves the
   directory, finds no tasks and runs nothing without complaining.
2. The assert guarding system_upgrade references download_run_once / download_force_cache,
   which Refactor download (container) #12937 removes, so it would raise AnsibleUndefinedVariable. It is also moot by then.

I also hit three unrelated issues while doing this and have commented on them in the
respective PRs.

[yankay]

/ok-to-test

[Copilot]

Pull request overview

This PR adds an opt-in worker upgrade mode for upgrade_cluster.yml that avoids serial: batch lockstep by using Ansible’s host_pinned strategy plus controller-side action plugins to enforce a sliding concurrency window, improving upgrades under node-duration variance and avoiding some PodDisruptionBudget deadlocks. It also refactors cluster-scoped kube-proxy patching so it only runs under a safe (linear) context.

Changes:

• Add upgrade_strategy: graceful_rolling with upgrade_node_concurrency and optional per-group ceilings enforced via new acquire_upgrade_slot / release_upgrade_slot action plugins.
• Move interactive prompts behind dynamic includes and replace timed pauses with wait_for to remain compatible with free/host_pinned.
• Split kube-proxy cluster-scoped patching into an opt-in task file (kubeadm_patch_kube_proxy) and run it once in a dedicated linear play; add docs + CI coverage + unit tests.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/unit/plugins/action/test_upgrade_slots.py	Adds unit tests for the slot action plugins (concurrency resolution, leases, per-group limits, failure marker).
tests/files/ubuntu24-calico-graceful-rolling-upgrade.yml	New CI scenario config exercising upgrade_strategy=graceful_rolling end-to-end.
tests/files/ubuntu24-calico-graceful-rolling-upgrade	New CI selector/marker for the graceful rolling upgrade job.
tests/ansible.cfg	Ensures test runs can load action plugins from ../plugins/action.
roles/upgrade/pre-upgrade/tasks/main.yml	Moves interactive pause into a dynamic include; replaces sleep pause with wait_for.
roles/upgrade/pre-upgrade/tasks/confirm_upgrade.yml	New dynamically-included pause prompt task file.
roles/upgrade/post-upgrade/tasks/main.yml	Same prompt/include + wait_for change for post-upgrade confirmation/pause.
roles/upgrade/post-upgrade/tasks/confirm_uncordon.yml	New dynamically-included uncordon pause prompt task file.
roles/kubespray_defaults/defaults/main/upgrade.yml	Introduces upgrade strategy and window/lease/failure-control defaults.
roles/kubernetes/kubeadm/tasks/main.yml	Gates kube-proxy patch tasks behind kubeadm_patch_kube_proxy and moves them to an include file.
roles/kubernetes/kubeadm/tasks/kube_proxy_kubeconfig.yml	New task file containing the cluster-scoped kube-proxy ConfigMap rewrite + restart logic.
roles/kubernetes/kubeadm/defaults/main.yml	Adds kubeadm_patch_kube_proxy default (off) with rationale.
plugins/action/acquire_upgrade_slot.py	New controller-side semaphore action plugin to acquire an upgrade slot with window/per-group constraints.
plugins/action/release_upgrade_slot.py	New controller-side action plugin to release a slot and optionally record the first node failure.
playbooks/upgrade_cluster.yml	Implements validation play + rolling worker upgrade block using host_pinned + slot plugins; runs kube-proxy patch in a separate linear play.
playbooks/scale.yml	Enables kubeadm_patch_kube_proxy: true in a linear context.
playbooks/cluster.yml	Enables kubeadm_patch_kube_proxy: true in a linear context.
docs/operations/upgrades.md	Documents the new rolling strategy and required forks vs concurrency relationship.
docs/operations/upgrade-strategies.md	New documentation describing strategy tradeoffs, settings, and operational guidance.
docs/developers/upgrade-slots.md	New design notes explaining constraints/ansible-core behaviors and test strategy.
docs/_sidebar.md	Adds navigation entries for the new docs pages.
ansible.cfg	Adds action_plugins = ./plugins/action so playbooks can find the new action plugins.
.pre-commit-config.yaml	Adds a pre-commit hook to run the Python plugin unit tests.
.gitlab-ci/kubevirt.yml	Adds the new kubevirt CI job to the PR pipeline list.

Suppressed comments (2)

plugins/action/acquire_upgrade_slot.py:572

• lease_directory() claims to use Ansible’s per-run controller tmpdir, but ansible_local_tmp() currently just returns C.DEFAULT_LOCAL_TMP when it exists. With the repo’s ansible.cfg not setting local_tmp, that path is typically a stable base dir, so using a constant subdir (…/kubespray-upgrade) can make concurrent ansible-playbook runs share slots and failure markers.

    local_tmp = ansible_local_tmp()
    if local_tmp:
        return local_tmp / "kubespray-upgrade"

plugins/action/release_upgrade_slot.py:248

• release_upgrade_slot.lease_directory() mirrors acquire’s logic and also uses ansible_local_tmp() + a constant 'kubespray-upgrade' subdir. If DEFAULT_LOCAL_TMP resolves to a shared base directory, separate playbook runs on the same controller can interfere (shared slots / shared UPGRADE_FAILED marker).

        return ansible_home_tmp() / ("kubespray-upgrade-%s" % safe_run_id(run_id))

    local_tmp = ansible_local_tmp()
    if local_tmp:
        return local_tmp / "kubespray-upgrade"

---

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