Merge #584

584: Use unified Docker image. r=epgts a=epgts Co-authored-by: Eric Gillespie <epg@timescale.com>
timescale · Oct 31, 2022 · 62581ba · 62581ba
2 parents 6791f1b + 73d8730
commit 62581ba
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 73 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -20,7 +20,7 @@ jobs:
     name: Test Postgres
     runs-on: ubuntu-20.04
     container:
-      image: ${{ inputs.container-image || 'timescaledev/rust-pgx:latest' }}
+      image: ${{ inputs.container-image || 'timescaledev/toolkit-builder:debian-11-amd64' }}
     strategy:
       matrix:
         pgversion: [12, 13, 14]
@@ -38,34 +38,25 @@ jobs:
       #  https://doc.rust-lang.org/cargo/reference/environment-variables.html
       CI: 1
       RUST_BACKTRACE: short
-      # TODO We don't seem to run rustup, nor does it seem like we should.
-      RUSTUP_MAX_RETRIES: 10
 
     steps:
     - name: Checkout Repository
       uses: actions/checkout@v2
       with:
         ref: ${{ github.event.pull_request.head.sha }}
 
-    # ~postgres/.pgx/config.toml comes in on the image.
-    # Setting HOME=~postgres (which is /home/postgres) in cargo's environment
-    # would work, but sticking with this approach for now.
+    # Github Actions provides a bind mounted working directory for us, where
+    # the above checkout happens, and where caches are read from and restored
+    # to, and it's all owned by 1001.  Our container image is `USER root` so
+    # we have no problem writing anywhere, but we run some things as user
+    # 'postgres', which used to be user 1000 but is now 1001.  Hoping in the
+    # future to make our container image `USER postgres` and further simplify
+    # this file and the packaging Actions file, but it's non-trivial.
     - name: chown Repository
       run: chown -R postgres .
 
-    # CARGO_HOME is /usr/local/cargo which looks like something we'd need to
-    # stay root to update, but the parts we need to update (.crates.toml,
-    # .crates2.json, .package-cache, git, registry) are already owned by
-    # `postgres` in the image.
-    - name: Cache cargo directories
-      uses: actions/cache@v2
-      with:
-        path: |
-          /usr/local/cargo/registry
-          /usr/local/cargo/git
-        key: ${{ runner.os }}-test-pg${{ matrix.pgversion }}-cargo-${{ hashFiles('Cargo.lock', '.github/workflows/ci.yml') }}
-        restore-keys: ${{ runner.os }}-test-pg${{ matrix.pgversion }}-cargo-
-
+    # TODO After the container image contains a primed target dir, is this still worth it?
+    #   Only possible advantage is this one is per-pg-version but what's the impact?
     - name: Cache cargo target dir
       uses: actions/cache@v2
       with:
@@ -89,13 +80,12 @@ jobs:
     name: Test Crates
     runs-on: ubuntu-20.04
     container:
-      image: ${{ inputs.container-image || 'timescaledev/rust-pgx:latest' }}
+      image: ${{ inputs.container-image || 'timescaledev/toolkit-builder:debian-11-amd64' }}
       env:
         CARGO_INCREMENTAL: 0
         CARGO_NET_RETRY: 10
         CI: 1
         RUST_BACKTRACE: short
-        RUSTUP_MAX_RETRIES: 10
 
     steps:
     - name: Checkout Repository
@@ -106,15 +96,6 @@ jobs:
     - name: chown Repository
       run: chown -R postgres .
 
-    - name: Cache cargo directories
-      uses: actions/cache@v2
-      with:
-        path: |
-          /usr/local/cargo/registry
-          /usr/local/cargo/git
-        key: ${{ runner.os }}-test-crates-cargo-${{ hashFiles('Cargo.lock', '.github/workflows/ci.yml') }}
-        restore-keys: ${{ runner.os }}-test-crates-cargo-
-
     - name: Cache cargo target dir
       uses: actions/cache@v2
       with:
@@ -129,15 +110,14 @@ jobs:
     name: Test Updates
     runs-on: ubuntu-20.04
     container:
