fix: exclude ACL from node-exporter and post-deprovision WALinuxAgent install#8036
Open
aadhar-agarwal wants to merge 2 commits intomainfrom
Open
fix: exclude ACL from node-exporter and post-deprovision WALinuxAgent install#8036aadhar-agarwal wants to merge 2 commits intomainfrom
aadhar-agarwal wants to merge 2 commits intomainfrom
Conversation
aadhar-agarwal
force-pushed
the
aadagarwal/acl-node-exporter
branch
from
fc73382 to
2892cda
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Excludes Azure Container Linux (ACL) from node-exporter VHD build steps to prevent Packer file-copy failures introduced when node-exporter was added to the VHD build pipeline.
Changes:
- Add ACL handling to skip node-exporter file installation/copy steps during VHD build.
- Update VHD content tests to skip node-exporter assertions on ACL.
- Update e2e validator documentation/comments to reflect node-exporter installation scope.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
vhdbuilder/packer/vhd-image-builder-acl.json |
Uploads node-exporter-related scripts/assets into the ACL Packer build context. |
vhdbuilder/packer/test/linux-vhd-content-test.sh |
Skips node-exporter content validation on ACL. |
vhdbuilder/packer/packer_source.sh |
Skips copying node-exporter assets on ACL (avoids ERR_PACKER_COPY_FILE). |
vhdbuilder/packer/install-dependencies.sh |
Updates node-exporter skip comment to include ACL. |
e2e/validators.go |
Updates node-exporter validator comments to reflect supported VHDs. |
PR #7704 missed ACL when adding node-exporter, causing exit code 113 (ERR_PACKER_COPY_FILE) on ACL VHD builds. Follows the Flatcar pattern: guard in packer_source.sh, file uploads in ACL packer JSON, and skip in VHD content tests. Signed-off-by: Aadhar Agarwal <aadagarwal@microsoft.com>
PR #7987 added post-deprovision WALinuxAgent install but the guard in packer_source.sh only excluded Flatcar, not ACL. ACL also does not upload these files via its packer JSON, causing the same exit code 113 failure pattern. Adds isACL to the guard and skips testWALinuxAgentInstalled for AzureContainerLinux. Signed-off-by: Aadhar Agarwal <aadagarwal@microsoft.com>
aadhar-agarwal
force-pushed
the
aadagarwal/acl-node-exporter
branch
from
2892cda to
ce27b33
Compare
aadhar-agarwal
changed the title
aadhar-agarwal
changed the title
aadhar-agarwal
requested review from
AbelHu,
Devinwong,
awesomenix,
calvin197,
cameronmeissner,
djsly,
ganeshkumarashok,
juan-lee,
junjiezhang1997,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
timmy-wright and
zachary-bailey
as code owners
Comment on lines
1414
to
1417
| // Check if node-exporter is installed on this VHD by looking for the skip sentinel file. | ||
| // The skip file is only present on VHDs that have node-exporter installed (Ubuntu, Mariner, Azure Linux). | ||
| // Flatcar, OSGuard, and older VHDs do not have node-exporter installed and will not have the skip file. | ||
| // Flatcar, ACL, OSGuard, and older VHDs do not have node-exporter installed and will not have the skip file. | ||
| if !fileExist(ctx, s, skipFile) { |
There was a problem hiding this comment.
This comment states node-exporter is installed on “Mariner”, but the VHD build/test logic elsewhere skips node-exporter on Mariner (components.json has an empty versions list for mariner, and linux-vhd-content-test.sh treats CBLMariner as a skip SKU). Please correct the comment to avoid suggesting Mariner VHDs should have the sentinel file.
Comment on lines
501
to
504
| "node-exporter") | ||
| # Skipping is handled by empty versionsV2 arrays in components.json | ||
| # for mariner, flatcar, and osguard. Kata is skipped explicitly here. | ||
| # for mariner, flatcar, acl, and osguard. Kata is skipped explicitly here. | ||
| if [ "${IS_KATA}" = "true" ]; then |
There was a problem hiding this comment.
The comment says skipping node-exporter is handled by empty versionsV2 arrays for "acl", but parts/common/components.json has no ACL/azurecontainerlinux entry for node-exporter at all (only ubuntu/azurelinux/mariner/flatcar). For ACL the skip is likely because there’s no matching downloadURIs key, not because an empty versions list was provided—please update the comment to reflect the actual mechanism to avoid future confusion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #7704 missed ACL when adding node-exporter, causing exit code 113 (ERR_PACKER_COPY_FILE) on ACL VHD builds. Follows the Flatcar pattern: guard in packer_source.sh, file uploads in ACL packer JSON, and skip in VHD content tests.
PR #7987 added post-deprovision WALinuxAgent install but the guard in packer_source.sh only excluded Flatcar, not ACL. ACL also does not upload these files via its packer JSON, causing the same exit code 113 failure pattern. Adds isACL to the guard and skips testWALinuxAgentInstalled for AzureContainerLinux.
[TEST All VHDs] AKS Linux VHD Build - Msft Tenant - https://msazure.visualstudio.com/CloudNativeCompute/_build/results?buildId=155800738&view=results
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #