Skip to content

fix: start ask node controller within boothook instead of service for non flatcar/acl#8993

Open
awesomenix wants to merge 1 commit into
mainfrom
nishp/fix/flakytests
Open

fix: start ask node controller within boothook instead of service for non flatcar/acl#8993
awesomenix wants to merge 1 commit into
mainfrom
nishp/fix/flakytests

Conversation

@awesomenix

@awesomenix awesomenix commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
  1. Start aks node controller immediately instad of queueing the service start which is about 3-4s slower
  2. Add no delay to dhcpd temporary call made during cloud init which gets DHCP lease, there is an artifical 1.9s jitter for no reason
  3. Disable aks node controller service from auto starting.
  4. Flatcar/ACL uses service still since there is no easy way to start scripts in boot

After all these changes we gain about 5-6s improvement in boot times.

Network online happens during cloud init local phase so when boothook runs which is part of cloud init phase network is definitely online so no check is required.

Before:

  • Kernel boot -> CSE start: 15.000s
  • Boot -> CSE start (journalctl hostname): 8.000s
  • CloudInit -> CSE start: 6.000s

After:

  • Kernel boot -> CSE start: 10.000s
  • Boot -> CSE start (journalctl hostname): 4.000s
  • CloudInit -> CSE start: 1.000s

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Linux bootstrapping flow to reduce node boot time by launching aks-node-controller earlier from the cloud-boothook (instead of waiting on systemd’s normal target transactions), and adjusts VHD/e2e validation to match the new launch semantics.

Changes:

  • Launch aks-node-controller-wrapper.sh directly from the boothook (with output redirected to /var/log/azure/aks-node-controller.output) and ensure required log directories exist.
  • Keep aks-node-controller.service disabled in the VHD image and adjust VHD content tests accordingly; update the unit to DefaultDependencies=no to reduce start delays when it is used.
  • Update e2e tooling/tests to inject files based on the wrapper invocation, validate wrapper output appropriately, and collect additional cloud-init logs.

Reviewed changes

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

Show a summary per file
File Description
vhdbuilder/packer/test/linux-vhd-content-test.sh Updates VHD validation to expect aks-node-controller.service to be disabled.
vhdbuilder/packer/pre-install-dependencies.sh Disables ANC systemd unit in the baked image; adds DHCP/cache-warmup related boot optimizations.
vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml Moves ANC into the “disable” service list for OSGuard customization.
pkg/agent/baker.go Changes boothook to create /var/log/azure and launch the ANC wrapper via nohup with file redirection; updates Flatcar/ACL ignition template to enable the unit.
pkg/agent/baker_test.go Extends ignition JSON assertions for systemd unit enablement and link creation.
parts/linux/cloud-init/artifacts/aks-node-controller.service Adds DefaultDependencies=no to reduce implicit systemd ordering delays.
e2e/vmss.go Adds cloud-init logs to extraction; updates boothook injection insertion point to match wrapper invocation.
e2e/validators.go Adds a helper to validate ANC wrapper output across different launch mechanisms.
e2e/scenario_test.go Uses the new validator helper for ANC hotfix binary selection scenario.

Comment thread e2e/validators.go Outdated
Comment thread pkg/agent/baker.go Outdated
Comment thread parts/linux/cloud-init/artifacts/aks-node-controller.service
Comment thread vhdbuilder/packer/test/linux-vhd-content-test.sh
Comment thread vhdbuilder/packer/pre-install-dependencies.sh
Comment thread vhdbuilder/packer/pre-install-dependencies.sh
Comment thread vhdbuilder/packer/pre-install-dependencies.sh Outdated
Comment thread e2e/vmss.go Outdated
Comment thread vhdbuilder/packer/pre-install-dependencies.sh Outdated
@Devinwong

Copy link
Copy Markdown
Collaborator

