chore(cks): include new script to generate cks images with cilium as …#12619
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% 17.90% -0.01%
+ Complexity 16094 16091 -3
============================================
Files 5938 5938
Lines 532864 532864
Branches 65192 65192
============================================
- Hits 95392 95385 -7
- Misses 426793 426801 +8
+ Partials 10679 10678 -1
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.
| echo "Downloading etcd ${ETCD_VERSION}..." | ||
| curl -sS -L "https://github.com/etcd-io/etcd/releases/download/${ETCD_VERSION}/etcd-${ETCD_VERSION}-linux-amd64.tar.gz" -o ${etcd_dir}/etcd-linux-amd64.tar.gz | ||
|
|
There was a problem hiding this comment.
etcd tarball download is hard-coded to linux-amd64, which will produce a broken ISO when ARCH is arm64/aarch64. Use the selected ARCH when building the etcd download URL and output filename (and ensure the upstream release asset name matches the arch mapping you use).
| echo "Downloading etcd ${ETCD_VERSION}..." | |
| curl -sS -L "https://github.com/etcd-io/etcd/releases/download/${ETCD_VERSION}/etcd-${ETCD_VERSION}-linux-amd64.tar.gz" -o ${etcd_dir}/etcd-linux-amd64.tar.gz | |
| # Determine etcd architecture based on ARCH/script argument, matching etcd release asset naming. | |
| # Default to amd64 if ARCH is not set or unrecognized. | |
| ETCD_ARCH="amd64" | |
| if [ -n "${ARCH:-}" ]; then | |
| case "${ARCH}" in | |
| amd64|x86_64) | |
| ETCD_ARCH="amd64" | |
| ;; | |
| arm64|aarch64) | |
| ETCD_ARCH="arm64" | |
| ;; | |
| armv7*|armhf|arm) | |
| ETCD_ARCH="arm" | |
| ;; | |
| *) | |
| echo "Warning: Unknown ARCH '${ARCH}', defaulting etcd architecture to 'amd64'" >&2 | |
| ;; | |
| esac | |
| elif [ -n "${6:-}" ]; then | |
| case "${6}" in | |
| amd64|x86_64) | |
| ETCD_ARCH="amd64" | |
| ;; | |
| arm64|aarch64) | |
| ETCD_ARCH="arm64" | |
| ;; | |
| armv7*|armhf|arm) | |
| ETCD_ARCH="arm" | |
| ;; | |
| *) | |
| echo "Warning: Unknown ARCH '${6}', defaulting etcd architecture to 'amd64'" >&2 | |
| ;; | |
| esac | |
| fi | |
| echo "Downloading etcd ${ETCD_VERSION} for architecture linux-${ETCD_ARCH}..." | |
| curl -sS -L "https://github.com/etcd-io/etcd/releases/download/${ETCD_VERSION}/etcd-${ETCD_VERSION}-linux-${ETCD_ARCH}.tar.gz" -o "${etcd_dir}/etcd-linux-${ETCD_ARCH}.tar.gz" |
| 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.
| build_name="${5}-${ARCH_SUFFIX}.iso" | ||
| [ -z "${build_name}" ] && build_name="setup-${RELEASE}-${ARCH_SUFFIX}.iso" |
There was a problem hiding this comment.
The fallback default for build_name is effectively dead code: build_name is always non-empty because it appends "-${ARCH_SUFFIX}.iso" even when BUILD_NAME is empty. If you want a default name, check whether parameter $5 is empty (or validate it explicitly) before constructing build_name.
| build_name="${5}-${ARCH_SUFFIX}.iso" | |
| [ -z "${build_name}" ] && build_name="setup-${RELEASE}-${ARCH_SUFFIX}.iso" | |
| if [ -z "${5}" ]; then | |
| build_name="setup-${RELEASE}-${ARCH_SUFFIX}.iso" | |
| else | |
| build_name="${5}-${ARCH_SUFFIX}.iso" | |
| fi |
| cni_status_code=$(curl -sS -L --write-out "%{http_code}\n" "https://github.com/containernetworking/plugins/releases/download/${CNI_VERSION}/cni-plugins-linux-${ARCH}-${CNI_VERSION}.tgz" -o "${cni_dir}/cni-plugins-${ARCH}.tgz") | ||
| if [[ ${cni_status_code} -eq 404 ]]; then | ||
| curl -sS -L --write-out "%{http_code}\n" "https://github.com/containernetworking/plugins/releases/download/${CNI_VERSION}/cni-plugins-${ARCH}-${CNI_VERSION}.tgz" -o "${cni_dir}/cni-plugins-${ARCH}.tgz" | ||
| fi |
There was a problem hiding this comment.
CNI download only special-cases the first URL returning 404; if the fallback URL also returns a non-200 status (or the first call returns 500/403), the script will continue with an invalid/corrupt tarball. Consider using curl -f (or checking for 200) on both attempts and exiting with a clear error if neither URL succeeds.
| cni_status_code=$(curl -sS -L --write-out "%{http_code}\n" "https://github.com/containernetworking/plugins/releases/download/${CNI_VERSION}/cni-plugins-linux-${ARCH}-${CNI_VERSION}.tgz" -o "${cni_dir}/cni-plugins-${ARCH}.tgz") | |
| if [[ ${cni_status_code} -eq 404 ]]; then | |
| curl -sS -L --write-out "%{http_code}\n" "https://github.com/containernetworking/plugins/releases/download/${CNI_VERSION}/cni-plugins-${ARCH}-${CNI_VERSION}.tgz" -o "${cni_dir}/cni-plugins-${ARCH}.tgz" | |
| fi | |
| cni_status_code=$(curl -sS -L --write-out "%{http_code}" -o "${cni_dir}/cni-plugins-${ARCH}.tgz" "https://github.com/containernetworking/plugins/releases/download/${CNI_VERSION}/cni-plugins-linux-${ARCH}-${CNI_VERSION}.tgz") | |
| if [[ "${cni_status_code}" -eq 404 ]]; then | |
| cni_status_code=$(curl -sS -L --write-out "%{http_code}" -o "${cni_dir}/cni-plugins-${ARCH}.tgz" "https://github.com/containernetworking/plugins/releases/download/${CNI_VERSION}/cni-plugins-${ARCH}-${CNI_VERSION}.tgz") | |
| fi | |
| if [[ "${cni_status_code}" -ne 200 ]]; then | |
| echo "ERROR: Failed to download CNI plugins (HTTP status: ${cni_status_code}) from both primary and fallback URLs." | |
| exit 1 | |
| fi |
| iso_dir="/tmp/iso" | ||
| working_dir="${iso_dir}/" | ||
| mkdir -p "${working_dir}" |
There was a problem hiding this comment.
Using a fixed working directory under /tmp ("/tmp/iso") can collide between concurrent runs and leaves no cleanup on failure. Prefer creating a unique temp dir (e.g., via mktemp -d) and registering a trap to remove it on exit/error, rather than relying on a final rm -rf that won't run if the script exits early.
| iso_dir="/tmp/iso" | |
| working_dir="${iso_dir}/" | |
| mkdir -p "${working_dir}" | |
| iso_dir="$(mktemp -d /tmp/iso.XXXXXX)" | |
| trap 'rm -rf "${iso_dir}"' EXIT | |
| working_dir="${iso_dir}/" |
| DASHBOARD_VERSION="2.7.0" | ||
| echo "Downloading Kubernetes Dashboard v${DASHBOARD_VERSION}..." | ||
| dashboard_conf_file="${working_dir}/dashboard.yaml" | ||
| curl -sSl "https://raw.githubusercontent.com/kubernetes/dashboard/v${DASHBOARD_VERSION}/aio/deploy/recommended.yaml" -o ${dashboard_conf_file} |
There was a problem hiding this comment.
This curl invocation uses -sSl (lowercase -l) rather than -sSL (uppercase -L). -l is an FTP-specific flag and does not enable redirect following; if the URL ever redirects, this download will fail. Use -L here for consistency with the other downloads in this script.
| curl -sSl "https://raw.githubusercontent.com/kubernetes/dashboard/v${DASHBOARD_VERSION}/aio/deploy/recommended.yaml" -o ${dashboard_conf_file} | |
| curl -sSL "https://raw.githubusercontent.com/kubernetes/dashboard/v${DASHBOARD_VERSION}/aio/deploy/recommended.yaml" -o ${dashboard_conf_file} |
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. |
5 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/