From 764d519079fccb242df01e8e7758fa939e126011 Mon Sep 17 00:00:00 2001 From: Michael Lin Date: Wed, 22 Jul 2026 19:38:18 -0700 Subject: [PATCH 1/2] Parameterize CloudWatch log group name to fix multi-fleet collisions The executor and docker-mirror AMIs' CloudWatch agent ships host syslog to a hard-coded log group ("executors" / "executors_docker_mirror"). Running two executor fleets in one account/region therefore collided on a single shared log group: the second `terraform apply` failed with ResourceAlreadyExistsException. `randomize_resource_names` only affected the resource's tag, never the actual log group name or the on-host agent target. Make the log group name configurable end to end: - aws_cloudwatch_log_group.syslogs now uses the resolved name (randomized when randomize_resource_names=true, else the legacy fixed name for backward compatibility) in both modules. - Pass the resolved name into the launch template / instance user_data and reconfigure the on-host CloudWatch agent at boot so it writes to the configured group. Prefer the AMI helper (/usr/local/bin/configure-cloudwatch-agent-log-group) when present, else fall back to rewriting the agent config and running fetch-config (works with existing AMIs). Gate the fallback on the agent binary, since fetch-config consumes the source config file at build time. - docker-mirror startup-script.sh becomes a templatefile (.tpl) to receive the name. - Add cloudwatch_log_group_name outputs to both modules and document the randomize_resource_names requirement for multi-fleet deployments. Requires the matching AMI change (parameterized helper) to exercise the helper path; the fallback keeps existing AMIs working immediately. Co-Authored-By: Claude Opus 4.8 (1M context) --- examples/multiple-executors/README.md | 1 + modules/docker-mirror/main.tf | 16 +++-- modules/docker-mirror/outputs.tf | 4 ++ modules/docker-mirror/startup-script.sh | 34 ---------- modules/docker-mirror/startup-script.sh.tpl | 74 +++++++++++++++++++++ modules/executors/main.tf | 14 +++- modules/executors/outputs.tf | 4 ++ modules/executors/startup-script.sh.tpl | 37 +++++++++++ 8 files changed, 143 insertions(+), 41 deletions(-) delete mode 100644 modules/docker-mirror/startup-script.sh create mode 100644 modules/docker-mirror/startup-script.sh.tpl create mode 100644 modules/executors/outputs.tf diff --git a/examples/multiple-executors/README.md b/examples/multiple-executors/README.md index ec6cc34..f10d4b5 100644 --- a/examples/multiple-executors/README.md +++ b/examples/multiple-executors/README.md @@ -6,6 +6,7 @@ The following variables must be supplied: - `sourcegraph_external_url`, `sourcegraph_executor_proxy_password`, `queue_name`, `metrics_environment_label`, and `instance_tag`: Analogous to the `executor_*` variables in the `single-executor` example. - `resource_prefix`: A prefix unique to each set of compute resources. This prevents collisions between two uses of the `executors` module. We recommend this value be constructed the same way `instance_tag` is constructed. +- `randomize_resource_names`: Must be `true` when running more than one `executors` (or `docker-mirror`) module in the same account and region. The executor AMI's CloudWatch agent ships host logs to a log group named `executors` by default; randomizing gives each fleet a unique log group (and reconfigures the on-host agent to match at boot) so the two deployments don't collide on that shared log group. - `docker_registry_mirror`: This variable is given the value `"http://${module.docker-mirror.ip_address}:5000"`, which converts the raw external IP address to an address resolvable by the executor instances. If your deployment environment already has a Docker registry that can be used, only the `executor` submodule must be used (and references to the `networking` and `docker-mirror` modules can be dropped). The Docker registry mirror address can be supplied along with its containing VPC and subnet as pre-existing identifier literals. diff --git a/modules/docker-mirror/main.tf b/modules/docker-mirror/main.tf index 5cd2321..2571180 100644 --- a/modules/docker-mirror/main.tf +++ b/modules/docker-mirror/main.tf @@ -2,7 +2,11 @@ locals { resource_prefix = (var.resource_prefix == "" || substr(var.resource_prefix, -1, -2) == "-") ? var.resource_prefix : "${var.resource_prefix}-" cloudwatch_log_group = { - name = var.randomize_resource_names ? "${local.resource_prefix}sourcegraph-executors-docker-registry-mirror-${random_id.cloudwatch_log_group[0].hex}" : null + # The docker-mirror AMI's CloudWatch agent ships syslog to this group. When + # randomizing, use a unique name so multiple deployments in the same + # account/region don't collide; the boot-time startup script reconfigures + # the agent to match. Otherwise keep the legacy fixed name the AMI defaults to. + name = var.randomize_resource_names ? "${local.resource_prefix}sourcegraph-executors-docker-registry-mirror-${random_id.cloudwatch_log_group[0].hex}" : "executors_docker_mirror" } instance = { name = var.randomize_resource_names ? "${local.resource_prefix}sourcegraph-executors-docker-registry-mirror-${random_id.instance[0].hex}" : "sourcegraph-executors-docker-registry-mirror" @@ -28,8 +32,10 @@ resource "random_id" "cloudwatch_log_group" { # Create a log group in CloudWatch. This is where the docker mirror will ingest # its logs to. resource "aws_cloudwatch_log_group" "syslogs" { - # TODO: This is hardcoded in the executor docker mirror image. - name = "executors_docker_mirror" + # The docker-mirror AMI's CloudWatch agent defaults to a group literally named + # "executors_docker_mirror". The startup script reconfigures the agent to use + # this name at boot, so randomized deployments get a unique log group. + name = local.cloudwatch_log_group.name retention_in_days = 7 tags = { @@ -121,7 +127,9 @@ resource "aws_instance" "default" { iam_instance_profile = aws_iam_instance_profile.instance.name - user_data_base64 = base64encode(file("${path.module}/startup-script.sh")) + user_data_base64 = base64encode(templatefile("${path.module}/startup-script.sh.tpl", { + cloudwatch_log_group_name = local.cloudwatch_log_group.name + })) } # Reserve a fixed disk to retain docker mirror data across rollouts. diff --git a/modules/docker-mirror/outputs.tf b/modules/docker-mirror/outputs.tf index e69de29..0397e3c 100644 --- a/modules/docker-mirror/outputs.tf +++ b/modules/docker-mirror/outputs.tf @@ -0,0 +1,4 @@ +output "cloudwatch_log_group_name" { + value = aws_cloudwatch_log_group.syslogs.name + description = "The name of the CloudWatch log group that the docker-mirror host ships syslog to." +} diff --git a/modules/docker-mirror/startup-script.sh b/modules/docker-mirror/startup-script.sh deleted file mode 100644 index d6c3821..0000000 --- a/modules/docker-mirror/startup-script.sh +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/bin/env bash -set -euxo pipefail - -# Wait until the block device is attached. -while true; do - sleep 1 - test -e /dev/nvme1n1 && break -done - -# Alias the nvme devices to /dev/sdX. -# https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/device_naming.html#available-ec2-device-names -VOLUMES_NAME="$(find /dev -maxdepth 1 | grep -i 'nvme[0-21]n1$')" -for VOLUME in ${VOLUMES_NAME}; do - ALIAS=$(sudo nvme id-ctrl -H -v "${VOLUME}" | { grep -Po '/dev/(sd[b-z]|xvd[b-z])' || test $? = 1; }) - if [ -n "${ALIAS}" ]; then - sudo ln -s "${VOLUME}" "${ALIAS}" - fi -done - -REALPATH="$(realpath /dev/sdh)" -# Ensure /dev/sdh has a file system. -if [ "$(sudo file -s "$REALPATH")" == "$REALPATH: data" ]; then - sudo mkfs.ext4 /dev/sdh -fi - -# Mount /dev/sdh to /mnt/registry. -sudo mkdir -p /mnt/registry -echo "/dev/sdh /mnt/registry ext4 defaults 0 2" | sudo tee -a /etc/fstab -sudo mount -a -sudo chown ubuntu:ubuntu /mnt/registry - -# Enable and start the registry service. -sudo systemctl enable docker_registry -sudo systemctl start docker_registry diff --git a/modules/docker-mirror/startup-script.sh.tpl b/modules/docker-mirror/startup-script.sh.tpl new file mode 100644 index 0000000..bbeac8c --- /dev/null +++ b/modules/docker-mirror/startup-script.sh.tpl @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +set -euxo pipefail + +# Pull the terraform-provided CloudWatch log group name into the environment. +CLOUDWATCH_LOG_GROUP_NAME="${cloudwatch_log_group_name}" + +# Point the host's CloudWatch agent at the configured log group. The docker-mirror +# AMI ships syslog to a fixed "executors_docker_mirror" group by default; when a +# specific (e.g. randomized) log group name is configured we reconfigure the agent +# at boot so multiple deployments in the same account/region don't collide on one +# shared log group. +if [ "$${CLOUDWATCH_LOG_GROUP_NAME}" != '' ]; then + CLOUDWATCH_CONFIG_FILE_PATH=/opt/aws/amazon-cloudwatch-agent/etc/amazon-cloudwatch-agent.json + if [ -x /usr/local/bin/configure-cloudwatch-agent-log-group ]; then + # Newer AMIs ship a helper that owns the agent config layout. + /usr/local/bin/configure-cloudwatch-agent-log-group "$${CLOUDWATCH_LOG_GROUP_NAME}" + elif [ -x /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl ]; then + # Fallback for AMIs predating the helper: (re)write the agent config with the + # desired log group name and reload. fetch-config consumes this file into the + # agent's own managed config, so we always write it fresh rather than assume + # the build-time source file is still present. + cat <"$${CLOUDWATCH_CONFIG_FILE_PATH}" +{ + "logs": { + "logs_collected": { + "files": { + "collect_list": [ + { + "file_path": "/var/log/syslog", + "log_group_name": "$${CLOUDWATCH_LOG_GROUP_NAME}", + "timezone": "UTC" + } + ] + } + }, + "log_stream_name": "{instance_id}-syslog" + } +} +CWCONFIG + /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl -a fetch-config -m ec2 -s -c file:"$${CLOUDWATCH_CONFIG_FILE_PATH}" + fi +fi + +# Wait until the block device is attached. +while true; do + sleep 1 + test -e /dev/nvme1n1 && break +done + +# Alias the nvme devices to /dev/sdX. +# https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/device_naming.html#available-ec2-device-names +VOLUMES_NAME="$(find /dev -maxdepth 1 | grep -i 'nvme[0-21]n1$')" +for VOLUME in $${VOLUMES_NAME}; do + ALIAS=$(sudo nvme id-ctrl -H -v "$${VOLUME}" | { grep -Po '/dev/(sd[b-z]|xvd[b-z])' || test $? = 1; }) + if [ -n "$${ALIAS}" ]; then + sudo ln -s "$${VOLUME}" "$${ALIAS}" + fi +done + +REALPATH="$(realpath /dev/sdh)" +# Ensure /dev/sdh has a file system. +if [ "$(sudo file -s "$REALPATH")" == "$REALPATH: data" ]; then + sudo mkfs.ext4 /dev/sdh +fi + +# Mount /dev/sdh to /mnt/registry. +sudo mkdir -p /mnt/registry +echo "/dev/sdh /mnt/registry ext4 defaults 0 2" | sudo tee -a /etc/fstab +sudo mount -a +sudo chown ubuntu:ubuntu /mnt/registry + +# Enable and start the registry service. +sudo systemctl enable docker_registry +sudo systemctl start docker_registry diff --git a/modules/executors/main.tf b/modules/executors/main.tf index 699ad25..61930c5 100644 --- a/modules/executors/main.tf +++ b/modules/executors/main.tf @@ -11,7 +11,12 @@ locals { name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.security_group[0].hex}" : "${var.resource_prefix}SourcegraphExecutorsMetricsAccess" } cloudwatch_log_group = { - name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.cloudwatch_log_group[0].hex}" : null + # The executor AMI's CloudWatch agent ships syslog to this group. When + # randomizing, use a unique name so multiple executor fleets in the same + # account/region don't collide on one shared log group; the boot-time + # startup script reconfigures the agent to match (see startup-script.sh.tpl). + # Otherwise keep the legacy fixed name the AMI defaults to. + name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.cloudwatch_log_group[0].hex}" : "executors" } iam_instance_profile = { name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.iam_instance_profile[0].hex}" : "${local.prefix}_executors" @@ -137,8 +142,10 @@ resource "random_id" "cloudwatch_log_group" { } resource "aws_cloudwatch_log_group" "syslogs" { - # TODO: This is hardcoded in the executor image. - name = "executors" + # The executor AMI's CloudWatch agent defaults to a group literally named + # "executors". The startup script reconfigures the agent to use this name at + # boot, so randomized deployments get a unique, non-colliding log group. + name = local.cloudwatch_log_group.name retention_in_days = 7 tags = { @@ -267,6 +274,7 @@ resource "aws_launch_template" "executor" { "EXECUTOR_USE_FIRECRACKER" = var.use_firecracker "EXECUTOR_DOCKER_AUTH_CONFIG" = var.docker_auth_config "PRIVATE_CA_CERTIFICATE" = var.private_ca_cert_path != "" ? file(var.private_ca_cert_path) : "" + "CLOUDWATCH_LOG_GROUP_NAME" = local.cloudwatch_log_group.name } })) diff --git a/modules/executors/outputs.tf b/modules/executors/outputs.tf new file mode 100644 index 0000000..a6528d1 --- /dev/null +++ b/modules/executors/outputs.tf @@ -0,0 +1,4 @@ +output "cloudwatch_log_group_name" { + value = aws_cloudwatch_log_group.syslogs.name + description = "The name of the CloudWatch log group that the executor hosts ship syslog to." +} diff --git a/modules/executors/startup-script.sh.tpl b/modules/executors/startup-script.sh.tpl index 957f4f6..2110a2d 100644 --- a/modules/executors/startup-script.sh.tpl +++ b/modules/executors/startup-script.sh.tpl @@ -50,6 +50,43 @@ EXECUTOR_DOCKER_AUTH_CONFIG="$${EXECUTOR_DOCKER_AUTH_CONFIG}" $${DOCKER_REGISTRY_NODE_EXPORTER_URL_LINE} EOF +# Point the host's CloudWatch agent at the configured log group. The executor +# AMI ships syslog to a fixed "executors" group by default; when a specific +# (e.g. randomized) log group name is configured we reconfigure the agent at +# boot so multiple executor deployments in the same account/region don't collide +# on one shared log group. +if [ "$${CLOUDWATCH_LOG_GROUP_NAME}" != '' ]; then + CLOUDWATCH_CONFIG_FILE_PATH=/opt/aws/amazon-cloudwatch-agent/etc/amazon-cloudwatch-agent.json + if [ -x /usr/local/bin/configure-cloudwatch-agent-log-group ]; then + # Newer executor AMIs ship a helper that owns the agent config layout. + /usr/local/bin/configure-cloudwatch-agent-log-group "$${CLOUDWATCH_LOG_GROUP_NAME}" + elif [ -x /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl ]; then + # Fallback for AMIs predating the helper: (re)write the agent config with the + # desired log group name and reload. fetch-config consumes this file into the + # agent's own managed config, so we always write it fresh rather than assume + # the build-time source file is still present. + cat <"$${CLOUDWATCH_CONFIG_FILE_PATH}" +{ + "logs": { + "logs_collected": { + "files": { + "collect_list": [ + { + "file_path": "/var/log/syslog", + "log_group_name": "$${CLOUDWATCH_LOG_GROUP_NAME}", + "timezone": "UTC" + } + ] + } + }, + "log_stream_name": "{instance_id}-syslog" + } +} +CWCONFIG + /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl -a fetch-config -m ec2 -s -c file:"$${CLOUDWATCH_CONFIG_FILE_PATH}" + fi +fi + # Enable and start the executor service systemctl enable executor systemctl start executor From dc28fca09af6ad359c814b8be844557dbe21aa27 Mon Sep 17 00:00:00 2001 From: Michael Lin Date: Fri, 24 Jul 2026 12:46:46 -0700 Subject: [PATCH 2/2] Also avoid log group collision when randomize_resource_names=false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When not randomizing, the executor log group name fell back to the literal "executors", so two fleets distinguished only by resource_prefix still collided on it — even though every other non-randomized executor resource (IAM role, launch template, ASG, security group, alarms) already includes the prefix via local.prefix. Make the non-randomized log group name include the prefix too: - randomize_resource_names: unique via random suffix (unchanged) - resource_prefix set: "_sourcegraph_executors" (now unique) - neither: legacy "executors" (single default fleet, unchanged) Update the multiple-executors README: a distinct resource_prefix per fleet (with or without randomize_resource_names) is sufficient to avoid the collision. Co-Authored-By: Claude Opus 4.8 (1M context) --- examples/multiple-executors/README.md | 4 ++-- modules/executors/main.tf | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/examples/multiple-executors/README.md b/examples/multiple-executors/README.md index f10d4b5..3bd3fac 100644 --- a/examples/multiple-executors/README.md +++ b/examples/multiple-executors/README.md @@ -5,8 +5,8 @@ This example uses [networking](https://registry.terraform.io/modules/sourcegraph The following variables must be supplied: - `sourcegraph_external_url`, `sourcegraph_executor_proxy_password`, `queue_name`, `metrics_environment_label`, and `instance_tag`: Analogous to the `executor_*` variables in the `single-executor` example. -- `resource_prefix`: A prefix unique to each set of compute resources. This prevents collisions between two uses of the `executors` module. We recommend this value be constructed the same way `instance_tag` is constructed. -- `randomize_resource_names`: Must be `true` when running more than one `executors` (or `docker-mirror`) module in the same account and region. The executor AMI's CloudWatch agent ships host logs to a log group named `executors` by default; randomizing gives each fleet a unique log group (and reconfigures the on-host agent to match at boot) so the two deployments don't collide on that shared log group. +- `resource_prefix`: A prefix unique to each set of compute resources. This prevents collisions between two uses of the `executors` module — including the CloudWatch log group each fleet's hosts ship syslog to (the on-host agent is reconfigured at boot to match). Give each fleet a distinct `resource_prefix` (or set `randomize_resource_names = true`) so two fleets in the same account/region don't collide on a single shared log group. We recommend this value be constructed the same way `instance_tag` is constructed. +- `randomize_resource_names`: When `true`, appends a random suffix to resource names. Either this or a distinct `resource_prefix` per fleet is required to run more than one `executors` module in the same account and region. - `docker_registry_mirror`: This variable is given the value `"http://${module.docker-mirror.ip_address}:5000"`, which converts the raw external IP address to an address resolvable by the executor instances. If your deployment environment already has a Docker registry that can be used, only the `executor` submodule must be used (and references to the `networking` and `docker-mirror` modules can be dropped). The Docker registry mirror address can be supplied along with its containing VPC and subnet as pre-existing identifier literals. diff --git a/modules/executors/main.tf b/modules/executors/main.tf index 61930c5..ffc8b68 100644 --- a/modules/executors/main.tf +++ b/modules/executors/main.tf @@ -11,12 +11,15 @@ locals { name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.security_group[0].hex}" : "${var.resource_prefix}SourcegraphExecutorsMetricsAccess" } cloudwatch_log_group = { - # The executor AMI's CloudWatch agent ships syslog to this group. When - # randomizing, use a unique name so multiple executor fleets in the same - # account/region don't collide on one shared log group; the boot-time - # startup script reconfigures the agent to match (see startup-script.sh.tpl). - # Otherwise keep the legacy fixed name the AMI defaults to. - name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.cloudwatch_log_group[0].hex}" : "executors" + # The executor AMI's CloudWatch agent ships syslog to this group; the + # boot-time startup script reconfigures the agent to this name (see + # startup-script.sh.tpl). Multiple fleets in one account/region must not + # share one group, so the name carries the same prefix/random suffix as the + # other executor resources (IAM role, launch template, ASG, ...): + # - randomize_resource_names: unique per fleet via the random suffix + # - resource_prefix set: unique per fleet via the prefix + # - neither: the legacy fixed "executors" name (single fleet) + name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.cloudwatch_log_group[0].hex}" : (var.resource_prefix != "" ? "${local.prefix}executors" : "executors") } iam_instance_profile = { name = var.randomize_resource_names ? "${local.prefix}executors-${random_id.iam_instance_profile[0].hex}" : "${local.prefix}_executors"