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

extract docker build / push make targets to helper script #10872

Merged
merged 10 commits into from
May 27, 2024

Conversation

alexrashed
Copy link
Member

@alexrashed alexrashed commented May 22, 2024

Motivation

This PR supersedes #10276.
I created a new PR because the branch of #10276 is already used in a PoC project.

This PR extracts the complex Makefile targets to push our Docker images to a clean and tested bash script.
In addition this PR adds explicit tests, as well as a test pipeline for this script using bats and GitHub actions.

Changes

  • Extracts the following make targets into bin/docker-helper.sh:
    • docker-build
    • docker-build-multiarch
    • docker-push-master
    • docker-create-push-manifests
    • docker-save-image
  • Refactors the usages of the make targets to directly use the helper script.
  • Adds bats tests to tests/bin.
  • Adds a pipeline tests-bin.yaml which executes these bats tests on PRs and on master if the new helper is changed.

TODO

  • Migrate docker-save-image and its usage to docker-helper.sh.
  • Add load function to unify loading the images.
  • Parameterize latest tag.
  • Wait for move source code into localstack-core #10800 to be merged (to avoid any merge conflicts).

@alexrashed alexrashed added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label May 22, 2024
@alexrashed alexrashed added this to the 3.5 milestone May 22, 2024
@alexrashed alexrashed self-assigned this May 22, 2024
Copy link

github-actions bot commented May 22, 2024

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 39m 37s ⏱️ + 2m 24s
2 985 tests ±0  2 675 ✅ ±0  310 💤 ±0  0 ❌ ±0 
2 987 runs  ±0  2 675 ✅ ±0  312 💤 ±0  0 ❌ ±0 

Results for commit bf76448. ± Comparison against base commit 054ac89.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented May 23, 2024

Helper Script Tests

16 tests   16 ✅  0s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit bf76448.

♻️ This comment has been updated with latest results.

@alexrashed alexrashed force-pushed the docker-helper-new branch 2 times, most recently from 86dfeff to d0eae7b Compare May 23, 2024 10:10
Copy link

github-actions bot commented May 23, 2024

S3 Image Test Results (AMD64 / ARM64)

  2 files  ±0    2 suites  ±0   3m 14s ⏱️ +2s
399 tests ±0  347 ✅ ±0   52 💤 ±0  0 ❌ ±0 
798 runs  ±0  694 ✅ ±0  104 💤 ±0  0 ❌ ±0 

Results for commit bf76448. ± Comparison against base commit 054ac89.

♻️ This comment has been updated with latest results.

@alexrashed alexrashed force-pushed the docker-helper-new branch 6 times, most recently from 00e4829 to 5b6fac2 Compare May 23, 2024 12:52
@alexrashed alexrashed marked this pull request as ready for review May 23, 2024 13:23
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great set of changes!
The our build steps have really outgrown he makefile targets, so moving everything into a shell scripts makes it much more maintainable and easier to understand. The helper script is well structured and documented 👌 And neat approach to testing with bats and mocking the docker command 🧠

Approved!

@alexrashed alexrashed merged commit 7c31136 into master May 27, 2024
41 checks passed
@alexrashed alexrashed deleted the docker-helper-new branch May 27, 2024 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants