Switch to GHA and GHCR for image building and hosting #31

urschrei · 2024-04-02T18:25:49Z

Closes #30

urschrei · 2024-04-03T11:24:41Z

@michaelkirk This isn't going terribly, but I have two questions:

I'm confused about how the local build commands work, given that three of them are multi-stage builds which pull the latest libproj-builder from Docker Hub in order to build. This obviously works locally, but I don't understand how.
What are we going to do about testing, given that we don't have e.g. geo available on the test runners?

urschrei · 2024-04-03T13:43:52Z

The current setup works as follows:

For each Rust version in the matrix

A libproj-builder image is built using the specific libproj and tagged
It's tested (currently a no-op)
It's pushed to ghcr.
Upon success of that job, the three dependent images are built, tagged, tested (also no-op) and pushed to ghcr

We have some images for inspection / testing:

https://github.com/orgs/georust/packages?repo_name=docker-images

We need to figure out what testing looks like for these images, though.

Things like tagging something 'latest' can be done on push of a tag.

See: docker/buildx#59

michaelkirk · 2024-04-03T19:11:33Z

I'm confused about how the local build commands work, given that three of them are multi-stage builds which pull the latest libproj-builder from Docker Hub in order to build. This obviously works locally, but I don't understand how.

It seems like you've since figured this out - but just to be clear: The build process was not pulling the earlier stages from Docker Hub. They were building and tagging them locally. Once built, the images are available from the local registry. Only after successfully running the tests against the built containers would I push anything to Docker Hub.

What are we going to do about testing, given that we don't have e.g. geo available on the test runners?

With the current system, I can run tests against the new containers before pushing them to be reasonably confident that the work. To maintain that kind of testing, my assumption is we'd clone down the necessary repos (georust/geo, georust/proj) as part of this new CI job, and then run the tests similarly to how they're run now before the containers are pushed to ghcr.io

I know this is still WIP, but it'd be helpful to update the README to clarify what the new process for getting up-to-date containers in, e.g., georust/geo CI looks like. And relatedly, is the tagging scheme going to change from what's in the current container build scripts?

.github/workflows/imagebuild.yml

urschrei · 2024-04-03T19:45:19Z

With the current system, I can run tests against the new containers before pushing them to be reasonably confident that the work. To maintain that kind of testing, my assumption is we'd clone down the necessary repos (georust/geo, georust/proj) as part of this new CI job, and then run the tests similarly to how they're run now before the containers are pushed to ghcr.io

Oh that's a good idea. No clue how I'm gonna make that work in the test step, but I'll see how I fare.

know this is still WIP, but it'd be helpful to update the README to clarify what the new process for getting up-to-date containers in, e.g., georust/geo CI looks like. And relatedly, is the tagging scheme going to change from what's in the current container build scripts?

Yeah, I just haven't gotten around to it yet – no point in doing it til we know that this works the way we want.

Adding new Rust versions involves updating two matrices in the workflow with the version you want, and updating the libproj version involves updating the env var. If the containers build and upload, the action passes and they're immediately available to us using the same naming scheme we've been using, e.g. ghcr.io/georust/libproj-builder:proj-9.4.0-rust-1.75. We could pull in Rust and libproj versions from text files at action run time, but it'll add some complexity.

urschrei · 2024-04-03T21:23:19Z

Tests are passing in the built images, and README is updated.

Two things:

Are we comfortable pushing new versions when tests pass, or would we prefer a "manual" publishing step, e.g. when we push a new git tag? Successfully built images can be uploaded as Github artifacts, and the "publish" step can download them and push them to the registry. It's more work to configure, but should work fine.
I simplified the tests in proj-ci-without-system-proj because it's going to be pain to run three times to simulate bundled / non-bundled libproj. We currently just test the bundled-proj feature, which is the point of the image anyway. Tests for the other two images have been reproduced from the Makefile.

urschrei · 2024-04-03T21:25:37Z

