Add Dockerfile and workflow for building and pushing Docker image to GHCR #25

jeancochrane · 2023-10-24T14:30:29Z

This PR adds a Dockerfile and a new GitHub Actions workflow to perform the following operations:

Run on every pull request, commit to the main branch, or commit to a long-lived yearly assessment branch
Build a Docker image for the model
Publish it to the GitHub Container Registry

Closes #22.

…ghcr

.github/workflows/deploy.yaml

jeancochrane · 2023-10-24T19:09:54Z

.github/workflows/deploy.yaml

+          # Tag the following types of images:
+          #   * On a branch, tag with the branch name (e.g. `master`)
+          #   * On a PR, tag with the PR number (e.g. `pr-12`)
+          #   * On all events, tag with the short git SHA (e.g. `e956384`)
+          tags: |
+            type=ref,event=branch
+            type=ref,event=pr
+            type=sha


Does this seem like the correct tagging scheme to you?

One subtlety here is that images persist indefinitely in GHCR, so by tagging every image with its sha we run the risk of using a very large amount of storage for images that correspond to individual commits. This strikes me as a good thing for the main branches (master and *-assessment-year) since we want to be able to recover old models that were used on those branches, but I bet we could reduce the storage use by not producing SHA-tagged images on pull requests.

That being said, my read of the pricing for GitHub Packages suggests that storage is free and unlimited for public packages, so maybe we just push everything and don't worry too much about it. I'm not sure, curious what you think!

suggestion: Instead of tagging and creating an image for every commit, let's create an image for each git tag. That way, when we tag assessment-year-2024, we'll make a long-lived image with the same name that has the dependencies necessary to run the 2024 model already built/bundled.

Good call! I switched this to type=ref,event=tag in 4699510.

…mmands

.github/workflows/deploy.yaml

jeancochrane · 2023-10-25T14:31:34Z

Dockerfile

+ENV RENV_CONFIG_REPOS_OVERRIDE "https://cloud.r-project.org/"
+ENV RENV_PATHS_LIBRARY renv/library


I copied these variables over from the GitLab version of the workflow, but I'm not 100% confident they're necessary.

RENV_PATHS_LIBRARY is likely necessary. If we want to setup package caching using Actions instead of (or in addition to) the Docker build cache, then we should set the other ones too. RENV_CONFIG_REPOS_OVERRIDE was the product of some weird bugs on GitLab's runners that I now can't recall.

suggestion: We might want to set RENV_CONFIG_REPOS_OVERRIDE to point to the Posit package mirror, since they have pre-built linux binaries. That would speed up our build times a lot.

Good thinking RE: PPM! I refactored to use it in 35b15ed and reduced the cache miss build time to 8min (see this workflow).

I think I'll leave off the cache variables until we decide to implement package manager caching.

jeancochrane · 2023-10-25T14:40:41Z

Dockerfile

+RUN apt-get update && apt-get install --no-install-recommends -y \
+    libcurl4-openssl-dev libssl-dev libxml2-dev libgit2-dev git \
+    libudunits2-dev python3-dev python3-pip


These system dependencies were also copied over from GitLab.

suggestion: Let's add the following as well, as they'll be needed down the road:

libgdal-dev libgeos-dev libproj-dev libfontconfig1-dev libharfbuzz-dev libfribidi-dev pandoc

Cool, I added these in c41c3a7 👍🏻

dfsnow · 2023-10-25T15:03:16Z

Dockerfile

+# Install renv for R dependencies
+RUN Rscript -e "install.packages('renv')"


nitpick: This is probably redundant since renv will bootstrap itself in the presence of .Rprofile files.

suggestion: I would change L29 to copy renv.lock, .Rprofile and renv/ to get the full bootstrap going.

Ah, that makes sense! Done in 3edd026 and d1e9ff7.

dfsnow · 2023-10-25T15:08:21Z

Dockerfile

+COPY renv.lock .
+
+# Install R dependencies
+RUN Rscript -e 'renv::restore()'
+
+# Copy the directory into the container
+ADD ./ model-res-avm/
+WORKDIR model-res-avm/


issue: You might run into issues here at runtime because renv (and all its libraries) are installed at ., while the working directory is model-res-avm. Thus if you do something like docker exec -it $CONTAINER_NAME R, it wouldn't load renv and the restored libraries.

Good call, I copied over the renv directory to model-res-avm in 3edd026.

dfsnow · 2023-10-25T15:13:04Z

Dockerfile

+# Copy pipenv files into the image. The reason this is a separate step from
+# the later step that adds files from the working directory is because we want
+# to avoid having to reinstall dependencies every time a file in the directory
+# changes, as Docker will bust the cache of every layer following a layer that
+# needs to change
+COPY Pipfile .
+COPY Pipfile.lock .
+
+# Install Python dependencies
+RUN pipenv install --system --deploy


