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

feat: Unify containers #953

Open
wants to merge 103 commits into
base: master
Choose a base branch
from
Open

feat: Unify containers #953

wants to merge 103 commits into from

Conversation

daler
Copy link
Member

@daler daler commented Feb 11, 2024

This addresses bioconda/bioconda-containers#74 (comment) by unifying all container builds within this repo, driven by a single bash script.

  • the dependencies among containers to be more formally declared
  • containers are built the same way with manifests containing multi-arch images
  • lots of comments to make it easier for the community to maintain over the long term.
  • tests use any images built during the CI run, falling back to quay.io if none were built
  • everything is in a single GitHub Actions workflow, build-images.yml
  • an additional build.sh script is provided for local development

xrefs:

@daler daler changed the title feature: Unify containers feat: Unify containers Feb 11, 2024
.github/workflows/build-images.yml Outdated Show resolved Hide resolved
generic_build.bash Show resolved Hide resolved
generic_build.bash Outdated Show resolved Hide resolved
@martin-g
Copy link
Contributor

The current CI error could be fixed by moving ARG BUSYBOX_IMAGE to the top, before the first FROM - martin-g#8
But now the error is the same as yesterday:

...
finished create-env for /usr/local
[3/5] STEP 1/3: FROM localhost/tmp-busybox
[3/5] STEP 2/3: COPY --from=build_bioconda_package /usr/local /usr/local
[3/5] STEP 3/3: RUN set -x &&     /usr/local/env-execute       catfasta2phyml --version     &&     [ ! "${CONDA_PREFIX}" = /usr/local ]     &&     { set -x && . /usr/local/env-activate.sh && set +x ; }     &&     [ "${CONDA_PREFIX}" = /usr/local ]     &&     catfasta2phyml --version
time="2024-02-13T09:42:19Z" level=warning msg="Failed to decode the keys [\"machine\"] from \"/usr/share/containers/containers.conf\"."
+ /usr/local/env-execute catfasta2phyml --version
/lib/ld-linux-aarch64.so.1: No such file or directory

@daler
Copy link
Member Author

daler commented Feb 13, 2024

Yep, I think that's showing that it's actually a good test -- it's installing an existing x86_64 package from bioconda channel into the ARM container.

I think the fix is to build both amd and arm test packages on the runner (using build-env), and then get them to the create-env's Dockerfile.test. Haven't quite worked out how best to do that yet.

Another big step is to get the main bioconda-utils tests to use these new containers when doing all the unit tests. Also haven't gotten there yet...

@martin-g
Copy link
Contributor

At martin-g#8 I worked on another solution - pass --arch=xyz when running the test image.

The latest build fails with:

+ buildah bud --build-arg=base=1fce627f9bd3 --arch=arm64 --pull-never --file=Dockerfile.test
time="2024-02-13T14:53:20Z" level=warning msg="Failed to decode the keys [\"machine\"] from \"/usr/share/containers/containers.conf\"."
time="2024-02-13T14:53:20Z" level=warning msg="Failed to decode the keys [\"machine\"] from \"/usr/share/containers/containers.conf\"."
STEP 1/4: FROM 1fce627f9bd3
error creating build container: pull policy is always but image has been referred to by ID (1fce627f9bd3)

I pass --pull-never but it seems to be ignored...

@daler
Copy link
Member Author

daler commented Feb 19, 2024

The current issue seems to be that we need to get the images built by buildah/podman over into docker storage so they can be used by bioconda-utils to test.

I'm unable to get podman manifest push --all localhost/$IMAGE_NAME docker-daemon:$IMAGE_NAME to work. Documentation is really hard to find, but https://docs.podman.io/en/stable/markdown/podman-manifest-push.1.html is the best I could find.

One possible workaround might be to run a more recent version of podman. But the Ubuntu 22.04 LTS repos don't support that. So this workaround would mean building podman from source, which I think would have to happen on every build which I don't think is appropriate.

Another workaround might be to push the just-built images to GitHub container registry, and then pull from there. That also has the minor benefit of being able to parallelize a little more and split the workflow into jobs. However we'd need to figure out what the storage situation is like, and if we can/should delete images immediately after testing.

And of course the other option is to not test at all, but that seems a shame now that we have just about everything in place to do that.

Any ideas? Pinging @mbargull for podman advice...

Edit: here's the error from podman manifest push

@bgruening
Copy link
Member

Is there a fedora/redhat image that you can use instead of ubuntu? This should maybe have a newer podman version.

@daler
Copy link
Member Author

daler commented Feb 19, 2024

Based on GitHub runners, doesn't look like it.

@daler
Copy link
Member Author

daler commented Feb 19, 2024

For the record, I think I'm going to take the approach of pushing to ghcr.io and then pulling back down when needed for docker. Seems to take something like ~25s total for push and pull per image.

@daler
Copy link
Member Author

daler commented Feb 21, 2024

After some code review (thanks @mbargull) here is the plan moving forward to simplify:

  • use a separate build config file containing conda, conda-build, and mamba versions that are used across all containers. Store this in bioconda-common unless there's a good reason not to. This avoids one kind of inter-image dependencies
  • use a base debian container to create a locale, and copy that out and into the build config for use in other images. This only needs to happen once (but should still run) and avoids another kind of inter-image dependency
  • bioconda-utils does not actually need to be installed in the build-env container. It is currently installed just to get the version of conda/mamba/conda-build that would be installed outside the container. Using a build config with versions will avoid the need to install it into a container.
  • modularize the now-monolithic generic_build.bash:
    • factor out the quay.io tag check into separate script
    • put as much as possible in a prepare.sh script that lives next to Dockerfiles, that is sourced — this mechanism will allow e.g. build args to be populated in an image-specific manner, but the logic lives next to the Dockerfiles, not interleaved with everything else in generic_build.bash.
    • clean up labeling section. This was originally used for debugging but is not actually required by anything (except the required label for ghcr.io permission inheritance)
    • put inspect calls behind debug flag
  • while the above changes will allow independent building of the images, the tests are still dependent (and should be). So the github action will need to set up those dependencies.

@daler
Copy link
Member Author

daler commented Feb 22, 2024

Since the immediate focus is on ARM container support handled in a unified way, and these changes do that, I propose we start using this as-is and then separately as the next stage do the proposed simplification work. @mbargull, thoughts?

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.

None yet

3 participants