Merge pull request #54 from python-discord/ci-improvements

CI Improvements

6 files changed, 219 insertions, 212 deletions

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 3559031..573e3cc 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -1,209 +1,20 @@
 # https://aka.ms/yaml
 
 jobs:
-  - job: test
-    displayName: 'Lint & Test'
+  - job: build_lint_test_push
+    displayName: 'Build, Lint, Test, & Push'
 
     pool:
-      vmImage: 'ubuntu-16.04'
+      vmImage: 'ubuntu-18.04'
 
-    steps:
-      - task: ShellScript@2
-        displayName: 'Check If Images Need to Be Built'
-        name: check
-        inputs:
-          scriptPath: scripts/check_dockerfiles.sh
-          disableAutoCwd: true
-
-      # Without a login the following Docker build tasks won't add image tags.
-      - task: Docker@1
-        displayName: 'Log into Docker Hub'
-        inputs:
-          command: login
-          containerregistrytype: 'Container Registry'
-          dockerRegistryEndpoint: 'DockerHub'
-
-      # The venv image depends on this image. Build it if it can't be pulled
-      # from Docker Hub, which will be the case if the base Dockerfile has had
-      # changes.
-      - script: |
-          docker build \
-            -f docker/base.Dockerfile \
-            -t pythondiscord/snekbox-base:latest \
-            .
-        displayName: 'Build Base Image'
-        condition: and(succeeded(), ne(variables['check.BASE_PULL'], True))
-
-      # The dev image is never pushed and therefore is always built.
-      - script: |
-          docker build \
-            -f docker/venv.Dockerfile \
-            -t pythondiscord/snekbox-venv:dev \
-            --build-arg DEV=1 \
-            .
-        displayName: 'Build Development Image'
-
-      # The linter and all tests run inside this container.
-      - script: |
-          docker run \
-            --tty \
-            --detach \
-            --name snekbox_test \
-            --privileged \
-            --network host \
-            --hostname pdsnk-dev \
-            -e PYTHONDONTWRITEBYTECODE=1 \
-            -e PIPENV_PIPFILE="/snekbox/Pipfile" \
-            -e ENV="${PWD}/scripts/.profile" \
-            --volume "${PWD}":"${PWD}" \
-            --workdir "${PWD}"\
-            --entrypoint /bin/bash \
-            pythondiscord/snekbox-venv:dev
-        displayName: 'Start Container'
-
-      - script: |
-          docker exec snekbox_test /bin/bash -c \
-            'pipenv run lint --format junit-xml --output-file test-lint.xml'
-        displayName: 'Run Linter'
-
-      - task: PublishTestResults@2
-        displayName: 'Publish Lint Results'
-        condition: succeededOrFailed()
-        inputs:
-          testResultsFiles: '**/test-lint.xml'
-          testRunTitle: 'Lint Results'
-
-      # Memory limit tests would fail if this isn't disabled.
-      - script: sudo swapoff -a
-        displayName: 'Disable Swap Memory'
-
-      - script: |
-          docker exec snekbox_test /bin/bash -c \
-            'pipenv run coverage run -m xmlrunner'
-        displayName: 'Run Unit Tests'
-
-      - task: PublishTestResults@2
-        displayName: 'Publish Test Results'
-        condition: succeededOrFailed()
-        inputs:
-          testResultsFiles: '**/TEST-*.xml'
-          testRunTitle: 'Test Results'
-
-      # Run report too because the XML report doesn't output to stdout.
-      - script: |
-          docker exec snekbox_test /bin/bash -c \
-            'pipenv run /bin/bash -c "coverage report && coverage xml"'
-        displayName: 'Generate Coverage Report'
-
-      - task: PublishCodeCoverageResults@1
-        displayName: 'Publish Coverage Results'
-        condition: succeededOrFailed()
-        inputs:
-          codeCoverageTool: Cobertura
-          summaryFileLocation: '**/coverage.xml'
-
-  # When a pull request, only perform this job if images need to be built.
-  # It's always performed for non-PRs because the final image will always need
-  # to be built.
-  - job: build
-    displayName: 'Build'
-    condition: >
-      and(
-        succeeded(),
-        or(
-          ne(variables['Build.Reason'], 'PullRequest'),
-          eq(coalesce(dependencies.test.outputs['check.BASE_CHANGED'], True), True),
-          eq(coalesce(dependencies.test.outputs['check.VENV_CHANGED'], True), True)
-        )
-      )
-    dependsOn: test
-
-    # coalesce() gives variables default values if they are null (i.e. unset).
     variables:
