-
Notifications
You must be signed in to change notification settings - Fork 250
feat: cache versioned kubelet kubectl package binaries #8287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package toolkit | ||
|
|
||
| import ( | ||
| "github.com/Masterminds/semver" | ||
| ) | ||
|
|
||
| func CheckK8sConstraint(kubernetesVersion string, constraintStr string) (bool, error) { | ||
| version, err := semver.NewVersion(kubernetesVersion) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| constraint, err := semver.NewConstraint(constraintStr) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| return constraint.Check(version), nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,27 +6,30 @@ stub() { | |
|
|
||
| installKubeletKubectlFromPkg() { | ||
| local desiredVersion="${1}" | ||
| installRPMPackageFromFile "kubelet" $desiredVersion || exit $ERR_KUBELET_INSTALL_FAIL | ||
| installRPMPackageFromFile "kubectl" $desiredVersion || exit $ERR_KUBECTL_INSTALL_FAIL | ||
|
|
||
| installRPMPackageFromFile "kubelet" "${desiredVersion}" "/opt/bin/kubelet" || exit "$ERR_KUBELET_INSTALL_FAIL" | ||
| installRPMPackageFromFile "kubectl" "${desiredVersion}" "/opt/bin/kubectl" || exit "$ERR_KUBECTL_INSTALL_FAIL" | ||
| } | ||
|
|
||
| installRPMPackageFromFile() { | ||
| local packageName="${1}" | ||
| local desiredVersion="${2}" | ||
| local targetBinDir="${3:-"/opt/bin"}" | ||
| local targetPath="${3:-/opt/bin/${packageName}}" | ||
| local downloadDir="/opt/${packageName}/downloads" | ||
| local rpmFile="" | ||
| local fullPackageVersion="" | ||
|
|
||
| echo "installing ${packageName} version ${desiredVersion} by manually unpacking the RPM" | ||
| if [ "${packageName}" != "kubelet" ] && [ "${packageName}" != "kubectl" ] && [ "${packageName}" != "azure-acr-credential-provider" ]; then | ||
| echo "Error: Unsupported package ${packageName}. Only kubelet, kubectl, and azure-acr-credential-provider installs are allowed on OSGuard." | ||
| exit 1 | ||
| fi | ||
| echo "installing ${packageName} version ${desiredVersion}" | ||
| downloadDir="/opt/${packageName}/downloads" | ||
|
|
||
| rpmFile=$(ls "${downloadDir}" | grep "${packageName}" | grep "${desiredVersion}" | sort -V | tail -n 1) || rpmFile="" | ||
| if [ -z "${rpmFile}" ] && { [ "${packageName}" = "kubelet" ] || [ "${packageName}" = "kubectl" ]; } && fallbackToKubeBinaryInstall "${packageName}" "${desiredVersion}"; then | ||
| echo "Successfully installed ${packageName} version ${desiredVersion} from binary fallback" | ||
| rm -rf ${downloadDir} | ||
| rm -rf "${downloadDir}" | ||
| return 0 | ||
| fi | ||
| if [ -z "${rpmFile}" ]; then | ||
|
|
@@ -37,7 +40,7 @@ installRPMPackageFromFile() { | |
| return 1 | ||
| fi | ||
| echo "Did not find cached rpm file, downloading ${packageName} version ${fullPackageVersion}" | ||
| downloadPkgFromVersion "${packageName}" ${fullPackageVersion} "${downloadDir}" | ||
| downloadPkgFromVersion "${packageName}" "${fullPackageVersion}" "${downloadDir}" | ||
| rpmFile=$(ls "${downloadDir}" | grep "${packageName}" | grep "${desiredVersion}" | sort -V | tail -n 1) || rpmFile="" | ||
| fi | ||
| if [ -z "${rpmFile}" ]; then | ||
|
|
@@ -46,17 +49,11 @@ installRPMPackageFromFile() { | |
| fi | ||
|
|
||
| rpmFile="${downloadDir}/${rpmFile}" | ||
| local rpmBinaryName="${packageName}" | ||
| local targetBinaryName="${packageName}" | ||
| if [ "${packageName}" = "azure-acr-credential-provider" ]; then | ||
| targetBinaryName="acr-credential-provider" | ||
| fi | ||
|
|
||
| echo "Unpacking usr/bin/${rpmBinaryName} from ${downloadDir}/${packageName}-${desiredVersion}*" | ||
| mkdir -p "${targetBinDir}" | ||
| echo "Unpacking usr/bin/${packageName} from ${downloadDir}/${packageName}-${desiredVersion}*" | ||
| mkdir -p "$(dirname "${targetPath}")" | ||
| # This assumes that the binary will either be in /usr/bin or /usr/local/bin, but not both. | ||
| rpm2cpio "${rpmFile}" | cpio -i --to-stdout "./usr/bin/${rpmBinaryName}" "./usr/local/bin/${rpmBinaryName}" | install -m0755 /dev/stdin "${targetBinDir}/${targetBinaryName}" | ||
| rm -rf ${downloadDir} | ||
| rpm2cpio "${rpmFile}" | cpio -i --to-stdout "./usr/bin/${packageName}" "./usr/local/bin/${packageName}" | install -m0755 /dev/stdin "${targetPath}" | ||
| rm -rf "${downloadDir}" | ||
|
Comment on lines
+52
to
+56
|
||
| } | ||
|
|
||
| downloadPkgFromVersion() { | ||
|
|
@@ -83,7 +80,7 @@ installCredentialProviderFromPkg() { | |
| echo "installing azure-acr-credential-provider package version: $packageVersion" | ||
| mkdir -p "${CREDENTIAL_PROVIDER_BIN_DIR}" | ||
| chown -R root:root "${CREDENTIAL_PROVIDER_BIN_DIR}" | ||
| installRPMPackageFromFile "azure-acr-credential-provider" "${packageVersion}" "${CREDENTIAL_PROVIDER_BIN_DIR}" || exit $ERR_CREDENTIAL_PROVIDER_DOWNLOAD_TIMEOUT | ||
| installRPMPackageFromFile "azure-acr-credential-provider" "${packageVersion}" "${CREDENTIAL_PROVIDER_BIN_DIR}/acr-credential-provider" || exit "$ERR_CREDENTIAL_PROVIDER_DOWNLOAD_TIMEOUT" | ||
| } | ||
|
|
||
| installDeps() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1074,14 +1074,16 @@ getLatestPkgVersionFromK8sVersion() { | |
| fallbackToKubeBinaryInstall() { | ||
| packageName="${1:-}" | ||
| packageVersion="${2:-}" | ||
| local targetPath="${3:-/opt/bin/${packageName}}" | ||
| if [ "${packageName}" = "kubelet" ] || [ "${packageName}" = "kubectl" ]; then | ||
| if [ "${SHOULD_ENFORCE_KUBE_PMC_INSTALL}" = "true" ]; then | ||
| echo "Kube PMC install is enforced, skipping fallback to kube binary install for ${packageName}" | ||
| return 1 | ||
| elif [ -f "/opt/bin/${packageName}-${packageVersion}" ]; then | ||
| mv "/opt/bin/${packageName}-${packageVersion}" "/opt/bin/${packageName}" | ||
| chmod a+x /opt/bin/${packageName} | ||
| rm -rf /opt/bin/${packageName}-* & | ||
| mv "/opt/bin/${packageName}-${packageVersion}" "${targetPath}" | ||
|
Comment on lines
1074
to
+1083
|
||
| chown root:root "${targetPath}" | ||
| chmod 0755 "${targetPath}" | ||
| rm -rf "/opt/bin/${packageName}-*" & | ||
| return 0 | ||
| else | ||
| echo "No binary fallback found for ${packageName} version ${packageVersion}" | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -213,8 +213,7 @@ installCredentialProviderFromPkg() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "installing azure-acr-credential-provider package version: $packageVersion" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mkdir -p "${CREDENTIAL_PROVIDER_BIN_DIR}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| chown -R root:root "${CREDENTIAL_PROVIDER_BIN_DIR}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installRPMPackageFromFile "azure-acr-credential-provider" "${packageVersion}" || exit $ERR_CREDENTIAL_PROVIDER_DOWNLOAD_TIMEOUT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ln -snf /usr/bin/azure-acr-credential-provider "$CREDENTIAL_PROVIDER_BIN_DIR/acr-credential-provider" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
awesomenix marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installRPMPackageFromFile "azure-acr-credential-provider" "${packageVersion}" "${CREDENTIAL_PROVIDER_BIN_DIR}/acr-credential-provider" || exit "$ERR_CREDENTIAL_PROVIDER_DOWNLOAD_TIMEOUT" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
213
to
+216
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getPackageCacheRoot() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -233,8 +232,9 @@ installCredentialProviderFromPkg() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installKubeletKubectlFromPkg() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local desiredVersion="${1}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installRPMPackageFromFile "kubelet" $desiredVersion || exit $ERR_KUBELET_INSTALL_FAIL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installRPMPackageFromFile "kubectl" $desiredVersion || exit $ERR_KUBECTL_INSTALL_FAIL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installRPMPackageFromFile "kubelet" "${desiredVersion}" "/opt/bin/kubelet" || exit "$ERR_KUBELET_INSTALL_FAIL" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
awesomenix marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installRPMPackageFromFile "kubectl" "${desiredVersion}" "/opt/bin/kubectl" || exit "$ERR_KUBECTL_INSTALL_FAIL" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| installToolFromLocalRepo() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -403,22 +403,60 @@ installNvidiaManagedExpPkgFromCache() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extractBinaryFromRPM() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local rpmFile="${1}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local packageName="${2}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local targetPath="${3:-/opt/bin/${packageName}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local extractDir | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local binaryPath="" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extractDir=$(mktemp -d) || return 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ! (cd "${extractDir}" && rpm2cpio "${rpmFile}" | cpio -idm >/dev/null 2>&1); then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rm -rf "${extractDir}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for candidate in "${extractDir}/usr/bin/${packageName}" "${extractDir}/usr/local/bin/${packageName}"; do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -f "${candidate}" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| binaryPath="${candidate}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+411
to
+422
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local binaryPath="" | |
| extractDir=$(mktemp -d) || return 1 | |
| if ! (cd "${extractDir}" && rpm2cpio "${rpmFile}" | cpio -idm >/dev/null 2>&1); then | |
| rm -rf "${extractDir}" | |
| return 1 | |
| fi | |
| for candidate in "${extractDir}/usr/bin/${packageName}" "${extractDir}/usr/local/bin/${packageName}"; do | |
| if [ -f "${candidate}" ]; then | |
| binaryPath="${candidate}" | |
| break | |
| local resolvedExtractDir | |
| local binaryPath="" | |
| local resolvedCandidate="" | |
| extractDir=$(mktemp -d) || return 1 | |
| resolvedExtractDir=$(readlink -f "${extractDir}") || { | |
| rm -rf "${extractDir}" | |
| return 1 | |
| } | |
| if ! (cd "${extractDir}" && rpm2cpio "${rpmFile}" | cpio -idm --no-absolute-filenames --no-preserve-owner >/dev/null 2>&1); then | |
| rm -rf "${extractDir}" | |
| return 1 | |
| fi | |
| for candidate in "${extractDir}/usr/bin/${packageName}" "${extractDir}/usr/local/bin/${packageName}"; do | |
| if [ -f "${candidate}" ]; then | |
| resolvedCandidate=$(readlink -f "${candidate}") || continue | |
| case "${resolvedCandidate}" in | |
| "${resolvedExtractDir}"/*) | |
| binaryPath="${resolvedCandidate}" | |
| break | |
| ;; | |
| esac |
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unquoted ${packageName} and ${desiredVersion} can cause word-splitting/globbing and also make grep interpret version characters as regex metacharacters. Quote variables and consider grep -F -- \"${desiredVersion}-\" (fixed string) to avoid regex surprises and reduce injection risk from unexpected input.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -217,14 +217,14 @@ installCredentialProviderFromPkg() { | |||||
| echo "installing azure-acr-credential-provider package version: $packageVersion" | ||||||
| mkdir -p "${CREDENTIAL_PROVIDER_BIN_DIR}" | ||||||
| chown -R root:root "${CREDENTIAL_PROVIDER_BIN_DIR}" | ||||||
| installPkgWithAptGet "azure-acr-credential-provider" "${packageVersion}" || exit $ERR_CREDENTIAL_PROVIDER_DOWNLOAD_TIMEOUT | ||||||
| ln -snf /usr/bin/azure-acr-credential-provider "$CREDENTIAL_PROVIDER_BIN_DIR/acr-credential-provider" | ||||||
| installPkgWithAptGet "azure-acr-credential-provider" "${packageVersion}" "${CREDENTIAL_PROVIDER_BIN_DIR}/acr-credential-provider" || exit "$ERR_CREDENTIAL_PROVIDER_DOWNLOAD_TIMEOUT" | ||||||
|
||||||
| } | ||||||
|
|
||||||
| installKubeletKubectlFromPkg() { | ||||||
| k8sVersion="${1}" | ||||||
| installPkgWithAptGet "kubelet" "${k8sVersion}" || exit $ERR_KUBELET_INSTALL_FAIL | ||||||
| installPkgWithAptGet "kubectl" "${k8sVersion}" || exit $ERR_KUBECTL_INSTALL_FAIL | ||||||
| local k8sVersion="${1}" | ||||||
|
|
||||||
| installPkgWithAptGet "kubelet" "${k8sVersion}" "/opt/bin/kubelet" || exit "$ERR_KUBELET_INSTALL_FAIL" | ||||||
| installPkgWithAptGet "kubectl" "${k8sVersion}" "/opt/bin/kubectl" || exit "$ERR_KUBECTL_INSTALL_FAIL" | ||||||
| } | ||||||
|
|
||||||
| installToolFromLocalRepo() { | ||||||
|
|
@@ -289,23 +289,55 @@ installCredentialProviderPackageFromBootstrapProfileRegistry() { | |||||
| fi | ||||||
| } | ||||||
|
|
||||||
| extractDebBinaryFromFile() { | ||||||
| local debFile="${1}" | ||||||
| local packageName="${2}" | ||||||
| local targetPath="${3:-/opt/bin/${packageName}}" | ||||||
| local extractDir | ||||||
|
|
||||||
| extractDir=$(mktemp -d) || return 1 | ||||||
| if ! dpkg-deb -x "${debFile}" "${extractDir}"; then | ||||||
| rm -rf "${extractDir}" | ||||||
| return 1 | ||||||
| fi | ||||||
|
|
||||||
| local sourceBinary="${extractDir}/usr/bin/${packageName}" | ||||||
| if [ ! -f "${sourceBinary}" ]; then | ||||||
| echo "Failed to locate usr/bin/${packageName} in ${debFile}" | ||||||
|
||||||
| echo "Failed to locate usr/bin/${packageName} in ${debFile}" | |
| echo "Failed to locate /usr/bin/${packageName} in ${debFile}" |
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
installPkgWithAptGet no longer installs a package with apt/dpkg; it now (a) optionally uses the versioned binary fallback, otherwise (b) locates/downloads a .deb and extracts/moves a single binary out of it. Recommendation (mandatory): rename/split the function to reflect the new behavior (e.g., installBinaryFromDeb / extractDebBinaryFromFile + a separate “download deb” step), and keep the old name only if it still performs apt-based installation.
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description says CSE should “fall back to existing package install behavior when the cache is not present” and that SHOULD_ENFORCE_KUBE_PMC_INSTALL=true still forces the “package path”. After this change, the non-cached path no longer installs packages via the package manager (it downloads the .deb then extracts only /usr/bin/<tool>), so it won’t execute the existing install behavior (and may skip package-provided dependencies/metadata). Recommendation (mandatory): keep the “cached versioned binary” shortcut, but when the versioned binary is not present (or when enforcement is enabled), revert to the prior dpkg/apt-based install path (or explicitly document and implement dependency handling if extraction is intended to replace package install).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CheckK8sConstraintis exported but has no GoDoc comment, which commonly fails repository linting and makes its intended input format (e.g., whether leadingvis allowed) unclear. Recommendation (nit): add a short doc comment describing expected version formatting and what the constraint represents.