(I haven't added the Rust versions 1.70 - 1.75 yet because it will quintuple the test-build cycle)

michaelkirk · 2024-04-03T23:07:24Z

It seems like all the container builds are running the same set of tests. e.g. the geo-ci container is running the proj tests, not the georust/geo tests.

Probably you can fix this in the workflow file. Another approach would be, rather than having 3 separate "set up RUN_CMD" steps, to instead have a test_$CONTAINER_NAME.sh script in the repository.

Either would be fine.

Also, it seems like we're not longer running the _PROJ_SYS_TEST_EXPECT_BUILD_FROM_SRC tests from the deleted Makefile. Is that intentional?

.github/workflows/imagebuild.yml

urschrei · 2024-04-04T14:35:48Z

I coupled the tests to the image name by using a map in the build matrix. Seems to work OK – I couldn't make the external test file work via -v for reasons that are totally unclear (worked fine when I ran the commands locally, variable substitutions were all correct, but the mounted file wouldn't appear in the running image. 🤷)

Regarding the _PROJ_SYS_TEST_EXPECT_BUILD_FROM_SRC tests, it's going to be difficult to run all three variations since each requires a separate docker run invocation. I suppose I could add extra test jobs conditional on the image name, but then extra keys will have to be added to the maps just for that image too. At the moment, I'm just running the

_PROJ_SYS_TEST_EXPECT_BUILD_FROM_SRC=1 + --features=bundled_proj test. How crucial are the other two variants?

michaelkirk · 2024-04-04T18:09:18Z

README.md

@@ -30,59 +30,30 @@ number tag as well, e.g. `rust:1.45.1`. Similarly we could publish a new tag
 for each minor patch (`geo-ci:rust-1.50.0`, `geo-ci:rust-1.50.1`, etc.), but
 running CI against each patch seems like overkill at this point.

-## How to Update Rust
+**New images are built by Github Actions and published to the Github Container Registry if tests pass**


The containers are published regardless of if the PR is merged, right? Ideally it'd be nice to not publish until merged into main but it seems non-trivial - building the three downstream containers across different jobs is nice for parallelism, but it means we can't rely on the local registry.

I can live with things as you have them now - but if you can think of an easy way to hold off on publishing until a merge into main, that'd be cool. Otherwise we can leave it for future work.

GHA now has the ability to trigger a job if a PR is merged.
In order to take advantage of that, we'd have to add a few new moving parts:

Upload built libproj-builder x rust versions (for now) images as artifacts

Upload each sub-image x rust version artifact

On successful merge, download all the above again, then push to the registry.

It can be done, but it'll be easier if we can implement your no-Rust-for-main-image idea first. It will also mean duplicating the rust version and image name matrices because matrices can't be shared between jobs. That strikes me as error-prone, so we might have to explore building matrices from JSON files at job run time.

My first though was to keep the publish step in the existing workflow, but gate it on a if branch == main check.

I'm happy to follow up with that later though.

Oh, that might be a lot easier…

README.md

michaelkirk · 2024-04-04T18:11:00Z

.github/workflows/imagebuild.yml

+
+    strategy:
+      matrix:
+        rust_version: [1.77]


I wonder if we actually need to specify rust_version for the libprojbuilder - it's not even using rust, right? It's fine to leave it this way for now, since that's how it's always been, but worth considering for future simplification. It'd speed up the build a bit and avoid having to track supported versions in two places.

Oooh. That's a good point.

The libproj-builder Docker image does expect a Rust version:

ARG RUST_VERSION

Unless I'm misunderstanding something about how the multi-stage builds work?

.github/workflows/imagebuild.yml

dockerfiles/proj-ci-without-system-proj

.github/workflows/imagebuild.yml

michaelkirk · 2024-04-04T19:25:02Z

indeed it does - but I don’t think it’s used for anything. We’d also have to update the dependent stages to reference the new tag for libprojbuilder who can wouldn’t include the rust portion of the tag id

urschrei · 2024-04-04T19:44:03Z

