KAAP-519: Download kubelet, kubectl and cri-tools binaries from k8s / cri repo not APT - #74
KAAP-519: Download kubelet, kubectl and cri-tools binaries from k8s / cri repo not APT#74hsri-pf9 wants to merge 4 commits into
Conversation
Changelist by BitoThis pull request implements the following key changes.
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #42a020
Actionable Suggestions - 1
-
installer/bundle_builder/ingredients/deb/download.sh - 1
- Double 'v' prefix in CNI_VERSION variable · Line 17-19
Review Details
-
Files reviewed - 3 · Commit Range:
581dddc..581dddc- installer/bundle_builder/build-bundle.sh
- installer/bundle_builder/ingredients/deb/Dockerfile
- installer/bundle_builder/ingredients/deb/download.sh
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at mithil@platform9.com.
Documentation & Help
AI Code Review powered by
| K8S_VERSION="v${KUBERNETES_VERSION%%-*}" | ||
| CRICTL_VERSION="v${CRITOOL_VERSION}" | ||
| CNI_VERSION="v${CNI_VERSION}" |
There was a problem hiding this comment.
The K8S_VERSION and CRICTL_VERSION variables are correctly prefixed with 'v', but CNI_VERSION is incorrectly prefixed with 'v' twice since the environment variable already includes the 'v' prefix. This will cause download failures for CNI plugins.
Code suggestion
Check the AI-generated fix before applying
@@ -17,7 +17,7 @@
+K8S_VERSION="v${KUBERNETES_VERSION%%-*}"
+CRICTL_VERSION="v${CRITOOL_VERSION}"
-+CNI_VERSION="v${CNI_VERSION}"
++CNI_VERSION="${CNI_VERSION}"
+mkdir -p /ingredients
Code Review Run #42a020
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Code Review Agent Run #472fa5Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Replace apt/.deb sourcing (installer/bundle_builder/ingredients/deb/download.sh) with direct upstream releases: - kubeadm, kubelet, kubectl: raw binaries from dl.k8s.io, each verified against its .sha256 before use (previously no integrity verification at all). - crictl: kubernetes-sigs/cri-tools release tarball. - CNI plugins: containernetworking/plugins release tarball. - containerd: the plain containerd-<version>-<os>-<arch>.tar.gz tarball instead of the cri-containerd-cni-* bundle, which containerd's own release notes mark deprecated since 1.6 (removed in 2.0) and which duplicated crictl/CNI plugins already sourced separately above. - runc: added as its own ingredient (opencontainers/runc releases), since the plain containerd tarball doesn't bundle it the way the deprecated one did. Also fixes CRITOOL_VERSION's default (1.32.0-1.1, a leftover Debian-revision suffix from the old apt-based versioning) which produced a 404 against cri-tools' real v1.32.0 tag; strips any such suffix defensively the same way KUBERNETES_VERSION already was. build-bundle.sh and .ci/build-push-bundle.sh are updated to match the new ingredient filenames and pass RUNC_VERSION through.
install.sh.tmpl and uninstall.sh.tmpl still assumed the old apt/.deb bundle layout (dpkg --install/--purge against kubeadm.deb, kubelet.deb, kubectl.deb, cri-tools.deb, kubernetes-cni.deb; containerd.tar). Neither matched what the bundle builder now actually produces, so install would fail outright on a real host: no such files exist in the bundle. Update both scripts to the layout the previous commit's bundle builder produces: install kubeadm/kubelet/kubectl as plain executables to /usr/bin, extract crictl.tar.gz to /usr/local/bin, extract cni-plugins.tgz to /opt/cni/bin, install runc to /usr/local/sbin, and extract the plain containerd.tar.gz to /usr/local (matching containerd's own docs convention, since the tarball's entries are now bin/containerd etc. instead of the deprecated bundle's absolute-path layout). uninstall.sh.tmpl removes the same set, deriving exact paths from each tarball's own file listing rather than a hardcoded package list.
Records the rationale for sourcing kubeadm/kubelet/kubectl/crictl/CNI plugins/containerd/runc from upstream releases instead of apt (previous two commits), and flags why this branch is not yet mergeable: the bundle OCI tag (quay.io/platform9/byoh-bundle-<os>_k8s:<k8s-version>) encodes only the Kubernetes version, not the bundle's internal file layout. Since install.sh.tmpl/uninstall.sh.tmpl are embedded in the manager binary and a bundle push to quay.io happens independently (a human running .ci/build-push-bundle.sh), there is no safe order to land the manager change and the bundle push in -- whichever lands second breaks every manager version still relying on the other format at that same mutable tag, including already-deployed production managers. Fixing that needs a bundle-format version folded into the OCI tag itself (installer/bundle_downloader.go, installer/registry.go, k8sinstallerconfig_controller.go) -- out of scope for this change, called out here as a blocker.
Adds two test cases to common_ubuntu_test.go asserting the rendered Install()/Uninstall() scripts reference the new bundle layout (raw kubeadm/kubelet/kubectl binaries, crictl.tar.gz, cni-plugins.tgz, runc, containerd.tar.gz) and contain no leftover dpkg/.deb references -- guarding against the exact regression this branch fixes (templates drifting from what the bundle builder actually packages) without needing a real bundle or host.
d0169d8 to
068a93e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
What this PR does / why we need it:
Changed the way of downloading kubelet, kubectl, kubeadm and cri-tools not from apt but directly downloading it from the repo via curl and adding them to the bundle.
Summary by Bito
This pull request transitions from apt-based package downloads to direct downloads from remote repositories, refining the bundle builder, Dockerfile, and download script. The changes improve command syntax, update environment variables, and implement curl for fetching binaries. The PR also corrects variable formatting, enhances docker run commands with additional environment variables, and explicitly sets CNI_VERSION in the Dockerfile for a more robust deployment process.