Skip to content
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.

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

bug(al2): skip empty container images #1926

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Conversation

ndbaker1
Copy link
Member

@ndbaker1 ndbaker1 commented Aug 16, 2024

Issue #, if available:

Description of changes:

follow up for #1810. when there are no images to cache during worker installation, the blank variable img will be passed to the pull-image.sh script.

the reason for the issue is the fallback "${VAR[@]:-}" still results in a single loop iteration over the empty string "", but would not do so with ${VAR[@]:-}

this PR short circuits when the loop variable is blank, which fixes this case and other cases where the image is actually blank, and also adds logging to help diagnose issues with the images in the future.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing Done

build with the fix alleviates the issue.

@ndbaker1 ndbaker1 added the changelog/exclude Exclude this PR from future changelog entries. label Aug 16, 2024
@ndbaker1
Copy link
Member Author

/ci build
+build cache_container_images=true

@ndbaker1 ndbaker1 force-pushed the fix-install branch 3 times, most recently from 73df233 to fe30a8a Compare August 16, 2024 20:06
@ndbaker1
Copy link
Member Author

had to redo the approach since the linter feel strongly about quoting array expansions

/ci build
+build cache_container_images=true

Copy link
Contributor

@ndbaker1 roger that! I've dispatched a workflow. 👍

Copy link
Contributor

@ndbaker1 the workflow that you requested has completed. 🎉

AMI variantBuildTest
1.21 / al2success ✅skipped ⏭️
1.22 / al2success ✅skipped ⏭️
1.23 / al2success ✅skipped ⏭️
1.24 / al2success ✅skipped ⏭️
1.25 / al2success ✅skipped ⏭️
1.26 / al2success ✅skipped ⏭️
1.27 / al2success ✅skipped ⏭️
1.28 / al2success ✅skipped ⏭️
1.29 / al2success ✅skipped ⏭️
1.30 / al2success ✅skipped ⏭️

@awslabs awslabs deleted a comment from github-actions bot Aug 16, 2024
@awslabs awslabs deleted a comment from github-actions bot Aug 16, 2024
@ndbaker1 ndbaker1 changed the title bug(al2): Remove empty string fallback on cached image list bug(al2): skip empty container images Aug 16, 2024
@Issacwww
Copy link
Member

Thanks for the quick fix! 🚀

@ndbaker1 ndbaker1 merged commit 4f6fb3c into awslabs:main Aug 16, 2024
10 checks passed
@ndbaker1 ndbaker1 deleted the fix-install branch August 16, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/exclude Exclude this PR from future changelog entries.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants