ci: Make the smoke test wait for health checks, and check more

AGENTS.md

@@ -22,8 +22,10 @@ CI runs these on every branch. `.tool-versions` pins Node and the GitHub CLI; CI
 
 `.buildkite/vagrant-run.sh` uses Vagrant to boot a GCP VM from the image named in `IMAGE` in `.buildkite/pipeline.yaml`, runs `test/smoke-test.sh` on it, and deletes it. It reads CI secrets and creates a billed VM, so take its result from the pull request's Buildkite build; run the script yourself only when a person asks.
 
-- `test/smoke-test.sh` starts the base file, and compares the number of running containers with `expect_containers`. Update that number in the same pull request that adds or removes a service.
+- `test/smoke-test.sh` starts the base file with `docker compose up --wait`, which fails if any container reports `unhealthy`. It then checks that every service has a running container, no container restarted, and the frontend responds. It counts the services itself, so a new long-running service needs no change to the test. A new service which exits when it is done, like `migrator`, must be left out of the count.
 - CI does not run `test/executor-smoke-test.sh`. Run it against a running deployment when you change `docker-compose/executors/`.
+- A service which is not ready until another service is needs a `start_period` on its health check, or `--wait` fails.
+- A test failure is not retried, and the step uploads every container's logs as artifacts. Only the image pull is retried: `test/smoke-test.sh` tries `docker compose pull` three times.
 - A branch which only changes files listed in the step's `if_changed` `exclude` skips the step. Builds on `main` always run it. When the smoke test starts reading a file on that list, remove it from the list.
 - The VM image is built from `gcp/images/e2e` in sourcegraph/infrastructure.
 

docker-compose/docker-compose.yaml

@@ -322,6 +322,8 @@ services:
       interval: 5s
       timeout: 10s
       retries: 3
+      # searcher is not ready until sourcegraph-frontend-internal is, which waits for the migrator
+      start_period: 300s
     volumes:
       - 'searcher-0:/mnt/cache'
     networks:
@@ -349,7 +351,8 @@ services:
       interval: 5s
       timeout: 5s
       retries: 3
-      start_period: 60s
+      # Not ready until sourcegraph-frontend-internal is, which waits for the migrator
+      start_period: 300s
     networks:
       - sourcegraph
     restart: always
@@ -402,7 +405,8 @@ services:
       interval: 5s
       timeout: 15s
       retries: 3
-      start_period: 60s
+      # Not ready until sourcegraph-frontend-internal is, which waits for the migrator
+      start_period: 300s
     networks:
       - sourcegraph
     restart: always

test/smoke-test.sh

@@ -3,48 +3,50 @@ set -euxfo pipefail
 
 configure_docker() {
   set +x
-  if [ -n "${DOCKER_USERNAME}" ] && [ -n "${DOCKER_PASSWORD}" ]; then
+  if [ -n "${DOCKER_USERNAME:-}" ] && [ -n "${DOCKER_PASSWORD:-}" ]; then
     docker login -u "${DOCKER_USERNAME}" --password-stdin <<<"$DOCKER_PASSWORD"
   fi
   set -x
   gcloud auth configure-docker
   gcloud auth configure-docker us-central1-docker.pkg.dev,us-docker.pkg.dev
 }
 
-expect_containers="25"
+compose_files=(--file docker-compose/docker-compose.yaml)
 
 deploy_sourcegraph() {
-	cd $(dirname "${BASH_SOURCE[0]}")/..
-	docker-compose --file docker-compose/docker-compose.yaml up -d -t 600
-
-	echo "Giving containers 90s to start..."
-	sleep 90
+	cd "$(dirname "${BASH_SOURCE[0]}")/.."
+	docker compose version
+	# `up` pulls images one at a time, `pull` pulls them in parallel.
+	# Docker Hub times out now and then; `up` pulls whatever is still missing after three attempts
+	for _ in 1 2 3; do
+		docker compose "${compose_files[@]}" pull --quiet && break
+		sleep 10
+	done
+	# --wait returns when every container is running, and healthy if it has a health check.
+	# It fails if a container reports unhealthy, or after the timeout
+	docker compose "${compose_files[@]}" up -d -t 600 --wait --wait-timeout 300
 }
 
 test_count() {
-	echo "TEST: Number of expected containers created"
-	containers_count=$(docker ps --format '{{.Names}}' | wc -l)
+	echo "TEST: Every service has a running container"
+	# migrator exits when it is done, every other service keeps running
+	expect_containers=$(docker compose "${compose_files[@]}" config --services | grep --count --invert-match '^migrator$')
+	containers_count=$(docker ps --filter status=running --format '{{.Names}}' | wc -l)
 	if [[ "$containers_count" -ne "$expect_containers" ]]; then
-		docker ps --format '{{.Names}}'
+		docker ps --all --format '{{.Names}}: {{.Status}}'
 		echo
-		echo "TEST FAILURE: expected $expect_containers containers, found $containers_count"
+		echo "TEST FAILURE: expected $expect_containers running containers, found $containers_count"
 		exit 1
 	fi
 }
 
 test_containers() {
-	echo "TEST: Checking every 10s that containers are running for 5 minutes..."
-	for i in {0..1}; do
-		containers=$(docker ps --format '{{.Names}}' | xargs -I{} -n1 sh -c "printf '{}: ' && docker inspect --format '{{.State.Status}}' {}")
-		containers_running=$(echo "$containers" | grep -c "running")
-		if [[ "$containers_running" -ne "$expect_containers" ]]; then
-			containers_failing=$(docker ps --format '{{.Names}}:{{.Status}}' | grep -v Up | cut -f 1 -d :)
-			echo "TEST FAILURE: expected $expect_containers containers running, found $containers_running. The following containers are failing: $containers_failing"
-			exit 1
-		fi
-		echo "Containers running OK.. waiting 10s"
-		sleep 1
-	done
+	echo "TEST: No container has restarted"
+	restarted_containers=$(docker ps --quiet | xargs docker inspect --format '{{.Name}} restarted {{.RestartCount}} times' | grep -v ' restarted 0 times' || true)
+	if [[ -n "$restarted_containers" ]]; then
+		echo "TEST FAILURE: $restarted_containers"
+		exit 1
+	fi
 
 	echo "TEST: Checking frontend is accessible"
 	curl -f http://localhost:80
@@ -53,20 +55,18 @@ test_containers() {
 	echo "ALL TESTS PASSED"
 }
 
-catch_errors() {
-	count=$(docker ps --format '{{.Names}}:{{.Status}}' | grep -c -v Up) || true
-	if [[ $count -ne 0 ]]; then
-		containers_failing=$(docker ps --format '{{.Names}}:{{.Status}}' | grep -v Up | cut -f 1 -d :)
-		echo
-		for cf in $containers_failing; do
-			echo "$cf is failing. Review the log files uploaded as artefacts to see errors."
-			docker logs -t "$cf" >"$cf".log 2>&1
+# On failure, save every container's logs, which the CI step uploads as artifacts
+save_logs_on_failure() {
+	exit_code=$?
+	if [[ $exit_code -ne 0 ]]; then
+		docker ps --all --format '{{.Names}}: {{.Status}}'
+		for container in $(docker ps --all --format '{{.Names}}'); do
+			docker logs --timestamps "$container" >"$container.log" 2>&1 || true
 		done
-		exit 1
 	fi
 }
 
-trap catch_errors EXIT
+trap save_logs_on_failure EXIT
 
 configure_docker
 deploy_sourcegraph