mirror of
https://github.com/storytold/craft-common-actions.git
synced 2026-11-02 18:06:16 +00:00
fix(install-apt-packages): time-limit and retry mirror downloads (#117)
* fix(install-apt-packages): time-limit and retry mirror downloads A SonarCloud job in vectorcraft (run 41253493430) sat in this step for 58 minutes: the Azure Ubuntu mirror timed out, apt fell back to archive.ubuntu.com, then a connection went silent and apt printed nothing until the run was cancelled. Acquire::http::Timeout doesn't end a connection that stays open, so the job's 90-minute timeout was the only bound. apt-get update and a new --download-only pass now each run under timeout(1), 300 s per attempt, up to 3 attempts. The install itself then works from downloaded files only, so dpkg is never killed mid-configure. Acquire::https::Timeout is set alongside the http one for the HTTPS fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(install-apt-packages): retry an update that fetched only some indexes apt-get update warns and exits 0 when an index fails to download, so the retry never ran for it. APT::Update::Error-Mode=any makes that a failure, which is retried. If indexes are still missing after the last attempt the step continues with a warning, as it did before: they often belong to a repository the packages don't need, and a package that needs one fails the download. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: FredApps <FredApps@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -54,6 +54,9 @@ runs:
|
||||
shell: bash
|
||||
env:
|
||||
PACKAGE_LIST: ${{ steps.key.outputs.list }}
|
||||
# Seconds one attempt at a mirror download may take, and how many attempts are made.
|
||||
APT_NETWORK_TIMEOUT: "300"
|
||||
APT_NETWORK_ATTEMPTS: "3"
|
||||
run: |
|
||||
set -euo pipefail
|
||||
debs="$HOME/.cache/craft-apt-debs"
|
||||
@@ -63,10 +66,38 @@ runs:
|
||||
# A slow or stalled mirror connection is retried instead of holding the job for minutes.
|
||||
# The cache directory belongs to the runner user, so downloads run as root rather than as
|
||||
# apt's _apt sandbox user, which could not write there.
|
||||
opts=(-o Acquire::Retries=3 -o Acquire::http::Timeout=30 -o "Dir::Cache::Archives=$debs"
|
||||
-o APT::Keep-Downloaded-Packages=true -o APT::Sandbox::User=root)
|
||||
sudo apt-get "${opts[@]}" update
|
||||
opts=(-o Acquire::Retries=3 -o Acquire::http::Timeout=30 -o Acquire::https::Timeout=30
|
||||
-o "Dir::Cache::Archives=$debs" -o APT::Keep-Downloaded-Packages=true
|
||||
-o APT::Sandbox::User=root)
|
||||
# Acquire timeouts don't end a connection that stays open and sends nothing: one such
|
||||
# stall held a job for 58 minutes until its own timeout. So each mirror download also has
|
||||
# a wall-clock limit and is retried. Only downloads are limited; dpkg is never killed.
|
||||
network() {
|
||||
local attempt status
|
||||
for ((attempt = 1; ; attempt++)); do
|
||||
status=0
|
||||
sudo timeout --kill-after=30 "$APT_NETWORK_TIMEOUT" apt-get "${opts[@]}" "$@" || status=$?
|
||||
if [ "$status" -eq 0 ] || [ "$attempt" -ge "$APT_NETWORK_ATTEMPTS" ]; then
|
||||
return "$status"
|
||||
fi
|
||||
echo "::warning::install-apt-packages: apt-get $1 attempt $attempt exited $status (124: over ${APT_NETWORK_TIMEOUT}s); retrying"
|
||||
sleep 5
|
||||
done
|
||||
}
|
||||
# Error-Mode=any: an index that failed to download fails the update, so it is retried
|
||||
# (apt-get update otherwise warns and exits 0). Indexes still missing after the last
|
||||
# attempt are only a warning, as before: often they belong to a repository the packages
|
||||
# don't need, and a package that does need one fails the download below.
|
||||
if ! network update -o APT::Update::Error-Mode=any; then
|
||||
echo "::warning::install-apt-packages: apt-get update failed $APT_NETWORK_ATTEMPTS times; continuing with the indexes it has"
|
||||
fi
|
||||
# shellcheck disable=SC2086 # the list is split into package names on purpose
|
||||
if ! network install -y --no-install-recommends --download-only -- $PACKAGE_LIST; then
|
||||
echo "::error::install-apt-packages: downloading the packages failed $APT_NETWORK_ATTEMPTS times"
|
||||
exit 1
|
||||
fi
|
||||
# Everything is downloaded: this only unpacks and configures, with no mirror to wait on.
|
||||
# shellcheck disable=SC2086
|
||||
sudo apt-get "${opts[@]}" install -y --no-install-recommends -- $PACKAGE_LIST
|
||||
sudo apt-get "${opts[@]}" autoclean
|
||||
sudo chown -R "$(id -u):$(id -g)" "$debs"
|
||||
|
||||
@@ -94,5 +94,69 @@ class InstallAptPackagesTests(unittest.TestCase):
|
||||
self.assertIn("::error::install-apt-packages:", res.stdout + res.stderr)
|
||||
|
||||
|
||||
|
||||
# apt-get that logs its operation and stalls (sends nothing, never exits) while STALLS says so.
|
||||
FAKE_APT = """#!/bin/bash
|
||||
op=update
|
||||
for arg in "$@"; do case "$arg" in update|install|autoclean) op=$arg ;; --download-only) op=download ;; esac; done
|
||||
echo "$op" >> "$APT_LOG"
|
||||
n=$(grep -c "^$op$" "$APT_LOG")
|
||||
if [ "$op" = "${STALL_OP:-}" ] && [ "$n" -le "${STALLS:-0}" ]; then exec /usr/bin/sleep 60; fi
|
||||
"""
|
||||
|
||||
|
||||
class StalledMirrorTests(unittest.TestCase):
|
||||
"""The install step against a fake apt-get, with a one-second limit per mirror download."""
|
||||
|
||||
def run_install(self, stall_op='', stalls=0):
|
||||
install_step = next(s for s in steps() if s.get('name') == 'Install packages')
|
||||
self.assertEqual(install_step['env']['APT_NETWORK_TIMEOUT'], '300')
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
bin_dir = Path(tmpdir) / 'bin'
|
||||
bin_dir.mkdir()
|
||||
for name, body in [('apt-get', FAKE_APT), ('sudo', '#!/bin/bash\nexec "$@"\n'), ('sleep', '#!/bin/bash\n')]:
|
||||
(bin_dir / name).write_text(body, newline='\n')
|
||||
(bin_dir / name).chmod(0o755)
|
||||
log = Path(tmpdir) / 'apt.log'
|
||||
log.touch()
|
||||
res = subprocess.run(
|
||||
['bash', '--noprofile', '--norc', '-e', '-o', 'pipefail', '-c', install_step['run']],
|
||||
env={**os.environ, 'PATH': f"{bin_dir}{os.pathsep}{os.environ['PATH']}", 'HOME': tmpdir,
|
||||
'PACKAGE_LIST': 'curl git ', 'APT_NETWORK_TIMEOUT': '1', 'APT_NETWORK_ATTEMPTS': '3',
|
||||
'APT_LOG': str(log), 'STALL_OP': stall_op, 'STALLS': str(stalls)},
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=120,
|
||||
)
|
||||
return res, log.read_text().split()
|
||||
|
||||
def test_a_stalled_update_is_retried(self):
|
||||
res, ops = self.run_install('update', stalls=1)
|
||||
self.assertEqual(res.returncode, 0, res.stdout + res.stderr)
|
||||
self.assertEqual(ops, ['update', 'update', 'download', 'install', 'autoclean'])
|
||||
self.assertIn('apt-get update attempt 1 exited 124', res.stdout)
|
||||
|
||||
def test_a_stalled_download_is_retried(self):
|
||||
res, ops = self.run_install('download', stalls=2)
|
||||
self.assertEqual(res.returncode, 0, res.stdout + res.stderr)
|
||||
self.assertEqual(ops, ['update', 'download', 'download', 'download', 'install', 'autoclean'])
|
||||
|
||||
def test_an_update_that_keeps_failing_continues_with_a_warning(self):
|
||||
res, ops = self.run_install('update', stalls=99)
|
||||
self.assertEqual(res.returncode, 0, res.stdout + res.stderr)
|
||||
self.assertEqual(ops, ['update', 'update', 'update', 'download', 'install', 'autoclean'])
|
||||
self.assertIn('::warning::install-apt-packages: apt-get update failed 3 times', res.stdout)
|
||||
|
||||
def test_a_download_that_keeps_stalling_fails_before_installing(self):
|
||||
res, ops = self.run_install('download', stalls=99)
|
||||
self.assertNotEqual(res.returncode, 0)
|
||||
self.assertEqual(ops, ['update', 'download', 'download', 'download'])
|
||||
self.assertIn('::error::install-apt-packages: downloading the packages failed 3 times', res.stdout)
|
||||
|
||||
def test_a_partly_failed_update_is_an_error_so_it_is_retried(self):
|
||||
install_step = next(s for s in steps() if s.get('name') == 'Install packages')
|
||||
self.assertIn('network update -o APT::Update::Error-Mode=any', install_step['run'])
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user