Resolve service name conflicts when the name is computed to the same value#308
Resolve service name conflicts when the name is computed to the same value#308nikola-jokic wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR resolves service container name conflicts when multiple services use the same Docker image. Previously, services were named based solely on the image name (e.g., "redis" from "redis:8.0.3-alpine"), causing Kubernetes pod creation to fail with duplicate container name errors when multiple services used the same image.
Changes:
- Implements a two-pass naming strategy: first counting occurrences of each base name, then appending index suffixes (-0, -1, etc.) only when conflicts exist
- Adds comprehensive test coverage for the collision scenario with two services using identical images
- Preserves backward compatibility by only adding suffixes when conflicts are detected (single services retain their original names)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/k8s/tests/prepare-job-test.ts | Adds test case verifying unique names ('redis-0', 'redis-1') are generated when two services use the same image |
| packages/k8s/src/hooks/prepare-job.ts | Implements conflict detection and resolution logic using occurrence counting and index tracking, passes resolved names through to response file generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (args.services?.length) { | ||
| const occurrences = new Map<string, number>() | ||
| for (const s of args.services) { | ||
| const base = generateContainerName(s.image) | ||
| occurrences.set(base, (occurrences.get(base) || 0) + 1) | ||
| } | ||
|
|
||
| const indices = new Map<string, number>() | ||
| services = args.services.map(service => { | ||
| return createContainerSpec( | ||
| service, | ||
| generateContainerName(service.image), | ||
| false, | ||
| extension | ||
| ) | ||
| const base = generateContainerName(service.image) | ||
| const total = occurrences.get(base) || 0 | ||
| const idx = indices.get(base) || 0 | ||
|
|
||
| let name: string | ||
| if (total > 1) { | ||
| name = `${base}-${idx}` | ||
| } else { | ||
| name = base | ||
| } | ||
|
|
||
| indices.set(base, idx + 1) | ||
| serviceNames.push(name) | ||
| return createContainerSpec(service, name, false, extension) | ||
| }) |
There was a problem hiding this comment.
The conflict resolution logic doesn't account for cases where a generated name (e.g., 'redis-0') might collide with another service's base name (e.g., a service using image 'redis-0:latest'). This could still result in duplicate container names. Consider using a two-pass approach: first detect all conflicts (including potential conflicts with suffixed names), then generate unique names that avoid all collisions. Alternatively, use a different separator or suffix format that's less likely to collide with valid image names (e.g., '--0' or '.0').
1 participant
The service name is generated the same way on the runner, based on the image. The change resolves the conflict by appending the index when the conflict exists.
Let's say there are two service containers using
redisimage.The service names would be
redis-0andredis-1. Index suffixes are only added for the names with conflicts.Fixes #241