indeed it does - but I don’t think it’s used for anything. We’d also have to update the dependent stages to reference the new tag for libprojbuilder who can wouldn’t include the rust portion of the tag id On Apr 4, 2024, at 11:51, Stephan Hügel @.***> wrote: @urschrei commented on this pull request. In .github/workflows/imagebuild.yml:

trying

pull_request: + merge_group: + +env: + LIBPROJ_VERSION: 9.4.0 + MAIN_IMAGE_NAME: libproj-builder + +jobs: + build_main_image: + name: Build the main image for geo and proj + runs-on: ubuntu-latest + + strategy: + matrix: + rust_version: [1.77] The libproj-builder Docker image does expect a Rust version: ARG RUST_VERSION Unless I'm misunderstanding something about how the multi-stage builds work? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

Happy to leave this as a follow-up action.

urschrei · 2024-04-04T20:23:30Z

Images for 1.70 (MSRV), 1.76, and 1.77 are now in the registry.

michaelkirk

just minor notes. Feel free to merge this.

michaelkirk · 2024-04-04T21:24:18Z

dockerfiles/proj-ci

@@ -12,6 +12,7 @@ RUN test -n "$PROJ_VERSION" || (echo "PROJ_VERSION ARG not set" && false)
 RUN apt-get update \
  && DEBIAN_FRONTEND="noninteractive" apt-get install -y --no-install-recommends \
    clang \
+    cmake \


Why this new dependency?

michaelkirk · 2024-04-04T21:24:22Z

dockerfiles/libproj-builder

@@ -14,6 +14,7 @@ RUN test -n "$PROJ_VERSION" || (echo "PROJ_VERSION ARG not set" && false)
 RUN apt-get update \
  && DEBIAN_FRONTEND="noninteractive" apt-get install -y \
    clang \
+    git \


Why this new dependency?

michaelkirk · 2024-04-04T21:24:27Z

dockerfiles/geo-ci

@@ -21,6 +21,7 @@ RUN apt-get update \
  && DEBIAN_FRONTEND="noninteractive" apt-get install -y --no-install-recommends \
    ca-certificates \
    clang \
+    cmake \


Why this new dependency?

michaelkirk · 2024-04-04T21:25:49Z

README.md

@@ -30,59 +30,30 @@ number tag as well, e.g. `rust:1.45.1`. Similarly we could publish a new tag
 for each minor patch (`geo-ci:rust-1.50.0`, `geo-ci:rust-1.50.1`, etc.), but
 running CI against each patch seems like overkill at this point.

-## How to Update Rust
+**New images are built by Github Actions and published to the Github Container Registry if tests pass**


My first though was to keep the publish step in the existing workflow, but gate it on a if branch == main check.

I'm happy to follow up with that later though.

urschrei force-pushed the switch_to_actions branch from a93ac36 to 8e67a75 Compare April 3, 2024 11:19

Switch to GHA

8d39a3d

urschrei force-pushed the switch_to_actions branch from 8e67a75 to 8d39a3d Compare April 3, 2024 11:20

urschrei added 12 commits April 3, 2024 12:51

Match makefile to renamed Dockerfiles for now

cfcf684

Try to push base image before building contingent images

b5b041a

Typo fix

de5d51d

Jobs are now dependent

f71188f

???

fa3251e

Fix base image name

6eb401d

Try to fix env base image name

7e144ca

More env fix

189a678

Try setting GHCR tag explicitly

f768980

Modify dependent image tag names with ghcr

55374a8

Simplify

e8f9984

Tag dependent images with ghcr so they're uploaded there

0a44736

urschrei added 9 commits April 3, 2024 14:48

Remove Makefile setup

93d23a2

Update Dockerfile URL comments

bd390b0

Add linux/arm64 platform

0813b2a

Define separate test and push steps

199712e

No multi-platform for now

5e8d668

See: docker/buildx#59

Don't mention multi-platform in output

4632b31

Try to fix multi-platform push

661c8de

Fix platforms typo

542dc1a

Disable multi-platform again

