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

Switch from docker-compose build to docker buildx bake #220

Merged
merged 5 commits into from
Aug 16, 2021

Conversation

eitsupi
Copy link
Member

@eitsupi eitsupi commented Aug 13, 2021

Related to rocker-org/rocker-versioned#211 rocker-org/rocker#349 rocker-org/rocker#398

Completely rewrite the Makefile to make it easier to build container images with docker buildx bake, and we will also use the Makefile for building on GitHub Actions.
The jq command is required and must be set to "experimental": "enabled" in ~/.docker/config.json to use buildx.

The build with the buildx bake command was once considered in #130, but the method using the existing docker-compose.yml considered in #130 does not do the right build.
This is because the current buildx bake command (v0.6) builds all targets in parallel without considering the dependencies between the targets. (docker/buildx#141, docker/buildx#447)
For example, since rocker/r-ver and rocker/rstudio start building at the same time, new rocker/rstudio will be based on the already published rocker/r-ver instead of the new rocker/r-ver.
Therefore, in this PR, the targets of docker-bake.json are taken out in order from the top by the jq command and build and push one by one.

This method does not take advantage of parallel builds, but it has the following improvements over the current build method.

  1. Remove the R version name that was hard-coded in the Makefile. This will make it unnecessary to edit the Makefile when a new version is released. (It is more advantageous to specify docker-bake.json as a matrix of GitHub Actions for parallelization, so I do not define a job that builds against all definition files in the directory as before.)
  2. Commit hash of the repository will be written as org.opencontainers.image.revision in the images. This will also show up in the reports published on the wiki, making it easier to compare the contents of the images' source.
    image
  3. Tags such as rocker/r-ver:latest, rocker/r-ver:4, rocker/r-ver:4.1 are automatically set in the stack files by make-stacks.R (Update for automated multi-tagging and reducing the number of stack files #214) and can be pushed without any additional steps.

We can also do multi-architecture builds in the future.
As mentioned in the command example in the Makefile, the following command will show the definitions for building rocker/r-ver:4.1.1 on amd64 and arm64 multi-arch build.
(After setting up the builder instance and disabling the --print option, will cause the build to take place, but please note that it will take several hours.)

$ BAKE_JSON=bakefiles/4.1.1.docker-bake.json BAKE_OPTION=--print\ -f\ build/platforms.docker-bake.override.json make bake-json/r-ver
docker buildx bake -f bakefiles/4.1.1.docker-bake.json --set=*.labels.org.opencontainers.image.revision=cc6d5bc1888b0d66b5dfa45eb73f759ab8b01fe9 --print -f build/platforms.docker-bake.override.json r-ver
[+] Building 0.0s (0/0)                                                                                             
{
   "target": {
      "r-ver": {
         "context": "./",
         "dockerfile": "dockerfiles/Dockerfile_r-ver_4.1.1",
         "labels": {
            "org.opencontainers.image.revision": "cc6d5bc1888b0d66b5dfa45eb73f759ab8b01fe9"
         },
         "tags": [
            "docker.io/rocker/r-ver:4.1.1",
            "docker.io/rocker/r-ver:4.1",
            "docker.io/rocker/r-ver:4",
            "docker.io/rocker/r-ver:latest"
         ],
         "platforms": [
            "linux/amd64",
            "linux/arm64"
         ]
      }
   }
}

If you want to check that the build actually takes place, you can build core-latest-daily as shown below for a quick check.
If we inspect the built image, we will see that the commit hash has been recorded.

$ BAKE_JSON=bakefiles/core-latest-daily.docker-bake.json BAKE_OPTION=--load make bake-json-all

@cboettig @noamross How about it? I'm new to Makefiles, so I'd appreciate it if you could point out the bad writing.

ToDo

  • GitHub Actions workflow definition file core.yml to use make
    • Update core.yml.
    • Test on my fork.

and add a docker-bake.override.json
@eitsupi eitsupi marked this pull request as ready for review August 14, 2021 08:25
@eitsupi
Copy link
Member Author

eitsupi commented Aug 14, 2021

The new workflow seems work well on my fork!
(Note that I am not able to push to DockerHub, so I have not been able to check if it works correctly when the --push option is enabled.)
image
image

@eitsupi
Copy link
Member Author

eitsupi commented Aug 14, 2021

This change also removes the hard-coding of image names in core.yml, so the matrix also allows we to build in parallel 4.1.1.docker-bake.json and ml-cuda11.1-4.1.1.docker-bake.json for example.
Since there are no dependencies between the images built from these two, I think it's better to build separate files in parallel with GitHub Actions rather than merging them into one bake file.

Or, we can merge them into a single file and define the target with dependencies in groups in a separate file docker-bake.override.json, and use the target name read by jq in docker-bake.override.json instead of docker-bake.json.
This method is likely to be more versatile. (For example, extracting the r-ver, rstudio, tidyverse, and verse that we want to build every day in devel.docker-bake.json)

@eitsupi
Copy link
Member Author

eitsupi commented Aug 16, 2021

I've thought about it a lot, and since the number of images (cuda11.1 is only available in 4.0.3 and later) and the architecture supported by the images (rstudio is expected to support arm64 in the near future, but older images will only support amd64) changes from version to version.
So I think It would be better to set group directly in the stack files and in X.Y.Z.docker-bake.json, not in other docker-bake.override.json and select them with a new matrix, like that.

{
  "r_version": [
    "4.0.2",
    "4.0.3",
    "4.0.4"
  ],
  "group": [
    "default"
  ],
  "include": [
    {
      "r_version": "4.0.3",
      "group": "cuda11"
    },
    {
      "r_version": "4.0.4",
      "group": "cuda11"
    }
  ]
}

I will work on that after this PR is merged and I would like to integrate cuda11.1 into X.Y.Z.json as well.

@cboettig cboettig merged commit 2ca3b22 into rocker-org:master Aug 16, 2021
@eitsupi eitsupi deleted the buildx-bake branch August 17, 2021 05:07
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.

2 participants