-      image: ${{ inputs.container-image || 'timescaledev/rust-pgx:latest' }}
+      image: ${{ inputs.container-image || 'timescaledev/toolkit-builder:debian-11-amd64' }}
       env:
         PGVERSION: 14
 
         CARGO_INCREMENTAL: 0
         CARGO_NET_RETRY: 10
         CI: 1
         RUST_BACKTRACE: short
-        RUSTUP_MAX_RETRIES: 10
 
     steps:
     - name: Checkout Repository
@@ -148,22 +128,14 @@ jobs:
     - name: chown Repository
       run: chown -R postgres .
 
-    - name: Cache cargo directories
-      uses: actions/cache@v2
-      with:
-        path: |
-          /usr/local/cargo/registry
-          /usr/local/cargo/git
-        key: ${{ runner.os }}-test-updates-cargo-${{ hashFiles('Cargo.lock', '.github/workflows/ci.yml') }}
-        restore-keys: ${{ runner.os }}-test-crates-cargo-
-
     - name: Cache cargo target dir
       uses: actions/cache@v2
       with:
         path: target
         key: ${{ runner.os }}-test-updates-target-${{ hashFiles('Cargo.lock', '.github/workflows/ci.yml') }}
         restore-keys: ${{ runner.os }}-test-crates-target-
 
+    # TODO Drop this.
     - name: Cache old versions dir
       uses: actions/cache@v2
       with:

diff --git a/.github/workflows/ci_image_build.yml b/.github/workflows/ci_image_build.yml
@@ -28,10 +28,9 @@ jobs:
     steps:
       - name: Run release-build-scripts job
         # Repeating the default here for 'pull_request'.  Keep in sync with above.
-        # TODO Change default from 'eg/prebuilt-image' to 'main' after merge.
         run: |
           gh workflow run toolkit-image.yml \
                 -R timescale/release-build-scripts \
-                -r ${{ inputs.builder-commit || 'eg/prebuilt-image' }} \
+                -r ${{ inputs.builder-commit || 'main' }} \
                 -f tag-base=${{ inputs.tag-base || 'timescaledev/toolkit-builder-test' }} \
                 -f toolkit-commit=${{ inputs.toolkit-commit || github.event.pull_request.head.sha }}
diff --git a/.github/workflows/clippy_rustfmt.yml b/.github/workflows/clippy_rustfmt.yml
@@ -6,24 +6,25 @@ on:
     - main
     - staging
     - trying
-  schedule:
-    # we build at 8am UTC, 3am Eastern, midnight Pacific
-    - cron:  '0 8 * * 1-4'
   workflow_dispatch:
+    inputs:
+      container-image:
+        description: 'Container image to pull from DockerHub'
+        required: false
 
 jobs:
   clippy:
     name: Clippy/rustfmt Test
     runs-on: ubuntu-20.04
     container:
-      image: timescaledev/rust-pgx:latest
+      # Duplicated from ci.yml
+      image: ${{ inputs.container-image || 'timescaledev/toolkit-builder:debian-11-amd64' }}
       env:
         # TODO: See TODOs on duplicate block in ci.yml
         CARGO_INCREMENTAL: 0
         CARGO_NET_RETRY: 10
         CI: 1
         RUST_BACKTRACE: short
-        RUSTUP_MAX_RETRIES: 10
 
     steps:
     - name: Checkout Repository
@@ -34,15 +35,6 @@ jobs:
     - name: chown Repository
       run: chown -R postgres .
 
-    - name: Cache cargo directories
-      uses: actions/cache@v2
-      with:
-        path: |
-          /usr/local/cargo/registry
-          /usr/local/cargo/git
-        key: ${{ runner.os }}-clippy-cargo-${{ hashFiles('Cargo.lock', '.github/workflows/clippy_rustfmt.yml') }}
-        restore-keys: ${{ runner.os }}-clippy-cargo-
-
     - name: Cache cargo target dir
       uses: actions/cache@v2
       with:
@@ -57,5 +49,4 @@ jobs:
       run: su postgres -c 'sh tools/build clippy 2>&1'
 
     - name: Verify formatting
-      # TODO Stop adding rustfmt component when we fix our image?  Or not:  it's a harmless no-op.
-      run: rustup component add rustfmt && cargo fmt --check
+      run: su postgres -c 'cargo fmt --check 2>&1'
diff --git a/docker/README.md b/docker/README.md
@@ -14,16 +14,23 @@ apt-get install docker.io
 
 ## Building multi-platform images
 
