Repository navigation
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds windows-exporter to the Windows VHD build so nodes can start the aks-windows-exporter service during CSE using VHD-baked assets (with a sentinel to keep aks-vm-extension in no-op mode for these VHDs).
Changes:
- Bake windows-exporter binaries/config/health script into the Windows VHD and create a sentinel file.
- Register/start
aks-windows-exporterat CSE time via newwindowsexporterfunc.ps1, with unit tests and VHD content tests. - Add e2e validation ensuring the service is running and serving metrics when the sentinel is present.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vhdbuilder/packer/windows/windows-vhd-builder-sig.json | Stages windows-exporter config + health script into c:\k\ during image build. |
| vhdbuilder/packer/windows/configure-windows-vhd.ps1 | Extracts/stages windows-exporter into C:\k\windows-exporter and creates sentinel. |
| vhdbuilder/packer/test/windows-vhd-content-test.ps1 | Validates windows-exporter assets + sentinel exist on the built VHD. |
| staging/cse/windows/windowsexporterfunc.ps1 | New CSE-time installer to register/start aks-windows-exporter via NSSM and health-check it. |
| staging/cse/windows/windowsexporterfunc.tests.ps1 | Adds Pester tests for sentinel/no-op behavior and error code on missing NSSM. |
| parts/windows/kuberneteswindowssetup.ps1 | Dot-sources exporter functions and invokes Install-WindowsExporter during BasePrep. |
| parts/windows/windowscsehelper.ps1 | Adds a new Windows CSE error code for windows-exporter start failure. |
| staging/cse/windows/kubernetesfunc.ps1 | Adds Defender exclusions for windows-exporter log file paths. |
| parts/windows/windowsexporter/windows-exporter-config.yml | New baked config (port 19182, enabled collectors, service include filter, debug logging). |
| parts/windows/windowsexporter/windows-exporter-health.ps1 | New baked health/version helper used by CSE health validation. |
| parts/common/components.json | Adds windows-exporter component download metadata for VHD caching. |
| e2e/validators.go | Adds Windows exporter validator (service + metrics) gated by sentinel presence. |
| e2e/scenario_win_test.go | Runs the new validator across Windows e2e scenarios. |
| e2e/config/config.go | Fixes a formatting issue in panic error rendering (%w -> %v). |
| staging/cse/windows/README | Updates the list of scripts to download, including windowsexporterfunc.ps1. |
github-actions
Bot
added
the
components
chmill-zz
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
Resolve compatibility gating for older CSE packages and restore the established exporter log paths.
1 open finding
1 resolved since last review
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
AgentBaker Linux gate detectiveRun: 184683343 Detective summary: Run AgentBaker E2E completed 194 scenarios with 2 deterministic failures. First, Ubuntu2404_TemporaryKubeletDisk reached node Ready and passed initial SSH/common validation but timed out waiting for SSH after reboot. Second, RCV1P_Ubuntu2604Minimal failed VMSS creation with ARM OSProvisioningInternalError because cloud-init failed to obtain a DHCP lease on eth0, followed by bastion SSH failures. Likely cause / signature: e2e-ubuntu2404-temporary-kubelet-disk-reboot-ssh-timeout and e2e-ubuntu2204-vmss-osprovisioning-dhcp-lease-failure. Both look like environment/provisioning flakes rather than a Linux VHD PR regression. Confidence: Medium-high: build report names only Run AgentBaker E2E as failed; failed test records show two failures; E2E log provides the reboot timeout and ARM DHCP lease error; PR changes are Windows exporter-focused, not Linux provisioning/reboot logic. Recommended owner/action: Treat as gate flakiness; no PR action unless this PR intentionally affected Linux reboot/provisioning paths. Track recurrences in the wiki and investigate WestUS3 VMSS/DHCP/reboot SSH behavior if frequency increases. Strongest alternative: PR-caused regression from exporter bake/provisioning changes, but the failures are Linux reboot/DHCP provisioning surfaces outside the Windows exporter change scope and one occurs before scenario validation. Evidence: build run, build report issues, E2E task log, failed test results run 676998803, and wiki tracker. Wiki: e2e-ubuntu2404-temporary-kubelet-disk-reboot-ssh-timeout, e2e-ubuntu2204-vmss-osprovisioning-dhcp-lease-failure |
| "default": { | ||
| "versionsV2": [ | ||
| { | ||
| "renovateTag": "<DO_NOT_UPDATE>", |
There was a problem hiding this comment.
It'd be nice to have a renovate tag to auto upgrade windows exporter - not sure if renovate can do this automatically from packages.aks.azure.com tho.
There was a problem hiding this comment.
Chase's AI reviewer: Agreed it'd be nice, but there's no index of the Windows Dalec packages on packages.aks.azure.com for Renovate to watch, so it stays <DO_NOT_UPDATE> like the other Windows packages for now.
| . "$WINDOWS_SCRIPTS_DIRECTORY\securetlsbootstrapfunc.ps1" | ||
| . "$WINDOWS_SCRIPTS_DIRECTORY\windowsciliumnetworkingfunc.ps1" | ||
| . "$WINDOWS_SCRIPTS_DIRECTORY\networkisolatedclusterfunc.ps1" | ||
| . "$WINDOWS_SCRIPTS_DIRECTORY\windowsexporterfunc.ps1" |
There was a problem hiding this comment.
I really don't like how we have so many of these files now. My preference is to move the windows exporter configuration into one of the others. But there isn't an obvious one 😢
There was a problem hiding this comment.
Chase's AI reviewer: Same feeling, but nothing existing was a natural home, so I kept the exporter logic in its own file. Happy to fold it in if you have a preference.
- Register the aks-windows-exporter service disabled in BasePrep, after Get-KubePackage provides nssm.exe, so PIS-cached images carry the service without starting it early. NodePrep enables, starts, and health-checks it, registering it first if BasePrep did not. - Move windows-exporter package URL selection into components_json_helpers.ps1 with unit tests for SKU overrides and single-URL enforcement. - Move VHD-only exporter config and health assets under vhdbuilder/packer/windows/windows-exporter.
Changes cached containers or packages on windows VHDsPlease get a Windows SIG member to approve. The following dif file shows any additions or deletions from what will be cached on windows VHDs organised by VHD type.
diff --git a/vhd_files/2022-containerd-gen2.txt b/vhd_files/2022-containerd-gen2.txt
index 8b912ca..522aad2 100644
--- a/vhd_files/2022-containerd-gen2.txt
+++ b/vhd_files/2022-containerd-gen2.txt
@@ -25,0 +26 @@ c:\akse-cache\win-vnet-cni\: https://packages.aks.azure.com/azure-cni/v1.6.21/bi
+c:\akse-cache\windows-exporter\: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
@@ -149,0 +151 @@ Windows 2022-containerd-gen2 package windows scripts default: https://packages.a
+Windows 2022-containerd-gen2 package windows-exporter default: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
diff --git a/vhd_files/2022-containerd.txt b/vhd_files/2022-containerd.txt
index d8c5c57..beee139 100644
--- a/vhd_files/2022-containerd.txt
+++ b/vhd_files/2022-containerd.txt
@@ -25,0 +26 @@ c:\akse-cache\win-vnet-cni\: https://packages.aks.azure.com/azure-cni/v1.6.21/bi
+c:\akse-cache\windows-exporter\: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
@@ -149,0 +151 @@ Windows 2022-containerd package windows scripts default: https://packages.aks.az
+Windows 2022-containerd package windows-exporter default: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
diff --git a/vhd_files/2025-gen2-tl.txt b/vhd_files/2025-gen2-tl.txt
index 74d381e..bc3ab52 100644
--- a/vhd_files/2025-gen2-tl.txt
+++ b/vhd_files/2025-gen2-tl.txt
@@ -24,0 +25 @@ c:\akse-cache\win-vnet-cni\: https://packages.aks.azure.com/azure-cni/v1.6.21/bi
+c:\akse-cache\windows-exporter\: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
@@ -85,0 +87 @@ Windows 2025-gen2-tl package windows scripts default: https://packages.aks.azure
+Windows 2025-gen2-tl package windows-exporter default: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
diff --git a/vhd_files/2025-gen2.txt b/vhd_files/2025-gen2.txt
index d3c9d54..61e4767 100644
--- a/vhd_files/2025-gen2.txt
+++ b/vhd_files/2025-gen2.txt
@@ -24,0 +25 @@ c:\akse-cache\win-vnet-cni\: https://packages.aks.azure.com/azure-cni/v1.6.21/bi
+c:\akse-cache\windows-exporter\: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
@@ -85,0 +87 @@ Windows 2025-gen2 package windows scripts default: https://packages.aks.azure.co
+Windows 2025-gen2 package windows-exporter default: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
diff --git a/vhd_files/2025.txt b/vhd_files/2025.txt
index a2f8f75..588027d 100644
--- a/vhd_files/2025.txt
+++ b/vhd_files/2025.txt
@@ -24,0 +25 @@ c:\akse-cache\win-vnet-cni\: https://packages.aks.azure.com/azure-cni/v1.6.21/bi
+c:\akse-cache\windows-exporter\: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip
@@ -85,0 +87 @@ Windows 2025 package windows scripts default: https://packages.aks.azure.com/aks
+Windows 2025 package windows-exporter default: https://packages.aks.azure.com/dalec-packages/windows-exporter/0.31.2/windows/amd64/windows-exporter_0.31.2-1_amd64.zip |
|
|
|
Summary
Bake windows-exporter binaries, configuration, health script, and
C:\k\skip_vhd_windows_exporterinto the Windows VHD. The baked skip marker disables aks-vm-extension management from the outset: there is no runtime ownership transfer or extension fallback on these VHDs.CSE registers the baked exporter as a disabled NSSM service in BasePrep (after the kube package provides nssm.exe), so PIS-cached images carry the service without running it. NodePrep enables and starts it, registering it first if BasePrep did not, and uses bounded health retries to tolerate asynchronous SERVICE_START_PENDING responses. Startup failures and exceptions are logged and node provisioning continues. CSE never creates or removes the skip marker.
New VHDs require CSE supporting this startup path. Older VHDs without exporter assets remain extension-managed. VHD tests require the skip marker; E2E independently validates the running service and metrics, so startup failures remain visible in CI.
Port Compatibility
AgentBaker-managed windows-exporter uses port
19100, matching Linux node-exporter, intentionally replacing the extension-managed Windows port19182. The prior description stated consumers were validated for this migration; production consumer readiness remains a release prerequisite.