Wanted to bring this up about old VHD launching ANC twice (but it probably won't happen)

  1. ANC.service baked in the old VHD
  2. Your new customdata which will directly run wrapper.sh

It probably won't happen because:

  • for scale up on existing nodepool, the vmss model is frozen, which means it will use whatever stored earlier.
  • for new nodepool, user will only be allowed to use the latest vhd

Just wanted to make sure this is in your radar

Comment thread pkg/agent/baker.go Outdated
@awesomenix awesomenix changed the title fix: start ask node controller within boothook instead of service fix: start ask node controller within boothook instead of service for non flatcar/acl Jul 23, 2026
Copilot AI review requested due to automatic review settings July 23, 2026 13:40
@awesomenix
awesomenix force-pushed the nishp/fix/flakytests branch from 8899814 to ba12a1e Compare July 23, 2026 13:40
@awesomenix
awesomenix force-pushed the nishp/fix/flakytests branch from ba12a1e to 28aee12 Compare July 23, 2026 13:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 14 comments.

Comments suppressed due to low confidence (7)

parts/linux/cloud-init/artifacts/aks-node-controller-wrapper.sh:26

  • LOGGER_TAG was updated to "aks-node-controller-launcher" but the script file is still named aks-node-controller-wrapper.sh, while other changes reference aks-node-controller-launcher.sh. This mismatch makes it easy to ship a VHD where the referenced launcher path does not exist.
    spec/parts/linux/cloud-init/artifacts/aks_node_controller_wrapper_spec.sh:5
  • This ShellSpec now targets ./parts/linux/cloud-init/artifacts/aks-node-controller-launcher.sh, but that file does not exist in the repo (the script is still named aks-node-controller-wrapper.sh). The test suite will fail until the script rename/addition is done.
    vhdbuilder/packer/vhd-image-builder-cvm.json:119
  • This packer file provisioner uploads parts/linux/cloud-init/artifacts/aks-node-controller-launcher.sh, but that artifact does not exist in the repo (only aks-node-controller-wrapper.sh exists). This will cause the VHD build to fail.
    {
      "type": "file",
      "source": "parts/linux/cloud-init/artifacts/aks-node-controller-launcher.sh",
      "destination": "/home/packer/aks-node-controller-launcher.sh"
    },
    {
      "type": "file",
      "source": "parts/linux/cloud-init/artifacts/cloud-init-status-check.sh",

vhdbuilder/packer/packer_source.sh:280

  • copyPackerFiles() copies $AKS_NODE_CONTROLLER_LAUNCHER_SRC to /opt/azure/containers/aks-node-controller-launcher.sh, but that source will not exist during packer builds unless the artifacts upload step provides it (the repo currently has only aks-node-controller-wrapper.sh).
  AKS_NODE_CONTROLLER_SRC=/home/packer/aks-node-controller
  AKS_NODE_CONTROLLER_DEST=/opt/azure/containers/aks-node-controller
  cpAndMode $AKS_NODE_CONTROLLER_SRC $AKS_NODE_CONTROLLER_DEST 755
  cpAndMode $AKS_NODE_CONTROLLER_LAUNCHER_SRC $AKS_NODE_CONTROLLER_LAUNCHER_DEST 0755

vhdbuilder/packer/pre-install-dependencies.sh:89

  • These comments say the boothook's explicit "systemctl start --no-block aks-node-controller.service" call is the sole trigger for ANC, but the boothook template now prefers directly nohup'ing aks-node-controller-launcher.sh when present. The comment should reflect the current launch mechanism to avoid confusion.
# the boothook's own explicit "systemctl start" would be a no-op and ANC would
# never actually run with the real config. The boothook's explicit
# "systemctl start --no-block aks-node-controller.service" call (issued only
# after those files exist) remains the sole trigger for this unit.

vhdbuilder/packer/pre-install-dependencies.sh:142

  • PR description says the boot-time improvement comes from removing an artificial delay in a DHCP dhcpd call during cloud-init, but the implementation here appends "nodelay" to /etc/dhcpcd.conf (dhcpcd). This either targets a different DHCP client than described, or the description needs updating.
if { isUbuntu "$OS" || isAzureLinux "$OS"; }; then
  echo "nodelay" | tee -a /etc/dhcpcd.conf

e2e/vmss.go:1494

  • The insertion point now searches for the launcher script invocation, but the error message still says "missing aks-node-controller service start", which is misleading when debugging boothook template changes.
	insertPos := strings.Index(boothookStr, "/bin/bash /opt/azure/containers/aks-node-controller-launcher.sh")
	if insertPos == -1 {
		insertPos = strings.Index(boothookStr, "systemctl start --no-block aks-node-controller-hack.service")
	}

Comment on lines 13 to 16
[Service]
Type=oneshot
ExecStart=/opt/azure/containers/aks-node-controller-wrapper.sh
ExecStart=/opt/azure/containers/aks-node-controller-launcher.sh
RemainAfterExit=yes
Comment thread vhdbuilder/packer/vhd-image-builder-base.json
Comment thread vhdbuilder/packer/vhd-image-builder-acl.json
Comment thread vhdbuilder/packer/vhd-image-builder-acl-arm64.json
Comment thread vhdbuilder/packer/vhd-image-builder-mariner.json
Comment thread vhdbuilder/packer/packer_source.sh
Comment thread vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml
Comment thread vhdbuilder/packer/pre-install-dependencies.sh
Comment thread vhdbuilder/packer/test/linux-vhd-content-test.sh
Comment thread aks-node-controller/pkg/nodeconfigutils/utils.go
Copilot AI review requested due to automatic review settings July 23, 2026 13:49
@awesomenix
awesomenix force-pushed the nishp/fix/flakytests branch from 28aee12 to e8e6f51 Compare July 23, 2026 13:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (6)

parts/linux/cloud-init/artifacts/aks-node-controller.service:16

  • ExecStart points to /opt/azure/containers/aks-node-controller-launcher.sh, but that script is not present anywhere under parts/linux/cloud-init/artifacts/ in this PR. As-is, VHD build/provisioning will fail (or ANC will never start) because the unit references a non-existent executable.
[Service]
Type=oneshot
ExecStart=/opt/azure/containers/aks-node-controller-launcher.sh
RemainAfterExit=yes

vhdbuilder/packer/vhd-image-builder-base.json:112

  • This Packer file provisioner copies parts/linux/cloud-init/artifacts/aks-node-controller-launcher.sh, but that file does not exist in the repo with the current PR contents. This will cause the VHD build to fail when Packer tries to upload the file.
    {
      "type": "file",
      "source": "parts/linux/cloud-init/artifacts/aks-node-controller-launcher.sh",
      "destination": "/home/packer/aks-node-controller-launcher.sh"
    },

vhdbuilder/packer/packer_source.sh:280

  • copyPackerFiles copies /home/packer/aks-node-controller-launcher.sh into /opt/azure/containers, but the upstream Packer templates now reference a launcher script that isn't present under parts/linux/cloud-init/artifacts/. Unless the launcher file is added, this cpAndMode will fail (and/or the resulting VHD will be missing the intended launcher).
  AKS_NODE_CONTROLLER_SRC=/home/packer/aks-node-controller
  AKS_NODE_CONTROLLER_DEST=/opt/azure/containers/aks-node-controller
  cpAndMode $AKS_NODE_CONTROLLER_SRC $AKS_NODE_CONTROLLER_DEST 755
  cpAndMode $AKS_NODE_CONTROLLER_LAUNCHER_SRC $AKS_NODE_CONTROLLER_LAUNCHER_DEST 0755

vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml:75

  • azlosguard imagecustomizer references /AgentBaker/parts/linux/cloud-init/artifacts/aks-node-controller-launcher.sh, but that script is not present in parts/linux/cloud-init/artifacts/ in this PR. This will break the image customization step (missing source file).
    - source: /AgentBaker/parts/linux/cloud-init/artifacts/aks-node-controller.service
      destination: /etc/systemd/system/aks-node-controller.service
      permissions: 600
    - source: /AgentBaker/parts/linux/cloud-init/artifacts/aks-node-controller-launcher.sh
      destination: /opt/azure/containers/aks-node-controller-launcher.sh
      permissions: 755

vhdbuilder/packer/pre-install-dependencies.sh:91

  • pre-install-dependencies.sh does not appear to run with set -e, and most systemctl calls here are guarded with || exit 1. This new systemctl disable is unguarded, so a failure (e.g., unit not present) would silently continue and potentially produce inconsistent VHDs.
# after those files exist) remains the sole trigger for this unit.
# Sometimes its also started diretly in boothook
systemctl disable aks-node-controller.service

vhdbuilder/packer/pre-install-dependencies.sh:143

  • This block introduces several new commands (tee writes, systemctl daemon-reload/enable) without any failure handling, even though this script generally expects to exit on failure (|| exit 1). Silent failures here can leave the VHD in a partially configured state (missing cache-warmup unit and/or dhcpcd config change).
if { isUbuntu "$OS" || isAzureLinux "$OS"; }; then
  echo "nodelay" | tee -a /etc/dhcpcd.conf
  tee /etc/systemd/system/cache-warmup.service > /dev/null << 'EOF'

Comment on lines 14 to 16
Type=oneshot
ExecStart=/opt/azure/containers/aks-node-controller-wrapper.sh
ExecStart=/opt/azure/containers/aks-node-controller-launcher.sh
RemainAfterExit=yes
Copilot AI review requested due to automatic review settings July 23, 2026 13:55
@awesomenix
awesomenix force-pushed the nishp/fix/flakytests branch from e8e6f51 to 4d424da Compare July 23, 2026 13:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (6)

vhdbuilder/packer/pre-install-dependencies.sh:91

  • systemctl disable aks-node-controller.service isn’t guarded with || exit 1 (and this script doesn’t use set -e), so a failure here would silently leave the unit enabled despite the comment above stating it must remain disabled in the VHD image.
systemctl disable aks-node-controller.service

vhdbuilder/packer/pre-install-dependencies.sh:142

  • This writes/creates /etc/dhcpcd.conf, but there are no other references to dhcpcd in the provisioning/VHD scripts, and the PR description mentions a dhcpd delay. Guarding this avoids creating an unused config file on images that don’t run dhcpcd and makes it clearer whether this change can have any effect.
  echo "nodelay" | tee -a /etc/dhcpcd.conf

vhdbuilder/packer/pre-install-dependencies.sh:157

  • The cache-warmup unit setup is intended to affect boot behavior, but systemctl daemon-reload / systemctl enable aren’t checked for failure (and this script doesn’t run with set -e). If either command fails, the image can be left in an unexpected state without failing the build.
  systemctl daemon-reload
  systemctl enable cache-warmup.service

vhdbuilder/packer/pre-install-dependencies.sh:86

  • The explanatory comment still refers to the "wrapper" even though the unit now executes aks-node-controller-launcher.sh. Updating terminology here avoids confusion when correlating the service behavior with the launcher script.
# boothook has written the provision config/nbc-cmd files, causing the wrapper's
# graceful no-op exit to mark the oneshot unit "active (exited)" - after which
# the boothook's own explicit "systemctl start" would be a no-op and ANC would

vhdbuilder/packer/pre-install-dependencies.sh:90

  • Spelling/grammar in this comment ("its"/"diretly") makes the intent harder to read in a boot-critical script.
# Sometimes its also started diretly in boothook

vhdbuilder/packer/pre-install-dependencies.sh:89

  • This comment says the boothook’s explicit systemctl start remains the “sole trigger” for the unit, but the updated boothook template now prefers launching aks-node-controller-launcher.sh directly (falling back to systemctl start only when the launcher is absent). Updating the wording would keep the rationale accurate.
# never actually run with the real config. The boothook's explicit
# "systemctl start --no-block aks-node-controller.service" call (issued only
# after those files exist) remains the sole trigger for this unit.

Comment on lines +40 to +44
logger -t aks-boothook "launching aks-node-controller $(date -Ins)"
if [ -f /opt/azure/containers/aks-node-controller-launcher.sh ]; then
nohup /bin/bash /opt/azure/containers/aks-node-controller-launcher.sh > /var/log/azure/aks-node-controller.output 2>&1 &
else
systemctl start --no-block aks-node-controller.service
Copilot AI review requested due to automatic review settings July 24, 2026 02:52
@awesomenix
awesomenix force-pushed the nishp/fix/flakytests branch from 4d424da to 7f29301 Compare July 24, 2026 02:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (4)

vhdbuilder/packer/pre-install-dependencies.sh:91

  • The comment has typos/grammar issues ("its"/"diretly") which makes the intent harder to read.
# "systemctl start --no-block aks-node-controller.service" call (issued only
# after those files exist) remains the sole trigger for this unit.
# Sometimes its also started diretly in boothook
systemctl disable aks-node-controller.service

vhdbuilder/packer/pre-install-dependencies.sh:143

  • This change appends nodelay to /etc/dhcpcd.conf, but the PR description/review thread discussion refers to removing delay from a dhcpd-based call during cloud-init. There are no dhcpd references anywhere in the repo, so it’s unclear whether this knob affects the described boot-time path or if the description/implementation is out of sync.
if { isUbuntu "$OS" || isAzureLinux "$OS"; }; then
  echo "nodelay" | tee -a /etc/dhcpcd.conf
  tee /etc/systemd/system/cache-warmup.service > /dev/null << 'EOF'

e2e/vmss.go:1497

  • The error message is now misleading: the code looks for the launcher start marker (or the hack service), but the error still says the boothook is missing the "service start".
	}
	if insertPos == -1 {
		return "", fmt.Errorf("cloud-boothook customData missing aks-node-controller service start")
	}

spec/parts/linux/cloud-init/artifacts/aks_node_controller_launcher_spec.sh:8

  • The spec was renamed to target the launcher, but the temp directory is still named aks-node-controller-wrapper, which is confusing when debugging failures.

@aks-node-assistant

Copy link
Copy Markdown
Contributor

AgentBaker Linux gate detective

Run: 173592672
Failed job/stage/task: e2e / Run AgentBaker E2E / Run AgentBaker E2E
First failing step: Run AgentBaker E2E / Test_ACL during VMSS creation / prepare AKS node

Detective summary: Multiple scenarios timed out around 1020s while creating/preparing VMSS before meaningful node validation.

Likely cause / signature: Known WestUS3 VMSS provisioning/context-deadline flake; this run matches the existing wiki signature.
Wiki signature: linux-vhd-gate-westus3-vmss-create-context-deadline-exceeded

Confidence: High

Recommended owner/action: AgentBaker E2E watcher to rerun and keep tracking under the existing signature.

Strongest alternative: PR #8993 boothook/ANC startup change delayed provisioning, but less likely because failures hit multiple unrelated scenarios at the VMSS creation/prep timeout boundary.

Evidence: timeline failed task exit 1; failed test run 544313106; task log 562 around line 849; build metadata/PR metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants