fix: cni-plugins backward compatibility - continue by fseldow · Pull Request #7881 · Azure/AgentBaker

fseldow · 2026-02-16T09:48:20Z

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Copilot

Pull request overview

This PR enhances backward compatibility for CNI plugin component detection during VM provisioning by updating the installFixedCNI() function to check for both "containernetworking-plugins" and "cni-plugins" component names in components.json.

Context: AgentBaker uses a dual naming convention for CNI plugins components. Newer VHDs may have the component named "cni-plugins" in components.json, while older VHDs use "containernetworking-plugins" (installed via package managers like apt/dnf). The VHD build process in vhdbuilder/packer/install-dependencies.sh installs "containernetworking-plugins" as the actual package, but components.json also maintains a separate "cni-plugins" entry for backward compatibility purposes (as noted in the test file comment at line 236-238).

The Fix: This change ensures that provisioning scripts running on newer VHDs with "cni-plugins" entries won't incorrectly fall back to downloading a hardcoded CNI version (v1.4.1) when the component is already present under a different name. This prevents unnecessary downloads and potential version conflicts.

Changes:

Updated jq query in installFixedCNI() to check for either "containernetworking-plugins" OR "cni-plugins" component names

parts/linux/cloud-init/artifacts/cse_install.sh

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 10 comments.

parts/linux/cloud-init/artifacts/cse_install.sh

Copilot · 2026-02-16T10:46:42Z

parts/linux/cloud-init/artifacts/cse_install.sh

