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

Multi-arch build-push helper script #84

Merged
merged 2 commits into from
Sep 13, 2021
Merged

Conversation

cevich
Copy link
Member

@cevich cevich commented Aug 16, 2021

This will partially address containers/buildah#3268 but the initial intent is to include this script in podman/buildah/skopeo automation. So there may be some initial end-user unfriendliness.

@github-actions
Copy link

github-actions bot commented Aug 16, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's df739c313efd86dc7f09ebcd463744c7b2d358d4.

@github-actions
Copy link

github-actions bot commented Aug 23, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's 5948acf506a7cf9eb8d98928917e9462df23083b.

@cevich cevich force-pushed the build-push branch 2 times, most recently from 48216ba to 0793518 Compare August 24, 2021 14:33
@cevich cevich force-pushed the build-push branch 6 times, most recently from 9064a33 to e05529f Compare August 25, 2021 18:18
@github-actions
Copy link

github-actions bot commented Aug 25, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's e05529f2532c19e308d53f7a8880295966d37de9.

@github-actions
Copy link

github-actions bot commented Aug 25, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's e1023710cac531c760b5cce784fbb51c2528f3cb.

@github-actions
Copy link

github-actions bot commented Aug 25, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's 84ea554532885b43dbebd4f0409aa6643b7af4d4.

@cevich
Copy link
Member Author

cevich commented Aug 25, 2021

@edsantiago this is still a WIP/prototype, but I've finally got the tests passing, and I'd appreciate your early feedback if you're able. Mainly just build-push/bin/build-push.sh and the README.md.

My intent (once this is more solid) is to incorporate it, along with specific tagging and modding scripts, into a VM image. That image can then be used from Cirrus-CI by the various projects to replace the fugly github-actions workflow. Thereby de-duplicating and simplifying the wizardry into unit and integration-tested process compatible with normal PR-workflows.

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

First pass, but not a 100% thorough one (I got tired!)

build-push/README.md Outdated Show resolved Hide resolved
build-push/README.md Outdated Show resolved Hide resolved
build-push/README.md Outdated Show resolved Hide resolved
build-push/README.md Outdated Show resolved Hide resolved
build-push/README.md Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
@cevich
Copy link
Member Author

cevich commented Aug 30, 2021

Thanks @edsantiago for so many helpful suggestions, especially on the CLI option expectations and related docs.

not a 100% thorough one (I got tired!)

Nono, this is great. I just needed an initial sanity-check over exactly what you looked at. I was afraid of getting too far and needing to re-architect due to some dumb choice (like if --tagcmd was a totally dumb idea). Thanks a bunch, I'll make all those fixes and press onward.

@cevich
Copy link
Member Author

cevich commented Sep 1, 2021

Major changes:

  • s/bud/build/g everywhere
  • Killed off --tagcmd completely, since it can easily be done as part of --modcmd as needed.
  • Renamed $NOPUSH to $PUSH along with other related readability improvements
  • Lots of README.md fixes and clarified examples
  • Lots of --help output cleanup
  • Re-worked registry login function to be called from multiple places w/o logging in multiple times
  • Implemented pushing function
  • Reworked tests as needed to pass (one test is temporarily disabled due to buildah open issue)

@github-actions
Copy link

github-actions bot commented Sep 1, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's dcf0f7fc8d81bf715af9d68580426ff984cab04b.

@cevich
Copy link
Member Author

cevich commented Sep 1, 2021

@edsantiago please don't make another pass yet, I need to work it over a few more times myself first. I'm positive there are a few "it's" I missed 😀

@cevich
Copy link
Member Author

cevich commented Sep 7, 2021

More changes:

  • Added "e2e" test with an actual multi-arch build and push to a test registry.
  • Additional README.md cleanup/fixes
  • Added a "stage notice" function to help humans distinguish pre, build, and post output. Otherwise it's a massive wall-of-text.
  • Fixed a typo of NAMESPASCE_USERNAME
  • Rearchitected use of $MANIFESTID - not reliable to use in practice, need to use the FQIN for manifest-lists.
  • Implemented workaround (for testing only!) to install qemu-user-static in CentOS stream-8 VM

@cevich cevich force-pushed the build-push branch 3 times, most recently from 8a38bfb to cd69bbb Compare September 7, 2021 20:55
@github-actions
Copy link

github-actions bot commented Sep 7, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's cd69bbbc0a291bd1eea08cfc5ab24174f3655098.

@cevich cevich force-pushed the build-push branch 3 times, most recently from cc40af9 to 62b6a62 Compare September 8, 2021 16:54
@github-actions
Copy link

github-actions bot commented Sep 8, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's 62b6a621325409e51d8313c038453108f97c8ecc.

@github-actions
Copy link

github-actions bot commented Sep 8, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's 87f6d9ac6dacbb69f59d21d52e57048ebb9aa0bd.

@cevich
Copy link
Member Author

cevich commented Sep 8, 2021

Pre-review final cleanup:

  • Added check on e2e test build, confirming all expected arches are present.
  • Moved/clarified comments in .cirrus.yml.
  • Switched to using a buildah namespace instead of a personal one for the test-push.
  • Pass in test FQIN from .cirrus.yml into test, placing definition close to needed credentials.
  • Added checks for required variables in testbuilds.sh.
  • build-push/README.md fixes/adjustments.

@cevich cevich marked this pull request as ready for review September 8, 2021 17:17
@cevich
Copy link
Member Author

cevich commented Sep 8, 2021

@edsantiago PTAL. Hopefully my changes summaries are helpful. The test-suite isn't perfect, but should be good n'uff for starting out. I plan to add more tests when incorporating this into a VM image for actual use in containers-org projects.

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

I only reviewed range-diff, but even so that was a lot: I ran out of steam near the end (tests).

LGTM, my comments are mostly typos. @rhatdan has a tool somewhere that does spellchecks in code, maybe you could add that to your pre-commit habits?

build-push/README.md Outdated Show resolved Hide resolved
build-push/README.md Outdated Show resolved Hide resolved
build-push/README.md Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
build-push/bin/build-push.sh Outdated Show resolved Hide resolved
@cevich
Copy link
Member Author

cevich commented Sep 9, 2021

I ran out of steam near the end (tests).

Still, I appreciate all the effort. The tests setup here is less than ideal and quite hacky given they need to do a lot with very little in the way of pre-installed tooling (i.e. none). There'll be more practical testing done at higher-levels when I work this script up the automation-food-chain.

@rhatdan has a tool somewhere that does spellchecks in code, maybe you could add that to your pre-commit habits?

Oh! That's a novel idea, thanks!

@github-actions
Copy link

github-actions bot commented Sep 13, 2021

Successfully triggered github-actions/success task to indicate successful run of cirrus-ci_retrospective integration and unit testing from this PR's de236cdc47c97e6c94e2d42caf17eed2b105698a.

@cevich cevich merged commit 4f304ba into containers:main Sep 13, 2021
@cevich cevich changed the title [WIP] Multi-arch build-push helper script Multi-arch build-push helper script Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants