From ebbfe774e9946f901e02e6e46f147e1eca9b6503 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9mence=20Lesn=C3=A9?= <clemence.lesne@microsoft.com>
Date: Fri, 7 Apr 2023 11:23:57 +0200
Subject: [PATCH 1/3] Fix: Properly handle pod termination

---
 .github/workflows/pipeline.yaml |  3 +++
 src/docker/Dockerfile-bullseye  |  9 ++++++++-
 src/docker/Dockerfile-focal     |  9 ++++++++-
 src/docker/Dockerfile-jammy     |  9 ++++++++-
 src/docker/Dockerfile-ubi8      |  9 ++++++++-
 src/docker/start.sh             | 14 +++-----------
 6 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/.github/workflows/pipeline.yaml b/.github/workflows/pipeline.yaml
index 12850e00..0751929d 100644
--- a/.github/workflows/pipeline.yaml
+++ b/.github/workflows/pipeline.yaml
@@ -23,6 +23,8 @@ env:
   AGENT_VERSION: 3.218.0
   # https://github.com/PowerShell/PowerShell/releases
   POWERSHELL_VERSION: 7.2.10
+  # https://github.com/krallin/tini/releases
+  TINI_VERSION: 0.19.0
   # https://github.com/mikefarah/yq/releases
   YQ_VERSION: 4.33.2
 
@@ -186,6 +188,7 @@ jobs:
           build-args: |
             "AGENT_VERSION=${{ env.AGENT_VERSION }}"
             "POWERSHELL_VERSION=${{ env.POWERSHELL_VERSION }}"
+            "TINI_VERSION=${{ env.TINI_VERSION }}"
             "YQ_VERSION=${{ env.YQ_VERSION }}"
           cache-from: type=gha
           cache-to: type=gha,mode=max
diff --git a/src/docker/Dockerfile-bullseye b/src/docker/Dockerfile-bullseye
index 750130fa..b5985bfa 100644
--- a/src/docker/Dockerfile-bullseye
+++ b/src/docker/Dockerfile-bullseye
@@ -75,6 +75,14 @@ RUN curl -LsSf https://github.com/mikefarah/yq/releases/download/v${YQ_VERSION}/
     && chmod +x /usr/bin/yq \
     && yq --version
 
+# Install Tini, then verify installation
+ARG TINI_VERSION
+ENV TINI_VERSION ${TINI_VERSION}
+RUN curl -LsSf https://github.com/krallin/tini/releases/download/v${TINI_VERSION}/tini-$(ARCH_AMD64=amd64 bash arch.sh) -o /tini \
+    && chmod +x /tini \
+    && /tini --version
+ENTRYPOINT ["/tini", "--"]
+
 # Cleanup helper script
 RUN rm arch.sh
 
@@ -84,5 +92,4 @@ WORKDIR ${AGENT_HOME}
 # Install Azure Pipelines Agent startup script
 COPY start.sh .
 RUN chmod +x start.sh
-
 CMD bash start.sh
diff --git a/src/docker/Dockerfile-focal b/src/docker/Dockerfile-focal
index fa4427b6..2b1a02e8 100644
--- a/src/docker/Dockerfile-focal
+++ b/src/docker/Dockerfile-focal
@@ -75,6 +75,14 @@ RUN curl -LsSf https://github.com/mikefarah/yq/releases/download/v${YQ_VERSION}/
     && chmod +x /usr/bin/yq \
     && yq --version
 
+# Install Tini, then verify installation
+ARG TINI_VERSION
+ENV TINI_VERSION ${TINI_VERSION}
+RUN curl -LsSf https://github.com/krallin/tini/releases/download/v${TINI_VERSION}/tini-$(ARCH_AMD64=amd64 bash arch.sh) -o /tini \
+    && chmod +x /tini \
+    && /tini --version
+ENTRYPOINT ["/tini", "--"]
+
 # Cleanup helper script
 RUN rm arch.sh
 
@@ -84,5 +92,4 @@ WORKDIR ${AGENT_HOME}
 # Install Azure Pipelines Agent startup script
 COPY start.sh .
 RUN chmod +x start.sh
-
 CMD bash start.sh
diff --git a/src/docker/Dockerfile-jammy b/src/docker/Dockerfile-jammy
index 47b211c2..587dc4be 100644
--- a/src/docker/Dockerfile-jammy
+++ b/src/docker/Dockerfile-jammy
@@ -75,6 +75,14 @@ RUN curl -LsSf https://github.com/mikefarah/yq/releases/download/v${YQ_VERSION}/
     && chmod +x /usr/bin/yq \
     && yq --version
 
+# Install Tini, then verify installation
+ARG TINI_VERSION
+ENV TINI_VERSION ${TINI_VERSION}
+RUN curl -LsSf https://github.com/krallin/tini/releases/download/v${TINI_VERSION}/tini-$(ARCH_AMD64=amd64 bash arch.sh) -o /tini \
+    && chmod +x /tini \
+    && /tini --version
+ENTRYPOINT ["/tini", "--"]
+
 # Cleanup helper script
 RUN rm arch.sh
 
