Fix CloudWatch log group collision for multiple executor fleets - #211
Open
michaellzc wants to merge 2 commits into
Open
Fix CloudWatch log group collision for multiple executor fleets#211michaellzc wants to merge 2 commits into
michaellzc wants to merge 2 commits into
Conversation
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) <noreply@anthropic.com>
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: "<prefix>_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) <noreply@anthropic.com>
DaedalusG
approved these changes
Jul 25, 2026
Contributor
|
@michaellzc these terraform repos don't have a "reease branch" concept since they only get releases during a minor/major sourcegraph release and theres currently no concept of a patch release in these repos |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The executor (and docker-mirror) AMIs' CloudWatch agent ships host
/var/log/syslogto a hard-coded log group —executors(andexecutors_docker_mirror). Running two executor fleets in one AWS account/region therefore collides on a single shared log group: the secondterraform applyfails withResourceAlreadyExistsException. This is the exact scenario inexamples/multiple-executors(e.g. code-intel + batches).randomize_resource_namesdidn't help — it only changed the resource'stags.Name, never the real log groupnameor the on-host agent's target.Fix (end-to-end name parameterization)
aws_cloudwatch_log_group.syslogs.namenow uses the resolved name — randomized (unique) whenrandomize_resource_names = true, else the legacy fixed name for backward compatibility — in bothmodules/executorsandmodules/docker-mirror.user_data, and the startup script reconfigures the on-host CloudWatch agent at boot to write to that group:/usr/local/bin/configure-cloudwatch-agent-log-groupwhen present (added in the matching AMI change).fetch-config(works with existing/older AMIs). The fallback is gated on the agent binary, becausefetch-configconsumes the source config file at build time so it isn't present on running instances.modules/docker-mirror/startup-script.sh→startup-script.sh.tpl(templatefile) to receive the name.cloudwatch_log_group_nameoutputs to both modules and documented therandomize_resource_names = truerequirement for multi-fleet deployments.Compatibility
randomize_resource_names = false) keeps the legacyexecutors/executors_docker_mirrornames → no replacement for existing single-fleet deployments.Testing
Validated e2e on a real AWS account (two fleets + shared docker-mirror, all
randomize_resource_names = true):terraform applytogether with no collision.{instance_id}-syslog.Companion AMI change: sourcegraph/sourcegraph (parameterized
configure-cloudwatch-agent-log-grouphelper).🤖 Generated with Claude Code