fix(deploy): mark auto-start, reload, and config-reload as internal deploy
Auto-start, manual reload, and config reload called Deploy with plain
context, so pullRefForDeploy saw IsInternalDeploy=false and skipped
the rewrite to localhost:{registryPort}. This caused pulls to go to
the public domain (port 443) instead of localhost:5000.
Wrap ctx with domain.WithInternalDeploy() in syncAndAutoStart,
ManualReloadHandler.Handle, and ConfigReloadHandler.Handle, matching
the pattern already used by ImagePushedHandler.
Fixes the reload half of #54.- commit
- ba7acd047b2ba39739193f17418ab3d290c09520
- author
- brice · · 2026-02-06 05:03 UTC
- committer
- brice · · 2026-02-06 05:07 UTC
- parent
- 1debceb
- signature
- unsigned
3 files · +46 -4
M internal/app/run.go +1 -1
| @@ -955,7 +955,7 @@ func syncAndAutoStart(ctx context.Context, svc *services, log zerowrap.Logger) { | |||
| 955 | 955 | ||
| 956 | 956 | if svc.configSvc.IsAutoRouteEnabled() { | |
| 957 | 957 | routes := svc.configSvc.GetRoutes(ctx) | |
| 958 | - | if err := svc.containerSvc.AutoStart(ctx, routes); err != nil { | |
| 958 | + | if err := svc.containerSvc.AutoStart(domain.WithInternalDeploy(ctx), routes); err != nil { | |
| 959 | 959 | log.Warn().Err(err).Msg("failed to auto-start containers") | |
| 960 | 960 | } | |
| 961 | 961 | } | |
| @@ -955,7 +955,7 @@ func syncAndAutoStart(ctx context.Context, svc *services, log zerowrap.Logger) { | |||||
| 955 | 955 | ||||
| 956 | if svc.configSvc.IsAutoRouteEnabled() { | 956 | if svc.configSvc.IsAutoRouteEnabled() { | ||
| 957 | routes := svc.configSvc.GetRoutes(ctx) | 957 | routes := svc.configSvc.GetRoutes(ctx) | ||
| 958 | - | if err := svc.containerSvc.AutoStart(ctx, routes); err != nil { | 958 | + | if err := svc.containerSvc.AutoStart(domain.WithInternalDeploy(ctx), routes); err != nil { |
| 959 | log.Warn().Err(err).Msg("failed to auto-start containers") | 959 | log.Warn().Err(err).Msg("failed to auto-start containers") | ||
| 960 | } | 960 | } | ||
| 961 | } | 961 | } | ||
M internal/usecase/container/events.go +3 -3
| @@ -159,7 +159,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||
| 159 | 159 | Str("new_image", route.Image). | |
| 160 | 160 | Msg("image changed for route, redeploying") | |
| 161 | 161 | ||
| 162 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | |
| 162 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { | |
| 163 | 163 | log.WrapErrWithFields(err, "failed to redeploy container", map[string]any{"domain": route.Domain}) | |
| 164 | 164 | } | |
| 165 | 165 | } | |
| @@ -171,7 +171,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||
| 171 | 171 | Str("image", route.Image). | |
| 172 | 172 | Msg("route missing container, deploying") | |
| 173 | 173 | ||
| 174 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | |
| 174 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { | |
| 175 | 175 | log.WrapErrWithFields(err, "failed to deploy container for route", map[string]any{"domain": route.Domain}) | |
| 176 | 176 | } | |
| 177 | 177 | } | |
| @@ -253,7 +253,7 @@ func (h *ManualReloadHandler) Handle(event domain.Event) error { | |||
| 253 | 253 | Str("image", route.Image). | |
| 254 | 254 | Msg("starting container for route") | |
| 255 | 255 | ||
| 256 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | |
| 256 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { | |
| 257 | 257 | log.WrapErrWithFields(err, "failed to start container", map[string]any{"domain": route.Domain}) | |
| 258 | 258 | errorCount++ | |
| 259 | 259 | continue | |
| @@ -159,7 +159,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||||
| 159 | Str("new_image", route.Image). | 159 | Str("new_image", route.Image). | ||
| 160 | Msg("image changed for route, redeploying") | 160 | Msg("image changed for route, redeploying") | ||
| 161 | 161 | ||||
| 162 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | 162 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { |
| 163 | log.WrapErrWithFields(err, "failed to redeploy container", map[string]any{"domain": route.Domain}) | 163 | log.WrapErrWithFields(err, "failed to redeploy container", map[string]any{"domain": route.Domain}) | ||
| 164 | } | 164 | } | ||
| 165 | } | 165 | } | ||
| @@ -171,7 +171,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||||
| 171 | Str("image", route.Image). | 171 | Str("image", route.Image). | ||
| 172 | Msg("route missing container, deploying") | 172 | Msg("route missing container, deploying") | ||
| 173 | 173 | ||||
| 174 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | 174 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { |
| 175 | log.WrapErrWithFields(err, "failed to deploy container for route", map[string]any{"domain": route.Domain}) | 175 | log.WrapErrWithFields(err, "failed to deploy container for route", map[string]any{"domain": route.Domain}) | ||
| 176 | } | 176 | } | ||
| 177 | } | 177 | } | ||
| @@ -253,7 +253,7 @@ func (h *ManualReloadHandler) Handle(event domain.Event) error { | |||||
| 253 | Str("image", route.Image). | 253 | Str("image", route.Image). | ||
| 254 | Msg("starting container for route") | 254 | Msg("starting container for route") | ||
| 255 | 255 | ||||
| 256 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | 256 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { |
| 257 | log.WrapErrWithFields(err, "failed to start container", map[string]any{"domain": route.Domain}) | 257 | log.WrapErrWithFields(err, "failed to start container", map[string]any{"domain": route.Domain}) | ||
| 258 | errorCount++ | 258 | errorCount++ | ||
| 259 | continue | 259 | continue | ||
M internal/usecase/container/service_test.go +42 -0
| @@ -1137,6 +1137,48 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||
| 1137 | 1137 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | |
| 1138 | 1138 | } | |
| 1139 | 1139 | ||
| 1140 | + | func TestService_AutoStart_UsesInternalDeployContext(t *testing.T) { | |
| 1141 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 1142 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 1143 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 1144 | + | ||
| 1145 | + | config := Config{ | |
| 1146 | + | RegistryDomain: "reg.example.com", | |
| 1147 | + | RegistryPort: 5000, | |
| 1148 | + | ReadinessDelay: time.Millisecond, | |
| 1149 | + | } | |
| 1150 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 1151 | + | ||
| 1152 | + | // Mark context as internal deploy — this is what syncAndAutoStart should do | |
| 1153 | + | ctx := domain.WithInternalDeploy(testContext()) | |
| 1154 | + | ||
| 1155 | + | routes := []domain.Route{ | |
| 1156 | + | {Domain: "app1.example.com", Image: "reg.example.com/myapp:latest"}, | |
| 1157 | + | } | |
| 1158 | + | ||
| 1159 | + | // Key assertion: PullImage should be called with localhost:5000 rewrite, | |
| 1160 | + | // NOT the original reg.example.com/myapp:latest | |
| 1161 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |
| 1162 | + | runtime.EXPECT().PullImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |
| 1163 | + | runtime.EXPECT().TagImage(mock.Anything, "localhost:5000/myapp:latest", "reg.example.com/myapp:latest").Return(nil) | |
| 1164 | + | runtime.EXPECT().UntagImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |
| 1165 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "reg.example.com/myapp:latest").Return([]int{8080}, nil) | |
| 1166 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "app1.example.com").Return([]string{}, nil) | |
| 1167 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "reg.example.com/myapp:latest").Return([]string{}, nil) | |
| 1168 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(&domain.Container{ | |
| 1169 | + | ID: "container-1", Name: "gordon-app1.example.com", Status: "created", | |
| 1170 | + | }, nil) | |
| 1171 | + | runtime.EXPECT().StartContainer(mock.Anything, "container-1").Return(nil) | |
| 1172 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "container-1").Return(true, nil).Times(2) | |
| 1173 | + | runtime.EXPECT().InspectContainer(mock.Anything, "container-1").Return(&domain.Container{ | |
| 1174 | + | Status: "running", | |
| 1175 | + | }, nil) | |
| 1176 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 1177 | + | ||
| 1178 | + | err := svc.AutoStart(ctx, routes) | |
| 1179 | + | assert.NoError(t, err) | |
| 1180 | + | } | |
| 1181 | + | ||
| 1140 | 1182 | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | |
| 1141 | 1183 | // does not remove the currently tracked container during zero-downtime deployment. | |
| 1142 | 1184 | // This is critical for preventing downtime - the old container must stay running | |
| @@ -1137,6 +1137,48 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||||
| 1137 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | 1137 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | ||
| 1138 | } | 1138 | } | ||
| 1139 | 1139 | ||||
| 1140 | + | func TestService_AutoStart_UsesInternalDeployContext(t *testing.T) { | |||
| 1141 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 1142 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 1143 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 1144 | + | ||||
| 1145 | + | config := Config{ | |||
| 1146 | + | RegistryDomain: "reg.example.com", | |||
| 1147 | + | RegistryPort: 5000, | |||
| 1148 | + | ReadinessDelay: time.Millisecond, | |||
| 1149 | + | } | |||
| 1150 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 1151 | + | ||||
| 1152 | + | // Mark context as internal deploy — this is what syncAndAutoStart should do | |||
| 1153 | + | ctx := domain.WithInternalDeploy(testContext()) | |||
| 1154 | + | ||||
| 1155 | + | routes := []domain.Route{ | |||
| 1156 | + | {Domain: "app1.example.com", Image: "reg.example.com/myapp:latest"}, | |||
| 1157 | + | } | |||
| 1158 | + | ||||
| 1159 | + | // Key assertion: PullImage should be called with localhost:5000 rewrite, | |||
| 1160 | + | // NOT the original reg.example.com/myapp:latest | |||
| 1161 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |||
| 1162 | + | runtime.EXPECT().PullImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |||
| 1163 | + | runtime.EXPECT().TagImage(mock.Anything, "localhost:5000/myapp:latest", "reg.example.com/myapp:latest").Return(nil) | |||
| 1164 | + | runtime.EXPECT().UntagImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |||
| 1165 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "reg.example.com/myapp:latest").Return([]int{8080}, nil) | |||
| 1166 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "app1.example.com").Return([]string{}, nil) | |||
| 1167 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "reg.example.com/myapp:latest").Return([]string{}, nil) | |||
| 1168 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(&domain.Container{ | |||
| 1169 | + | ID: "container-1", Name: "gordon-app1.example.com", Status: "created", | |||
| 1170 | + | }, nil) | |||
| 1171 | + | runtime.EXPECT().StartContainer(mock.Anything, "container-1").Return(nil) | |||
| 1172 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "container-1").Return(true, nil).Times(2) | |||
| 1173 | + | runtime.EXPECT().InspectContainer(mock.Anything, "container-1").Return(&domain.Container{ | |||
| 1174 | + | Status: "running", | |||
| 1175 | + | }, nil) | |||
| 1176 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 1177 | + | ||||
| 1178 | + | err := svc.AutoStart(ctx, routes) | |||
| 1179 | + | assert.NoError(t, err) | |||
| 1180 | + | } | |||
| 1181 | + | ||||
| 1140 | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | 1182 | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | ||
| 1141 | // does not remove the currently tracked container during zero-downtime deployment. | 1183 | // does not remove the currently tracked container during zero-downtime deployment. | ||
| 1142 | // This is critical for preventing downtime - the old container must stay running | 1184 | // This is critical for preventing downtime - the old container must stay running | ||