Skip to content

Remove duplicated retry logic for S3 artifacts #209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
NotTheEvilOne merged 2 commits into from
Oct 3, 2025
Merged

Remove duplicated retry logic for S3 artifacts #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce5ca49
Partly revert "proper mockups for s3-related tests + retrying setup"
NotTheEvilOne Oct 2, 2025
1efe214
Partly revert "retrying for metadata artifacts download"
NotTheEvilOne Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ pygit2 = "^1.18.2"
pygments = "^2.19.2"
PyYAML = "^6.0.2"
gitpython = "^3.1.45"
retrying = "^1.4.2"

[tool.poetry.group.dev.dependencies]
bandit = "^1.8.6"
Expand Down
5 changes: 0 additions & 5 deletions src/gardenlinux/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,3 @@
# https://github.com/gardenlinux/gardenlinux/issues/3044
# Empty string is the 'legacy' variant with traditional root fs and still needed/supported
IMAGE_VARIANTS = ["", "_usi", "_tpm2_trustedboot"]

# configuration for https://github.com/groodt/retrying
RETRYING_MAX_ATTEMPTS = 5
RETRYING_WAIT_EXPONENTIAL_MULTIPLIER = 1000
RETRYING_WAIT_EXPONENTIAL_MAX = 16000
5 changes: 0 additions & 5 deletions src/gardenlinux/s3/bucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
from typing import Any, Optional

import boto3
from retrying import retry

from ..constants import RETRYING_MAX_ATTEMPTS, RETRYING_WAIT_EXPONENTIAL_MAX, RETRYING_WAIT_EXPONENTIAL_MULTIPLIER
from ..logger import LoggerSetup


Expand Down Expand Up @@ -90,9 +88,6 @@ class tree for self).

return getattr(self._bucket, name)

@retry(stop_max_attempt_number=RETRYING_MAX_ATTEMPTS,
wait_exponential_multiplier=RETRYING_WAIT_EXPONENTIAL_MULTIPLIER,
wait_exponential_max=RETRYING_WAIT_EXPONENTIAL_MAX)
def download_file(self, key, file_name, *args, **kwargs):
"""
boto3: Download an S3 object to a file.
Expand Down
103 changes: 52 additions & 51 deletions tests/github/test_download_metadata_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from gardenlinux.constants import RETRYING_MAX_ATTEMPTS, S3_DOWNLOADS_DIR
from gardenlinux.constants import S3_DOWNLOADS_DIR
from gardenlinux.features import CName
from gardenlinux.github.release_notes.helpers import download_metadata_file
from gardenlinux.s3 import S3Artifacts
Expand All @@ -15,24 +15,34 @@


def test_download_metadata_file(downloads_dir, release_s3_bucket):
release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}")
release_s3_bucket.upload_file(
RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}",
)

s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME)
s3_artifacts._bucket = release_s3_bucket

cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format(TEST_GARDENLINUX_RELEASE, TEST_GARDENLINUX_COMMIT_SHORT))
download_metadata_file(s3_artifacts,
cname.cname,
TEST_GARDENLINUX_RELEASE,
TEST_GARDENLINUX_COMMIT_SHORT,
S3_DOWNLOADS_DIR)
cname = CName(
"test-aws-gardener_prod",
"amd64",
"{0}-{1}".format(TEST_GARDENLINUX_RELEASE, TEST_GARDENLINUX_COMMIT_SHORT),
)
download_metadata_file(
s3_artifacts,
cname.cname,
TEST_GARDENLINUX_RELEASE,
TEST_GARDENLINUX_COMMIT_SHORT,
S3_DOWNLOADS_DIR,
)
assert (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists()


def test_download_metadata_file_no_such_release(downloads_dir, release_s3_bucket):
release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}")
release_s3_bucket.upload_file(
RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}",
)
s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME)
s3_artifacts._bucket = release_s3_bucket

Expand All @@ -41,17 +51,19 @@ def test_download_metadata_file_no_such_release(downloads_dir, release_s3_bucket
cname = CName("aws-gardener_prod", "amd64", "{0}-{1}".format(release, commit))

with pytest.raises(IndexError):
download_metadata_file(s3_artifacts,
cname.cname,
release,
commit,
S3_DOWNLOADS_DIR)
assert not (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists()
download_metadata_file(
s3_artifacts, cname.cname, release, commit, S3_DOWNLOADS_DIR
)
assert not (
S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml"
).exists()


def test_download_metadata_file_no_such_commit(downloads_dir, release_s3_bucket):
release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}")
release_s3_bucket.upload_file(
RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}",
)

s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME)
s3_artifacts._bucket = release_s3_bucket
Expand All @@ -61,17 +73,21 @@ def test_download_metadata_file_no_such_commit(downloads_dir, release_s3_bucket)
cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format(release, commit))

with pytest.raises(IndexError):
download_metadata_file(s3_artifacts,
cname.cname,
release,
commit,
S3_DOWNLOADS_DIR)
assert not (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists()


def test_download_metadata_file_no_such_release_and_commit(downloads_dir, release_s3_bucket):
release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}")
download_metadata_file(
s3_artifacts, cname.cname, release, commit, S3_DOWNLOADS_DIR
)
assert not (
S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml"
).exists()


def test_download_metadata_file_no_such_release_and_commit(
downloads_dir, release_s3_bucket
):
release_s3_bucket.upload_file(
RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml",
f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}",
)

s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME)
s3_artifacts._bucket = release_s3_bucket
Expand All @@ -81,24 +97,9 @@ def test_download_metadata_file_no_such_release_and_commit(downloads_dir, releas
cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format(release, commit))

with pytest.raises(IndexError):
download_metadata_file(s3_artifacts,
cname.cname,
release,
commit,
S3_DOWNLOADS_DIR)
assert not (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists()


def test_download_metadata_uses_retrying_strategy(downloads_dir, blackhole_s3_bucket):
s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME)
s3_artifacts._bucket._bucket = blackhole_s3_bucket

cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format("foo", "bar"))

with pytest.raises(IOError) as exn:
download_metadata_file(s3_artifacts,
cname.cname,
"foo",
"bar",
S3_DOWNLOADS_DIR)
assert str(exn.value) == f"Download attempt # {RETRYING_MAX_ATTEMPTS} failed"
download_metadata_file(
s3_artifacts, cname.cname, release, commit, S3_DOWNLOADS_DIR
)
assert not (
S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml"
).exists()
Loading