29eb3c9

michaelkirk reviewed Apr 3, 2024

View reviewed changes

.github/workflows/imagebuild.yml Show resolved Hide resolved

michaelkirk reviewed Apr 3, 2024

View reviewed changes

.github/workflows/imagebuild.yml Outdated Show resolved Hide resolved

urschrei added 7 commits April 3, 2024 21:01

Fix test command setup

133c2da

Ensure cmake is available in geo-ci

df5bf1c

More specific tests

04c1459

Try not using slim

d6b5f8e

Fix test typos

63efd15

Ensure sqlite3 is present

36da62e

README update

c8bd434

urschrei marked this pull request as ready for review April 3, 2024 21:23

urschrei added 2 commits April 3, 2024 22:30

README typo fix

e60a8ca

Use same sqlite lib in all images

5719f4b

michaelkirk reviewed Apr 3, 2024

View reviewed changes

Use map of image name / test commands for sub-images

df0ff8e

urschrei force-pushed the switch_to_actions branch from fe4e78f to df0ff8e Compare April 4, 2024 13:40

Try to remove QEMU

75a9d69

michaelkirk approved these changes Apr 4, 2024

View reviewed changes

urschrei added 4 commits April 4, 2024 19:26

Remove SQlite3 deps

b47710c

Add proj-sys tests

1378cff

Test proj-sys

cfe241c

Fix typo

cda435b

urschrei added 2 commits April 4, 2024 20:59

Build rustc MSRV + two most recent versions

830de1c

Don't trim trailing 0 in Rust 1.70

f97d761

michaelkirk approved these changes Apr 4, 2024

View reviewed changes

urschrei merged commit 162a091 into main Apr 4, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to GHA and GHCR for image building and hosting #31

Switch to GHA and GHCR for image building and hosting #31

urschrei commented Apr 2, 2024

urschrei commented Apr 3, 2024 •

edited

Loading

urschrei commented Apr 3, 2024 •

edited

Loading

michaelkirk commented Apr 3, 2024

urschrei commented Apr 3, 2024

urschrei commented Apr 3, 2024

urschrei commented Apr 3, 2024

michaelkirk commented Apr 3, 2024 •

edited

Loading

urschrei commented Apr 4, 2024

michaelkirk Apr 4, 2024 •

edited

Loading

urschrei Apr 4, 2024

michaelkirk Apr 4, 2024

urschrei Apr 4, 2024

michaelkirk Apr 4, 2024

urschrei Apr 4, 2024

urschrei Apr 4, 2024

michaelkirk commented Apr 4, 2024 via email •

edited

Loading

urschrei commented Apr 4, 2024

urschrei commented Apr 4, 2024

michaelkirk left a comment

michaelkirk Apr 4, 2024

michaelkirk Apr 4, 2024

michaelkirk Apr 4, 2024

michaelkirk Apr 4, 2024

Switch to GHA and GHCR for image building and hosting #31

Switch to GHA and GHCR for image building and hosting #31

Conversation

urschrei commented Apr 2, 2024

urschrei commented Apr 3, 2024 • edited Loading

urschrei commented Apr 3, 2024 • edited Loading

michaelkirk commented Apr 3, 2024

urschrei commented Apr 3, 2024

urschrei commented Apr 3, 2024

urschrei commented Apr 3, 2024

michaelkirk commented Apr 3, 2024 • edited Loading

urschrei commented Apr 4, 2024

michaelkirk Apr 4, 2024 • 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

michaelkirk commented Apr 4, 2024 via email • edited Loading

urschrei commented Apr 4, 2024

urschrei commented Apr 4, 2024

michaelkirk left a 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

urschrei commented Apr 3, 2024 •

edited

Loading

urschrei commented Apr 3, 2024 •

edited

Loading

michaelkirk commented Apr 3, 2024 •

edited

Loading

michaelkirk Apr 4, 2024 •

edited

Loading

michaelkirk commented Apr 4, 2024 via email •

edited

Loading