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

Merge and change the structure of stack files and update build strategy, and some improvements #227

Merged
merged 32 commits into from
Sep 1, 2021

Conversation

eitsupi
Copy link
Member

@eitsupi eitsupi commented Aug 17, 2021

Completely merge the stack files that exist for each version into X.Y.Z.json(version-fixed images) or extra.json(special images that build only the latest version; geospatial dev images).
To shorten the build time, change the structure of the matrix used in GitHub Actions so that images that do not have dependencies within the same version can be set as separate groups and built in parallel for each group.
To do such a build on GitHub Actions, modify the structure of the matrix files to include the group on docker-bake.json.
The new matrix files are automatically generated from docker-bake.json files.

image

image

Some image names (IMAGE in stack files, which is used as Dockerfiles' name etc.) changed in order to avoid duplicate key names in docker-bake.json, the compose files will not be generated normally from the integrated stack file, so update the write-compose.R not to generate compose files from the integrated stack files.
This also affects some Dockerfiles' name.

And, this PR makes some small improvements to stack files or files generated from stack files.

ToDo

  • Set group in each stack files and docker-bake.json files.
  • Add a script generate new matrix files, which contains group.
  • Merge stack files to X.Y.Z.json or extra.json (geospatial dev images) and to be updated by sync-template-vars.R and make-stacks.R.
    Note: Due to the trigger of GitHub Actions, make-stacks.R now also updates devel.json which is a template of X.Y.Z.json (Create releases and build non-latest images automatically #223), so there is no point in separating sync-template-vars.R and make-stacks.R, I integrate sync-template-vars.R functionality into make-stacks.R.
    • cuda11
    • ubuntu18.04
    • geospatial ubuntugis
    • geospatial dev-osgeo
  • Add a new workflow extra.yml to build images defined by extra.json.
  • Add anotations in docker-bake.json
    • org.opencontainers.image.version
    • org.opencontainers.image.title
    • org.opencontainers.image.description
    • org.opencontainers.image.base.name
  • Divide the COPY section and the RUN section in the stack files and Dockerfiles into two parts, and use cache when building images.
  • Set DEFAULT_USER to ENV.
  • Add new builds job which build each group to the Makefile, and use them in GitHub Actions workflow.
  • Workflow tests on my fork.

@eitsupi
Copy link
Member Author

eitsupi commented Aug 17, 2021

As for the build time of ubuntugis, if I remember correctly when I looked at the log before it disappeared, it was about 30 minutes, so I think it won't take more than 3 hours if I add the current build time of less than 2 hours.
Multi-architecture builds of rocker/r-ver will take an hour and a half, but I don't think it will take more than 5 hours total, so we should be able to build it.
(GitHubActions are capped at 6 hours.)

By the way, when the arm64 version of RStudio Server is released, it probably won't even be able to muli-arch build up to rocker/verse unless RSPM provides the arm64 binary...

which contains group from docker-bake.json
and update the script to copy labels to docker-bake.json
@eitsupi
Copy link
Member Author

eitsupi commented Aug 20, 2021

In 32b1548, I have added labels to the devel stack file, which is a description of the container images.
This will be transcribed into docker-bake.json and written to the image in the same way as LABEL in Dockerfiles.
(I think it would be useful to be able to show them in reports in the future.)

I used the explanations on Docker Hub, but I would appreciate it if you could give me instructions if there is a more appropriate text.

@eitsupi eitsupi changed the title [WIP] Merge and change the structure of stack files and update build strategy [WIP] Merge and change the structure of stack files and update build strategy, and some improvements Aug 28, 2021
@eitsupi eitsupi changed the title [WIP] Merge and change the structure of stack files and update build strategy, and some improvements Merge and change the structure of stack files and update build strategy, and some improvements Aug 29, 2021
@eitsupi eitsupi marked this pull request as ready for review August 29, 2021 03:17
@eitsupi
Copy link
Member Author

eitsupi commented Aug 29, 2021

This PR has become very huge as the stack files and almost all the files generated from them have changed.
However, most files (especially Dockerfiles) have only minor changes, so I'd appreciate it if you could check and merge them.

This should be the last huge PR in a series of build system automation (#181), and I think that automation is almost over when it is merged.

@eitsupi eitsupi mentioned this pull request Aug 29, 2021
8 tasks
@eitsupi
Copy link
Member Author

eitsupi commented Aug 29, 2021

I didn't understand what the geospatial-dev-osgeo was for, so I thought all the work was done in its current state, but I noticed that it was mentioned in #214 (comment).

geospatial-dev-osgeo doesn't currently have a version and only the latest image exists. Should I follow this and include it only in the latest stack file (currently 4.1.1.json)?
Or should I embed it in devel.json and transfer a copy to the latest two versions, just like any other?
The concern was the build time, but the current 4.1.X build of the default group takes about 2 hours, so I think there is still room to add geospatial-dev-osgeo.
(Although it is undesirable in terms of dependency, it is possible to add it to the cuda11images group and leave the default group as it is, taking advantage of the fact that the build time of a cuda11images is likely to be longer than the build time of rocker/verse.)

Update: I've reconsidered about geospatial-dev-osgeo. Please see the next comment. Also, the build time for this image is over 2 hours, so integrating it into the default group may be triggering the build time limit.

I also left core-4.0.0-ubuntu18.04.json and geospatial-4.0.0-ubuntu18.04.json, but these can be incorporated into 4.0.0.json as a new group. Is it a good idea to do this?

@eitsupi
Copy link
Member Author

eitsupi commented Aug 30, 2021

It's a late question, but is there any point in continuing to build old version of geospatial-ubuntugis?

Unlike other images, the default CRAN is not date-fixed, so I think it is better to separate geospatial-dev-osgeo and geospatial-ubuntugis from the stack X.Y.Z.json for each version and build only the latest version and build them at the base timing (eg once a month), like rocker/rstudio:latest-daily.

@eitsupi eitsupi changed the title Merge and change the structure of stack files and update build strategy, and some improvements [WIP] Merge and change the structure of stack files and update build strategy, and some improvements Aug 30, 2021
@eitsupi
Copy link
Member Author

eitsupi commented Aug 30, 2021

To recap, I would prefer to make the following modifications.

  • Integrate core-4.0.0-ubuntu18.04.json into 4.0.0.json. Thus, it will be built for each release of R.
  • geospatial-dev-osgeo.json, and geospatial-ubuntugis in X.Y.Z.json together to make extra.json. extra.json is updated by make-stacks.R to keep up with the latest R version.
  • Create a new workflow extra.yml to build images from extra.json and do scheduled builds on the first day of each month. The base image is rocker/verse:X.Y.Z.

Originally, I should have thought this far before starting the work... Do you think these changes make sense?

@cboettig
Copy link
Member

Nice, these proposed changes make sense to me. yeah, sometimes the better way to do things is only obvious after trying some alternatives!

@cboettig
Copy link
Member

Oh, and in reply to the above -- right, I don't think it makes sense to build old versions of the geospatial dev images (ubuntugis and dev-osgeo). Maintaining those may be tricky if those sources change (though I think our config is relatively stable for them), and dev-osgeo in particular is an intensive build. We might drop ubuntugis if dev-osgeo is proving stable, since these two are a bit redundant. Still, many users and CI platforms elsewhere use the PPA-based ubuntugis strategy it's faster (though not being able to use RSPM binaries somewhat defeats that anyway), and so it can be nice to be able to replicate that. On the flip side the value-add of building the latest osgeo sources is much larger, since they are both slow and somewhat tricky to configure properly to talk to the R packages...

@eitsupi eitsupi changed the title [WIP] Merge and change the structure of stack files and update build strategy, and some improvements Merge and change the structure of stack files and update build strategy, and some improvements Aug 31, 2021
@eitsupi
Copy link
Member Author

eitsupi commented Aug 31, 2021

I changed the structure of the stack file as suggested above and also set up automatic updates and build workflow.

After create a new release (when the new R version is released)

image

New workflow extra.yml runs at 0:00 on the first day of every month and build geospatial dev images, independent of core.yml.

image

Please note that the image information defined in extra.json is not uploaded to the wiki at this time. I would like to update the wiki template by another PR at a later date to resolve this.

changed `make-bakejson.R` to create base image name and version labels
@cboettig
Copy link
Member

cboettig commented Sep 1, 2021

@eitsupi This looks good to me. I think we're ready to merge?

@eitsupi
Copy link
Member Author

eitsupi commented Sep 1, 2021

@cboettig Thank you for your review.
Yes, I think it will work fine if you merge it.

Note that merging this PR will trigger the release workflow, but ignore it as the workflow will be canceled as the tag R4.1.1 already exists.

@cboettig cboettig merged commit daf1b2f into rocker-org:master Sep 1, 2021
@eitsupi eitsupi deleted the update-bakefiles branch September 1, 2021 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants