Skip to content

fix: fetch cse from protectedsettings during boothook, latency back to normal - #9079

Open
awesomenix wants to merge 1 commit into
mainfrom
nishp/useslowcse/scriptlessphase2
Open

fix: fetch cse from protectedsettings during boothook, latency back to normal#9079
awesomenix wants to merge 1 commit into
mainfrom
nishp/useslowcse/scriptlessphase2

Conversation

@awesomenix

Copy link
Copy Markdown
Contributor

We had to recently move boothook into cse because of known limitations. But now

  1. Restore original boothook
  2. Download configuration from CSE using special scripts
  3. execute aks node controller with that script

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Windows Unit Test Results

  3 files   11 suites   50s ⏱️
381 tests 381 ✅ 0 💤 0 ❌
384 runs  384 ✅ 0 💤 0 ❌

Results for commit b5c7cfd.

♻️ This comment has been updated with latest results.

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 reintroduces a cloud-init #cloud-boothook path for scriptless NBC to reduce provisioning latency by fetching/decrypting the CustomScript protected settings early (preferring HostGAPlugin, falling back to WireServer), then launching aks-node-controller using the extracted boothook payload.

Changes:

  • Add fetch_provision_config.py and ensure it is baked into multiple VHD build pipelines (Packer + OSGuard).
  • Update pkg/agent/baker.go scriptless flow to generate an early boothook and a new phase2 commandToExecute payload format.
  • Update unit tests and e2e VMSS scaffolding to match the new scriptless behavior.

Reviewed changes

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

Show a summary per file
File Description
vhdbuilder/packer/vhd-image-builder-mariner.json Bake fetch_provision_config.py into Mariner VHD build inputs.
vhdbuilder/packer/vhd-image-builder-mariner-cvm.json Bake fetch_provision_config.py into Mariner CVM VHD build inputs.
vhdbuilder/packer/vhd-image-builder-mariner-arm64.json Bake fetch_provision_config.py into Mariner ARM64 VHD build inputs.
vhdbuilder/packer/vhd-image-builder-cvm.json Bake fetch_provision_config.py into CVM VHD build inputs.
vhdbuilder/packer/vhd-image-builder-base.json Bake fetch_provision_config.py into base VHD build inputs.
vhdbuilder/packer/vhd-image-builder-arm64-gen2.json Bake fetch_provision_config.py into ARM64 Gen2 VHD build inputs.
vhdbuilder/packer/vhd-image-builder-arm64-gb.json Bake fetch_provision_config.py into ARM64 GB VHD build inputs.
vhdbuilder/packer/vhd-image-builder-acl.json Bake fetch_provision_config.py into ACL VHD build inputs.
vhdbuilder/packer/vhd-image-builder-acl-arm64.json Bake fetch_provision_config.py into ACL ARM64 VHD build inputs.
vhdbuilder/packer/packer_source.sh Copy fetch_provision_config.py from packer staging into /opt/azure/containers/.
vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml Ensure OSGuard images also include fetch_provision_config.py.
pkg/agent/baker.go Switch scriptless custom data to an early boothook; generate a phase2 command payload that can be extracted/executed.
pkg/agent/baker_test.go Update tests for new scriptless custom data / phase2 payload behavior.
parts/linux/cloud-init/artifacts/fetch_provision_config.py New utility to fetch/decrypt CustomScript protected settings and materialize boothook payload early.
e2e/vmss.go Adjust e2e scriptless compilation/hotfix injection to the updated scriptless flow.

Comment thread pkg/agent/baker.go
Comment thread pkg/agent/baker.go
Comment thread e2e/vmss.go
Copilot AI review requested due to automatic review settings July 30, 2026 15:34
@awesomenix
awesomenix force-pushed the nishp/useslowcse/scriptlessphase2 branch from 1ae65f5 to 6561452 Compare July 30, 2026 15:34

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 15 out of 15 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