-      BASE_CHANGED: $[ coalesce(dependencies.test.outputs['check.BASE_CHANGED'], True) ]
-      VENV_CHANGED: $[ coalesce(dependencies.test.outputs['check.VENV_CHANGED'], True) ]
-      BASE_PULL: $[ coalesce(dependencies.test.outputs['check.BASE_PULL'], False) ]
+      BASE_CHANGED: 'True'
+      VENV_CHANGED: 'True'
+      BASE_PULL: 'False'
+      VENV_PULL: 'False'
 
     steps:
-      - task: Docker@1
-        displayName: 'Log into Docker Hub'
-        inputs:
-          command: login
-          containerregistrytype: 'Container Registry'
-          dockerRegistryEndpoint: 'DockerHub'
-
-      # Because this is the base image for the venv image, if the venv needs to
-      # be built, this base image must also be present. Build it if it has
-      # changed or can't be pulled from Docker Hub.
-      - script: |
-          docker build \
-            -f docker/base.Dockerfile \
-            -t pythondiscord/snekbox-base:latest \
-            .
-        displayName: 'Build Base Image'
-        condition: >
-          and(
-            succeeded(),
-            ne(variables.BASE_PULL, True),
-            or(
-              eq(variables.BASE_CHANGED, True),
-              eq(variables.VENV_CHANGED, True)
-            )
-          )
-
-      # Also build this image if base has changed - even if this image hasn't.
-      - script: |
-          docker build \
-            -f docker/venv.Dockerfile \
-            -t pythondiscord/snekbox-venv:latest \
-            .
-        displayName: 'Build Virtual Environment Image'
-        condition: >
-          and(
-            succeeded(),
-            or(
-              eq(variables.BASE_CHANGED, True),
-              eq(variables.VENV_CHANGED, True)
-            )
-          )
-
-      # Always build this image unless it's for a pull request.
-      - script: |
-          docker build \
-            -f docker/Dockerfile \
-            -t pythondiscord/snekbox:latest \
-            .
-        displayName: 'Build Final Image'
-        condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
-
-      # Push images only after they've all successfully been built.
-      # These have the same conditions as the build tasks. However, for safety,
-      # a condition for not being a pull request is added.
-      - script: docker push pythondiscord/snekbox-base:latest
-        displayName: 'Push Base Image'
-        condition: >
-          and(
-            succeeded(),
-            ne(variables['Build.Reason'], 'PullRequest'),
-            ne(variables.BASE_PULL, True),
-            or(
-              eq(variables.BASE_CHANGED, True),
-              eq(variables.VENV_CHANGED, True)
-            )
-          )
-
-      - script: docker push pythondiscord/snekbox-venv:latest
-        displayName: 'Push Virtual Environment Image'
-        condition: >
-          and(
-            succeeded(),
-            ne(variables['Build.Reason'], 'PullRequest'),
-            or(
-              eq(variables.BASE_CHANGED, True),
-              eq(variables.VENV_CHANGED, True)
-            )
-          )
-
-      - script: docker push pythondiscord/snekbox:latest
-        displayName: 'Push Final Image'
-        condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
+      - template: ci/build.yml
+      - template: ci/setup.yml
+      - template: ci/lint-test.yml
+      - template: ci/push.yml
diff --git a/ci/build.yml b/ci/build.yml
new file mode 100644
index 0000000..7d51709
--- /dev/null
+++ b/ci/build.yml
@@ -0,0 +1,51 @@
+steps:
+  - task: ShellScript@2
+    displayName: 'Check If Images Need to Be Built'
+    inputs:
+      scriptPath: scripts/check_dockerfiles.sh
+      disableAutoCwd: true
+
+  # Without a login, the following Docker build steps wouldn't add image tags.
+  - task: Docker@1
+    displayName: 'Log into Docker Hub'
+    inputs:
+      command: login
+      containerregistrytype: 'Container Registry'
+      dockerRegistryEndpoint: 'DockerHub'
+
+  # Building the venv depends on this base image. Build the base if it can't
+  # pulled from Docker Hub, which will be the case if the base Dockerfile has
+  # has had changes.
+  - script: |
+      docker build \
+        -f docker/base.Dockerfile \
+        -t pythondiscord/snekbox-base:latest \
+        .
+    displayName: 'Build Base Image'
+    condition: >
+      and(
+        succeeded(),
+        or(
+          eq(variables.BASE_CHANGED, 'True'),
+          and(
+            eq(variables.VENV_CHANGED, 'True'),
+            eq(variables.BASE_PULL, 'False')
+          )
+        )
+      )
+
+  # Build the venv image if it's had changes or it can't be pulled.
+  - script: |
+      docker build \
+        -f docker/venv.Dockerfile \
+        -t pythondiscord/snekbox-venv:latest \
+        .
+    displayName: 'Build Virtual Environment Image'
+    condition: >
+      and(
+        succeeded(),
+        or(
+          eq(variables.VENV_CHANGED, 'True'),
+          eq(variables.VENV_PULL, 'False')
+        )
+      )
diff --git a/ci/lint-test.yml b/ci/lint-test.yml
new file mode 100644
index 0000000..2d70f6e
--- /dev/null
+++ b/ci/lint-test.yml
@@ -0,0 +1,41 @@
+steps:
+  - script: |
+      docker exec snekbox_test /bin/bash -c \
+        'flake8; flake8 --format junit-xml --output-file test-lint.xml'
+    displayName: 'Run Linter'
+
+  - task: PublishTestResults@2
+    displayName: 'Publish Lint Results'
+    condition: succeededOrFailed()
+    inputs:
+      testResultsFiles: '**/test-lint.xml'
+      testRunTitle: 'Lint Results'
+
+  # Memory limit tests would fail if this isn't disabled.
+  - script: sudo swapoff -a
+    displayName: 'Disable Swap Memory'
+
+  - script: |
+      docker exec snekbox_test /bin/bash -c \
+        'coverage run -m xmlrunner'
+    displayName: 'Run Unit Tests'
+
+  - task: PublishTestResults@2
+    displayName: 'Publish Test Results'
+    condition: succeededOrFailed()
+    inputs:
+      testResultsFiles: '**/TEST-*.xml'
+      testRunTitle: 'Test Results'
+
+  # Run report too because the XML report doesn't output to stdout.
+  - script: |
+      docker exec snekbox_test /bin/bash -c \
+        'coverage report && coverage xml'
+    displayName: 'Generate Coverage Report'
+
+  - task: PublishCodeCoverageResults@1
+    displayName: 'Publish Coverage Results'
+    condition: succeededOrFailed()
+    inputs:
+      codeCoverageTool: Cobertura
+      summaryFileLocation: '**/coverage.xml'
diff --git a/ci/push.yml b/ci/push.yml
new file mode 100644
index 0000000..9449df0
--- /dev/null
+++ b/ci/push.yml
@@ -0,0 +1,39 @@
+steps:
+  # Always build this image unless it's for a pull request.
+  - script: |
+      docker build \
+        -f docker/Dockerfile \
+        -t pythondiscord/snekbox:latest \
+        .
+    displayName: 'Build Final Image'
+    condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
+
+  # Push images only after they've all successfully been built.
+  - script: docker push pythondiscord/snekbox-base:latest
+    displayName: 'Push Base Image'
+    condition: >
+      and(
+        succeeded(),
+        ne(variables['Build.Reason'], 'PullRequest'),
+        ne(variables.BASE_PULL, 'True'),
+        or(
+          eq(variables.BASE_CHANGED, 'True'),
+          eq(variables.VENV_CHANGED, 'True')
+        )
+      )
+
+  - script: docker push pythondiscord/snekbox-venv:latest
+    displayName: 'Push Virtual Environment Image'
+    condition: >
+      and(
+        succeeded(),
+        ne(variables['Build.Reason'], 'PullRequest'),
+        or(
+          eq(variables.BASE_CHANGED, 'True'),
+          eq(variables.VENV_CHANGED, 'True')
+        )
+      )
+
+  - script: docker push pythondiscord/snekbox:latest
+    displayName: 'Push Final Image'
+    condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
diff --git a/ci/setup.yml b/ci/setup.yml
new file mode 100644
index 0000000..cf190de
--- /dev/null
+++ b/ci/setup.yml
@@ -0,0 +1,24 @@
+steps:
+  # The linter and all tests run inside this container.
+  # The venv image will be pulled if it doesn't exist locally.
+  - script: |
+      docker run \
+        --tty \
+        --detach \
+        --name snekbox_test \
+        --privileged \
+        --network host \
+        --hostname pdsnk-dev \
+        -e PYTHONDONTWRITEBYTECODE=1 \
+        -e PIPENV_PIPFILE="/snekbox/Pipfile" \
+        -e ENV="${PWD}/scripts/.profile" \
+        --volume "${PWD}":"${PWD}" \
+        --workdir "${PWD}"\
+        --entrypoint /bin/bash \
+        pythondiscord/snekbox-venv:latest
+    displayName: 'Start Container'
+
+  - script: |
+      docker exec snekbox_test /bin/bash -c \
+        'pipenv install --system --deploy --dev'
+    displayName: 'Install Development Dependencies'
diff --git a/scripts/check_dockerfiles.sh b/scripts/check_dockerfiles.sh
index c84c61f..88cb7cc 100755
--- a/scripts/check_dockerfiles.sh
+++ b/scripts/check_dockerfiles.sh
@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
 
 set -euo pipefail
