feat: add upgrade cluster playbook - #3102
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: redscholar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive cluster upgrade feature, including a new CLI command, an upgrade playbook, and specialized roles for component backups and version validations. The review identifies several technical improvements: addressing potential 'undefined variable' errors in CRI pre-checks for non-cluster hosts, correcting the use of 'run_once' on tasks relying on host-specific etcd data, removing redundant fact-gathering steps to improve performance, and replacing bash-specific syntax with portable shell scripts for better compatibility across environments.
| when: | ||
| - .cri.container_manager | eq "containerd" | ||
| - .upgrade.cri | not | ||
| - .upgrade | default dict | empty | not | ||
| - .containerd_current_version.stdout | empty | not |
There was a problem hiding this comment.
The variable .containerd_current_version is only registered for nodes in the k8s_cluster group (see builtin/core/roles/defaults/tasks/main.yaml). Since this precheck role is executed for all hosts in the playbook, it will cause an 'undefined variable' error on hosts that are not part of the Kubernetes cluster (e.g., standalone load balancers or external etcd nodes). You should add a check to ensure the host belongs to the k8s_cluster group before accessing this variable.
- name: CRI | Validate installed containerd version when not upgrading cri
when:
- .groups.k8s_cluster | default list | has .inventory_hostname
- .cri.container_manager | eq "containerd"
- .upgrade.cri | not
- .upgrade | default dict | empty | not
- .containerd_current_version.stdout | default "" | empty | not| run_once: true | ||
| when: | ||
| - .upgrade.etcd | not | ||
| - .upgrade | default dict | empty | not | ||
| - .etcd_install_version.error | empty |
There was a problem hiding this comment.
Using run_once: true on a task that relies on host-specific variables like .etcd_install_version is risky. If the first host in the play's inventory is not an etcd node, this validation will be skipped entirely. Additionally, if the first host is not an etcd node, .etcd_install_version will be undefined, leading to a playbook execution error. It is better to remove run_once: true and scope the task to the etcd group to ensure every etcd node is validated.
- name: ETCD | Validate installed etcd version is sufficient (when not upgrading etcd)
when:
- .groups.etcd | default list | has .inventory_hostname
- .upgrade.etcd | not
- .upgrade | default dict | empty | not
- .etcd_install_version.error | empty| - hosts: | ||
| - etcd | ||
| serial: 1 | ||
| gather_facts: true |
There was a problem hiding this comment.
The gather_facts: true directive is used in multiple plays throughout this playbook. Since facts are gathered for all hosts in the first play (line 4), they are cached and available for the remainder of the playbook execution. Removing redundant fact gathering in subsequent plays (lines 38, 57, 67, 84) will improve the overall performance of the upgrade process.
| for ((i=1; i<=30; i++)); do | ||
| if systemctl is-active kubelet.service >/dev/null 2>&1; then | ||
| echo "✅ kubelet is active" | ||
| exit 0 | ||
| fi | ||
| sleep 5 | ||
| done |
There was a problem hiding this comment.
The loop uses bash-specific syntax ((i=1; i<=30; i++)). While KubeKey environments typically have bash, it is safer and more portable to use a standard while loop or the seq command to ensure compatibility across different Linux distributions where /bin/sh might not be bash.
i=1
while [ $i -le 30 ]; do
if systemctl is-active kubelet.service >/dev/null 2>&1; then
echo "✅ kubelet is active"
exit 0
fi
i=$((i+1))
sleep 5
donef6922b9 to
4e85d62
Compare
48b400f to
0cd9d7f
Compare
3536b85 to
ac3fe8d
Compare
ac3fe8d to
09c4f02
Compare
|
816f8f7 to
e02c3d9
Compare
e02c3d9 to
aba0854
Compare
|
This PR has multiple commits, and the default merge method is: squash. DetailsInstructions 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. |
0f9ec0b to
6cd9dd4
Compare
6cd9dd4 to
a88a825
Compare
Signed-off-by: redscholar <blacktiledhouse@gmail.com>
Previously `kk upgrade cluster` always upgraded Kubernetes plus every component wired in the playbook, with no way to target a single component. This change makes Kubernetes the base of cluster upgrades and lets users scope the operation per component. - `kk upgrade cluster` now upgrades Kubernetes only by default. - `kk upgrade cluster --all` upgrades Kubernetes and all wired components (etcd, cri, cni, storage_class). - `kk upgrade cluster --set upgrade.<component>=true` enables an individual component without adding a dedicated flag. - `kk upgrade etcd|cri|cni|storageclass` are new top-level subcommands that upgrade a single component only (Kubernetes base disabled). - The kubernetes upgrade role is now gated behind `upgrade.kubernetes`, which is what makes single-component upgrades possible. - precheck scenario detection now keys off `upgrade.kubernetes` instead of the presence of `upgrade.all`, so component-only upgrades no longer fail the version comparison for an already-target Kubernetes. - A warning is added when `kk upgrade etcd` is run against a stacked (non-external) etcd cluster, which is managed by kubeadm. Signed-off-by: redscholar <blacktiledhouse@gmail.com>
KubeKey now computes the sequence of intermediate minor versions and upgrades the cluster one minor at a time, so a cross-many-minor upgrade (e.g. v1.23 -> v1.34) becomes a single 'kk upgrade cluster --with-kubernetes v1.34.3 --all' command instead of requiring an exact single-version target. - Add upgrade-path computation (builtin/core/upgrade_path.go) and auto-step orchestration in cmd/kk/app/builtin/upgrade.go and the upgrade_cluster playbook. - upgrade-kubernetes: drop the kubeadm --config flag for Kubernetes >= v1.30 (kubeadm no longer accepts the legacy dual-document config there) and run 'kubeadm upgrade apply' with --ignore-preflight-errors=all (KubeKey already performs its own prechecks). - Calico: pre-apply Calico CRDs before the Helm upgrade so CRDs newly introduced by the target version (e.g. Goldmane/Whisker in v3.31) exist before the chart renders. - defaults: make the running-version probe robust with a 3-tier fallback (kubectl get nodes -> apiserver static-pod image -> kubelet --version). - precheck: adapt version/etcd/inventory/cri checks for the auto-step flow. Signed-off-by: redscholar <blacktiledhouse@gmail.com>
Add unit tests covering the new auto-step upgrade flow, and consolidate the duplicated test helpers that lived in each module's internal package. - test/builtin/core: add upgrade_path_test.go (minor-version sequencing) and upgrade_logic_test.go (auto-step orchestration / --all flag handling). - cmd/kk/app/options/builtin: add upgrade_test.go; pkg/converter/tmpl: add calico_repro_test.go (Calico CRD pre-apply regression). - Refactor pkg/modules test helpers: move NewTestVariable out of each module's internal/test.go into the shared pkg/modules/testutil package, and update every module test to import it. This removes the per-module duplication while keeping connector types (ConnKey/ExecOptions) in pkg/modules/internal. Signed-off-by: redscholar <blacktiledhouse@gmail.com>
a88a825 to
91456bb
Compare
|





What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for reviewers:
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: