Skip to content

Conversation

@awesomenix
Copy link
Collaborator

What this PR does / why we need it:

Switch to using scriptless but with CSE for Phase 1

Which issue(s) this PR fixes:

Fixes #

Copy link
Contributor

Copilot AI left a comment

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 implements Phase 1 of transitioning to scriptless node bootstrapping by moving LocalDNS configuration from CustomData to CSE environment variables, while maintaining backward compatibility during the migration period.

Changes:

  • LocalDNS configuration moved from cloud-init CustomData to CSE environment variables (SHOULD_ENABLE_LOCALDNS, LOCALDNS_CPU_LIMIT, LOCALDNS_MEMORY_LIMIT, LOCALDNS_GENERATED_COREFILE)
  • Refactored LocalDNS enablement logic to support both legacy (corefile in VHD) and new (corefile generated at provision time) deployment modes
  • Modified GenerateLocalDNSCoreFile to return empty string instead of error when LocalDNS is disabled, allowing graceful handling
  • E2E tests updated to use minimal customData placeholder while CSE variables carry the configuration

Reviewed changes

Copilot reviewed 35 out of 133 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/agent/testdata/*/CSECommand Snapshot tests regenerated with new SHOULD_ENABLE_LOCALDNS, LOCALDNS_CPU_LIMIT, LOCALDNS_MEMORY_LIMIT, LOCALDNS_GENERATED_COREFILE environment variables
pkg/agent/baker_test.go Removed tests expecting errors when LocalDNSProfile is nil or disabled (now returns empty string)
pkg/agent/baker.go Changed GenerateLocalDNSCoreFile to return ("", nil) instead of errors when LocalDNS disabled
parts/linux/cloud-init/artifacts/cse_main.sh Simplified LocalDNS enablement to single conditional path
parts/linux/cloud-init/artifacts/cse_config.sh Refactored enableLocalDNS functions: extracted generateLocalDNSFiles, unified enableLocalDNS logic to handle both legacy and new modes
parts/linux/cloud-init/artifacts/cse_cmd.sh Added 4 new LocalDNS-related environment variables to CSE command
e2e/vmss.go Changed customData to minimal placeholder (base64 "#cloud-config\n") for scriptless testing

Copy link
Contributor

Copilot AI left a comment

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 35 out of 133 changed files in this pull request and generated 2 comments.

mkdir -p "$(dirname "${LOCALDNS_CORE_FILE}")"
touch "${LOCALDNS_CORE_FILE}"
chmod 0644 "${LOCALDNS_CORE_FILE}"
echo "${LOCALDNS_GENERATED_COREFILE}" | base64 -d > "${LOCALDNS_CORE_FILE}" || exit $ERR_LOCALDNS_FAIL
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

LOCALDNS_GENERATED_COREFILE is produced by GenerateLocalDNSCoreFile via getBase64EncodedGzippedCustomScriptFromStr (base64 of gzip content). Here it's only base64-decoded, so the on-disk corefile will be gzip-compressed bytes and localdns.sh/systemd will treat it as an invalid Corefile. Decode + gunzip (or change the generator to return plain base64 text for this path) so /opt/azure/containers/localdns/localdns.corefile is written as plain text.

Suggested change
echo "${LOCALDNS_GENERATED_COREFILE}" | base64 -d > "${LOCALDNS_CORE_FILE}" || exit $ERR_LOCALDNS_FAIL
echo "${LOCALDNS_GENERATED_COREFILE}" | base64 -d | gunzip > "${LOCALDNS_CORE_FILE}" || exit $ERR_LOCALDNS_FAIL

Copilot uses AI. Check for mistakes.
Comment on lines 171 to 176
nodeBootstrapping, err = ab.GetNodeBootstrapping(ctx, s.Runtime.NBC)
require.NoError(s.T, err)
cse = nodeBootstrapping.CSE
customData = nodeBootstrapping.CustomData
// #cloud-config\n
customData = "I2Nsb3VkLWNvbmZpZwoK"
}
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

In the NBC (BootstrapConfigMutator) path, this replaces the AgentBaker-provided nodeBootstrapping.CustomData with a hardcoded "#cloud-config" payload. Unless the VMSS model is guaranteed to no longer rely on CustomData to write required files/units, this will drop critical provisioning inputs and can make E2E scenarios fail in non-scriptless/older-VHD cases. Prefer using nodeBootstrapping.CustomData (or gate this change behind an explicit scriptless flag and ensure required artifacts are present on the image).

Copilot uses AI. Check for mistakes.
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.

1 participant