pkg/agent/baker_test.go:1721

  • This test no longer asserts that the ignition payload enables aks-node-controller.service (systemd units + basic.target.wants link). Given ignition can’t run cloud-boothooks (see e2e/validators.go:2231-2237) and the VHD build keeps the unit disabled by default, the ignition payload must still enable the service; otherwise provisioning won’t start on ACL/Flatcar.

Re-add the systemd/link assertions here so the test suite catches regressions in the ignition template.

		var ignition map[string]interface{}
		Expect(json.Unmarshal(decodedPayload, &ignition)).To(Succeed())

		storage, ok := ignition["storage"].(map[string]interface{})
		Expect(ok).To(BeTrue())

Comment thread pkg/agent/baker.go
Comment on lines 98 to 102
flatcarTemplate = `{
"ignition": { "version": "3.4.0" },
"systemd": {
"units": [{
"name": "aks-node-controller.service",
"enabled": true
}]
},
"storage": {
"files": [%s],
"links": [{
"path": "/etc/systemd/system/basic.target.wants/aks-node-controller.service",
"target": "/etc/systemd/system/aks-node-controller.service",
"overwrite": true
}]
"files": [%s]
}
Copilot AI review requested due to automatic review settings July 30, 2026 18:50
@awesomenix
awesomenix force-pushed the nishp/useslowcse/scriptlessphase2 branch from 6561452 to 3078219 Compare July 30, 2026 18:50

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 16 out of 16 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

e2e/validators.go:2790

  • There’s a duplicated assertion for "Using NBC command for scriptless phase 2" and the journalctl grep isn’t scoped to the current boot, which can make the check noisier/flakier when logs persist across boots.
		ValidateFileHasContent(ctx, s, "/var/log/azure/aks-node-controller.output", "Using NBC command for scriptless phase 2")
		execScriptOnVMForScenarioValidateExitCode(ctx, s, "sudo journalctl | grep -q 'starting /opt/bin/boothook.sh'", 0, "expected journalctl to contain 'starting /opt/bin/boothook.sh' for scriptless phase 2")
		ValidateFileHasContent(ctx, s, "/var/log/azure/aks-node-controller.output", "Using NBC command for scriptless phase 2")

pkg/agent/baker_test.go:1738

  • This inline comment no longer matches what the test is counting (the ignition custom data here is only validating nodecustomdata plus an optional hotfix file, not nbc-cmd/nodeconfig entries). Updating it will avoid misleading future readers.
		Expect(len(files)).To(Equal(expectedCount)) // nbc-cmd, nodecustomdata, aks-node-config (optional hotfix file present)

Copilot AI review requested due to automatic review settings July 30, 2026 21:14
@awesomenix
awesomenix force-pushed the nishp/useslowcse/scriptlessphase2 branch from 3078219 to b5c7cfd Compare July 30, 2026 21:14

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 16 out of 16 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

pkg/agent/baker.go:74

  • In the boothook, python3 ... || exit 0 can leave a stale /opt/bin/boothook.sh behind (from a previous boot) and then exit before launching it. Later, the CSE command checks [ -e /opt/bin/boothook.sh ] and will skip executing /opt/azure/containers/boothook.sh, so provisioning can hang waiting for provision-wait even though the controller was never started. Avoid exiting early on fetch failure; instead ensure the sentinel file is removed (or continue execution) so the CSE path can safely fall back.
if [ -f /opt/azure/containers/fetch_provision_config.py ]; then
	python3 /opt/azure/containers/fetch_provision_config.py --output /opt/bin/boothook.sh --timeout 60 >>/var/log/azure/aks-early-boothook.log 2>&1 || exit 0
fi

e2e/validators.go:2790

  • This validation does two identical ValidateFileHasContent checks back-to-back, and the journalctl call is unscoped (can be slow) and will also fail older VHDs that don’t ship fetch_provision_config.py (even though the runtime logic is designed to fall back when early boothook fetch isn’t available). Consider removing the duplicate assertion and only requiring the journal marker when the fetch script exists; also prefer journalctl --boot=0 --no-pager to keep output bounded and non-interactive.
		ValidateFileHasContent(ctx, s, "/var/log/azure/aks-node-controller.output", "Using NBC command for scriptless phase 2")
		execScriptOnVMForScenarioValidateExitCode(ctx, s, "sudo journalctl | grep -q 'starting /opt/bin/boothook.sh'", 0, "expected journalctl to contain 'starting /opt/bin/boothook.sh' for scriptless phase 2")
		ValidateFileHasContent(ctx, s, "/var/log/azure/aks-node-controller.output", "Using NBC command for scriptless phase 2")

pkg/agent/baker_test.go:1945

  • The getNodeBootstrappingCmd/scriptless-phase2 path still has a size-based fallback in getLinuxNodeBootstrappingPayload (it disables scriptless phase2 when the rendered custom data exceeds MaxCustomDataLength), but this spec block no longer covers that behavior. Without a test, it’s easy to regress the fallback and end up generating oversized customData that Azure rejects. Please add a deterministic test that forces the scriptless boothook payload over MaxCustomDataLength (e.g., by injecting an incompressible large CustomCATrust cert blob) and asserts the code falls back to the regular Linux CSE path.
	It("should use the aks-node-controller provision-wait command when scriptless phase2 is supported", func() {
		templateGenerator := InitializeTemplateGenerator()
		config := newScriptlessCmdTestConfig()
		config.EnableScriptlessNBCCSECmd = true
		config.PreProvisionOnly = false
		config.AKSNodeConfigJSON = `{"foo":"bar"}`

		cmd := templateGenerator.getNodeBootstrappingCmd(config)

		// Derive the wrapper from the template constant rather than hardcoding the command chain,
		// so adding/reordering steps in cseScriptlessPhase2Template doesn't break this spec.
		prefix, suffix, ok := strings.Cut(cseScriptlessPhase2Template, "%s")
		Expect(ok).To(BeTrue())

		// commandToExecute is run as a single shell string, so it must never span multiple lines.
		Expect(cmd).NotTo(ContainSubstring("\n"))
		Expect(cmd).To(HavePrefix(prefix))
		Expect(cmd).To(HaveSuffix(suffix))
		// whatever else the chain does, provisioning must be the last thing it waits on.
		Expect(cmd).To(HaveSuffix("/opt/azure/containers/aks-node-controller provision-wait"))

		// the blob piped into base64 -d is the scriptless boothook document itself.
		encoded := strings.TrimSuffix(strings.TrimPrefix(cmd, prefix), suffix)
		decoded, err := base64.StdEncoding.DecodeString(encoded)
		Expect(err).NotTo(HaveOccurred())
		decodedGzip, err := getGzipDecodedValue(decoded)
		Expect(err).NotTo(HaveOccurred())
		Expect(string(decodedGzip)).To(ContainSubstring(aksNbcCmdFilepath))
		Expect(string(decodedGzip)).To(ContainSubstring(aksNodeConfigFilepath))

		// custom data now carries the early boothook, which fetches and decrypts these
		// same CSE settings itself so provisioning can start ahead of the CSE handler.
		payload, err := base64.StdEncoding.DecodeString(templateGenerator.getNodeBootstrappingPayload(config))
		Expect(err).NotTo(HaveOccurred())
		Expect(string(payload)).To(HavePrefix("#cloud-boothook\n"))
	})

	It("should use the regular linux CSE command when EnableScriptlessNBCCSECmd is false", func() {
		templateGenerator := InitializeTemplateGenerator()
		config := newScriptlessCmdTestConfig()
		config.EnableScriptlessNBCCSECmd = false
		config.PreProvisionOnly = false

		Expect(templateGenerator.getNodeBootstrappingCmd(config)).To(Equal(templateGenerator.getLinuxNodeCSECommand(config)))
		Expect(templateGenerator.getNodeBootstrappingCmd(config)).NotTo(Equal("/opt/azure/containers/aks-node-controller provision-wait"))
	})

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.

2 participants