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

Let FROM <base_image> in the Dockerfile template be configurable #909

Merged
merged 34 commits into from
Jun 9, 2023

Conversation

yuvipanda
Copy link
Collaborator

@yuvipanda yuvipanda commented Jun 9, 2020

This PR makes the base image configurable via a traitlet that can be passed in
to the commandline when calling repo2docker. It keeps the current default (bionic),
but open to making the default change in the future.

This is the first time we're allowing traitlets (other than .appendix) to directly
influence the contents of the built image, definitely affecting reproducibility. I
am not sure if there is a way around that, unfortunately.

References

@yuvipanda yuvipanda changed the title Set ubuntu 20.04 as new base image [WIP] Set ubuntu 20.04 as new base image Jun 9, 2020
@yuvipanda yuvipanda marked this pull request as draft June 9, 2020 05:56
@betatim
Copy link
Member

betatim commented Jun 9, 2020

Can we use this PR to make not only the tag but the full base image name configurable via traitlet?

This has come up as an idea in #487 (I think) or a related issue about making the base image configurable. I think allowing the person running repo2docker to change this would be a nice feature in general and would allow us to run tests with the old and the new version.

The other bit of homework to do is finding the ticket/docs where we wrote how long we will use this base image/when we will update. So that we can be consistent with what we promised.

@yuvipanda
Copy link
Collaborator Author

I just rebased this. We hard code a bunch of packages - particularly in the R ecosystem - to bionic. I think making the base image configurable would mean we'll have to restrict it to one of two choices (bionic, focal) so the R buildpack knows which versions of things to use. But that seems like a lot of added complexity...

@manics
Copy link
Member

manics commented Nov 29, 2021

Can we install R and rstudio from conda instead of depending on the underlying OS?

@yuvipanda
Copy link
Collaborator Author

@manics unfortunately we can't, as we'll then stop being able to install binary packages from packagemanager.rstudio.com

@yuvipanda
Copy link
Collaborator Author

Although, perhaps we can get it from https://docs.rstudio.com/resources/install-r/

@yuvipanda
Copy link
Collaborator Author

I'm trying out getting R from RStudio tho (#1161) maybe that'll help remove anything distro specific

@yuvipanda
Copy link
Collaborator Author

Now based on #1161, and I'll try make the base image configurable as well once tests pass.

@yuvipanda yuvipanda changed the title [WIP] Set ubuntu 20.04 as new base image Set ubuntu 20.04 as new base image Jun 3, 2022
@yuvipanda
Copy link
Collaborator Author

tests pass, yay!

@yuvipanda yuvipanda force-pushed the feat/new-base branch 2 times, most recently from b5d5342 to d26c1f1 Compare June 25, 2022 00:59
@yuvipanda yuvipanda changed the title Set ubuntu 20.04 as new base image Make the base image configurable Jun 25, 2022
@yuvipanda yuvipanda changed the title Make the base image configurable Make the base image configurable at build time Jun 25, 2022
@mahluk
Copy link

mahluk commented Jul 22, 2022

When do you plan to merge this PR? Thank you @yuvipanda for this as we need to update base image because of public docker registry pull limits

@yuvipanda
Copy link
Collaborator Author

@mahluk slow going :) Hopefully the tests are fixed up now, and we can figure out how to run these tests.

@yuvipanda yuvipanda marked this pull request as ready for review July 25, 2022 19:41
@yuvipanda
Copy link
Collaborator Author

@minrk @betatim @manics @consideRatio would love some review on this one now - I think it's ready to go. This doesn't actually change any behavior, just makes that configurable. We can eventually parameterize the base image tests so they run on multiple ones, but I don't think that's needed in this PR

yuvipanda added a commit to yuvipanda/VICTOR-notebook that referenced this pull request Jan 7, 2023
Tests this repo2docker action PR:
jupyterhub/repo2docker-action#96

to test this repo2docker PR:
jupyterhub/repo2docker#909
@ryanlovett
Copy link
Collaborator

ryanlovett commented May 18, 2023

We investigated using this PR now that Ubuntu 18.04 is past its maintenance period. (although still within the extended security maintenance period). It successfully built the image for a jupyterhub deployment when I specified buildpack-deps:jammy.

I did not investigate how the R environment was affected by repo2docker's hardcoding of https://packagemanager.rstudio.com/all/__linux__/bionic/. We wouldn't use it in production for images that rely on R when the R package manager is not matched to the distro. (Yuvi fixed this in a separate PR.)

Since repo2docker is locked into Ubuntu Bionic right now for non-Dockerfile builds, would it be more palatable to permit people to change just the Ubuntu version, and let that update the RStudio package manager URL?

Configuration via runtime.txt would make this more explicit, e.g. ubuntu-<codename> to mimic python-x.y or r-<RVERSION>.... "runtime" makes me think it is not "build time", although this file does determine how the image is built. Using runtime.txt would mean that you wouldn't have to invent a new file.

@minrk
Copy link
Member

minrk commented May 22, 2023

I think outstanding issues are addressed via the docs. To me, making it configurable from the CLI lets repo2docker be configured and there are no changes for the reproducibility of repos via services like binder, where the base image doesn't enter into it.

I think we do also need to bump the default distro used in a separate PR, because 18.04 is super old now.

@yuvipanda
Copy link
Collaborator Author

Yay, would love to get this merged before it hits the 3 year anniversary of June 9 :D

@yuvipanda
Copy link
Collaborator Author

@minrk aaah, the R test might still be failing because MRAN doesn't work on newer ubuntu?

@cboettig
Copy link

cboettig commented May 22, 2023

@yuvipanda MRAN has is slated for imminent retirement July 1, and may not still be doing any snapshorts?

I recommend switching over to Posit package manager, which has snapshots (and binary builds! though it goes back only to Oct 2017; MRAN dated back to 2014).

@manics manics closed this May 31, 2023
@manics manics reopened this May 31, 2023
@shaneknapp
Copy link

Yay, would love to get this merged before it hits the 3 year anniversary of June 9 :D

we got one more day lol! XD

@minrk
Copy link
Member

minrk commented Jun 9, 2023

Everything passes but the MRAN tests, fixed by #1285

Merging on 3 years to the day!

@minrk minrk merged commit e8eab15 into jupyterhub:main Jun 9, 2023
17 of 18 checks passed
@yuvipanda
Copy link
Collaborator Author

\o/ thank you, @minrk!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use with Podman when short-name resolution is enforced Make it possible to configure the base image