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

ci: Automate generation and update of docs-builder image #24121

Merged
merged 3 commits into from Aug 9, 2023

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Mar 1, 2023

The docs-builder image is used for the CI wofkflow for testing documentation. Each time we change the list of requirements (Sphinx dependencies etc.) or the Dockerfile for that image, we have to generate a new image to use in the CI workflow.

So far these images have been generated and pushed to Docker manually (thanks Joe!). But this is not fun or practical [citation needed]. This commit introduces a new CI workflow to generate and push a new image when relevant, and to update the documentation workflow with the new tag and digest.

For Pull Requests from branches of the cilium/cilium repository, we can add a new commit directly to update the image. For branches from external forks, we require an additional step from the PR author to update the image reference.

Fixes: #24019

Status: Currently a draft, because we're missing scripts and docs regarding the update for PRs with branches on external forks. Also, nothing has been tested yet.

@qmonnet qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Mar 1, 2023
@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from 675926f to 03e6b59 Compare March 1, 2023 16:57
@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from 061bcf1 to 9749855 Compare March 1, 2023 17:18
@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from 9749855 to 7f7a454 Compare March 1, 2023 17:34
@joestringer
Copy link
Member

joestringer commented Mar 2, 2023

Failed to log into dockerhub. I'm not sure whether we're going to be able to get the level of flexibility around bot write access to docker repos with DockerHub. Probably the next steps are then to switch to Quay.io instead:

  • Initialize a docs-builder repo on quay.io/cilium
  • Create new bot credentials for accessing that repo
  • Create a new environment with these credentials
  • Update this PR to change references for docs-builder to the new repo location
  • Update this PR to reference the new environment + credential names

(cc @aanm)

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 3, 2023
@qmonnet qmonnet added pinned These issues are not marked stale by our issue bot. and removed stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Apr 3, 2023
@joestringer
Copy link
Member

I've created https://quay.io/repository/cilium/docs-builder. If you configure the environment for the workflow for docs-builder and use the credentials from QUAY_DOCS_BUILDER_USERNAME and QUAY_DOCS_BUILDER_PASSWORD, it should be possible to push docs-builder images into that repository from here (via the environment, which requires @cilium/docs-structure approval to run the workflow).

@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from 7f7a454 to 7e4eb86 Compare July 28, 2023 00:52
@qmonnet
Copy link
Member Author

qmonnet commented Jul 28, 2023

Incremental diff
diff --git a/.github/workflows/build-images-docs-builder.yaml b/.github/workflows/build-images-docs-builder.yaml
index 01d1a7525dc5..bab24db224af 100644
--- a/.github/workflows/build-images-docs-builder.yaml
+++ b/.github/workflows/build-images-docs-builder.yaml
@@ -23,11 +23,11 @@ concurrency:
 jobs:
   build-and-push:
     timeout-minutes: 45
-    environment: release-base-images # To get access to AUTO_COMMITTER_*
+    environment: docs-builder
     runs-on: ubuntu-20.04
     steps:
-      - name: Checkout master branch to access local actions
-        uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
+      - name: Checkout main branch to access local actions
+        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
         with:
           ref: ${{ github.event.repository.default_branch }}
           persist-credentials: false
@@ -35,11 +35,11 @@ jobs:
       - name: Set environment variables
         uses: ./.github/actions/set-env-variables
 
-      - name: Set up docker buildx
-        uses: docker/setup-buildx-action@f03ac48505955848960e80bbb68046aa35c7b9e7 # v2.4.1
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@4c0219f9ac95b02789c1075625400b2acbff50b1 # v2.9.1
 
       - name: Checkout source code
-        uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
+        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
         with:
           persist-credentials: false
           ref: ${{ github.event.pull_request.head.sha }}
@@ -58,22 +58,23 @@ jobs:
         id: docs-builder-tag-in-repositories
         shell: bash
         run: |
-          if docker buildx imagetools inspect docker://${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }} &>/dev/null; then
+          if docker buildx imagetools inspect quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }} &>/dev/null; then
             echo exists="true" >> $GITHUB_OUTPUT
           else
             echo exists="false" >> $GITHUB_OUTPUT
           fi
 
-      - name: Login to DockerHub
+      - name: Login to quay.io
         if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
-        uses: docker/login-action@f4ef78c080cd8ba55a85445d5b36e214a81df20a # v2.1.0
+        uses: docker/login-action@465a07811f14bebb1938fbed4728c6a1ff8901fc # v2.2.0
         with:
-          username: ${{ secrets.DOCKER_HUB_USERNAME }}
-          password: ${{ secrets.DOCKER_HUB_PASSWORD }}
+          registry: quay.io
+          username: ${{ secrets.QUAY_DOCS_BUILDER_USERNAME }}
+          password: ${{ secrets.QUAY_DOCS_BUILDER_PASSWORD }}
 
       - name: Build docs-builder image
         if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
