workflows: build and push indexes #230
Conversation
|
🎉 All dependencies have been resolved ! |
phlogistonjohn
force-pushed
the
jjm-indexes
branch
from
b662a53 to
f4ced68
Compare
|
|
||
| env: | ||
| CONTAINER_CMD: docker | ||
| CONTAINER_CMD: podman |
There was a problem hiding this comment.
I agree here, and we should probably align to podman in all of samba-in-kubernetes sub-projects. However, it would be nice to add to the checks stage something like run: command -v ${{ env.CONTAINER_CMD }}
synarete
left a comment
synarete
left a comment
There was a problem hiding this comment.
Overall, looks good. My only concern is how we (developers) can test those images (outside of CI), but this discussion is out-of-scope for this PR.
Indeed. I am hoping that once the new Ceph lab is sorted out there will be arm64 boxes to hop on and test these builds. Esp. since my plan is to eventually switch the cephadm code to make use of the -any indexes :-) |
anoopcs9
left a comment
anoopcs9
left a comment
There was a problem hiding this comment.
On top of the suggested changes I recommend enabling some CentOS based arm64 jobs -- anoopcs9@0cdfe83.
phlogistonjohn
force-pushed
the
jjm-indexes
branch
from
f4ced68 to
782d704
Compare
mergify
bot
dismissed
synarete’s stale review
Pull request has been modified.
phlogistonjohn
force-pushed
the
jjm-indexes
branch
from
782d704 to
623d418
Compare
Discussed in a call. I do believe that the PR has satisfied what we discussed and the follow up PR #234 ought to complete the needed changes suggested in the above patch. |
phlogistonjohn
force-pushed
the
jjm-indexes
branch
from
623d418 to
47c9fe7
Compare
anoopcs9
left a comment
anoopcs9
left a comment
There was a problem hiding this comment.
On top of the suggested changes I recommend enabling some CentOS based arm64 jobs -- anoopcs9@0cdfe83.
Discussed in a call. I do believe that the PR has satisfied what we discussed and the follow up PR #234 ought to complete the needed changes suggested in the above patch.
Fine, still I think the other inline suggestions needs to be fixed (see below)
|
FWIW I plan on adding a new option that allows secifiying an input file instead of a bunch of -i options in the future. That will rid the script of the duplication and allow us to use one "list". |
|
I don't know if it's github being weird or I'm just a blockhead today but I tried to make sense of your suggestions but couldn't. I used unix tools to ensure there were no more duplicates in the images and then I went over the samba-client "subsection" and tried fixing it up (and then copying it to the repeated sections). I'm testing it but I can't be sure it exactly covers what you are asking for. |
| exclude: | ||
| - os: centos | ||
| arch: arm64 | ||
| runs-on: ubuntu-latest | ||
| arch: {name: arm64, host: ubuntu-24.04-arm} |
There was a problem hiding this comment.
samba-client:default-centos-arm64 is mentioned down the line. Did you forget to remove the exclude altogether here?
There was a problem hiding this comment.
Updated. PTAL
phlogistonjohn
force-pushed
the
jjm-indexes
branch
from
b29e6a3 to
64cb875
Compare
|
@Mergifyio rebase |
The lines were causing flake8 to complain so if we want to use the tool to validate things we should make it happy. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Avoid syntax errors and style/formatting drift by adding a very basic rule for flake8 and pyflakes to run in our CI. Signed-off-by: John Mulligan <jmulligan@redhat.com>
In preparation for being able to create indexes (aka manifests) which can combine one or more "true" images the main function being purely per-target will become limiting. Create a new helper class structure that handles per target and then global actions with a simple shim class for adapting the legacy functions without needing to change them. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add a new --index action that creates indexes/manifests based on the inputted targets. For now the indexes abstract away the architecture. To preserve the FQIN pattern our indexes and indexes alone will use 'any' in the 3rd position of the FQIN tag. This will allow consumers to eventually run something like `podman pull quay.io/samba.org/samba-server:default-fedora-any` and it will automatically select the correct image for the architecture if said arch image exists. This does NOT change the existing per-arch FQIN for "real" images. My experiments with manifest commands with podman have shown some quirky behavior so you will see a more belt-and-suspenders approach to some of these commands. I have only tested it on podman because I don't care about docker. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Swap the push function to a new Pusher class to make use of the new "main function" handling in preparation for extending push to also push indexes/manifests. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Enhance the QMatcher class to help be a more generic "selector" for what to push. This is in preparation for pushing indexes. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add support to push indexes/manifests along with "real" images. This can be invoked by specifying either "index" or "index-multiarch" to the --push-kinds/--push-what option. For example: `--push-kinds=mixed,index-multiarch`. Both options enable pushing indexes (aka manifests) but "index-multiarch" will only push those that refer to 2 or more "real" images - implying images that actually have support for multiple architectures. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Use podman in our CI because it matches what we (I?) use to test the scripts and what I use to release images. I'm not against having things work with docker but I don't like having to constantly second guess what I am doing when working on these features. Signed-off-by: John Mulligan <jmulligan@redhat.com>
GitHub has supported native arm64 for almost a year, but that had somehow flown under my radar until recently. Tweak the matrix to choose those builders when building for arm64 instead of relying on emulation. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Indexes, aka manifests, make it easier for clients to specify a tag name that will automatically select between architectures for you when choosing an image from a registry. Signed-off-by: John Mulligan <jmulligan@redhat.com>
We test the opensuse images but we hadn't been pushing them before but somehow some of the arm64 (and only arm64!) images got added to the list of images to publish to our quay.io registry. Stop that. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Try to make the list of images that we publish a bit more obvious by sorting the lists. Signed-off-by: John Mulligan <jmulligan@redhat.com>
It was excluded without needing to be. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
✅ Branch has been successfully rebased |
anoopcs9
force-pushed
the
jjm-indexes
branch
from
64cb875 to
0bd96c8
Compare
|
@synarete ptal, thanks |
|
Reminder: the jobs names change so we'll need to manually merge it. Once it's merged we (I) need to update the branch protection rules with the new names. |
It won’t be just the branch‑protection rules; the Mergify configuration would also need an update (if so, it’s better to include those changes here). Otherwise, we could explicitly define job names as: |
3 participants
Depends on: #229
Update the hack/build-image script to create new indexes/manifests that can represent >1 image with different architecture. In order to keep things simple and the changes to the script and CI yaml file smaller, I've extended the "FQIN" tagging to include a new 'any' arch value that is only used for indexes. Thus an index that covers both amd64 and arm64 builds might look like:
quay.io/samba.org/samba-server:default-fedora-any.These indexes are constructed with a new
--indexoption. The code for the--pushcode has been updated to support indexes with the new "QMatcher" flag values ofindexorindex-multiarchthe former pushes all indexes possible and the latter only pushes indexes if it contains images of >1 architecture.The updates to the script are followed by updates to the CI YAML file to enable building and pushing the manifests. This also includes a switch to always use podman for building (for consistency) and to use native arm64 builders rather than emulation.
I'm using the
wipbranch at https://github.com/phlogistonjohn/samba-container to test these patches. You can see results of pushing towipand the CI runs at https://github.com/phlogistonjohn/samba-container/actionsand the results get pushed to https://quay.io/repository/phlogistonjohn/samba-server?tab=tags and related repos under my user at quay.io.