+shopt -s inherit_errexit
 exec 3>&1 # New file descriptor to stdout
 
 BASE_URL="https://dev.azure.com/\
@@ -12,10 +13,18 @@ repositoryType=${BUILD_REPOSITORY_PROVIDER}&\
 repositoryId=${BUILD_REPOSITORY_NAME}&\
 api-version=5.0"
 
-get_build() {
-    set -e # Poor Ubuntu LTS doesn't have Bash 4.4's inherit_errexit
+declare -A build_cache
 
+get_build() {
     local branch="${1:?"get_build: argument 1 'branch' is unset"}"
+
+    # Attempt to use cached value
+    if [[ -v build_cache["${branch}"] ]]; then
+        printf '%s\n' "Retrieving build for ${branch} from cache." >&3
+        printf '%s' "${build_cache[$branch]}"
+        return 0
+    fi
+
     local url="${BASE_URL}&branchName=${branch}"
 
     printf '%s\n' "Retrieving the latest successful build using ${url}" >&3
@@ -29,10 +38,37 @@ get_build() {
     then
         return 1
     else
+        # Cache the response
+        build_cache["${branch}"]="${response}"
         printf '%s' "${response}"
     fi
 }
 
+can_pull() {
+    local image="${1:?"can_pull: argument 1 'image' is unset"}"
+
+    local master_commit
+    if master_commit="$(
+            get_build "refs/heads/master" \
+            | jq -re '.value[0].sourceVersion'
+        )" \
+        && git diff --quiet "${master_commit}" -- "${@:2}"
+    then
+        printf \
+            '%s\n' \
+            "Can pull ${image} image from Docker Hub; no changes since master."
+
+        printf '%s\n' "##vso[task.setvariable variable=${image^^}_PULL]True"
+    else
+        printf \
+            '%s\n' \
+            "Cannot pull ${image} image from Docker Hub due to detected " \
+            "changes; the ${image} image will be built."
+
+        return 1
+    fi
+}
+
 # Get the previous commit
 if [[ "${BUILD_REASON}" = "PullRequest" ]]; then
     if ! prev_commit="$(
@@ -65,22 +101,27 @@ printf '%s\n' "Comparing HEAD (${head}) against ${prev_commit}."
 
 if git diff --quiet "${prev_commit}" -- docker/base.Dockerfile; then
     echo "No changes detected in docker/base.Dockerfile."
-    echo "##vso[task.setvariable variable=BASE_CHANGED;isOutput=true]False"
+    echo "##vso[task.setvariable variable=BASE_CHANGED]False"
 else
     # Always rebuild the venv if the base changes.
+    echo "Changes detected in docker/base.Dockerfile; all images will be built."
     exit 0
 fi
 
 if git diff --quiet "${prev_commit}" -- docker/venv.Dockerfile Pipfile*; then
     echo "No changes detected in docker/venv.Dockerfile or the Pipfiles."
-    echo "##vso[task.setvariable variable=VENV_CHANGED;isOutput=true]False"
-elif master_commit="$(
-        get_build "refs/heads/master" \
-        | jq -re '.value[0].sourceVersion'
-    )" \
-    && git diff --quiet "${master_commit}" -- docker/base.Dockerfile
-then
+    echo "##vso[task.setvariable variable=VENV_CHANGED]False"
+
+    if ! can_pull venv docker/venv.Dockerfile Pipfile*; then
+        # Venv image can't be pulled so it needs to be built.
+        # Therefore, the base image is needed too.
+        can_pull base docker/base.Dockerfile || true
+    fi
+else
+    echo \
+        "Changes detected in docker/venv.Dockerfile or the Pipfiles;" \
+        "the venv image will be built."
+
     # Though base image hasn't changed, it's still needed to build the venv.
-    echo "Can pull base image from Docker Hub; no changes made since master."
-    echo "##vso[task.setvariable variable=BASE_PULL;isOutput=true]True"
+    can_pull base docker/base.Dockerfile || true
 fi