Merge pull request #33 from python-discord/ci

CI Improvements

3 files changed, 316 insertions, 145 deletions

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 98d64bf..424b1a3 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -1,144 +1,228 @@
 # https://aka.ms/yaml
 
 jobs:
-- job: test
-  displayName: 'Lint & Test'
-
-  pool:
-    vmImage: 'Ubuntu-16.04'
-
-  steps:
-  - script: docker build -t pythondiscord/snekbox-base:latest -f docker/base.Dockerfile .
-    displayName: 'Build Base Image'
-
-  - script: |
-      docker build -t pythondiscord/snekbox-venv:dev -f docker/venv.Dockerfile --build-arg DEV=1 .
-    displayName: 'Build Development Image'
-
-  - script: |
-      docker run \
-        -td \
-        --name snekbox_test \
-        --privileged \
-        --network host \
-        -h pdsnk-dev \
-        -e PYTHONDONTWRITEBYTECODE=1 \
-        -e PIPENV_PIPFILE="/snekbox/Pipfile" \
-        -e ENV="${PWD}/scripts/.profile" \
-        -v "${PWD}":"${PWD}" \
-        -w "${PWD}"\
-        --entrypoint /bin/ash \
-        pythondiscord/snekbox-venv:dev
-    displayName: 'Start Container'
-
-  - script: |
-      docker exec snekbox_test /bin/ash -c \
-        'pipenv run lint --format junit-xml --output-file test-lint.xml'
-    displayName: 'Run Linter'
-
-  - task: PublishTestResults@2
-    condition: succeededOrFailed()
-    displayName: 'Publish Lint Results'
-    inputs:
-      testResultsFiles: '**/test-lint.xml'
-      testRunTitle: 'Lint Results'
-
-  - script: sudo swapoff -a
-    displayName: 'Disable swap memory'
-
-  - script: |
-      docker exec snekbox_test /bin/ash -c \
-        'pipenv run coverage run -m xmlrunner'
-    displayName: 'Run Unit Tests'
-
-  - task: PublishTestResults@2
-    condition: succeededOrFailed()
-    displayName: 'Publish Test Results'
-    inputs:
-      testResultsFiles: '**/TEST-*.xml'
-      testRunTitle: 'Test Results'
-
-  - script: |
-      docker exec snekbox_test /bin/ash -c \
-        'pipenv run coverage xml'
-    displayName: 'Generate Coverage Report'
-
-  - task: PublishCodeCoverageResults@1
-    displayName: 'Publish Coverage Results'
-    condition: succeededOrFailed()
-    inputs:
-      codeCoverageTool: Cobertura
-      summaryFileLocation: '**/coverage.xml'
-
-- job: build
-  displayName: 'Build'
-  dependsOn: test
-
-  variables:
-    BASE_CHANGED: true
-    VENV_CHANGED: true
-
-  steps:
-  - task: Docker@1
-    displayName: 'Login: Docker Hub'
-
-    inputs:
-      containerregistrytype: 'Container Registry'
-      dockerRegistryEndpoint: 'DockerHub'
-      command: 'login'
-
-  - script: |
-      REQUEST_URL="https://dev.azure.com/python-discord/${SYSTEM_TEAMPROJECTID}/_apis/build/builds?queryOrder=finishTimeDescending&resultFilter=succeeded&\$top=1&repositoryType=${BUILD_REPOSITORY_PROVIDER}&repositoryId=${BUILD_REPOSITORY_NAME}&branchName=${BUILD_SOURCEBRANCH}&api-version=5.0"
-      echo "Retrieving previous build's commit using $REQUEST_URL"
-      RESPONSE="$(curl -sSL "${REQUEST_URL}")"
-
-      if [[ $BUILD_REASON = "PullRequest" ]]; then
-        PREV_COMMIT="$(echo "${RESPONSE}" | grep -Po '"pr\.sourceSha"\s*:\s*"\K.*?[^\\](?="\s*[,}])')"
-        if [[ -z $PREV_COMMIT ]]; then
-          echo "Could not retrieve the previous build's commit. Falling back to the head of the target branch."
-          PREV_COMMIT="origin/$SYSTEM_PULLREQUEST_TARGETBRANCH"
-        fi
-      else
-        PREV_COMMIT="$(echo "${RESPONSE}" | grep -Po '"sourceVersion"\s*:\s*"\K.*?[^\\](?="\s*[,}])')"
-      fi
-
-      if [[ -n $PREV_COMMIT ]]; then
-        echo "Using $PREV_COMMIT to compare diffs."
-
-        if [[ -z "$(git diff $PREV_COMMIT -- docker/base.Dockerfile)" ]]; then
-          echo "No changes detected in docker/base.Dockerfile. The base image will not be built."
-          echo "##vso[task.setvariable variable=BASE_CHANGED]false"
-        fi
-
-        if [[ -z "$(git diff $PREV_COMMIT -- docker/venv.Dockerfile Pipfile*)" ]]; then
-          echo "No changes detected in docker/venv.Dockerfile or the Pipfiles. The venv image will not be built."
-          echo "##vso[task.setvariable variable=VENV_CHANGED]false"
-        fi
-      else
-        echo "No previous commit was retrieved. Either the previous build is too old and was deleted or the branch was empty before this build. All images will be built."
-      fi
-    displayName: 'Check Changed Files'
-
-  - script: docker build -t pythondiscord/snekbox-base:latest -f docker/base.Dockerfile .
-    displayName: 'Build Base Image'
-    condition: and(succeeded(), eq(variables.BASE_CHANGED, 'true'))
-
-  - script: docker build -t pythondiscord/snekbox-venv:latest -f docker/venv.Dockerfile .
-    displayName: 'Build Virtual Environment Image'
-    condition: and(succeeded(), or(eq(variables.BASE_CHANGED, 'true'), eq(variables.VENV_CHANGED, 'true')))
-
-  - script: docker build -t pythondiscord/snekbox:latest -f docker/Dockerfile .
-    displayName: 'Build Final Image'
-    condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
-
-  - script: docker push pythondiscord/snekbox-base:latest
-    displayName: 'Push Base Image to Dockerhub'
-    condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'), eq(variables.BASE_CHANGED, 'true'))
-
-  - script: docker push pythondiscord/snekbox-venv:latest
-    displayName: 'Push Virtual Environment Image to Dockerhub'
-    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 to Dockerhub'
-    condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
+  - job: test
+    displayName: 'Lint & Test'
+
+    pool:
+      vmImage: 'ubuntu-16.04'
+
+    steps:
+      - task: ShellScript@2
+        displayName: 'Check If Images Need to Be Built'
+        name: check
+        inputs:
+          scriptPath: scripts/check_dockerfiles.sh
+
+      # Without a login the following Docker build tasks won't add image tags.
+      - task: Docker@2
+        displayName: 'Log into Docker Hub'
+        inputs:
+          command: login
+          containerRegistry: DockerHubV2
+
+      # 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.
+      - task: Docker@2
+        displayName: 'Build Base Image'
+        condition: and(succeeded(), ne(variables['check.BASE_PULL'], True))
+        inputs:
+          command: build
+          repository: pythondiscord/snekbox-base
+          tags: latest
+          Dockerfile: docker/base.Dockerfile
+          buildContext: .
+
+      # The dev image is never pushed and therefore is always built.
+      - task: Docker@2
+        displayName: 'Build Development Image'
+        inputs:
+          command: build
+          repository: pythondiscord/snekbox-venv
+          tags: dev
+          Dockerfile: docker/venv.Dockerfile
+          buildContext: .
+          arguments: --build-arg DEV=1
+
+      # 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/ash \
+            pythondiscord/snekbox-venv:dev
+        displayName: 'Start Container'
+
+      - script: |
+          docker exec snekbox_test /bin/ash -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/ash -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/ash -c \
+            'pipenv run /bin/ash -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) ]
+
+    steps:
+      - task: Docker@2
+        displayName: 'Log into Docker Hub'
+        inputs:
+          command: login
+          containerRegistry: DockerHubV2
+
+      # 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.
+      - task: Docker@2
+        displayName: 'Build Base Image'
+        condition: >
+          and(
+            succeeded(),
+            ne(variables.BASE_PULL, True),
+            or(
+              eq(variables.BASE_CHANGED, True),
+              eq(variables.VENV_CHANGED, True)
+            )
+          )
+        inputs:
+          command: build
+          repository: pythondiscord/snekbox-base
+          tags: latest
+          Dockerfile: docker/base.Dockerfile
+          buildContext: .
+
+      # Also build this image if base has changed - even if this image hasn't.
+      - task: Docker@2
+        displayName: 'Build Virtual Environment Image'
+        condition: >
+          and(
+            succeeded(),
+            or(
+              eq(variables.BASE_CHANGED, True),
+              eq(variables.VENV_CHANGED, True)
+            )
+          )
+        inputs:
+          command: build
+          repository: pythondiscord/snekbox-venv
+          tags: latest
+          Dockerfile: docker/venv.Dockerfile
+          buildContext: .
+
+      # Always build this image unless it's for a pull request.
+      - task: Docker@2
+        displayName: 'Build Final Image'
+        condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
+        inputs:
+          command: build
+          repository: pythondiscord/snekbox
+          tags: latest
+          Dockerfile: docker/Dockerfile
+          buildContext: .
+
+      # 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.
+      - task: Docker@2
+        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)
+            )
+          )
+        inputs:
+          command: push
+          repository: pythondiscord/snekbox-base
+          tags: latest
+
+      - task: Docker@2
+        displayName: 'Push Virtual Environment Image'
+        condition: >
+          and(
+            succeeded(),
+            ne(variables['Build.Reason'], 'PullRequest'),
+            or(
+              eq(variables.BASE_CHANGED, True),
+              eq(variables.VENV_CHANGED, True)
+            )
+          )
+        inputs:
+          command: push
+          repository: pythondiscord/snekbox-venv
+          tags: latest
+
+      - task: Docker@2
+        displayName: 'Push Final Image'
+        condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
+        inputs:
+          command: push
+          repository: pythondiscord/snekbox
+          tags: latest
diff --git a/scripts/check_dockerfiles.sh b/scripts/check_dockerfiles.sh
new file mode 100755
index 0000000..c84c61f
--- /dev/null
+++ b/scripts/check_dockerfiles.sh
@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+exec 3>&1 # New file descriptor to stdout
+
+BASE_URL="https://dev.azure.com/\
+python-discord/${SYSTEM_TEAMPROJECTID}/_apis/build/builds?\
+queryOrder=finishTimeDescending&\
+resultFilter=succeeded&\
+\$top=1&\
+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
+
+    local branch="${1:?"get_build: argument 1 'branch' is unset"}"
+    local url="${BASE_URL}&branchName=${branch}"
+
+    printf '%s\n' "Retrieving the latest successful build using ${url}" >&3
+
+    local response
+    response="$(curl -sSL "${url}")"
+
+    if [[ -z "${response}" ]] \
+        || ! count="$(printf '%s' "${response}" | jq -re '.count')" \
+        || (( "${count}" < 1 ))
+    then
+        return 1
+    else
+        printf '%s' "${response}"
+    fi
+}
+
+# Get the previous commit
+if [[ "${BUILD_REASON}" = "PullRequest" ]]; then
+    if ! prev_commit="$(
+            get_build "${BUILD_SOURCEBRANCH}" \
+            | jq -re '.value[0].triggerInfo."pr.sourceSha"'
+        )"
+    then
+        echo \
+            "Could not retrieve the previous build's commit." \
+            "Falling back to the head of the target branch."
+
+        prev_commit="origin/${SYSTEM_PULLREQUEST_TARGETBRANCH}"
+    fi
+elif ! prev_commit="$(
+        get_build "${BUILD_SOURCEBRANCH}" \
+        | jq -re '.value[0].sourceVersion'
+    )"
+then
+    echo \
+        "No previous build was found." \
+        "Either the previous build is too old and was deleted" \
+        "or the branch was empty before this build." \
+        "All images will be built."
+    exit 0
+fi
+
+# Compare diffs
+head="$(git rev-parse HEAD)"
+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"
+else
+    # Always rebuild the venv if the base changes.
+    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
+    # 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"
+fi
diff --git a/scripts/dev.sh b/scripts/dev.sh
index 097690b..8f5b24f 100755
--- a/scripts/dev.sh
+++ b/scripts/dev.sh
@@ -34,16 +34,17 @@ fi
 # The volume is mounted to same the path in the container as the source
 # directory on the host to ensure coverage can find the source files.
 docker run \
-    -td \
+    --tty \
+    --detach \
     --name snekbox_test \
     --privileged \
     --network host \
-    -h pdsnk-dev \
+    --hostname pdsnk-dev \
     -e PYTHONDONTWRITEBYTECODE=1 \
     -e PIPENV_PIPFILE="/snekbox/Pipfile" \
     -e ENV="${PWD}/scripts/.profile" \
-    -v "${PWD}":"${PWD}" \
-    -w "${PWD}"\
+    --volume "${PWD}":"${PWD}" \
+    --workdir "${PWD}"\
     --entrypoint /bin/ash \
     pythondiscord/snekbox-venv:dev \
     >/dev/null \