issue (maybe blocking): We might be able to dump a huge proportion of the pip requirements, or even drop them entirely, depending on how things will run on AWS.

Basically, the only things using Python in this repo are sales val (which will shortly be removed) and DVC. If AWS is going to use the container image directly, then we should keep pip/DVC. If AWS is going to use a separate runner and then use the Docker container only to run the R pipeline, then we can remove the pip reqs here and move them to the runner.

Either way, we should only need to install the boto + DVC stuff from here on out.

Awesome, I switched to just installing dvc[s3] in Pipfile in 62c144a and that seems to keep DVC and the S3 stuff 👍🏻

dfsnow · 2023-10-25T15:17:26Z

Dockerfile

+ENV RENV_CONFIG_REPOS_OVERRIDE "https://cloud.r-project.org/"
+ENV RENV_PATHS_LIBRARY renv/library


RENV_PATHS_LIBRARY is likely necessary. If we want to setup package caching using Actions instead of (or in addition to) the Docker build cache, then we should set the other ones too. RENV_CONFIG_REPOS_OVERRIDE was the product of some weird bugs on GitLab's runners that I now can't recall.

suggestion: We might want to set RENV_CONFIG_REPOS_OVERRIDE to point to the Posit package mirror, since they have pre-built linux binaries. That would speed up our build times a lot.

dfsnow · 2023-10-25T15:26:23Z

Dockerfile

+RUN apt-get update && apt-get install --no-install-recommends -y \
+    libcurl4-openssl-dev libssl-dev libxml2-dev libgit2-dev git \
+    libudunits2-dev python3-dev python3-pip


suggestion: Let's add the following as well, as they'll be needed down the road:

libgdal-dev libgeos-dev libproj-dev libfontconfig1-dev libharfbuzz-dev libfribidi-dev pandoc

dfsnow · 2023-10-25T15:30:39Z

.github/workflows/deploy.yaml

+    permissions:
+      contents: read
+      packages: write


question: Are these strictly necessary? I haven't set them up in the other GHA Docker builds and they seem to work fine.

Interesting, I got these permissions from the docs on publishing a package using an action! Perhaps they're outdated? In any case, if app-shiny-rpie pushes successfully without these permissions then I think we don't need them. Removed in 4e3ab96.

dfsnow · 2023-10-25T15:36:57Z

.github/workflows/deploy.yaml

+          # Tag the following types of images:
+          #   * On a branch, tag with the branch name (e.g. `master`)
+          #   * On a PR, tag with the PR number (e.g. `pr-12`)
+          #   * On all events, tag with the short git SHA (e.g. `e956384`)
+          tags: |
+            type=ref,event=branch
+            type=ref,event=pr
+            type=sha


suggestion: Instead of tagging and creating an image for every commit, let's create an image for each git tag. That way, when we tag assessment-year-2024, we'll make a long-lived image with the same name that has the dependencies necessary to run the 2024 model already built/bundled.

.github/workflows/deploy.yaml

…orkflow

…file

dfsnow

Looks good @jeancochrane. Eager to see this in action!

Two quick asks:

It might be worthwhile to clear out the tagged images here once this is merged: https://github.com/ccao-data/model-res-avm/pkgs/container/model-res-avm
In the Packages sidebar, it doesn't correctly display the container type if attestation is enabled. You can turn it off by setting provenance: false like this: https://github.com/ccao-data/app-shiny-rpie/blob/master/.github/workflows/docker-build.yaml#L42C11-L42C11

jeancochrane · 2023-10-25T20:15:21Z

In the Packages sidebar, it doesn't correctly display the container type if attestation is enabled. You can turn it off by setting provenance: false like this: https://github.com/ccao-data/app-shiny-rpie/blob/master/.github/workflows/docker-build.yaml#L42C11-L42C11

Good call @dfsnow, done in edcfeeb!

It might be worthwhile to clear out the tagged images here once this is merged: https://github.com/ccao-data/model-res-avm/pkgs/container/model-res-avm

Hm, I don't seem to have permissions to delete images from our Packages repository -- are you able to either do that or grant me permissions to do so?

jeancochrane linked an issue Oct 24, 2023 that may be closed by this pull request

[Infra updates] Build and push a Docker image for the model #22

Closed

jeancochrane force-pushed the jeancochrane/22-infra-updates-build-and-push-a-docker-image-for-the-model branch 5 times, most recently from 3488fe1 to 60dfd2c Compare October 24, 2023 18:35

Add Dockerfile and workflow for building and pushing Docker image to …