-        uses: docker/build-push-action@3b5e8027fcad23fda98b2e3ac259d8d67585f671 # v4.0.0
+        uses: docker/build-push-action@2eb1c1961a95fc15694676618e422e8ba1d63825 # v4.1.1
         id: docker_build_docs_builder
         with:
           provenance: false
@@ -81,7 +82,7 @@ jobs:
           file: ./Documentation/Dockerfile
           push: true
           tags: |
-            docker://${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}
+            quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}
 
       - name: Retrieve image digest
         if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
@@ -90,7 +91,7 @@ jobs:
           mkdir -p image-digest/
           echo "## docs-builder" > image-digest/docs-builder.txt
           echo "" >> image-digest/docs-builder.txt
-          echo "\`docker://${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}@${{ steps.docker_build_docs_builder.outputs.digest }}\`" >> image-digest/docs-builder.txt
+          echo "\`quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}@${{ steps.docker_build_docs_builder.outputs.digest }}\`" >> image-digest/docs-builder.txt
           echo "" >> image-digest/docs-builder.txt
 
       - name: Upload artifact digests
@@ -104,22 +105,24 @@ jobs:
       - name: Update docs-builder image reference in CI workflow
         if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
         run: |
-          Documentation/update-docs-builder-image.sh "docker://${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}@${{ steps.docker_build_docs_builder.outputs.digest }}"
+          Documentation/update-docs-builder-image.sh "quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}@${{ steps.docker_build_docs_builder.outputs.digest }}"
           git commit -sam "ci: update docs-builder"
 
       - name: Get token
         if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
         id: get_token
-        uses: cilium/actions-app-token@350a14155dd9be79227f081310f4d77fdf792e76 # v0.21.0
+        uses: cilium/actions-app-token@61a6271ce92ba02f49bf81c755685d59fb25a59a # v0.21.1
         with:
           APP_PEM: ${{ secrets.AUTO_COMMITTER_PEM }}
           APP_ID: ${{ secrets.AUTO_COMMITTER_APP_ID }}
 
       - name: Push changes into PR
+        env:
+          REF: ${{ github.event.pull_request.head.ref }}
         if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
         run: |
           git diff HEAD^
-          git push https://x-access-token:${{ steps.get_token.outputs.app_token }}@github.com/${{ env.QUAY_ORGANIZATION }}/cilium.git HEAD:${{ github.event.pull_request.head.ref }}
+          git push https://x-access-token:${{ steps.get_token.outputs.app_token }}@github.com/${{ env.QUAY_ORGANIZATION }}/cilium.git HEAD:"$REF"
 
   image-digests:
     name: Display digests

@qmonnet
Copy link
Member Author

qmonnet commented Jul 28, 2023

Looks like I've got something wrong

image

@qmonnet
Copy link
Member Author

qmonnet commented Aug 1, 2023

@joestringer Can you take a look, please? I can't find any obvious mistake in my YAML. Maybe I've got something wrong anyway, or maybe it has to do with the setup on either GH or quay.io? Let me know if there's any way I can help.

@joestringer
Copy link
Member

I updated the username and password again, maybe last time I got them around the wrong way or something. After re-triggering the workflow it appears to log in this time 🎉

@joestringer joestringer added the dont-merge/preview-only Only for preview or testing, don't merge it. label Aug 1, 2023
@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from 7e4eb86 to 9e52805 Compare August 2, 2023 11:17
@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from 8993449 to bd41ecf Compare August 8, 2023 12:46
@qmonnet qmonnet temporarily deployed to docs-builder August 8, 2023 12:46 — with GitHub Actions Inactive
@qmonnet qmonnet temporarily deployed to docs-builder August 8, 2023 12:48 — with GitHub Actions Inactive
@auto-committer auto-committer bot temporarily deployed to docs-builder August 8, 2023 12:50 Inactive
@qmonnet
Copy link
Member Author

qmonnet commented Aug 8, 2023

OK so I got it working at last - sorry for the wall of logs on this page.

Changes include:

  • We now have 4 jobs instead of 1. The drawback is that we need to approve the deployment twice, given that two of these jobs, running in a succession, use the docs-builder environment.

  • I also run the script in the Docker image that we have just generated (because it has both bash and git). This Docker image is modified by the PR author, but then so can be the script in the first place, and my understanding is that whatever the image contains, it still cannot access the environment from inside the container.