+        updatePackageVersions "${cniPackage}" "${os}" "${os_version}" "${OS_VARIANT}"
+
+        #should change to ne
+        # shellcheck disable=SC3010
+        if [[ ${#PACKAGE_VERSIONS[@]} -gt 1 ]]; then
+            echo "WARNING: containerd package versions array has more than one element. Installing the last element in the array."
+            exit $ERR_CONTAINERD_VERSION_INVALID
+        fi
+        packageVersion=${PACKAGE_VERSIONS[0]}


Missing validation for empty PACKAGE_VERSIONS array or value. The code directly accesses PACKAGE_VERSIONS[0] without checking if the array is empty or contains the special value. Following the pattern in installContainerdWithComponentsJson (lines 77-80), this code should include a check like:

if [[ ${#PACKAGE_VERSIONS[@]} -eq 0 || ${PACKAGE_VERSIONS[0]} == "<SKIP>" ]]; then echo "INFO: cni-plugins package versions array is either empty or the first element is <SKIP>. Skipping cni-plugins installation." return 0 fi

Without this check, the script will fail with an unbound variable error or use an invalid version if the array is empty.

Copilot · 2026-02-16T10:46:43Z

parts/linux/cloud-init/artifacts/cse_install.sh

+        os_version="${UBUNTU_RELEASE}"
+        if isMarinerOrAzureLinux "${OS}" && [ "${IS_KATA}" = "true" ]; then
+            os=${MARINER_KATA_OS_NAME}
+        fi


The logic in line 162-163 only handles the Kata case for setting os to MARINER_KATA_OS_NAME, but it doesn't handle the Azure Linux Kata case (AZURELINUX_KATA_OS_NAME). The containerd installation function (lines 62-67) checks both isMariner and isAzureLinux separately for Kata. This code should follow the same pattern:

if isMarinerOrAzureLinux "${OS}" && [ "${IS_KATA}" = "true" ]; then os=${MARINER_KATA_OS_NAME} fi if isAzureLinux "${OS}" && [ "${IS_KATA}" = "true" ]; then os=${AZURELINUX_KATA_OS_NAME} fi

Note that isMarinerOrAzureLinux returns true for both Mariner and Azure Linux, so the current code sets os to MARINER_KATA_OS_NAME even when OS is Azure Linux, which is incorrect.

Suggested change

fi

fi

if isAzureLinux "${OS}" && [ "${IS_KATA}" = "true" ]; then

os=${AZURELINUX_KATA_OS_NAME}

fi

Copilot · 2026-02-16T10:46:43Z

parts/linux/cloud-init/artifacts/cse_install.sh

+        fi
+        packageVersion=${PACKAGE_VERSIONS[0]}
+
+        # Is there a ${arch} variable I can use instead of the iff


Typo in comment: "iff" should be "if". The comment currently reads "Is there a ${arch} variable I can use instead of the iff" but should say "Is there a ${arch} variable I can use instead of the if".

Suggested change

# Is there a ${arch} variable I can use instead of the iff

# Is there a ${arch} variable I can use instead of the if

Copilot · 2026-02-16T10:46:43Z

parts/linux/cloud-init/artifacts/cse_install.sh

+        if [ -z "$UBUNTU_RELEASE" ]; then
+            os=${OS}
+            os_version="current"
+        fi
+        os_version="${UBUNTU_RELEASE}"


The os_version assignment logic has a bug. Line 161 unconditionally overwrites the os_version set in line 159. This means when UBUNTU_RELEASE is empty (non-Ubuntu systems), os_version will be set to an empty string instead of "current". The logic should match the pattern used in installContainerdWithComponentsJson (lines 53-59) where os_version is only set to UBUNTU_RELEASE when it's not empty. The corrected logic should be:

os=${UBUNTU_OS_NAME} os_version="${UBUNTU_RELEASE}" if [ -z "$UBUNTU_RELEASE" ]; then os=${OS} os_version="current" fi

Suggested change

if [ -z "$UBUNTU_RELEASE" ]; then

os=${OS}

os_version="current"

fi

os_version="${UBUNTU_RELEASE}"

os_version="${UBUNTU_RELEASE}"

if [ -z "$UBUNTU_RELEASE" ]; then

os=${OS}

os_version="current"

fi

Copilot · 2026-02-16T10:46:43Z

parts/linux/cloud-init/artifacts/cse_install.sh

+        #should change to ne
+        # shellcheck disable=SC3010
+        if [[ ${#PACKAGE_VERSIONS[@]} -gt 1 ]]; then
+            echo "WARNING: containerd package versions array has more than one element. Installing the last element in the array."


The error message refers to "containerd" but this code is installing CNI plugins. The message should say "CNI plugins package versions array has more than one element" instead of "containerd package versions array has more than one element".

Suggested change

echo "WARNING: containerd package versions array has more than one element. Installing the last element in the array."

echo "WARNING: CNI plugins package versions array has more than one element. Installing the last element in the array."

parts/linux/cloud-init/artifacts/cse_install.sh

Copilot · 2026-02-16T10:46:44Z

parts/linux/cloud-init/artifacts/cse_install.sh

+    #always just use what is listed in components.json so we don't have to sync.
+    if [ -z "$(jq -r '.Packages[] | select(.name == "containernetworking-plugins") | .name' < $COMPONENTS_FILEPATH)" ]; then
+        cniPackage=$(jq ".Packages" "$COMPONENTS_FILEPATH" | jq ".[] | select(.name == \"cni-plugins\")") || exit $ERR_CNI_VERSION_INVALID
+
+        #CNI doesn't really care about this but wanted to reuse updatePackageVersions which requires it.
+        os=${UBUNTU_OS_NAME}
+        if [ -z "$UBUNTU_RELEASE" ]; then
+            os=${OS}
+            os_version="current"
+        fi
+        os_version="${UBUNTU_RELEASE}"
+        if isMarinerOrAzureLinux "${OS}" && [ "${IS_KATA}" = "true" ]; then
+            os=${MARINER_KATA_OS_NAME}
+        fi
+        updatePackageVersions "${cniPackage}" "${os}" "${os_version}" "${OS_VARIANT}"
+
+        #should change to ne
+        # shellcheck disable=SC3010
+        if [[ ${#PACKAGE_VERSIONS[@]} -gt 1 ]]; then
+            echo "WARNING: containerd package versions array has more than one element. Installing the last element in the array."
+            exit $ERR_CONTAINERD_VERSION_INVALID
+        fi
+        packageVersion=${PACKAGE_VERSIONS[0]}
+
+        # Is there a ${arch} variable I can use instead of the iff
+        if [ "$(isARM64)" -eq 1 ]; then
+            CNI_DIR_TMP="cni-plugins-linux-arm64-v${packageVersion}"
+        else
+            CNI_DIR_TMP="cni-plugins-linux-amd64-v${packageVersion}"
+        fi
+
+        if [ -d "$CNI_DOWNLOADS_DIR/${CNI_DIR_TMP}" ]; then
+            #not clear to me when this would ever happen. assume its related to the line above Latest VHD should have the untar, older should have the tgz.
+            mv ${CNI_DOWNLOADS_DIR}/${CNI_DIR_TMP}/* $CNI_BIN_DIR
+        else
+            echo "CNI tarball should already be unzipped by components.json"
+            exit $ERR_CNI_VERSION_INVALID
+        fi
+
+        chown -R root:root $CNI_BIN_DIR
+    fi


Logic error: This code assumes "cni-plugins" package is cached in the VHD downloads directory, but according to the VHD build script (vhdbuilder/packer/install-dependencies.sh:335-473), there is NO case handler for "cni-plugins", which means it's never downloaded or extracted during VHD build. The test file (vhdbuilder/packer/test/linux-vhd-content-test.sh:236-238) explicitly states: "cni-plugins is a special case that it is still in components.json for backward compatibility but we don't cache it."

This means the check at line 182 will always fail (directory doesn't exist), and the code will always exit with ERR_CNI_VERSION_INVALID at line 187. The entire code block (lines 152-191) appears to be attempting to handle a "cni-plugins" entry that was never installed on the VHD, making it non-functional.

If the intent is backward compatibility with old VHDs that have "containernetworking-plugins" but new CSE scripts that reference "cni-plugins", the logic is backwards. The code should check if "cni-plugins" exists in components.json AND was actually cached on the VHD during build. Since "cni-plugins" is never cached, this entire code path is broken.

Copilot · 2026-02-16T10:46:44Z

parts/linux/cloud-init/artifacts/cse_install.sh

+        fi
+        updatePackageVersions "${cniPackage}" "${os}" "${os_version}" "${OS_VARIANT}"
+
+        #should change to ne


Unclear comment. Line 167 says "#should change to ne" but it's not clear what "ne" means or what should be changed. This appears to be an incomplete thought or TODO that should either be completed or removed.

Suggested change

#should change to ne

Copilot

Pull request overview

Copilot reviewed 52 out of 66 changed files in this pull request and generated 7 comments.

parts/linux/cloud-init/artifacts/cse_install.sh

Copilot

Pull request overview

Copilot reviewed 52 out of 66 changed files in this pull request and generated 5 comments.

parts/linux/cloud-init/artifacts/cse_install.sh

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: cni-plugins backward compatibility - continue

7790e27

Copilot AI review requested due to automatic review settings February 16, 2026 09:48

fseldow requested review from AbelHu, Devinwong, awesomenix, cameronmeissner, djsly, ganeshkumarashok, juan-lee, junjiezhang1997, lilypan26, mxj220, pdamianov-dev, phealy, r2k1, timmy-wright and zachary-bailey as code owners February 16, 2026 09:48

fseldow temporarily deployed to test February 16, 2026 09:48 — with GitHub Actions Inactive

Copilot started reviewing on behalf of fseldow February 16, 2026 09:48 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

parts/linux/cloud-init/artifacts/cse_install.sh Outdated Show resolved Hide resolved

Update parts/linux/cloud-init/artifacts/cse_install.sh

cb01226

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 16, 2026 09:54

fseldow temporarily deployed to test February 16, 2026 09:55 — with GitHub Actions Inactive

Copilot started reviewing on behalf of fseldow February 16, 2026 09:55 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

f

94299c6

fseldow temporarily deployed to test February 16, 2026 10:21 — with GitHub Actions Inactive

ff

2ac15ec

Copilot AI review requested due to automatic review settings February 16, 2026 10:40

fseldow temporarily deployed to test February 16, 2026 10:40 — with GitHub Actions Inactive

Copilot started reviewing on behalf of fseldow February 16, 2026 10:40 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

data

daae386

fseldow requested review from YaoC and yewmsft as code owners February 16, 2026 10:57

fseldow temporarily deployed to test February 16, 2026 10:57 — with GitHub Actions Inactive

mini change

d04709c

Copilot AI review requested due to automatic review settings February 16, 2026 11:06

fseldow temporarily deployed to test February 16, 2026 11:06 — with GitHub Actions Inactive

Copilot started reviewing on behalf of fseldow February 16, 2026 11:07 View session

data

c5c3618

fseldow temporarily deployed to test February 16, 2026 11:08 — with GitHub Actions Inactive

word

67f45bf

Copilot AI reviewed Feb 16, 2026

View reviewed changes

data

680e1c2

fseldow temporarily deployed to test February 16, 2026 11:27 — with GitHub Actions Inactive

update

3b6d89a

djsly approved these changes Feb 16, 2026

View reviewed changes

data

cf37a68

Copilot AI review requested due to automatic review settings February 16, 2026 14:11

fseldow temporarily deployed to test February 16, 2026 14:11 — with GitHub Actions Inactive

Copilot started reviewing on behalf of fseldow February 16, 2026 14:11 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

djsly approved these changes Feb 16, 2026

View reviewed changes

djsly merged commit 05d9af9 into main Feb 16, 2026
32 of 34 checks passed

djsly deleted the xinhl/cniplugins branch February 16, 2026 16:49

djsly pushed a commit that referenced this pull request Feb 16, 2026

fix: cni-plugins backward compatibility - continue (#7881)

74a21e1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

djsly pushed a commit that referenced this pull request Feb 16, 2026

fix: cni-plugins backward compatibility - continue (#7881)

edbf028

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

-        fi
+        fi
+        if isAzureLinux "${OS}" && [ "${IS_KATA}" = "true" ]; then
+            os=${AZURELINUX_KATA_OS_NAME}
+        fi

	# Is there a ${arch} variable I can use instead of the iff
	# Is there a ${arch} variable I can use instead of the if

	echo "WARNING: containerd package versions array has more than one element. Installing the last element in the array."
	echo "WARNING: CNI plugins package versions array has more than one element. Installing the last element in the array."

Conversation

fseldow commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants