From 2207a7bce33a3dd8e8ab7059f1eee98437499970 Mon Sep 17 00:00:00 2001 From: nfebe Date: Wed, 18 Mar 2026 01:03:19 +0100 Subject: [PATCH] fix(docker): Set explicit container_name on all services EnsureContainerName only named the primary service, leaving others with Docker Compose auto-generated names. This caused cron jobs and backups to fail when targeting secondary services using the {deploymentName}-{serviceName} convention. EnsureContainerNames now sets container_name on every service: primary gets {deploymentName}, others get {deploymentName}-{service}. Existing container_name values are preserved. The compose file is patched automatically on start/restart/rebuild, so existing deployments pick up correct names on next restart. Closes #82 Signed-off-by: nfebe --- internal/api/server.go | 2 +- internal/docker/compose_yaml.go | 42 ++++++++++++++++---------- internal/docker/compose_yaml_test.go | 45 +++++++++++++++++----------- internal/docker/manager.go | 23 ++++++++++++++ 4 files changed, 77 insertions(+), 35 deletions(-) diff --git a/internal/api/server.go b/internal/api/server.go index a6b0862..d0a24fc 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -670,7 +670,7 @@ func (s *Server) createDeployment(c *gin.Context) { } // Ensure container name is set for proxy DNS resolution - if content, err := docker.EnsureContainerName(req.ComposeContent, req.Name); err == nil { + if content, err := docker.EnsureContainerNames(req.ComposeContent, req.Name); err == nil { req.ComposeContent = content } diff --git a/internal/docker/compose_yaml.go b/internal/docker/compose_yaml.go index a2a00cd..af1c211 100644 --- a/internal/docker/compose_yaml.go +++ b/internal/docker/compose_yaml.go @@ -2,6 +2,7 @@ package docker import ( "bytes" + "fmt" "strings" "gopkg.in/yaml.v3" @@ -156,9 +157,11 @@ func ParseComposeYAML(content string) (map[string]interface{}, error) { return compose, nil } -// EnsureContainerName ensures the first service has container_name set to deploymentName. -// This is required for nginx reverse proxy DNS resolution on shared networks. -func EnsureContainerName(content string, deploymentName string) (string, error) { +// EnsureContainerNames ensures all services have explicit container_name set. +// The primary service (preferring "app") gets deploymentName as its container name. +// All other services get "{deploymentName}-{serviceName}". +// Existing container_name values are preserved. +func EnsureContainerNames(content string, deploymentName string) (string, error) { if deploymentName == "" { return content, nil } @@ -173,27 +176,34 @@ func EnsureContainerName(content string, deploymentName string) (string, error) return content, nil } - // Find the first service (prefer "app" if exists, otherwise take any) - var targetService string + // Find the primary service (prefer "app" if exists, otherwise take first) + var primaryService string for name := range services { if name == "app" { - targetService = "app" + primaryService = "app" break } - if targetService == "" { - targetService = name + if primaryService == "" { + primaryService = name } } - service, ok := services[targetService].(map[string]interface{}) - if !ok { - return content, nil - } + for name := range services { + service, ok := services[name].(map[string]interface{}) + if !ok { + continue + } - // Only set if not already specified - if _, hasContainerName := service["container_name"]; !hasContainerName { - service["container_name"] = deploymentName - services[targetService] = service + if _, hasContainerName := service["container_name"]; hasContainerName { + continue + } + + if name == primaryService { + service["container_name"] = deploymentName + } else { + service["container_name"] = fmt.Sprintf("%s-%s", deploymentName, name) + } + services[name] = service } result, err := MarshalComposeYAML(compose) diff --git a/internal/docker/compose_yaml_test.go b/internal/docker/compose_yaml_test.go index 8082fdd..194497e 100644 --- a/internal/docker/compose_yaml_test.go +++ b/internal/docker/compose_yaml_test.go @@ -506,13 +506,13 @@ func TestMarshalComposeYAML_UnknownServiceKeys(t *testing.T) { } } -func TestEnsureContainerName_AddsToAppService(t *testing.T) { +func TestEnsureContainerNames_AddsToAppService(t *testing.T) { input := `name: myapp services: app: image: nginx ` - result, err := EnsureContainerName(input, "my-deployment") + result, err := EnsureContainerNames(input, "my-deployment") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -522,15 +522,17 @@ services: } } -func TestEnsureContainerName_PrefersAppService(t *testing.T) { +func TestEnsureContainerNames_NamesAllServices(t *testing.T) { input := `name: myapp services: web: image: nginx app: image: node + db: + image: postgres ` - result, err := EnsureContainerName(input, "my-deployment") + result, err := EnsureContainerNames(input, "my-deployment") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -543,22 +545,26 @@ services: services := compose["services"].(map[string]interface{}) appService := services["app"].(map[string]interface{}) webService := services["web"].(map[string]interface{}) + dbService := services["db"].(map[string]interface{}) if appService["container_name"] != "my-deployment" { - t.Errorf("app service should have container_name, got: %v", appService["container_name"]) + t.Errorf("app service should have container_name 'my-deployment', got: %v", appService["container_name"]) + } + if webService["container_name"] != "my-deployment-web" { + t.Errorf("web service should have container_name 'my-deployment-web', got: %v", webService["container_name"]) } - if _, hasName := webService["container_name"]; hasName { - t.Errorf("web service should not have container_name") + if dbService["container_name"] != "my-deployment-db" { + t.Errorf("db service should have container_name 'my-deployment-db', got: %v", dbService["container_name"]) } } -func TestEnsureContainerName_FallsBackToFirstService(t *testing.T) { +func TestEnsureContainerNames_FallsBackToFirstService(t *testing.T) { input := `name: myapp services: web: image: nginx ` - result, err := EnsureContainerName(input, "my-deployment") + result, err := EnsureContainerNames(input, "my-deployment") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -568,33 +574,36 @@ services: } } -func TestEnsureContainerName_PreservesExisting(t *testing.T) { +func TestEnsureContainerNames_PreservesExisting(t *testing.T) { input := `name: myapp services: app: image: nginx container_name: custom-name + db: + image: postgres + container_name: custom-db ` - result, err := EnsureContainerName(input, "my-deployment") + result, err := EnsureContainerNames(input, "my-deployment") if err != nil { t.Fatalf("unexpected error: %v", err) } if !strings.Contains(result, "container_name: custom-name") { - t.Errorf("should preserve existing container_name, got:\n%s", result) + t.Errorf("should preserve existing container_name on app, got:\n%s", result) } - if strings.Contains(result, "container_name: my-deployment") { - t.Errorf("should not override existing container_name") + if !strings.Contains(result, "container_name: custom-db") { + t.Errorf("should preserve existing container_name on db, got:\n%s", result) } } -func TestEnsureContainerName_EmptyName(t *testing.T) { +func TestEnsureContainerNames_EmptyName(t *testing.T) { input := `name: myapp services: app: image: nginx ` - result, err := EnsureContainerName(input, "") + result, err := EnsureContainerNames(input, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -604,10 +613,10 @@ services: } } -func TestEnsureContainerName_NoServices(t *testing.T) { +func TestEnsureContainerNames_NoServices(t *testing.T) { input := `name: myapp ` - result, err := EnsureContainerName(input, "my-deployment") + result, err := EnsureContainerNames(input, "my-deployment") if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/docker/manager.go b/internal/docker/manager.go index 9c61053..669a926 100644 --- a/internal/docker/manager.go +++ b/internal/docker/manager.go @@ -3,6 +3,7 @@ package docker import ( "encoding/json" "fmt" + "os" "path/filepath" "strings" "sync" @@ -142,6 +143,22 @@ func (m *Manager) DeleteDeployment(name string) error { return m.discovery.DeleteDeployment(name) } +// ensureContainerNames patches the compose file to set explicit container_name on all services. +func (m *Manager) ensureContainerNames(name string) { + content, filename, err := m.discovery.GetComposeFile(name) + if err != nil || content == "" { + return + } + + updated, err := EnsureContainerNames(content, name) + if err != nil || updated == content { + return + } + + composePath := filepath.Join(m.basePath, name, filename) + _ = os.WriteFile(composePath, []byte(updated), 0644) +} + func (m *Manager) StartDeployment(name string) (string, error) { m.mu.RLock() deployment, err := m.discovery.GetDeployment(name) @@ -151,6 +168,8 @@ func (m *Manager) StartDeployment(name string) (string, error) { return "", err } + m.ensureContainerNames(name) + output, err := m.executor.Up(deployment.Path) if err != nil { return output, err @@ -258,6 +277,8 @@ func (m *Manager) RestartDeployment(name string) (string, error) { return "", err } + m.ensureContainerNames(name) + return m.executor.Restart(deployment.Path) } @@ -270,6 +291,8 @@ func (m *Manager) RebuildDeployment(name string) (string, error) { return "", err } + m.ensureContainerNames(name) + output, err := m.executor.Rebuild(deployment.Path) if err != nil { return output, err