You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Ubuntu 26.04 CVM images are built on AMD SEV-SNP and therefore contain the existing Hyper-V PHC Chrony configuration. The same image can also be used on Intel TDX nodes, where the node must use network NTP instead of the inherited PHC configuration.
This change detects the confidential VM platform at the start of node provisioning and selects the appropriate Chrony source without changing the resulting configuration for SEV-SNP nodes.
Changes
• Scope platform-specific time synchronization to Ubuntu 26.04 FDE images during normal node initialization.
• Detect the confidential VM platform with systemd-detect-virt --cvm
• Preserve the existing Chrony PHC configuration for AMD SEV-SNP refclock PHC /dev/ptp0 poll 3 dpoll -2 offset 0
• Reconfigure Intel TDX nodes to use only the approved Ubuntu NTP pools: pool ntp.ubuntu.com iburst maxsources 4 pool 0.ubuntu.pool.ntp.org iburst maxsources 1 pool 1.ubuntu.pool.ntp.org iburst maxsources 1 pool 2.ubuntu.pool.ntp.org iburst maxsources 2
• Fail node provisioning with dedicated exit code 245 when Chrony cannot synchronize with the configured Ubuntu NTP pools.
• Log chronyc sources -v and chronyc tracking diagnostics to the provisioning log when synchronization fails.
• Fail with dedicated exit code 244 if the node cannot be safely identified as SEV-SNP or TDX.
• Fail with dedicated exit code 246 if Chrony cannot be installed, configured, written, or restarted for the detected CVM platform.
• Query the systemd-timesyncd unit LoadState, skipping service control only when the unit is not-found; loaded but inactive units are still stopped and disabled.
• Continue attempting the remaining Chrony configuration steps after an individual operation fails, while recording the overall result and logging each specific error to stderr .
• Preserve the existing best-effort provisioning behavior for non-target Ubuntu and Flatcar images while making Chrony configuration failures fatal for the targeted Ubuntu 26.04 CVM path.
• Add CSE events for platform selection, successful NTP synchronization, NTP failure, platform-detection failure, and Chrony configuration failure.
• Propagate platform-detection, NTP-synchronization, and Chrony-configuration failures through the generated CSE command.
• Refactor the existing inline Ubuntu and Flatcar Chrony setup into a reusable function while preserving the default PHC configuration for non-target images.
• Preserve the separate native Chrony paths used by Azure Linux, Mariner, and ACL, along with Flatcar-specific service handling.
• Add focused coverage for Ubuntu 26.04 CVM platform selection, SEV-SNP PHC behavior, TDX NTP behavior, synchronization and setup failures, removed and loaded-but-inactive systemd-timesyncd units, best-effort non-target behavior, other Ubuntu releases, and Azure Linux/Mariner routing.
Which issue(s) this PR fixes:
TDX SKUs cannot use the inherited Hyper-V PHC time source and must instead synchronize through the approved Ubuntu NTP pools.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
NTP failures use the wrong exit-code contract, and Chrony setup failures can allow provisioning to continue.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
parts/linux/cloud-init/artifacts/cse_cmd.sh:27
🔴 High Risk — Script Logic: this wrapper only exits for 244/245. configure_ubuntu_2604_cvm_time_sync can return status 1 when configure_chrony fails (the TDX branch explicitly maps that failure to return 1, and the SEV-SNP branch returns the helper status), so this branch falls through and continues provisioning a CVM node without the required time source. Propagate any nonzero result or map configuration failures to a code handled here.
if [ "$initAKSCloudExitCode" -eq 245 ]; then
echo "NTP not reachable; init-aks-cloud failed with NTP synchronization error code ${initAKSCloudExitCode}" >> ${PROVISION_OUTPUT};
exit ${initAKSCloudExitCode};
elif [ "$initAKSCloudExitCode" -eq 244 ]; then
echo "Unable to determine confidential VM platform; init-aks-cloud failed with exit code ${initAKSCloudExitCode}" >> ${PROVISION_OUTPUT};
This returns 245, but the requested classification for an NTP connectivity failure is the existing outbound-connectivity code 50 (cse_helpers.sh:37, used by cse_main.sh:460). Because cse_cmd.sh exits immediately on this status before cse_main runs, TDX NTP failures are reported as 245 and never use the established outbound-connectivity code. Return 50 for this path and update the CSE command's special-case handling and tests accordingly.
[High Risk] The PR contract says an NTP synchronization failure must use the existing outbound-connectivity exit code 50 (ERR_OUTBOUND_CONN_FAIL), but this path returns the new 245 (ERR_NTP_UNREACHABLE), and cse_cmd.sh propagates 245. As a result, consumers that classify exit 50 (including the e2e retry handling) will not recognize this failure as the outbound-connectivity case. Return 50 here and update the related command/tests, or revise the stated contract.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
NTP failure handling still conflicts with the documented outbound-connectivity exit-code contract.
Review details
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
parts/linux/cloud-init/artifacts/cse_cmd.sh:31
The new tests only assert that fragments of this exit-code dispatch appear in the rendered command. They do not execute the flattened command, so a quoting or control-flow error could still allow the main CSE to start after init-aks-cloud.sh returns 244, 245, or 246. Add an execution test in pkg/agent/baker_test.go, using temporary mock init/CSE scripts, that verifies each new failure code exits before the main CSE and that the zero exit path continues.
🔴 High Risk — Script Logic: The PR contract says an NTP synchronization failure must use the outbound-connectivity exit code 50, but this defines and propagates 245. The E2E harness only recognizes ERR_OUTBOUND_CONN_FAIL/50 as retryable (e2e/scenario/vmss.go:248-256), so this failure will be classified differently and will not receive the intended retry handling. Use 50 consistently across both constant definitions, cse_cmd.sh, and the tests, or update the stated contract if 245 is intentional.
ERR_NTP_UNREACHABLE=245 # Chrony could not synchronize with the configured NTP pools
[🔴 High Risk][bug] Ubuntu VHD creation already treats a missing/removed systemd-timesyncd unit as normal (vhdbuilder/scripts/linux/ubuntu/tool_installs_ubuntu.sh:117-124). On those 26.04 FDE images, these unconditional stop/disable calls can return nonzero, so the new CVM path returns 246 before it writes the PHC/NTP configuration. Use the existing SubState/dead guard (or tolerate unit-not-found) while still failing on genuine systemctl errors.
[🔴 High Risk][discrepancy_with_pr_description] The PR description requires NTP synchronization failures to use the existing outbound-connectivity exit code 50 (ERR_OUTBOUND_CONN_FAIL), but this path returns 245 and cse_cmd.sh propagates 245 instead. That changes the stated provisioning contract; align the implementation and generated command with code 50, or update the requirement and all consumers/tests.
🔴 High Risk — Script Logic: The PR contract says an NTP failure must use the existing outbound-connectivity exit code 50 (ERR_OUTBOUND_CONN_FAIL in cse_helpers.sh:37), but this returns 245 and cse_cmd.sh propagates 245. Consumers that classify provisioning failures by code 50 will miss an NTP outage. Use code 50 for this path and update the wrapper/tests to propagate that established code.
🟡 Medium Risk — Script Logic: The refactor now treats stopping/disabling systemd-timesyncd as a hard failure. The Ubuntu VHD build path explicitly supports this unit being absent (vhdbuilder/scripts/linux/ubuntu/tool_installs_ubuntu.sh:116-124), so a CVM/minimal image can enter this branch with no unit; configure_chrony then returns before writing the NTP/PHC config, and TDX provisioning fails with 246. Preserve the prior behavior by checking the unit state first and only stopping/disabling it when present.
if ! systemctl stop systemd-timesyncd; then
echo "ERROR: failed to stop systemd-timesyncd" >&2
🔴 High Risk — Script Logic. This assigns the NTP failure to 245, but the repository's established outbound-connectivity code is ERR_OUTBOUND_CONN_FAIL=50 (cse_helpers.sh:37), and the e2e retry path only recognizes exit 50 (e2e/scenario/vmss.go:193-197). As a result, a transient NTP reachability failure will not follow the outbound-connectivity retry/classification path and also contradicts the PR's stated exit code. Use the established 50 code and update the CSE dispatch/tests consistently.
ERR_NTP_UNREACHABLE=245 # Chrony could not synchronize with the configured NTP pools
🔴 High Risk — Script Logic. Ubuntu VHD setup explicitly supports images where systemd-timesyncd is absent by checking its state and skipping stop/disable (vhdbuilder/scripts/linux/ubuntu/tool_installs_ubuntu.sh:116-124), but this refactored path treats either command failure as fatal. On such an image, Ubuntu 26.04 CVM provisioning exits 246 before writing the selected Chrony configuration. Guard these calls with the same missing-unit check used by the VHD builder, while still failing on real service-operation errors.
return 1
fi
if ! systemctl disable systemd-timesyncd; then
echo "ERROR: failed to disable systemd-timesyncd" >&2
return 1
The refactor changes failure handling for every non-target Ubuntu and Flatcar node. configure_chrony now returns immediately if service control, package installation, config removal/write, or restart fails, but this caller ignores that status. Unlike the previous inline best-effort sequence, a transient early failure can now skip writing and restarting Chrony while provisioning still succeeds, leaving the node with stale or incomplete time-source configuration. Preserve the old best-effort behavior for this path, or explicitly propagate the failure if making Chrony setup fatal for these platforms is intended.
🟡 Medium Risk — 🔄 Backward Compatibility:SubState=dead means the unit is inactive, not that its unit file is removed. An existing, enabled-but-stopped systemd-timesyncd therefore bypasses disable and can start alongside Chrony after a reboot. Check LoadState and skip service control only for not-found; update the removed-unit ShellSpec mock accordingly.
timesyncd_status="$(systemctl show -p SubState --value systemd-timesyncd 2>/dev/null || true)"
if [ "$timesyncd_status" = "dead" ]; then
echo "systemd-timesyncd is removed, no need to disable"
SubState=dead is not a presence check: an installed but inactive systemd-timesyncd unit also reports dead, so this branch can leave an enabled unit untouched and let it start again on the next boot, racing Chrony. Conversely, a removed unit may produce no SubState value and then fall into stop, causing the new 246 failure instead of being skipped. Check the unit's load/presence state separately and skip only for not-found, while preserving real systemctl errors.
timesyncd_status="$(systemctl show -p SubState --value systemd-timesyncd 2>/dev/null || true)"
if [ "$timesyncd_status" = "dead" ]; then
echo "systemd-timesyncd is removed, no need to disable"
else
🟡 Medium Risk — Script Logic: SubState=dead is the inactive state for both a missing unit and an installed but stopped unit, so this check does not prove that systemd-timesyncd was removed. For an installed unit that is merely inactive, the code skips disable, allowing it to be started later while Chrony is active; query LoadState (for not-found) or otherwise check unit presence instead. The added test mocks dead, so it does not cover this distinction.
timesyncd_status="$(systemctl show -p SubState --value systemd-timesyncd 2>/dev/null || true)"
if [ "$timesyncd_status" = "dead" ]; then
echo "systemd-timesyncd is removed, no need to disable"
parts/linux/cloud-init/artifacts/cse_cmd.sh:32
🔴 High Risk — Script Logic: This block has no fallback for other non-zero statuses. init-aks-cloud.sh still exits 1 for certificate and custom-cloud repository failures (for example, its exit 1 paths around lines 807, 819, and 924), but those failures are now ignored and provisioning proceeds to provision_start.sh; the previous || exit $? propagated them. Retain the dedicated messages, but add a final non-zero branch that logs and exits with the original status.
🔴 High Risk — Script Logic: SubState=dead means the unit is inactive, not necessarily removed; an installed-but-stopped, still-enabled systemd-timesyncd unit also reports dead. This branch therefore skips systemctl disable, allowing timesyncd to start later and race Chrony. Check the unit's LoadState/enablement and skip only when it is genuinely not found, while preserving stop/disable failures.
timesyncd_status="$(systemctl show -p SubState --value systemd-timesyncd 2>/dev/null || true)"
if [ "$timesyncd_status" = "dead" ]; then
echo "systemd-timesyncd is removed, no need to disable"
else
if ! systemctl stop systemd-timesyncd; then
The reason will be displayed to describe this comment to others. Learn more.
I think it's still relevant in cases where we may need to deliver a hotfix of this script? Nishchay (@awesomenix)Devinwonglilypan26 to keep me honest here, that might've just been for phase 2 I can't remember at this point
🟡 Medium Risk — 🧪 Test Coverage: The new ShellSpec cases call the helper functions directly, but none executes the top-level action=init gate. A regression connecting Ubuntu/FDE detection to configure_ubuntu_2604_cvm_time_sync, or propagating exit codes 244–246, could bypass the platform-specific setup while all current tests pass. Add an execution-level test that runs the script in init mode with isolated OS/kernel and Chrony commands, then verifies the selected configuration and final exit status.
Detective summary: E2E completed 113 passed, 0 flaky, 70 skipped, 1 failed. The only failed scenario is AzureLinuxV2, which failed at VMSS create before node validation because ARM rejected osProfile.customData: it must be Base64 and no more than 87380 characters. A separate Test/Scan warning is non-fatal and the E2E failure is the gate-blocking error.
Likely cause / signature: $sig.
Confidence: High.
Recommended owner/action: Continue repair item #39729730; PR author and CSE/init-aks-cloud reviewers should verify this change does not increase or corrupt customData beyond ARM limits for AzureLinuxV2.
Strongest alternative: transient ARM/VMSS validation issue; less likely because the error is deterministic input validation on osProfile.customData, and this exact signature has recurred across multiple PR-gate runs.
Evidence: E2E task log reports the AzureLinuxV2 VMSS create PUT failed with InvalidParameter targeting osProfile.customData; summary reports only one failed scenario and zero flaky tests; timeline marks the Run AgentBaker E2E task failed; build metadata maps to PR #9591; PR changed files include CSE/init-aks-cloud scripts that can affect customData content.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
Existing public-cloud VHDs retain the baked PHC-only script, so affected TDX nodes remain unsupported without a compatible delivery or image rollout strategy.
Get a fresh assessment by requesting another Copilot review.
Detective summary: The gate-blocking failure is the E2E scenario Ubuntu2604Minimal_Taints: post-provisioning validation detected a critical kernel PANIC/CRASH call trace at drm_atomic_helper_wait_for_vblanks.part.0. The suite reports 113 passed, 0 flaky, 70 skipped, 1 failed. The same run also repeated the known Ubuntu 22.04 FIPS CIS baseline diff for rules 5.1.18 and 5.1.6.
Likely cause / signature: $kernelSig (secondary: $cisSig).
Confidence: Medium-high.
Recommended owner/action: Treat as recurring kernel-validation noise unless a rerun reproduces a PR-specific CSE/init path; separately track the existing CIS baseline drift.
Strongest alternative: PR #9591 CSE/init-aks-cloud changes caused kernel/CIS regressions; less likely because the kernel trace and CIS rule diff both match existing cross-PR signatures.
Evidence: E2E task log final summary names Ubuntu2604Minimal_Taints and the drm_atomic_helper_wait_for_vblanks panic trace; timeline marks Run AgentBaker E2E failed; Test/Scan log reports CIS 5.1.18|pass->fail and 5.1.6|pass->fail; wiki already tracks both signatures.
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
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.
What this PR does / why we need it:
Ubuntu 26.04 CVM images are built on AMD SEV-SNP and therefore contain the existing Hyper-V PHC Chrony configuration. The same image can also be used on Intel TDX nodes, where the node must use network NTP instead of the inherited PHC configuration.
This change detects the confidential VM platform at the start of node provisioning and selects the appropriate Chrony source without changing the resulting configuration for SEV-SNP nodes.
Changes
• Scope platform-specific time synchronization to Ubuntu 26.04 FDE images during normal node initialization.
• Detect the confidential VM platform with
systemd-detect-virt --cvm• Preserve the existing Chrony PHC configuration for AMD SEV-SNP
refclock PHC /dev/ptp0 poll 3 dpoll -2 offset 0• Reconfigure Intel TDX nodes to use only the approved Ubuntu NTP pools:
pool ntp.ubuntu.com iburst maxsources 4pool 0.ubuntu.pool.ntp.org iburst maxsources 1pool 1.ubuntu.pool.ntp.org iburst maxsources 1pool 2.ubuntu.pool.ntp.org iburst maxsources 2• Validate TDX network synchronization with
chronyc waitsync 12 0 0 5• Fail node provisioning with dedicated exit code 245 when Chrony cannot synchronize with the configured Ubuntu NTP pools.
• Log
chronyc sources -vandchronyc trackingdiagnostics to the provisioning log when synchronization fails.• Fail with dedicated exit code 244 if the node cannot be safely identified as SEV-SNP or TDX.
• Fail with dedicated exit code 246 if Chrony cannot be installed, configured, written, or restarted for the detected CVM platform.
• Query the systemd-timesyncd unit LoadState, skipping service control only when the unit is not-found; loaded but inactive units are still stopped and disabled.
• Continue attempting the remaining Chrony configuration steps after an individual operation fails, while recording the overall result and logging each specific error to stderr .
• Preserve the existing best-effort provisioning behavior for non-target Ubuntu and Flatcar images while making Chrony configuration failures fatal for the targeted Ubuntu 26.04 CVM path.
• Add CSE events for platform selection, successful NTP synchronization, NTP failure, platform-detection failure, and Chrony configuration failure.
• Propagate platform-detection, NTP-synchronization, and Chrony-configuration failures through the generated CSE command.
• Refactor the existing inline Ubuntu and Flatcar Chrony setup into a reusable function while preserving the default PHC configuration for non-target images.
• Preserve the separate native Chrony paths used by Azure Linux, Mariner, and ACL, along with Flatcar-specific service handling.
• Add focused coverage for Ubuntu 26.04 CVM platform selection, SEV-SNP PHC behavior, TDX NTP behavior, synchronization and setup failures, removed and loaded-but-inactive systemd-timesyncd units, best-effort non-target behavior, other Ubuntu releases, and Azure Linux/Mariner routing.
Which issue(s) this PR fixes:
TDX SKUs cannot use the inherited Hyper-V PHC time source and must instead synchronize through the approved Ubuntu NTP pools.