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

Using base image with python version 3.9.13 #318

Merged
merged 3 commits into from
Oct 26, 2022
Merged

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Oct 13, 2022

As mentioned #316 (comment),
The new base jupyter/minimal-notebook start to support linux/arm64/v8 which is not for the old base image. This is also the wired reason that the arm64 build pass and what is being downloaded in my arm64 machine is actually amd64.
I propose to remove the support to arm64 and add it back after I also start and test by using the self-hosted CI runner.

@danielhollas
Copy link
Contributor

The new base jupyter/minimal-notebook start to support linux/arm64/v8 which is not for the old base image.

I don't think I am qualified to review this, but in any case I am quite confused by this sentence.

@unkcpz
Copy link
Member Author

unkcpz commented Oct 13, 2022

I don't think I am qualified to review this

Sure you are, this is an open source project, and you have exceeded qualification to review this from my perspective. :)

The new base jupyter/minimal-notebook start to support linux/arm64/v8 which is not for the old base image.

but in any case I am quite confused by this sentence.

My bad. I mean jupyter/minimal-notebook:python-3.9.4 which is used by base image only has amd64 [1].
In this PR I change it to jupyter/minimal-notebook:python-3.9.13 has both linux/amd64 and liunx/arm64/v8 supported.[2]

[1] https://hub.docker.com/r/jupyter/minimal-notebook/tags?page=1&name=3.9.4
[2] https://hub.docker.com/r/jupyter/minimal-notebook/tags?page=1&name=3.9.13

Copy link
Contributor

@danielhollas danielhollas left a comment

Choose a reason for hiding this comment

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

Got it, thanks. Got confused since you're removing the linux/arm64 support while at the same time bumping to the image that supports it. :-)

Sure you are, this is an open source project, and you have exceeded qualification to review this from my perspective. :)
😊

Leaving approve for others to weigh in whether support should be removed here.

@unkcpz
Copy link
Member Author

unkcpz commented Oct 13, 2022

Leaving approve for others to weigh in whether support should be removed here.

I'd say it has to be removed otherwise the build will fail in any case. The original support is a "fake" support, the docker buildx will fall to use the one with wrong architecture and set the image digest to wrong image.

@yakutovicha
Copy link
Member

Got it, thanks. Got confused since you're removing the linux/arm64 support while at the same time bumping to the image that supports it. :-)

I am also a bit confused here. Why does switching to the base image that supports the arm architecture require switching off the arm build?

@csadorf csadorf removed their request for review October 14, 2022 13:00
@unkcpz
Copy link
Member Author

unkcpz commented Oct 14, 2022

I am also a bit confused here. Why does switching to the base image that supports the arm architecture require switching off the arm build?

If the image jupyter/minimal-notebook not support arm64, when building for arm64 the amd64 will be used and all build passes.
When there is the image support arm64, it will use the correct one but our stack doesn't support arm64 yet such as the rabbitmq don't have conda forge for arm64.

Copy link
Contributor

@danielhollas danielhollas left a comment

Choose a reason for hiding this comment

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

When there is the image support arm64, it will use the correct one but our stack doesn't support arm64 yet such as the rabbitmq don't have conda forge for arm64.

So if I understand correctly, arm64 support will be added back in some future PR and besides upgrading the base image (done here) is blocked on other dependencies (RabbitMQ?).

Approving under the assumption that my understanding is correct. :-)

@unkcpz
Copy link
Member Author

unkcpz commented Oct 26, 2022

So if I understand correctly, arm64 support will be added back in some future PR and besides upgrading the base image (done here) is blocked on other dependencies (RabbitMQ?).

Thanks, @danielhollas. Yes, this is what my plan is. But for rabbitmq, I don't think it will have conda forge package supported for arm64 soon, even if it has, it may not support <3.8.15 as we needed for AiiDA. Probably the best way to install rabbitmq is to roll back to using apt with the latest version and change the timeout setting in the configuration.

@unkcpz unkcpz enabled auto-merge (squash) October 26, 2022 08:43
Copy link
Member

@yakutovicha yakutovicha left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @unkcpz!

@unkcpz unkcpz merged commit 25c65ea into main Oct 26, 2022
@unkcpz unkcpz deleted the fix/py-ver-to-3.9.13 branch October 26, 2022 14:39
unkcpz pushed a commit that referenced this pull request Nov 24, 2022
This was an omission in #318. The Python version specified in build.json is used to automatically generate the image tags on Dockerhub. So currently, for example, the image aiidalab/full-stack:python-3.9.4 actually corresponds to python-3.9.13.
The default BASE in `stack/base/Dockerfile` is set in docker-bake.hcl for building the image. 
Replace the `ARG BASE` with using the arg from the environment should be fine and good to have only one place to set the version.
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.

3 participants