From 48519e30de97699e686137632f64bf53e258345e Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:15:05 -0600 Subject: [PATCH 1/7] ci: Speed up the smoke test Skip waiting on the VM deletion, poll container health instead of sleeping 90 seconds, pull images in parallel, install fewer Vagrant plugins, and skip the smoke test on docs-only builds. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c731-7931-7091-9b5e-664cc2379c79 Co-authored-by: Amp --- .buildkite/pipeline.yaml | 4 ++++ .buildkite/vagrant-run.sh | 20 +++++++++++++++----- test/Vagrantfile | 1 - test/smoke-test.sh | 19 +++++++++++++++++-- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index efa4e4125..afabeac5c 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -16,6 +16,10 @@ steps: command: .buildkite/vagrant-run.sh docker-test artifact_paths: ./*.log agents: { queue: "vagrant" } + # Skip the smoke test on builds which only change docs + if_changed: + include: "**" + exclude: "**.md" # Please keep in mind that the release manifest uses specific branch names when creating releases. # Therefore, if you update them, or if you decide to change how we detect what kind of build we're dealing diff --git a/.buildkite/vagrant-run.sh b/.buildkite/vagrant-run.sh index a54aeb97d..1403e0dd8 100755 --- a/.buildkite/vagrant-run.sh +++ b/.buildkite/vagrant-run.sh @@ -5,23 +5,32 @@ set -euo pipefail box="$1" exit_code=0 +KEY_PATH="/tmp/e2e-builder.json" pushd "test" +# Ask GCP to delete the VM, without waiting the 1-2 minutes the deletion takes +request_instance_deletion() ( + export CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE="$KEY_PATH" + instance_name="$(cat ".vagrant/machines/${box}/google/id" 2>/dev/null)" && + instance_url="$(gcloud compute instances list --project=sourcegraph-ci --filter="name=${instance_name}" --format="value(selfLink)")" && + curl --fail --silent --show-error --output /dev/null --request DELETE \ + --header "Authorization: Bearer $(gcloud auth print-access-token)" "$instance_url" +) + +# Fall back to a blocking destroy, so a failed deletion request cannot leak a VM cleanup() { - vagrant destroy -f "$box" + request_instance_deletion || vagrant destroy -f "$box" } echo --- ":vagrant: installing plugins" vagrant --version -vagrant plugin install vagrant-google --plugin-version '2.7.0' -vagrant plugin install vagrant-env -vagrant plugin install vagrant-scp +# Agents on the same host share ~/.vagrant.d, so only the first job on a new host installs +[[ "$(vagrant plugin list)" == *"vagrant-google (2.7.0"* ]] || vagrant plugin install vagrant-google --plugin-version '2.7.0' trap cleanup EXIT echo --- ":lock: builder account key" -KEY_PATH="/tmp/e2e-builder.json" if [ ! -f ${KEY_PATH} ]; then gcloud secrets versions access latest --secret=e2e-builder-sa-key --quiet --project=sourcegraph-ci > "${KEY_PATH}" fi @@ -33,6 +42,7 @@ echo --- ":vagrant: starting box $box" vagrant up "$box" --provider=google || exit_code=$? if [ "$exit_code" != 0 ]; then + vagrant plugin install vagrant-scp vagrant scp "${box}:/deploy-sourcegraph-docker/*.log" ../ exit $exit_code fi diff --git a/test/Vagrantfile b/test/Vagrantfile index 09bd66030..ef3901e09 100644 --- a/test/Vagrantfile +++ b/test/Vagrantfile @@ -13,7 +13,6 @@ servers = YAML.load_file('servers.yaml') Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| # Iterate through entries in YAML file servers.each do |server| - config.env.enable config.vm.define server['name'] do |srv| srv.vm.box = server['box'] srv.vm.synced_folder '../', '/deploy-sourcegraph-docker' diff --git a/test/smoke-test.sh b/test/smoke-test.sh index 5aac87dca..6eb98584e 100755 --- a/test/smoke-test.sh +++ b/test/smoke-test.sh @@ -15,10 +15,24 @@ expect_containers="25" deploy_sourcegraph() { cd $(dirname "${BASH_SOURCE[0]}")/.. + docker-compose version + # `up` pulls images one at a time, `pull` pulls them in parallel + docker-compose --file docker-compose/docker-compose.yaml pull --quiet docker-compose --file docker-compose/docker-compose.yaml up -d -t 600 +} - echo "Giving containers 90s to start..." - sleep 90 +wait_until_healthy() { + echo "Waiting up to 5 minutes for the frontend to respond, and container health checks to pass..." + for _ in {1..60}; do + if curl --fail --silent --output /dev/null http://localhost:80/healthz && + [[ -z "$(docker ps --quiet --filter health=starting --filter health=unhealthy)" ]]; then + return + fi + sleep 5 + done + docker ps --format '{{.Names}}: {{.Status}}' + echo "TEST FAILURE: containers were not healthy after 5 minutes" + exit 1 } test_count() { @@ -70,5 +84,6 @@ trap catch_errors EXIT configure_docker deploy_sourcegraph +wait_until_healthy test_count test_containers From c37ba4671950b1a5565945a6081ae87d772a148b Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:24:25 -0600 Subject: [PATCH 2/7] ci: Boot the smoke test VM from a pd-balanced disk Amp-Thread-ID: https://ampcode.com/threads/T-01a1c731-7931-7091-9b5e-664cc2379c79 Co-authored-by: Amp --- test/Vagrantfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Vagrantfile b/test/Vagrantfile index ef3901e09..4806a15ef 100644 --- a/test/Vagrantfile +++ b/test/Vagrantfile @@ -36,6 +36,8 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| srv.vm.provider :google do |g, o| g.machine_type = server['machine_type'] + # The default, pd-standard, is slow to boot from and to pull images onto + g.disk_type = 'pd-balanced' g.image = ENV['IMAGE'] g.image_project_id = 'sourcegraph-ci' g.google_project_id = project_id From c759e83868b6ccafaab7e73b44cd31bd246f7ee8 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:29:49 -0600 Subject: [PATCH 3/7] Revert "ci: Boot the smoke test VM from a pd-balanced disk" Measured on build 16813: image pulls took 90s, against 93s on pd-standard in build 16811, so the disk is not what limits them. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c731-7931-7091-9b5e-664cc2379c79 Co-authored-by: Amp --- test/Vagrantfile | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Vagrantfile b/test/Vagrantfile index 4806a15ef..ef3901e09 100644 --- a/test/Vagrantfile +++ b/test/Vagrantfile @@ -36,8 +36,6 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| srv.vm.provider :google do |g, o| g.machine_type = server['machine_type'] - # The default, pd-standard, is slow to boot from and to pull images onto - g.disk_type = 'pd-balanced' g.image = ENV['IMAGE'] g.image_project_id = 'sourcegraph-ci' g.google_project_id = project_id From 1bb61d08c48add5b76d3a16c2f8f297ff2fa430c Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:53:29 -0600 Subject: [PATCH 4/7] ci: Always run the smoke test on main, and fail it on container restarts if_changed compares a build against origin/main. For a commit which is already on main, and is not the newest, that comparison is empty, so the smoke test was skipped. Only apply if_changed on other branches. The health check wait is shorter than the sleep it replaced, so a crash-looping container was more likely to be seen as running. Fail the test if any container has restarted. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c731-7931-7091-9b5e-664cc2379c79 Co-authored-by: Amp --- .buildkite/pipeline.yaml | 11 +++++++++-- test/smoke-test.sh | 7 +++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index afabeac5c..94570eee9 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -12,11 +12,18 @@ steps: command: .buildkite/prettier-check.sh agents: { queue: "vagrant" } - - label: ":rice: docker-compose-test" + # Builds on main always run the smoke test. if_changed compares against origin/main, + # so it finds no changes in a commit which is already on main, unless it is the newest + - &smoke_test + label: ":rice: docker-compose-test" command: .buildkite/vagrant-run.sh docker-test artifact_paths: ./*.log agents: { queue: "vagrant" } - # Skip the smoke test on builds which only change docs + if: build.branch == pipeline.default_branch + + # Builds on other branches skip the smoke test if they only change docs + - <<: *smoke_test + if: build.branch != pipeline.default_branch if_changed: include: "**" exclude: "**.md" diff --git a/test/smoke-test.sh b/test/smoke-test.sh index 6eb98584e..1c558b21e 100755 --- a/test/smoke-test.sh +++ b/test/smoke-test.sh @@ -60,6 +60,13 @@ test_containers() { 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 curl -f http://localhost:80/healthz From bf45be823133812dd48a4444b4f43b973272fc31 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:59:38 -0600 Subject: [PATCH 5/7] ci: Name the main branch in the smoke test conditions The Buildkite pipeline's default branch is still "master", so pipeline.default_branch never matched a build on main. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c731-7931-7091-9b5e-664cc2379c79 Co-authored-by: Amp --- .buildkite/pipeline.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index 94570eee9..c9a6acd17 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -13,17 +13,18 @@ steps: agents: { queue: "vagrant" } # Builds on main always run the smoke test. if_changed compares against origin/main, - # so it finds no changes in a commit which is already on main, unless it is the newest + # so it finds no changes in a commit which is already on main, unless it is the newest. + # The branch is named here, because the Buildkite pipeline's default branch is still "master" - &smoke_test label: ":rice: docker-compose-test" command: .buildkite/vagrant-run.sh docker-test artifact_paths: ./*.log agents: { queue: "vagrant" } - if: build.branch == pipeline.default_branch + if: build.branch == "main" # Builds on other branches skip the smoke test if they only change docs - <<: *smoke_test - if: build.branch != pipeline.default_branch + if: build.branch != "main" if_changed: include: "**" exclude: "**.md" From 71401cfce7b51b42db6a5bf5727fb5f9dc747746 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:19:06 -0600 Subject: [PATCH 6/7] ci: Delete the smoke test VM by its Buildkite job label, and sweep stale VMs A build cancelled while GCP was still creating the VM leaked it, because Vagrant only records the VM once the creation has finished, so the cleanup found nothing to delete. The VM now carries its Buildkite job ID as a label, and the cleanup deletes by that label. As a backstop for jobs which die before their cleanup runs, every job deletes labelled VMs older than an hour. The step times out after 30 minutes, so no running job owns a VM that old. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c731-7931-7091-9b5e-664cc2379c79 Co-authored-by: Amp --- .buildkite/pipeline.yaml | 2 ++ .buildkite/vagrant-run.sh | 16 ++++++++++------ test/Vagrantfile | 6 ++++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index c9a6acd17..eff33a8b5 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -20,6 +20,8 @@ steps: command: .buildkite/vagrant-run.sh docker-test artifact_paths: ./*.log agents: { queue: "vagrant" } + # vagrant-run.sh deletes VMs older than an hour, so no job may run that long + timeout_in_minutes: 30 if: build.branch == "main" # Builds on other branches skip the smoke test if they only change docs diff --git a/.buildkite/vagrant-run.sh b/.buildkite/vagrant-run.sh index 1403e0dd8..920b753e1 100755 --- a/.buildkite/vagrant-run.sh +++ b/.buildkite/vagrant-run.sh @@ -9,18 +9,18 @@ KEY_PATH="/tmp/e2e-builder.json" pushd "test" -# Ask GCP to delete the VM, without waiting the 1-2 minutes the deletion takes +# Ask GCP to delete the VMs matching a filter, without waiting the 1-2 minutes a deletion takes request_instance_deletion() ( export CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE="$KEY_PATH" - instance_name="$(cat ".vagrant/machines/${box}/google/id" 2>/dev/null)" && - instance_url="$(gcloud compute instances list --project=sourcegraph-ci --filter="name=${instance_name}" --format="value(selfLink)")" && - curl --fail --silent --show-error --output /dev/null --request DELETE \ - --header "Authorization: Bearer $(gcloud auth print-access-token)" "$instance_url" + gcloud compute instances list --project=sourcegraph-ci --filter="$1" --format="value(selfLink)" | tee /dev/stderr | + xargs --no-run-if-empty --max-args=1 curl --fail --silent --show-error --output /dev/null \ + --request DELETE --header "Authorization: Bearer $(gcloud auth print-access-token)" ) +# Find this job's VM by its label, because Vagrant only knows about the VM once GCP has finished creating it. # Fall back to a blocking destroy, so a failed deletion request cannot leak a VM cleanup() { - request_instance_deletion || vagrant destroy -f "$box" + request_instance_deletion "labels.buildkite-job-id=${BUILDKITE_JOB_ID}" || vagrant destroy -f "$box" } echo --- ":vagrant: installing plugins" @@ -38,6 +38,10 @@ export GOOGLE_JSON_KEY_LOCATION="${KEY_PATH}" export DOCKER_USERNAME=sourcegraph export DOCKER_PASSWORD="$(gcloud secrets versions access latest --secret=DOCKER_PASSWORD_BUILDKITE_AGENT --quiet --project=sourcegraph-ci)" +echo --- ":wastebasket: deleting VMs left behind by jobs which died before their cleanup" +# The smoke test step times out after 30 minutes, so a VM older than an hour has no job +request_instance_deletion "labels.buildkite-job-id:* AND creationTimestamp<-PT1H" || echo "Could not delete stale VMs" + echo --- ":vagrant: starting box $box" vagrant up "$box" --provider=google || exit_code=$? diff --git a/test/Vagrantfile b/test/Vagrantfile index ef3901e09..bf738b5cf 100644 --- a/test/Vagrantfile +++ b/test/Vagrantfile @@ -46,8 +46,10 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| g.service_account = server['service_account'] g.labels = { "team" => "dev-infra", - "type" => "vagrant" - } + "type" => "vagrant", + # .buildkite/vagrant-run.sh deletes VMs by this label + "buildkite-job-id" => ENV['BUILDKITE_JOB_ID'] + }.compact g.scopes = [ "compute-rw", "storage-ro" From a7429ce2e41661927c5c6a2314f7d1783166de58 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:25:57 -0600 Subject: [PATCH 7/7] ci: Name the smoke test VM after its job, and delete orphaned disks too Vagrant creates the boot disk before the VM, and the disk carries no labels. A build cancelled between the two left the disk behind, and the label lookup from the previous commit could not find it. sourcegraph-ci holds three such disks, from September. The VM and its disk are now named docker-test-job-. The cleanup and the stale sweep delete VMs by that name, and disks by that name which no VM uses. This replaces the label. Amp-Thread-ID: https://ampcode.com/threads/T-01a1c731-7931-7091-9b5e-664cc2379c79 Co-authored-by: Amp --- .buildkite/pipeline.yaml | 2 +- .buildkite/vagrant-run.sh | 24 ++++++++++++++++-------- test/Vagrantfile | 9 ++++----- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index eff33a8b5..65b37cc6e 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -20,7 +20,7 @@ steps: command: .buildkite/vagrant-run.sh docker-test artifact_paths: ./*.log agents: { queue: "vagrant" } - # vagrant-run.sh deletes VMs older than an hour, so no job may run that long + # vagrant-run.sh deletes VMs and disks older than an hour, so no job may run that long timeout_in_minutes: 30 if: build.branch == "main" diff --git a/.buildkite/vagrant-run.sh b/.buildkite/vagrant-run.sh index 920b753e1..d6fd33380 100755 --- a/.buildkite/vagrant-run.sh +++ b/.buildkite/vagrant-run.sh @@ -9,18 +9,26 @@ KEY_PATH="/tmp/e2e-builder.json" pushd "test" -# Ask GCP to delete the VMs matching a filter, without waiting the 1-2 minutes a deletion takes -request_instance_deletion() ( +# Vagrant creates a disk, then a VM, both with this name, and only knows about them once GCP has finished +# creating the VM. Naming them after the job lets the cleanup find them if the job is cancelled before that +instance_name_prefix="${box}-job-" +export VAGRANT_INSTANCE_NAME="${instance_name_prefix}${BUILDKITE_JOB_ID}" + +# Ask GCP to delete the VMs matching a filter, and the matching disks which no VM uses, +# without waiting the 1-2 minutes a deletion takes. GCP deletes a VM's disk with the VM +request_deletion() ( export CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE="$KEY_PATH" - gcloud compute instances list --project=sourcegraph-ci --filter="$1" --format="value(selfLink)" | tee /dev/stderr | + { + gcloud compute instances list --project=sourcegraph-ci --filter="$1" --format="value(selfLink)" && + gcloud compute disks list --project=sourcegraph-ci --filter="($1) AND -users:*" --format="value(selfLink)" + } | tee /dev/stderr | xargs --no-run-if-empty --max-args=1 curl --fail --silent --show-error --output /dev/null \ --request DELETE --header "Authorization: Bearer $(gcloud auth print-access-token)" ) -# Find this job's VM by its label, because Vagrant only knows about the VM once GCP has finished creating it. # Fall back to a blocking destroy, so a failed deletion request cannot leak a VM cleanup() { - request_instance_deletion "labels.buildkite-job-id=${BUILDKITE_JOB_ID}" || vagrant destroy -f "$box" + request_deletion "name=${VAGRANT_INSTANCE_NAME}" || vagrant destroy -f "$box" } echo --- ":vagrant: installing plugins" @@ -38,9 +46,9 @@ export GOOGLE_JSON_KEY_LOCATION="${KEY_PATH}" export DOCKER_USERNAME=sourcegraph export DOCKER_PASSWORD="$(gcloud secrets versions access latest --secret=DOCKER_PASSWORD_BUILDKITE_AGENT --quiet --project=sourcegraph-ci)" -echo --- ":wastebasket: deleting VMs left behind by jobs which died before their cleanup" -# The smoke test step times out after 30 minutes, so a VM older than an hour has no job -request_instance_deletion "labels.buildkite-job-id:* AND creationTimestamp<-PT1H" || echo "Could not delete stale VMs" +echo --- ":wastebasket: deleting VMs and disks left behind by jobs which died before their cleanup" +# The smoke test step times out after 30 minutes, so a VM or disk older than an hour has no job +request_deletion "name~^${instance_name_prefix} AND creationTimestamp<-PT1H" || echo "Could not delete stale VMs and disks" echo --- ":vagrant: starting box $box" vagrant up "$box" --provider=google || exit_code=$? diff --git a/test/Vagrantfile b/test/Vagrantfile index bf738b5cf..671f25254 100644 --- a/test/Vagrantfile +++ b/test/Vagrantfile @@ -39,17 +39,16 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| g.image = ENV['IMAGE'] g.image_project_id = 'sourcegraph-ci' g.google_project_id = project_id - g.name = "#{server['name']}-"+"#{Time.now.to_i}"+"#{rand(100000)}" + # .buildkite/vagrant-run.sh names the VM after its job, so it can delete the VM by name + g.name = ENV['VAGRANT_INSTANCE_NAME'] || "#{server['name']}-"+"#{Time.now.to_i}"+"#{rand(100000)}" g.network = server['network'] g.external_ip = external_ip g.use_private_ip = use_private_ip g.service_account = server['service_account'] g.labels = { "team" => "dev-infra", - "type" => "vagrant", - # .buildkite/vagrant-run.sh deletes VMs by this label - "buildkite-job-id" => ENV['BUILDKITE_JOB_ID'] - }.compact + "type" => "vagrant" + } g.scopes = [ "compute-rw", "storage-ro"