-To build a new Docker image `rust-pgx` for multiple platforms and push
+To build a new Docker image `toolkit-builder` for multiple platforms and push
 it to the development repository:
 
 ```bash
 ARCH=amd64
 OS_NAME=debian
 OS_VERSION=11
 OS_CODE_NAME=bullseye
-DOCKER_BUILDKIT=1 docker build --platform $ARCH --build-arg ARCH=$ARCH --build-arg OS_NAME=$OS_NAME --build-arg OS_VERSION=$OS_VERSION --build-arg OS_CODE_NAME=$OS_CODE_NAME -f docker/ci/Dockerfile -t timescaledev/toolkit-builder-test:$OS_NAME-$OS_VERSION-$ARCH .
-docker build --tag timescaledev/rust-pgx-test:latest --push .
+DOCKER_BUILDKIT=1 docker build \
+                --platform $ARCH \
+                --build-arg ARCH=$ARCH \
+                --build-arg OS_NAME=$OS_NAME \
+                --build-arg OS_VERSION=$OS_VERSION \
+                --build-arg OS_CODE_NAME=$OS_CODE_NAME \
+                -f docker/ci/Dockerfile \
+                -t timescaledev/toolkit-builder-test:$OS_NAME-$OS_VERSION-$ARCH \
+                .
 ```
 
 We publish the images as `timescaledev/toolkit-builder` instead of
@@ -34,14 +41,14 @@ We publish the images as `timescaledev/toolkit-builder` instead of
 If you get the following error when pushing:
 
 ```
-$ docker buildx build --platform linux/arm64/v8,linux/amd64 --tag timescaledev/rust-pgx-test:latest --push .
-[+] Building 487.0s (54/54) FINISHED                                                                                                                                                                          
+$ docker buildx build --platform linux/arm64/v8,linux/amd64 --tag timescaledev/toolkit-builder-test:latest --push .
+[+] Building 487.0s (54/54) FINISHED
  => [internal] load .dockerignore                                                                                                                                                                        0.0s
- => => transferring context: 2B                                                                                                                                                                          0.0s 
+ => => transferring context: 2B                                                                                                                                                                          0.0s
     .
     .
     .
-=> [auth] timescaledev/rust-pgx-test:pull,push token for registry-1.docker.io                                                                                                                           0.0s 
+=> [auth] timescaledev/toolkit-builder-test:pull,push token for registry-1.docker.io                                                                                                                           0.0s
 ------
  > exporting to image:
 ------
@@ -53,5 +60,5 @@ Install from https://github.com/docker/buildx and then:
 
 ```bash
 export DOCKER_BUILDKIT=0
-docker buildx build --platform linux/arm64/v8,linux/amd64 --tag timescaledev/rust-pgx-test:latest --push .
+docker buildx build --platform linux/arm64/v8,linux/amd64 --tag timescaledev/toolkit-builder-test:latest --push .
 ```
diff --git a/docker/ci/setup.sh b/docker/ci/setup.sh
@@ -54,11 +54,6 @@ if $privileged; then
         centos | rockylinux)
             case $OS_VERSION in
                 7)
-                    # TODO There's actually more to it than just the pgx bug:  it's unclear how to
-                    # even build because it can't find libclang.so without
-                    #   scl enable llvm-toolset-7 'tools/build -pg14 test-extension'
-                    # We may have to just add a centos7 check to the packaging script, too.
-
                     # Postgresql packages require both
                     # - llvm-toolset-7-clang from centos-release-scl-rh
                     # - llvm5.0-devel from epel-release
@@ -182,7 +177,6 @@ EOF
             #   of bothering with apt-key...
             curl -Ls https://packagecloud.io/timescale/timescaledb/gpgkey | apt-key add -
             mkdir -p /etc/apt/sources.list.d
-            # TODO Don't duplicate os name and version here.  Deduplicate with packaging scripts.
             cat > /etc/apt/sources.list.d/timescaledb.list <<EOF
 deb https://packagecloud.io/timescale/timescaledb/$OS_NAME/ $OS_CODE_NAME main
 EOF