@@ -84,5 +92,4 @@ WORKDIR ${AGENT_HOME}
 # Install Azure Pipelines Agent startup script
 COPY start.sh .
 RUN chmod +x start.sh
-
 CMD bash start.sh
diff --git a/src/docker/Dockerfile-ubi8 b/src/docker/Dockerfile-ubi8
index 04bd98ea..9c4b918e 100644
--- a/src/docker/Dockerfile-ubi8
+++ b/src/docker/Dockerfile-ubi8
@@ -73,6 +73,14 @@ RUN curl -LsSf https://github.com/mikefarah/yq/releases/download/v${YQ_VERSION}/
     && chmod +x /usr/bin/yq \
     && yq --version
 
+# Install Tini, then verify installation
+ARG TINI_VERSION
+ENV TINI_VERSION ${TINI_VERSION}
+RUN curl -LsSf https://github.com/krallin/tini/releases/download/v${TINI_VERSION}/tini-$(ARCH_AMD64=amd64 bash arch.sh) -o /tini \
+    && chmod +x /tini \
+    && /tini --version
+ENTRYPOINT ["/tini", "--"]
+
 # Cleanup helper script
 RUN rm arch.sh
 
@@ -82,5 +90,4 @@ WORKDIR ${AGENT_HOME}
 # Install Azure Pipelines Agent startup script
 COPY start.sh .
 RUN chmod +x start.sh
-
 CMD bash start.sh
diff --git a/src/docker/start.sh b/src/docker/start.sh
index 533210fb..f1f49309 100644
--- a/src/docker/start.sh
+++ b/src/docker/start.sh
@@ -25,7 +25,7 @@ fi
 print_header() {
   lightcyan='\033[1;36m'
   nocolor='\033[0m'
-  echo -e "${lightcyan}$1${nocolor}"
+  echo -e "${lightcyan}➡️ $1${nocolor}"
 }
 
 print_header "Configuring agent..."
@@ -44,17 +44,9 @@ bash config.sh \
   --token $(cat "$AZP_TOKEN_FILE") \
   --unattended \
   --url "$AZP_URL" \
-  --work "${AZP_WORK:-_work}" &
-
-# Fake the exit code of the agent for the prevent Kubernetes to detect the pod as failed (this is intended)
-# See: https://stackoverflow.com/a/62183992/12732154
-wait $!
+  --work "${AZP_WORK:-_work}"
 
 print_header "Running agent..."
 
 # Running it with the --once flag at the end will shut down the agent after the build is executed
-bash run-docker.sh "$@" --once &
-
-# Fake the exit code of the agent for the prevent Kubernetes to detect the pod as failed (this is intended)
-# See: https://stackoverflow.com/a/62183992/12732154
-wait $!
+exec bash run-docker.sh "$@" --once

From 75d7ef2c6eca150c9d459265a58a994fae9091a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9mence=20Lesn=C3=A9?= <clemence.lesne@microsoft.com>
Date: Fri, 7 Apr 2023 12:50:03 +0200
Subject: [PATCH 2/3] Fix: Clean-up the stopped agents from the remote agents
 list

---
 src/docker/start.sh                           | 22 +++++--------------
 .../templates/deployment.yaml                 |  8 +++++++
 .../azure-pipelines-agent/templates/hpa.yaml  |  8 +++++++
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/docker/start.sh b/src/docker/start.sh
index f1f49309..ed2bebf9 100644
--- a/src/docker/start.sh
+++ b/src/docker/start.sh
@@ -6,18 +6,11 @@ if [ -z "$AZP_URL" ]; then
   exit 1
 fi
 
-if [ -z "$AZP_TOKEN_FILE" ]; then
-  if [ -z "$AZP_TOKEN" ]; then
-    echo 1>&2 "error: missing AZP_TOKEN environment variable"
-    exit 1
-  fi
-
-  AZP_TOKEN_FILE=/azp/.token
-  echo -n $AZP_TOKEN >"$AZP_TOKEN_FILE"
+if [ -z "$AZP_TOKEN" ]; then
+  echo 1>&2 "error: missing AZP_TOKEN environment variable"
+  exit 1
 fi
 
-unset AZP_TOKEN
-
 if [ -n "$AZP_WORK" ]; then
   mkdir -p "$AZP_WORK"
 fi