c565cd1

…ghcr

jeancochrane force-pushed the jeancochrane/22-infra-updates-build-and-push-a-docker-image-for-the-model branch from 60dfd2c to c565cd1 Compare October 24, 2023 18:42

jeancochrane commented Oct 24, 2023

View reviewed changes

jeancochrane added 2 commits October 24, 2023 15:54

Cache R and pip packages

b93c3a6

Temporarily simplify Dockerfile to test caching

d74fb4b

jeancochrane force-pushed the jeancochrane/22-infra-updates-build-and-push-a-docker-image-for-the-model branch from fe65257 to fac8fc5 Compare October 24, 2023 21:12

jeancochrane added 2 commits October 24, 2023 16:25

Attempt to fix up cache paths for GitHub workflow

efcf7e6

Temporarily try a simpler R package for testing cache

92c2b1e

jeancochrane force-pushed the jeancochrane/22-infra-updates-build-and-push-a-docker-image-for-the-model branch from be14c44 to 92c2b1e Compare October 24, 2023 21:25

jeancochrane added 2 commits October 24, 2023 16:32

Try using cache dir env vars to pass mount paths to Dockerfile RUN co…

644e916

…mmands

Revert pip/renv caching for now

afa0c8d

jeancochrane commented Oct 24, 2023

View reviewed changes

.github/workflows/deploy.yaml Show resolved Hide resolved

jeancochrane commented Oct 25, 2023

View reviewed changes

jeancochrane marked this pull request as ready for review October 25, 2023 14:41

jeancochrane requested review from dfsnow and wrridgeway as code owners October 25, 2023 14:41

dfsnow reviewed Oct 25, 2023

View reviewed changes

jeancochrane and others added 7 commits October 25, 2023 10:56

Remove explicit permissions from publish-docker-image job in deploy w…

4e3ab96

…orkflow

Change SHA Docker tagging scheme to follow GitHub tags instead

4699510

Switch to PPM for R package installs

35b15ed

Install more system dependencies into Dockerfile

c41c3a7

Fixup renv install in Dockerfile

3edd026

Simplify Pipfile to only keep dvc dependency

62c144a

Try explicitly copying renv path to make sure it gets added in Docker…

d1e9ff7

…file

jeancochrane requested a review from dfsnow October 25, 2023 18:54

jeancochrane mentioned this pull request Oct 25, 2023

Update the deploy.yaml workflow to run the model on CI #26

Merged

dfsnow approved these changes Oct 25, 2023

View reviewed changes

Disable build provenance

edcfeeb

jeancochrane merged commit 5d117e9 into master Oct 25, 2023
2 checks passed

jeancochrane deleted the jeancochrane/22-infra-updates-build-and-push-a-docker-image-for-the-model branch October 25, 2023 20:13

jeancochrane mentioned this pull request Nov 9, 2023

Add Dockerfile and build-and-run-model workflow for CI model runs ccao-data/model-condo-avm#9

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Dockerfile and workflow for building and pushing Docker image to GHCR #25

Add Dockerfile and workflow for building and pushing Docker image to GHCR #25

jeancochrane commented Oct 24, 2023 •

edited

Loading

jeancochrane Oct 24, 2023

dfsnow Oct 25, 2023

jeancochrane Oct 25, 2023

jeancochrane Oct 25, 2023

dfsnow Oct 25, 2023

jeancochrane Oct 25, 2023

jeancochrane Oct 25, 2023

dfsnow Oct 25, 2023

jeancochrane Oct 25, 2023

dfsnow Oct 25, 2023

jeancochrane Oct 25, 2023

dfsnow Oct 25, 2023

jeancochrane Oct 25, 2023

dfsnow Oct 25, 2023

jeancochrane Oct 25, 2023

dfsnow Oct 25, 2023

dfsnow Oct 25, 2023

dfsnow Oct 25, 2023

jeancochrane Oct 25, 2023

dfsnow Oct 25, 2023

dfsnow left a comment •

edited

Loading

jeancochrane commented Oct 25, 2023

		ENV RENV_CONFIG_REPOS_OVERRIDE "https://cloud.r-project.org/"
		ENV RENV_PATHS_LIBRARY renv/library

		# Install renv for R dependencies
		RUN Rscript -e "install.packages('renv')"

Add Dockerfile and workflow for building and pushing Docker image to GHCR #25

Add Dockerfile and workflow for building and pushing Docker image to GHCR #25

Conversation

jeancochrane commented Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfsnow left a comment • edited Loading

Choose a reason for hiding this comment

jeancochrane commented Oct 25, 2023

jeancochrane commented Oct 24, 2023 •

edited

Loading

dfsnow left a comment •

edited

Loading