fix: add per-attempt timeout and cse budget guard to _apt_get_update

[awesomenix]

Description:

_apt_get_update was the only retry helper without a per-attempt timeout or CSE budget
guard. On broken DNS/NSG, 10 uncapped attempts could burn the entire 780s CSE budget and
starve later provisioning steps, surfacing as exit 99 (ERR_APT_UPDATE_TIMEOUT) — a
timeout the function never actually enforced.

Changes

• Wrap each apt-get update attempt in timeout, defaulting to 180s (override via
  APT_GET_UPDATE_TIMEOUT_SECONDS), capped to the remaining CSE budget.
• Add check_cse_timeout before each attempt and before each sleep; return 2 to let
  callers short-circuit, matching _apt_get_install and _retrycmd_internal.
• Success now requires apt-get to exit 0 and emit no W:/E: lines. Required by the
  above: a timeout-killed apt-get may flush no diagnostics, which the previous grep-only
  check would have misread as success.

Guards are only active when CSE_STARTTIME_SECONDS is set, so VHD build behavior is
unchanged. All callers already use || exit $ERR_APT_UPDATE_TIMEOUT, so return 2 is handled.

180s (not the 90s originally proposed) since apt-get update fetches index files from
several repos sequentially; a tighter cap risks failing slow-but-recoverable mirrors.

[github-actions]

Windows Unit Test Results

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

Results for commit 3d054b1.

♻️ This comment has been updated with latest results.

[Copilot]

Pull request overview

This PR hardens the Ubuntu _apt_get_update retry helper used during node provisioning by preventing individual apt-get update attempts from monopolizing the global CSE execution budget, and by making “success” depend on both exit status and diagnostics output.

Changes:

• Wrap each apt-get update attempt with a per-attempt timeout (default 180s; configurable via APT_GET_UPDATE_TIMEOUT_SECONDS) and cap it to the remaining CSE budget when running under CSE.
• Add check_cse_timeout guards before each attempt and before sleeping between retries; return 2 to signal “stop retrying due to CSE budget”.
• Add ShellSpec coverage validating timeout capping, early-exit semantics, and timeout failure behavior.

Reviewed changes

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

File	Description
spec/parts/linux/cloud-init/artifacts/cse_helpers_ubuntu_spec.sh	Adds ShellSpec tests for _apt_get_update budget-guard and per-attempt timeout behavior.
parts/linux/cloud-init/artifacts/ubuntu/cse_helpers_ubuntu.sh	Implements per-attempt timeouts, remaining-budget capping, and CSE timeout guarding for _apt_get_update.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

parts/linux/cloud-init/artifacts/ubuntu/cse_helpers_ubuntu.sh:31

• _apt_get_update now documents/returns status 2 to allow callers to short-circuit when CSE time is nearly exhausted, but at least one caller (apt_get_install_from_local_repo in this same file) invokes it via if ! _apt_get_update ...; then ... return 1, which collapses a 2 into a generic failure. That loses the early-exit signal and can cause the CSE to continue into slower fallback paths instead of stopping immediately. Consider preserving the exact return code (e.g., capture $? and return $rc when rc==2) in wrappers/callers that currently use ! _apt_get_update.

# Core update function used by apt_get_update and apt_get_install_from_local_repo
# returns 0 if successful, 1 if retries are exhausted, 2 if the CSE timeout is approaching
# and the caller should short-circuit.
_apt_get_update() {

parts/linux/cloud-init/artifacts/ubuntu/cse_helpers_ubuntu.sh:82

• Quote $apt_update_output in the redirection/cat/grep uses to avoid accidental word-splitting or glob expansion if this path ever changes (and to match other code in this file that consistently quotes paths).

        timeout "$effectiveTimeout" apt-get ${apt_opts} update > $apt_update_output 2>&1
        aptStatus=$?
        cat $apt_update_output
        # apt-get must both exit 0 and emit no W:/E: lines. Checking the exit status matters now that
        # attempts are wrapped in timeout: a killed apt-get may flush no diagnostics at all, which the

[Devinwong]

The caller should also handle return 2 if you mean to distinguish between 1 & 2, right?

[Devinwong]

Problematic codes

if ! _apt_get_update 10 "${opts}"; then
        echo "Failed to update apt cache from local repo ${local_repo_dir}"
        rm -f "${tmp_list}"
        rmdir "${tmp_dir}"
        return 1
    fi

[Devinwong]

but not a blocker

[awesomenix]

wont that be handled with retries, meaning if it failed it will retry X times and then returns 1?

[awesomenix]

i think the idea here is that we are returning exit code 99 rather than exit code 124

[Devinwong]

approve with non-blocking suggestions