@@ -30,18 +23,13 @@ print_header() {
 
 print_header "Configuring agent..."
 
-# Allow the agent to run as root (only feasible because the agent is running in a not-reused container)
-export AGENT_ALLOW_RUNASROOT="1"
-# Let the agent ignore the token env variables
-export VSO_AGENT_IGNORE=AZP_TOKEN,AZP_TOKEN_FILE
-
 bash config.sh \
   --acceptTeeEula \
   --agent "${AZP_AGENT_NAME:-$(hostname)}" \
   --auth PAT \
   --pool "${AZP_POOL:-Default}" \
   --replace \
-  --token $(cat "$AZP_TOKEN_FILE") \
+  --token "$AZP_TOKEN" \
   --unattended \
   --url "$AZP_URL" \
   --work "${AZP_WORK:-_work}"
@@ -49,4 +37,4 @@ bash config.sh \
 print_header "Running agent..."
 
 # Running it with the --once flag at the end will shut down the agent after the build is executed
-exec bash run-docker.sh "$@" --once
+bash run-docker.sh "$@" --once
diff --git a/src/helm/azure-pipelines-agent/templates/deployment.yaml b/src/helm/azure-pipelines-agent/templates/deployment.yaml
index ada394bb..9692d30b 100644
--- a/src/helm/azure-pipelines-agent/templates/deployment.yaml
+++ b/src/helm/azure-pipelines-agent/templates/deployment.yaml
@@ -39,7 +39,15 @@ spec:
             {{- toYaml .Values.securityContext | nindent 12 }}
           image: "{{ .Values.image.repository | required "A value for .Values.image.repository is required" }}:{{ .Values.image.flavor | required "A value for .Values.image.flavor is required" }}-{{ .Values.image.version | default .Chart.Version }}"
           imagePullPolicy: {{ .Values.image.pullPolicy }}
+          lifecycle:
+            preStop:
+              exec:
+                command: [bash, -c, "./config.sh remove --auth PAT --token $AZP_TOKEN"]
           env:
+            - name: VSO_AGENT_IGNORE
+              value: AZP_TOKEN
+            - name: AGENT_ALLOW_RUNASROOT
+              value: "1"
             - name: AZP_AGENT_NAME
               valueFrom:
                 fieldRef:
diff --git a/src/helm/azure-pipelines-agent/templates/hpa.yaml b/src/helm/azure-pipelines-agent/templates/hpa.yaml
index 81802f87..4e9e254d 100644
--- a/src/helm/azure-pipelines-agent/templates/hpa.yaml
+++ b/src/helm/azure-pipelines-agent/templates/hpa.yaml
@@ -47,7 +47,15 @@ spec:
               {{- toYaml .Values.securityContext | nindent 14 }}
             image: "{{ .Values.image.repository | required "A value for .Values.image.repository is required" }}:{{ .Values.image.flavor | required "A value for .Values.image.flavor is required" }}-{{ .Values.image.version | default .Chart.Version }}"
             imagePullPolicy: {{ .Values.image.pullPolicy }}
+            lifecycle:
+              preStop:
+                exec:
+                  command: [bash, -c, "./config.sh remove --auth PAT --token $AZP_TOKEN"]
             env:
+              - name: VSO_AGENT_IGNORE
+                value: AZP_TOKEN
+              - name: AGENT_ALLOW_RUNASROOT
+                value: "1"
               - name: AZP_AGENT_NAME
                 valueFrom:
                   fieldRef:

From aa795dad5bbe96e0ca1322d902f747b6bb83de7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9mence=20Lesn=C3=A9?= <clemence.lesne@microsoft.com>
Date: Fri, 7 Apr 2023 12:53:56 +0200
Subject: [PATCH 3/3] Doc: Clarify zoombies agents with capabilities

---
 README.md | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 740e3191..74c8dacb 100644
--- a/README.md
+++ b/README.md
@@ -70,7 +70,9 @@ helm upgrade --install agent clemlesne-azure-pipelines-agent/azure-pipelines-age
 
 Capabilities are declarative variables you can add to the agents, to allow developers to select the right agent for their pipeline ([official documentation](https://learn.microsoft.com/en-us/azure/devops/pipelines/process/demands?view=azure-devops&tabs=yaml)).
 
-Note, you can add multiple Helm instances to the same agent pool. Then, disctinct them by capabilities. For examples:
+> Note, you can add multiple Helm instances to the same agent pool. It will result in a single pool with multiple capabilities. Be warning, if a capability is not unique accross the pool, all the agents will scale. This will create "zoombies" agents, scaled for nothing, waiting their timeout.
+
+Disctinct the agents by capabilities. For examples:
 
 - A pool of AMD64 agents, and a pool of ARM64 agents
 - A pool of agents with GPU, and a pool of agents without GPU
@@ -84,6 +86,8 @@ Take the assumption we want to host a specific instance pool to ARM servers.
 # values.yaml
 pipelines:
   pool: Kubernetes
+  capabiliies:
+    - arch-arm64
 
 affinity:
   nodeAffinity:
@@ -110,7 +114,7 @@ pool:
   name: Kubernetes
   demands:
     - Agent.OS -equals Linux
-    - Agent.OSArchitecture -equals ARM64
+    - arch-arm64
 
 stages:
   ...