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

ENH Add AlmaLinux 8 based image #235

Merged
merged 6 commits into from
Jun 15, 2023
Merged

Conversation

jakirkham
Copy link
Member

  • Adds an AlmaLinux based image for use in conda-forge builds
  • Starts by using version 8 (though makes it easy to update this version later)
  • Does a multiarch build (should simplify handling different supported architectures)

@beckermr
Copy link
Member

We definitely want "8" or maybe conda_2_28 somewhere in the image name.

@jakirkham
Copy link
Member Author

Currently that is handled with the DOCKERTAG, which is set to 8 (as opposed to latest). This follows a similar strategy that has been used with the CUDA images in the past

@jakirkham jakirkham force-pushed the add_alma8 branch 2 times, most recently from 6c7242d to f2b8aa0 Compare May 31, 2023 21:47
Co-authored-by: jakirkham <jakirkham@gmail.com>
Co-authored-by: jakirkham <jakirkham@gmail.com>
@beckermr beckermr changed the title [WIP] Add AlmaLinux 8 based image ENH Add AlmaLinux 8 based image Jun 12, 2023
@beckermr
Copy link
Member

@jakirkham Shall we merge?

@beckermr beckermr marked this pull request as ready for review June 12, 2023 12:56
@jakirkham
Copy link
Member Author

SGTM. Thanks for helping get this across the finish line Matt! 🙏

@@ -0,0 +1,93 @@
ARG DISTRO_ARCH
Copy link
Member

@isuruf isuruf Jun 12, 2023

Choose a reason for hiding this comment

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

Why is a new Dockerfile needed?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Because they are different OSes and versions, which resulted in subtle differences

Also when we run into issues specific to this distro & version, it will be helpful to have them siloed

Most of the things that can be reused already are through refactored scripts that all Dockerfiles use

Think it would be better to keep this distinct

Copy link
Member

Choose a reason for hiding this comment

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

What are these subtle differences? It would good to have a diff to see what changed for easier review.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sorry, but I don't plan to pick up this task

From the conda-forge meeting earlier it sound like others don't care or (in some cases don't want) this change

Would suggest converting this to an issue if we want to discuss further

Copy link
Member

Choose a reason for hiding this comment

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

Here is the diff

0a1
> ARG DISTRO_ARCH
3c4
< FROM ${DISTRO_NAME}:${DISTRO_VER}
---
> FROM --platform=linux/${DISTRO_ARCH} ${DISTRO_NAME}:${DISTRO_VER}
6a8
> ARG DISTRO_ARCH
9c11,12
< ENV DISTRO_NAME=${DISTRO_NAME} \
---
> ENV DISTRO_ARCH=${DISTRO_ARCH} \
>     DISTRO_NAME=${DISTRO_NAME} \
25c28
< RUN rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-${DISTRO_VER}
---
> RUN rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-AlmaLinux
31a35,36
>         glibc-minimal-langpack \
>         glibc-langpack-en \
41,61d45
< # Download and cache new compiler packages.
< # Should speedup installation of them on CIs.
< RUN source /opt/conda/etc/profile.d/conda.sh && \
<     conda activate && \
<     conda create -n test --yes --quiet --download-only \
<         conda-forge::binutils_impl_linux-64 \
<         conda-forge::binutils_linux-64 \
<         conda-forge::gcc_impl_linux-64 \
<         conda-forge::gcc_linux-64 \
<         conda-forge::gfortran_impl_linux-64 \
<         conda-forge::gfortran_linux-64 \
<         conda-forge::gxx_impl_linux-64 \
<         conda-forge::gxx_linux-64 \
<         conda-forge::libgcc-ng \
<         conda-forge::libgfortran-ng \
<         conda-forge::libstdcxx-ng && \
<     conda remove --yes --quiet -n test --all && \
<     conda clean -tiy && \
<     chgrp -R lucky /opt/conda && \
<     chmod -R g=u /opt/conda
< 
65c49
< COPY linux-anvil-cos7-x86_64/entrypoint_source /opt/docker/bin/entrypoint_source
---
> COPY linux-anvil-alma/entrypoint_source /opt/docker/bin/entrypoint_source

Copy link
Member

Choose a reason for hiding this comment

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

Entrypoint scripts are identical.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that different linux distros should be siloed FWIW. Future alma versions should likely use the same image under the assumption that they won't break or change too many things.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM. Merge? Or do we need any more changes here?

Copy link
Member

Choose a reason for hiding this comment

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

Let's merge.

linux-anvil-alma/Dockerfile Outdated Show resolved Hide resolved
@ocefpaf ocefpaf merged commit ca60436 into conda-forge:main Jun 15, 2023
45 of 46 checks passed
@jakirkham jakirkham deleted the add_alma8 branch October 10, 2023 18:42
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

4 participants