[CKS] script to generate cks images with cilium as default CNI#12619
[CKS] script to generate cks images with cilium as default CNI#12619ewerton-silva00 wants to merge 3 commits intoapache:mainfrom
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
|
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12619 +/- ##
============================================
+ Coverage 17.90% 19.03% +1.13%
- Complexity 16094 16154 +60
============================================
Files 5938 5485 -453
Lines 532864 494612 -38252
Branches 65192 58103 -7089
============================================
- Hits 95392 94170 -1222
+ Misses 426793 389890 -36903
+ Partials 10679 10552 -127
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 16769 |
There was a problem hiding this comment.
Pull request overview
Adds a new utility script to generate CloudStack Kubernetes Service (CKS) “binaries ISO” images that bundle Kubernetes components plus manifests/images needed to run with Cilium as the default CNI, addressing the requested Cilium support in #9304.
Changes:
- Introduces
create-kubernetes-binaries-iso-with-cilium.shto build an ISO with Kubernetes binaries, CNI/crictl, addons/manifests, and pre-pulled container images. - Generates the Cilium manifest via Helm with a set of default Cilium configuration values.
- Updates image collection/pulling logic to include images referenced by generated Cilium and Dashboard manifests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sudo $PKG_MGR --assumeyes remove docker-common docker container-selinux docker-selinux docker-engine | ||
| sudo $PKG_MGR --assumeyes install lvm2 device-mapper device-mapper-persistent-data device-mapper-event device-mapper-libs device-mapper-event-libs | ||
| sudo $PKG_MGR --assumeyes install http://mirror.centos.org/centos/7/extras/x86_64/Packages/container-selinux-2.107-3.el7.noarch.rpm | ||
| sudo $PKG_MGR --assumeyes install containerd.io | ||
| elif [ -f /etc/debian_version ] || command -v apt-get > /dev/null 2>&1; then |
There was a problem hiding this comment.
RedHat install path downloads a container-selinux RPM from a CentOS 7 x86_64 URL. This will fail (and possibly install an incompatible package) on aarch64/arm64, even though the script advertises arm support. Consider selecting the correct arch/repo or using distro-provided packages/repos instead of a hard-coded x86_64 RPM URL.
shwstppr
left a comment
shwstppr
left a comment
There was a problem hiding this comment.
@ewerton-silva00 can you please check comments from Copilot and look into failing pre-commit GHA?
|
@shwstppr, yes. I am reviewing the points reported by Copilot. I will make the necessary adjustments, test, and submit the changes. |
DaanHoogland
changed the title
* etcd download: use arch variable instead of hard-coded amd64 Replace hard-coded linux-amd64 in etcd tarball download URL and output filename with linux- to support both amd64 and arm64 architectures. * container-selinux: install from distro repos instead of CentOS 7 mirror Remove hard-coded CentOS 7 x86_64 RPM URL for container-selinux and install directly from the system's configured repositories, which handles architecture and version automatically. * build_name: fix dead-code fallback default Check whether parameter is empty before constructing build_name, so the fallback default setup--.iso is actually reachable. * CNI download: fail on HTTP errors for both URL attempts Use curl -f on both primary and legacy CNI download URLs and exit with a clear error if neither succeeds, instead of only checking for 404 on the first attempt. * temp directory: use mktemp and trap for cleanup Replace fixed /tmp/iso with mktemp -d to avoid collisions between concurrent runs, and register a trap EXIT to ensure cleanup on failure or early exit.
|
@shwstppr and @DaanHoogland, could you please review this Pull Request again? |
| echo "Downloading etcd ${ETCD_VERSION}..." | ||
| curl -sS -L "https://github.com/etcd-io/etcd/releases/download/${ETCD_VERSION}/etcd-${ETCD_VERSION}-linux-${ARCH}.tar.gz" -o "${etcd_dir}/etcd-linux-${ARCH}.tar.gz" | ||
|
|
||
| mkisofs -o "${output_dir}/${build_name}" -J -R -l "${iso_dir}" |
There was a problem hiding this comment.
| mkisofs -o "${output_dir}/${build_name}" -J -R -l "${iso_dir}" | |
| mkisofs -o "${output_dir}/${build_name}" -J -R -l "${iso_dir}" | |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ested by Copilot. cks: harden ISO build script with pipefail, --fail, and strict mode: - Replace 'set -e' with 'set -o errexit', 'set -o nounset', 'set -o pipefail'; - Add TRACE env var support for debug with 'set -o xtrace'; - Add --fail (-f) flag to curl commands in pipelines to prevent silent; - Add --fail and tar.gz integrity validation for etcd download; - Fix ARCH validation error message to list all accepted values (x86_64, amd64, aarch64, arm64). cks: fix semver comparison using sort -V instead of awk: - Replace awk numeric comparison with a sort -V based version_lt() function. The previous approach treated '1.9.0' as 1.9 and '1.18.0' as 1.18, making 1.9 > 1.18 and selecting the wrong source for kubelet.service and 10-kubeadm.conf. cks: vendor kubelet.service and 10-kubeadm.conf to reduce supply-chain risk: - Vendor kubelet.service and 10-kubeadm.conf from shapeblue/cloudstack-nonoss into the repository instead of fetching them at build time from a mutable remote branch. These files are executed with root privileges as systemd units, and pinning to a mutable branch without checksum verification posed a supply-chain risk. cks: only add Cilium Helm repo if not already configured: - Replace 'helm repo add ... || true' with an explicit check via 'helm repo list'. The previous approach suppressed all failures (including network/DNS errors), which could cause a later, less-clear failure during 'helm template'. cks: move Cilium ISO script to scripts/util/cks/ and add download progress: - Move create-kubernetes-binaries-iso-with-cilium.sh to scripts/util/cks/ alongside vendored kubelet.service and 10-kubeadm.conf; - Download kubeadm, kubelet, kubectl individually with --progress-bar for visibility during long downloads.
|
@DaanHoogland, could you please review this Pull Request again? |
6 participants
…a default CNI
Description
This PR fixes #9304.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
The script in question generates CloudStack Kubernetes Service (CKS) images using Cilium as the default CNI.
Additionally, the other components are up-to-date.
You can also use ready-made images from my repository.. See: https://download.suanuvem.io/cks/