Incremental diff
diff --git a/.github/workflows/build-images-docs-builder.yaml b/.github/workflows/build-images-docs-builder.yaml
index 191f9d4c9657..b4157a621f0d 100644
--- a/.github/workflows/build-images-docs-builder.yaml
+++ b/.github/workflows/build-images-docs-builder.yaml
@@ -2,12 +2,13 @@ name: Docs-builder Image Build
 
 # Any change in triggers needs to be reflected in the concurrency group.
 on:
-  pull_request_target:
+  pull_request:
     types:
       - opened
       - synchronize
       - reopened
     paths:
+      - .github/workflows/build-images-docs-builder.yaml
       - Documentation/Dockerfile
       - Documentation/requirements.txt
 
@@ -21,9 +22,13 @@ concurrency:
 
 jobs:
   build-and-push:
-    timeout-minutes: 45
-    environment: docs-builder
+    name: Build and push image
     runs-on: ubuntu-20.04
+    timeout-minutes: 30
+    environment: docs-builder
+    outputs:
+      tag: ${{ steps.docs-builder-tag.outputs.tag }}
+      digest: ${{ steps.docker-build-docs-builder.outputs.digest }}
     steps:
       - name: Checkout default branch (trusted)
         uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
@@ -45,11 +50,6 @@ jobs:
           persist-credentials: false
           ref: ${{ github.event.pull_request.head.sha }}
 
-      - name: Set up git
-        run: |
-          git config user.name "Cilium Imagebot"
-          git config user.email "noreply@cilium.io"
-
       - name: Generate image tag for docs-builder
         id: docs-builder-tag
         run: |
@@ -72,11 +72,12 @@ jobs:
           registry: quay.io
           username: ${{ secrets.QUAY_DOCS_BUILDER_USERNAME }}
           password: ${{ secrets.QUAY_DOCS_BUILDER_PASSWORD }}
+          logout: true
 
       - name: Build docs-builder image
         if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
         uses: docker/build-push-action@2eb1c1961a95fc15694676618e422e8ba1d63825 # v4.1.1
-        id: docker_build_docs_builder
+        id: docker-build-docs-builder
         with:
           provenance: false
           context: ./Documentation
@@ -85,32 +86,48 @@ jobs:
           tags: |
             quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}
 
-      - name: Retrieve image digest
-        if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
-        shell: bash
+  # Use a separate job for the steps below, to ensure we're no longer logged
+  # into Quay.io.
+  update-pr:
+    name: Update Pull Request with new image reference
+    needs: build-and-push
+    if: needs.build-and-push.outputs.digest
+    runs-on: ubuntu-20.04
+    timeout-minutes: 10
+    environment: docs-builder
+    steps:
+      - name: Checkout default branch (trusted)
+        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+        with:
+          ref: ${{ github.event.repository.default_branch }}
+          persist-credentials: false
+
+      - name: Set environment variables
+        uses: ./.github/actions/set-env-variables
+
+      # Warning: since this is a privileged workflow, subsequent workflow job
+      # steps must take care not to execute untrusted code.
+      - name: Checkout pull request branch (NOT TRUSTED)
+        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+        with:
+          persist-credentials: false
+          ref: ${{ github.event.pull_request.head.sha }}
+
+      - name: Set up git
         run: |
-          mkdir -p image-digest/
-          echo "## docs-builder" > image-digest/docs-builder.txt
-          echo "" >> image-digest/docs-builder.txt
-          echo "\`quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}@${{ steps.docker_build_docs_builder.outputs.digest }}\`" >> image-digest/docs-builder.txt
-          echo "" >> image-digest/docs-builder.txt
-
-      - name: Upload artifact digests
-        if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
-        uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2
-        with:
-          name: image-digest docs-builder
-          path: image-digest
-          retention-days: 1
+          git config user.name "Cilium Imagebot"
+          git config user.email "noreply@cilium.io"
 
       - name: Update docs-builder image reference in CI workflow
-        if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
         run: |
-          Documentation/update-docs-builder-image.sh "quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ steps.docs-builder-tag.outputs.tag }}@${{ steps.docker_build_docs_builder.outputs.digest }}"
+          NEW_IMAGE="quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ needs.build-and-push.outputs.tag }}@${{ needs.build-and-push.outputs.digest }}"
+          # Run in Docker to prevent the script from accessing the environment.
+          docker run --rm -v $PWD:/cilium -w /cilium "${NEW_IMAGE}" \
+              bash -c "git config --global --add safe.directory /cilium && \
+                       /cilium/Documentation/update-docs-builder-image.sh ${NEW_IMAGE}"
           git commit -sam "ci: update docs-builder"
 
       - name: Get token
-        if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
         id: get_token
         uses: cilium/actions-app-token@61a6271ce92ba02f49bf81c755685d59fb25a59a # v0.21.1
         with:
@@ -120,15 +137,47 @@ jobs:
       - name: Push changes into PR
         env:
           REF: ${{ github.event.pull_request.head.ref }}
-        if: ${{ steps.docs-builder-tag-in-repositories.outputs.exists == 'false' }}
         run: |
           git diff HEAD^
           git push https://x-access-token:${{ steps.get_token.outputs.app_token }}@github.com/${{ env.QUAY_ORGANIZATION }}/cilium.git HEAD:"$REF"
 
+  retrieve-digest:
+    name: Retrieve and upload image digest
+    needs: build-and-push
+    if: needs.build-and-push.outputs.digest
+    runs-on: ubuntu-20.04
+    timeout-minutes: 10
+    steps:
+      - name: Checkout default branch (trusted)
+        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+        with:
+          ref: ${{ github.event.repository.default_branch }}
+          persist-credentials: false
+
+      - name: Set environment variables
+        uses: ./.github/actions/set-env-variables
+
+      - name: Retrieve image digest
+        shell: bash
+        run: |
+          NEW_IMAGE="quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/docs-builder:${{ needs.build-and-push.outputs.tag }}@${{ needs.build-and-push.outputs.digest }}"
+          mkdir -p image-digest/
+          echo "## docs-builder" > image-digest/docs-builder.txt
+          echo "" >> image-digest/docs-builder.txt
+          echo "\`${NEW_IMAGE}\`" >> image-digest/docs-builder.txt
+          echo "" >> image-digest/docs-builder.txt
+
+      - name: Upload artifact digests
+        uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2
+        with:
+          name: image-digest docs-builder
+          path: image-digest
+          retention-days: 1
+
   image-digests:
     name: Display digests
+    needs: retrieve-digest
     runs-on: ubuntu-20.04
-    needs: build-and-push
     steps:
       - name: Create image digest directory
         shell: bash

Given that we've come this far, it would probably make sense to split the docs-builder environment into two distinct ones, the first with the Quay.io login secrets, the other one with the auto-committer credentials, and to pass only the necessary secrets to each job. Cc @aanm

@qmonnet qmonnet requested review from learnitall and aanm August 8, 2023 13:01
Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! One quick question about the last job, but LGTM.

@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from 0941fdb to 10262ba Compare August 8, 2023 16:33
@qmonnet qmonnet temporarily deployed to docs-builder August 8, 2023 16:33 — with GitHub Actions Inactive
@qmonnet qmonnet temporarily deployed to docs-builder August 8, 2023 16:35 — with GitHub Actions Inactive
@auto-committer auto-committer bot temporarily deployed to docs-builder August 8, 2023 16:40 Inactive
@qmonnet qmonnet requested a review from learnitall August 8, 2023 16:54
Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🥳

.github/workflows/build-images-docs-builder.yaml Outdated Show resolved Hide resolved
qmonnet and others added 3 commits August 9, 2023 09:11
We're switching to quay.io to host the docs-builder images. Among other
things, this switch brings the possibility to automatically update the
image from PRs on the Cilium repository, when relevant files are
updated.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
The docs-builder image is used for the CI wofkflow for testing
documentation. Each time we change the list of requirements (Sphinx
dependencies etc.) or the Dockerfile for that image, we have to generate
a new image to use in the CI workflow.

So far these images have been generated and pushed to Docker manually
(thanks Joe!). But this is not fun or practical [citation needed]. This
commit introduces a new CI workflow to generate and push a new image
when relevant, and to update the documentation workflow with the new tag
and digest.

For Pull Requests from branches of the cilium/cilium repository, we can
add a new commit directly to update the image. For branches from
external forks, we require an additional step from the PR author to
update the image reference.

Related: #24019
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Cilium Imagebot <noreply@cilium.io>
@qmonnet qmonnet force-pushed the pr/qmonnet/docs-builder-automation branch from ce90a18 to 40db2f0 Compare August 9, 2023 08:13
@qmonnet
Copy link
Member Author

qmonnet commented Aug 9, 2023

I removed the test commits, to prepare the PR for a merge. The docs-builder generation workflow is not supposed to run this time for this PR, given that we do use pull_request_target for the trigger.
/test

@qmonnet qmonnet removed the dont-merge/preview-only Only for preview or testing, don't merge it. label Aug 9, 2023
@qmonnet qmonnet removed the request for review from brlbil August 9, 2023 08:32
@qmonnet qmonnet merged commit 1739d09 into main Aug 9, 2023
193 checks passed
@qmonnet qmonnet deleted the pr/qmonnet/docs-builder-automation branch August 9, 2023 09:20
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. pinned These issues are not marked stale by our issue bot. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automate creation and publication of docs-builder image for CI documentation workflow
5 participants