root-project / root-docker

Docker recipes for ROOT
30 stars 33 forks source link

Automate generation of Docker images and testing via GitHub actions #44

Open lobis opened 2 years ago

lobis commented 2 years ago

This should close https://github.com/root-project/root-docker/issues/40 and close https://github.com/root-project/root-docker/issues/36.

After this PR the images should appear as a GitHub package as in https://github.com/lobis/root-docker/pkgs/container/root.

This PR creates GitHub actions to build most of the Dockerfiles. It builds all the "from binaries" images and also Ubuntu from source.

There is one action for each Dockerfile.

The actions take no parameters, they just build the images with the default configuration.

The actions can be triggered manually (individually) or via the aux action called "Trigger Build and Publish All Images", which just sends a trigger for all other actions (excluding "from source", since they take a long time). They are also triggered whenever there is a change to the corresponding directory (e.g. fedora action is triggered whenever something changes in the fedora/* directory).

Running the action will upload the image to the GitHub package repository with the corresponding tag. The tag is generated after building the image, so the root version will always be correct (taken directly from root-config --version. The tagging schema is the following:

$ROOT_VERSION_MAJOR.$ROOT_VERSION_MINOR.$ROOT_VERSION_PATCH-$OS_NAME$OS_VERSION_ID

For example the fedora action would produce: 6.26.02-fedora35. The OS version ID is only applied to Fedora, Ubuntu, and CentOS

The version of root / OS depends exclusively on the default parameters of the Dockerfile. Updating the contents of this file should automatically trigger the action and produce a newer image.

In order to work after merge, we need to add a secret to the repository under the name GHCR_WRITE_TOKEN that contains a token with write permission to packages

Also currently the Gentoo Dockerfile doesn't seem to work. The Ubuntu from source also doesn't seem to work, I don't understand why, but the corresponding acitons are fine.

eguiraud commented 2 years ago

Hey, I haven't forgotten about this, will take a look as soon as I manage -- meanwhile, thank you very much for working on this.

Runaway comments:

lobis commented 2 years ago
  • can we even run the "build ubuntu/centos from source" workflows on github CI? don't they take too long?

Yes you can, in my experience it can take ~ 2-4 hours but I never had any problems with it, even using the GitHub provided runners. Since its not going to be ran that often I don't think this is a problem.

Hey, I haven't forgotten about this, will take a look as soon as I manage -- meanwhile, thank you very much for working on this.

Runaway comments:

  • it looks like this is actually at least 3 different PRs? one to change the formatting of our existing Dockerfiles (why?), one to add Dockerfiles for building ROOT on Ubuntu/CentOS from source, one to add the actual github workflows. It might make sense to split the changes up to move things along more quickly

Yes, I agree.

Regarding the formatting there is no reason for it, for some reason I though it was inconsistent across Dockerfiles but I must have made a mistake, I will revert those changes.

I made a PR adding Ubuntu built from source (https://github.com/root-project/root-docker/pull/45). I had some inquiries about how to make this. I followed the choice to have a separate directory on the root level for the "built from source" images, but this means replicating the packages file and making sure it remains on sync. Other choice would be to have a new Dockerfile called DockerfileFromSource or whatever on the ubuntu directory and make use of the same packages file. I also don't like to not use the Dockerfile name, so I am wondering if there is a better way do to this.

Also on these images, it is possible for the user to choose the C++ standard via an argument, should this appear on the image tag? any ideas on the formatting? I guess something like 6.24.06-ubuntu20.04-cpp17? while also tagging, for example, 6.24.06-ubuntu20.04-cpp14 with 6.24.06-ubuntu20.04 as the default?

Or perhaps tags such as 6.24.06-ubuntu20.04 should only refer to the "from binaries" images, while tags such as 6.24.06-ubuntu20.04-cpp17 should only refer to the "from source" images? otherwise we would need a way to tag the "from source/binaries" images explicitly to tell them apart so I guess this is the choice I prefer.

I also have some comments regarding some of the images. Currently on the readme there are a few tags that I couldn't manage to reproduce using the currently available Dockerfiles. For example, the fedora image currently doesn't have any ARG. I made the fedora version an argument which the user can set, however if I only change the fedora version to, lets say, 34, I don't get the same root version as on the official tag for fedora34 found on the README. Basically I don't know how to reproduce all tags without modifying the Dockerfiles, so I guess they were made with older versions. I am of the opinion that all tags offered should be reproducible by the user or atleast the CI, what do you think?

eguiraud commented 2 years ago

Also on these images, it is possible for the user to choose the C++ standard via an argument, should this appear on the image tag? any ideas on the formatting? I guess something like 6.24.06-ubuntu20.04-cpp17? while also tagging, for example, 6.24.06-ubuntu20.04-cpp14 with 6.24.06-ubuntu20.04 as the default?

Or perhaps tags such as 6.24.06-ubuntu20.04 should only refer to the "from binaries" images, while tags such as 6.24.06-ubuntu20.04-cpp17 should only refer to the "from source" images? otherwise we would need a way to tag the "from source/binaries" images explicitly to tell them apart so I guess this is the choice I prefer.

Given that the ubuntu22.04 image now comes with C++17, I am not sure there is still a strong motivation for having builds that do not use the default C++ standard of the platform's compiler?

I also have some comments regarding some of the images. Currently on the readme there are a few tags that I couldn't manage to reproduce using the currently available Dockerfiles. For example, the fedora image currently doesn't have any ARG. I made the fedora version an argument which the user can set, however if I only change the fedora version to, lets say, 34, I don't get the same root version as on the official tag for fedora34 found on the README. Basically I don't know how to reproduce all tags without modifying the Dockerfiles, so I guess they were made with older versions. I am of the opinion that all tags offered should be reproducible by the user or atleast the CI, what do you think?

I agree that's a nice-to-have, but I am not sure that's possible for rolling releases like Arch, and for images that depend on system packages that get updated to newer and newer ROOT versions like on Fedora. Our primary goal in having GitHub actions though is automatization of image builds, not full reproducibility (which we never had). I can add a mention to the main README that only the *_from_source Dockerfiles lead to fully reproducible images.

lobis commented 2 years ago

Given that the ubuntu22.04 image now comes with C++17, I am not sure there is still a strong motivation for having builds that do not use the default C++ standard of the platform's compiler?

Agree, this is no longer needed.

I agree that's a nice-to-have, but I am not sure that's possible for rolling releases like Arch, and for images that depend on system packages that get updated to newer and newer ROOT versions like on Fedora. Our primary goal in having GitHub actions though is automatization of image builds, not full reproducibility (which we never had). I can add a mention to the main README that only the *_from_source Dockerfiles lead to fully reproducible images.

Makes sense to me.

eguiraud commented 2 years ago

Hi @lobis , thank you very much for your continuous help with this, much appreciated!

As you see GitHub can't figure out how to rebase this branch on master, can you please do it manually and fix the conflicts?

Other than that the PR looks great, I will try to find the time to play with the workflows a little bit and then merge it.

lobis commented 2 years ago

Hi @lobis , thank you very much for your continuous help with this, much appreciated!

As you see GitHub can't figure out how to rebase this branch on master, can you please do it manually and fix the conflicts?

Other than that the PR looks great, I will try to find the time to play with the workflows a little bit and then merge it.

No problem!

I think now it should be good to go 🀞🏻

I am not sure how you can play with the workflows without merging to be honest. Most likely you will need to fork this branch into a new repository, since even merging this branch into a separate branch instead of master won't work as the GitHub packages are shared accross branches I think. (if you don't mind about this then its OK I think)

Remember that you will need to create a github token with write permission for packages (and another one in your fork if you want to test it there!)

eguiraud commented 2 years ago

Yeah totally, I would play with this in my fork! GitHub is still unhappy:

image

lobis commented 2 years ago

Yeah totally, I would play with this in my fork! GitHub is still unhappy:

image

I am not sure if I did it properly but what I did is the following:

git remote -v

origin  git@github.com:lobis/root-docker.git (fetch)
origin  git@github.com:lobis/root-docker.git (push)
upstream        https://github.com/root-project/root-docker.git (fetch)
upstream        https://github.com/root-project/root-docker.git (push)

I had previously done a git merge upstream/master and then I did a git rebase upstream/master and it seems fine, not sure how to fix it. I will give it another try.

eguiraud commented 2 years ago

I see. It's likely the presence of that merge commit (master into feature branch) that confuses GitHub. Worst case scenario I'll fix this manually, do not spend too much time on it :)

lobis commented 2 years ago

I see. It's likely the presence of that merge commit (master into feature branch) that confuses GitHub. Worst case scenario I'll fix this manually, do not spend too much time on it :)

Okay, I think I cannot fix it, hopefully you can do it in much less time πŸ‘πŸ»