fix(container): skip redundant deploy when container already runs the same image
- commit
- 558622b4d113aa93c6dbff30d03f4bfea643272b
- author
- bnema · · 2026-02-09 20:41 UTC
- parent
- f5548f9
- signature
- unsigned
2 files · +270 -4
M internal/usecase/container/service.go +42 -0
| @@ -239,6 +239,16 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||
| 239 | 239 | return nil, err | |
| 240 | 240 | } | |
| 241 | 241 | ||
| 242 | + | // Skip redundant deploy: if the existing container is already running | |
| 243 | + | // the exact same image (by Docker image ID), return it immediately. | |
| 244 | + | // This prevents the double-deploy caused by the event-based deploy | |
| 245 | + | // (triggered by image.pushed) racing with the explicit CLI deploy call. | |
| 246 | + | if hasExisting && existing.ImageID != "" { | |
| 247 | + | if skip, container := s.skipRedundantDeploy(ctx, existing, resources.actualImageRef); skip { | |
| 248 | + | return container, nil | |
| 249 | + | } | |
| 250 | + | } | |
| 251 | + | ||
| 242 | 252 | newContainer, err := s.createStartedContainer(ctx, route, existing, resources) | |
| 243 | 253 | if err != nil { | |
| 244 | 254 | return nil, err | |
| @@ -261,6 +271,38 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||
| 261 | 271 | return newContainer, nil | |
| 262 | 272 | } | |
| 263 | 273 | ||
| 274 | + | // skipRedundantDeploy checks whether the existing container is already running | |
| 275 | + | // the same image (by Docker image ID) as the one we are about to deploy. | |
| 276 | + | // When a push triggers both an event-based deploy and an explicit CLI deploy, | |
| 277 | + | // the second one arrives after the first has already completed; this avoids | |
| 278 | + | // a full redundant create-start-readiness cycle. | |
| 279 | + | func (s *Service) skipRedundantDeploy(ctx context.Context, existing *domain.Container, actualImageRef string) (bool, *domain.Container) { | |
| 280 | + | log := zerowrap.FromCtx(ctx) | |
| 281 | + | ||
| 282 | + | newImageID, err := s.runtime.GetImageID(ctx, actualImageRef) | |
| 283 | + | if err != nil { | |
| 284 | + | log.Debug().Err(err).Msg("cannot resolve image ID for redundancy check, proceeding with deploy") | |
| 285 | + | return false, nil | |
| 286 | + | } | |
| 287 | + | ||
| 288 | + | if existing.ImageID != newImageID { | |
| 289 | + | return false, nil | |
| 290 | + | } | |
| 291 | + | ||
| 292 | + | // Verify the container is actually still running before skipping. | |
| 293 | + | running, err := s.runtime.IsContainerRunning(ctx, existing.ID) | |
| 294 | + | if err != nil || !running { | |
| 295 | + | return false, nil | |
| 296 | + | } | |
| 297 | + | ||
| 298 | + | log.Info(). | |
| 299 | + | Str("container_id", existing.ID). | |
| 300 | + | Str("image_id", newImageID). | |
| 301 | + | Msg("skipping redundant deploy: container already running this image") | |
| 302 | + | ||
| 303 | + | return true, existing | |
| 304 | + | } | |
| 305 | + | ||
| 264 | 306 | type deployResources struct { | |
| 265 | 307 | networkName string | |
| 266 | 308 | actualImageRef string | |
| @@ -239,6 +239,16 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||||
| 239 | return nil, err | 239 | return nil, err | ||
| 240 | } | 240 | } | ||
| 241 | 241 | ||||
| 242 | + | // Skip redundant deploy: if the existing container is already running | |||
| 243 | + | // the exact same image (by Docker image ID), return it immediately. | |||
| 244 | + | // This prevents the double-deploy caused by the event-based deploy | |||
| 245 | + | // (triggered by image.pushed) racing with the explicit CLI deploy call. | |||
| 246 | + | if hasExisting && existing.ImageID != "" { | |||
| 247 | + | if skip, container := s.skipRedundantDeploy(ctx, existing, resources.actualImageRef); skip { | |||
| 248 | + | return container, nil | |||
| 249 | + | } | |||
| 250 | + | } | |||
| 251 | + | ||||
| 242 | newContainer, err := s.createStartedContainer(ctx, route, existing, resources) | 252 | newContainer, err := s.createStartedContainer(ctx, route, existing, resources) | ||
| 243 | if err != nil { | 253 | if err != nil { | ||
| 244 | return nil, err | 254 | return nil, err | ||
| @@ -261,6 +271,38 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||||
| 261 | return newContainer, nil | 271 | return newContainer, nil | ||
| 262 | } | 272 | } | ||
| 263 | 273 | ||||
| 274 | + | // skipRedundantDeploy checks whether the existing container is already running | |||
| 275 | + | // the same image (by Docker image ID) as the one we are about to deploy. | |||
| 276 | + | // When a push triggers both an event-based deploy and an explicit CLI deploy, | |||
| 277 | + | // the second one arrives after the first has already completed; this avoids | |||
| 278 | + | // a full redundant create-start-readiness cycle. | |||
| 279 | + | func (s *Service) skipRedundantDeploy(ctx context.Context, existing *domain.Container, actualImageRef string) (bool, *domain.Container) { | |||
| 280 | + | log := zerowrap.FromCtx(ctx) | |||
| 281 | + | ||||
| 282 | + | newImageID, err := s.runtime.GetImageID(ctx, actualImageRef) | |||
| 283 | + | if err != nil { | |||
| 284 | + | log.Debug().Err(err).Msg("cannot resolve image ID for redundancy check, proceeding with deploy") | |||
| 285 | + | return false, nil | |||
| 286 | + | } | |||
| 287 | + | ||||
| 288 | + | if existing.ImageID != newImageID { | |||
| 289 | + | return false, nil | |||
| 290 | + | } | |||
| 291 | + | ||||
| 292 | + | // Verify the container is actually still running before skipping. | |||
| 293 | + | running, err := s.runtime.IsContainerRunning(ctx, existing.ID) | |||
| 294 | + | if err != nil || !running { | |||
| 295 | + | return false, nil | |||
| 296 | + | } | |||
| 297 | + | ||||
| 298 | + | log.Info(). | |||
| 299 | + | Str("container_id", existing.ID). | |||
| 300 | + | Str("image_id", newImageID). | |||
| 301 | + | Msg("skipping redundant deploy: container already running this image") | |||
| 302 | + | ||||
| 303 | + | return true, existing | |||
| 304 | + | } | |||
| 305 | + | ||||
| 264 | type deployResources struct { | 306 | type deployResources struct { | ||
| 265 | networkName string | 307 | networkName string | ||
| 266 | actualImageRef string | 308 | actualImageRef string | ||
M internal/usecase/container/service_test.go +228 -4
| @@ -313,11 +313,64 @@ func TestService_Deploy_ReplacesExistingContainer(t *testing.T) { | |||
| 313 | 313 | ||
| 314 | 314 | assert.NoError(t, err) | |
| 315 | 315 | assert.Equal(t, "new-container", result.ID) | |
| 316 | + | } | |
| 316 | 317 | ||
| 317 | - | // Old container should be replaced | |
| 318 | - | tracked, exists := svc.Get(ctx, "test.example.com") | |
| 319 | - | assert.True(t, exists) | |
| 320 | - | assert.Equal(t, "new-container", tracked.ID) | |
| 318 | + | func TestService_Deploy_SkipRedundantDeploy_GetImageIDError(t *testing.T) { | |
| 319 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 320 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 321 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 322 | + | ||
| 323 | + | config := Config{ | |
| 324 | + | ReadinessDelay: time.Millisecond, | |
| 325 | + | DrainDelay: time.Millisecond, | |
| 326 | + | } | |
| 327 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 328 | + | ctx := testContext() | |
| 329 | + | ||
| 330 | + | // Existing container has ImageID, but GetImageID will fail | |
| 331 | + | existingContainer := &domain.Container{ | |
| 332 | + | ID: "existing-container", | |
| 333 | + | Name: "gordon-test.example.com", | |
| 334 | + | Image: "myapp:latest", | |
| 335 | + | ImageID: "sha256:abc123", | |
| 336 | + | Status: "running", | |
| 337 | + | } | |
| 338 | + | svc.containers["test.example.com"] = existingContainer | |
| 339 | + | ||
| 340 | + | route := domain.Route{ | |
| 341 | + | Domain: "test.example.com", | |
| 342 | + | Image: "myapp:latest", | |
| 343 | + | } | |
| 344 | + | ||
| 345 | + | // prepareDeployResources | |
| 346 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |
| 347 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |
| 348 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |
| 349 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 350 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |
| 351 | + | ||
| 352 | + | // GetImageID fails => deploy proceeds normally (graceful degradation) | |
| 353 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("", errors.New("image inspect failed")) | |
| 354 | + | ||
| 355 | + | // Full deploy proceeds despite the error | |
| 356 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |
| 357 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(newContainer, nil) | |
| 358 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |
| 359 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |
| 360 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |
| 361 | + | ID: "new-container", Status: "running", | |
| 362 | + | }, nil) | |
| 363 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 364 | + | ||
| 365 | + | // Old container is finalized | |
| 366 | + | runtime.EXPECT().StopContainer(mock.Anything, "existing-container").Return(nil) | |
| 367 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "existing-container", true).Return(nil) | |
| 368 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |
| 369 | + | ||
| 370 | + | result, err := svc.Deploy(ctx, route) | |
| 371 | + | ||
| 372 | + | assert.NoError(t, err) | |
| 373 | + | assert.Equal(t, "new-container", result.ID) | |
| 321 | 374 | } | |
| 322 | 375 | ||
| 323 | 376 | func TestService_Deploy_WithNetworkIsolation(t *testing.T) { | |
| @@ -2003,3 +2056,174 @@ func TestService_Deploy_NilCacheInvalidator(t *testing.T) { | |||
| 2003 | 2056 | assert.NoError(t, err) | |
| 2004 | 2057 | assert.Equal(t, "new-container", result.ID) | |
| 2005 | 2058 | } | |
| 2059 | + | ||
| 2060 | + | func TestService_Deploy_SkipsRedundantDeploy(t *testing.T) { | |
| 2061 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 2062 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 2063 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 2064 | + | ||
| 2065 | + | config := Config{ | |
| 2066 | + | ReadinessDelay: time.Millisecond, | |
| 2067 | + | DrainDelay: time.Millisecond, | |
| 2068 | + | } | |
| 2069 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 2070 | + | ctx := testContext() | |
| 2071 | + | ||
| 2072 | + | // Pre-populate with existing container that has an ImageID (set by InspectContainer). | |
| 2073 | + | // This simulates the first deploy (from image.pushed event) having already completed. | |
| 2074 | + | existingContainer := &domain.Container{ | |
| 2075 | + | ID: "existing-container", | |
| 2076 | + | Name: "gordon-test.example.com", | |
| 2077 | + | Image: "myapp:latest", | |
| 2078 | + | ImageID: "sha256:abc123", | |
| 2079 | + | Status: "running", | |
| 2080 | + | } | |
| 2081 | + | svc.containers["test.example.com"] = existingContainer | |
| 2082 | + | ||
| 2083 | + | route := domain.Route{ | |
| 2084 | + | Domain: "test.example.com", | |
| 2085 | + | Image: "myapp:latest", | |
| 2086 | + | } | |
| 2087 | + | ||
| 2088 | + | // prepareDeployResources will run: orphan cleanup, image pull, etc. | |
| 2089 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |
| 2090 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |
| 2091 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |
| 2092 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 2093 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |
| 2094 | + | ||
| 2095 | + | // skipRedundantDeploy: resolve image ID and compare with existing container | |
| 2096 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:abc123", nil) | |
| 2097 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "existing-container").Return(true, nil) | |
| 2098 | + | ||
| 2099 | + | // No CreateContainer, StartContainer, or readiness calls should happen | |
| 2100 | + | ||
| 2101 | + | result, err := svc.Deploy(ctx, route) | |
| 2102 | + | ||
| 2103 | + | assert.NoError(t, err) | |
| 2104 | + | assert.NotNil(t, result) | |
| 2105 | + | assert.Equal(t, "existing-container", result.ID) | |
| 2106 | + | ||
| 2107 | + | // Container should still be tracked | |
| 2108 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |
| 2109 | + | assert.True(t, exists) | |
| 2110 | + | assert.Equal(t, "existing-container", tracked.ID) | |
| 2111 | + | } | |
| 2112 | + | ||
| 2113 | + | func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) { | |
| 2114 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 2115 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 2116 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 2117 | + | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | |
| 2118 | + | ||
| 2119 | + | config := Config{ | |
| 2120 | + | ReadinessDelay: time.Millisecond, | |
| 2121 | + | DrainDelay: time.Millisecond, | |
| 2122 | + | } | |
| 2123 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 2124 | + | svc.SetProxyCacheInvalidator(cacheInvalidator) | |
| 2125 | + | ctx := testContext() | |
| 2126 | + | ||
| 2127 | + | // Existing container with a DIFFERENT image ID than what's being deployed | |
| 2128 | + | existingContainer := &domain.Container{ | |
| 2129 | + | ID: "old-container", | |
| 2130 | + | Name: "gordon-test.example.com", | |
| 2131 | + | Image: "myapp:latest", | |
| 2132 | + | ImageID: "sha256:old-image", | |
| 2133 | + | Status: "running", | |
| 2134 | + | } | |
| 2135 | + | svc.containers["test.example.com"] = existingContainer | |
| 2136 | + | ||
| 2137 | + | route := domain.Route{ | |
| 2138 | + | Domain: "test.example.com", | |
| 2139 | + | Image: "myapp:latest", | |
| 2140 | + | } | |
| 2141 | + | ||
| 2142 | + | // prepareDeployResources | |
| 2143 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |
| 2144 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |
| 2145 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |
| 2146 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 2147 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |
| 2148 | + | ||
| 2149 | + | // skipRedundantDeploy: image IDs differ, so deploy proceeds | |
| 2150 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:new-image", nil) | |
| 2151 | + | ||
| 2152 | + | // Full deploy proceeds | |
| 2153 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |
| 2154 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(newContainer, nil) | |
| 2155 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |
| 2156 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |
| 2157 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |
| 2158 | + | ID: "new-container", Status: "running", | |
| 2159 | + | }, nil) | |
| 2160 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 2161 | + | cacheInvalidator.EXPECT().InvalidateTarget(mock.Anything, "test.example.com").Return() | |
| 2162 | + | runtime.EXPECT().StopContainer(mock.Anything, "old-container").Return(nil) | |
| 2163 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "old-container", true).Return(nil) | |
| 2164 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |
| 2165 | + | ||
| 2166 | + | result, err := svc.Deploy(ctx, route) | |
| 2167 | + | ||
| 2168 | + | assert.NoError(t, err) | |
| 2169 | + | assert.Equal(t, "new-container", result.ID) | |
| 2170 | + | } | |
| 2171 | + | ||
| 2172 | + | func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) { | |
| 2173 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 2174 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 2175 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 2176 | + | ||
| 2177 | + | config := Config{ | |
| 2178 | + | ReadinessDelay: time.Millisecond, | |
| 2179 | + | DrainDelay: time.Millisecond, | |
| 2180 | + | } | |
| 2181 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 2182 | + | ctx := testContext() | |
| 2183 | + | ||
| 2184 | + | // Existing container has same image ID but is NOT running (crashed) | |
| 2185 | + | existingContainer := &domain.Container{ | |
| 2186 | + | ID: "existing-container", | |
| 2187 | + | Name: "gordon-test.example.com", | |
| 2188 | + | Image: "myapp:latest", | |
| 2189 | + | ImageID: "sha256:abc123", | |
| 2190 | + | Status: "exited", | |
| 2191 | + | } | |
| 2192 | + | svc.containers["test.example.com"] = existingContainer | |
| 2193 | + | ||
| 2194 | + | route := domain.Route{ | |
| 2195 | + | Domain: "test.example.com", | |
| 2196 | + | Image: "myapp:latest", | |
| 2197 | + | } | |
| 2198 | + | ||
| 2199 | + | // prepareDeployResources | |
| 2200 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |
| 2201 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |
| 2202 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |
| 2203 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 2204 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |
| 2205 | + | ||
| 2206 | + | // skipRedundantDeploy: same image ID but container not running => proceed with deploy | |
| 2207 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:abc123", nil) | |
| 2208 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "existing-container").Return(false, nil) | |
| 2209 | + | ||
| 2210 | + | // Full deploy proceeds (replaces crashed container) | |
| 2211 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |
| 2212 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(newContainer, nil) | |
| 2213 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |
| 2214 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |
| 2215 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |
| 2216 | + | ID: "new-container", Status: "running", | |
| 2217 | + | }, nil) | |
| 2218 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 2219 | + | ||
| 2220 | + | // Old (exited) container is still finalized | |
| 2221 | + | runtime.EXPECT().StopContainer(mock.Anything, "existing-container").Return(nil) | |
| 2222 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "existing-container", true).Return(nil) | |
| 2223 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |
| 2224 | + | ||
| 2225 | + | result, err := svc.Deploy(ctx, route) | |
| 2226 | + | ||
| 2227 | + | assert.NoError(t, err) | |
| 2228 | + | assert.Equal(t, "new-container", result.ID) | |
| 2229 | + | } | |
| @@ -313,11 +313,64 @@ func TestService_Deploy_ReplacesExistingContainer(t *testing.T) { | |||||
| 313 | 313 | ||||
| 314 | assert.NoError(t, err) | 314 | assert.NoError(t, err) | ||
| 315 | assert.Equal(t, "new-container", result.ID) | 315 | assert.Equal(t, "new-container", result.ID) | ||
| 316 | + | } | |||
| 316 | 317 | ||||
| 317 | - | // Old container should be replaced | 318 | + | func TestService_Deploy_SkipRedundantDeploy_GetImageIDError(t *testing.T) { |
| 318 | - | tracked, exists := svc.Get(ctx, "test.example.com") | 319 | + | runtime := mocks.NewMockContainerRuntime(t) |
| 319 | - | assert.True(t, exists) | 320 | + | envLoader := mocks.NewMockEnvLoader(t) |
| 320 | - | assert.Equal(t, "new-container", tracked.ID) | 321 | + | eventBus := mocks.NewMockEventPublisher(t) |
| 322 | + | ||||
| 323 | + | config := Config{ | |||
| 324 | + | ReadinessDelay: time.Millisecond, | |||
| 325 | + | DrainDelay: time.Millisecond, | |||
| 326 | + | } | |||
| 327 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 328 | + | ctx := testContext() | |||
| 329 | + | ||||
| 330 | + | // Existing container has ImageID, but GetImageID will fail | |||
| 331 | + | existingContainer := &domain.Container{ | |||
| 332 | + | ID: "existing-container", | |||
| 333 | + | Name: "gordon-test.example.com", | |||
| 334 | + | Image: "myapp:latest", | |||
| 335 | + | ImageID: "sha256:abc123", | |||
| 336 | + | Status: "running", | |||
| 337 | + | } | |||
| 338 | + | svc.containers["test.example.com"] = existingContainer | |||
| 339 | + | ||||
| 340 | + | route := domain.Route{ | |||
| 341 | + | Domain: "test.example.com", | |||
| 342 | + | Image: "myapp:latest", | |||
| 343 | + | } | |||
| 344 | + | ||||
| 345 | + | // prepareDeployResources | |||
| 346 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |||
| 347 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |||
| 348 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |||
| 349 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 350 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |||
| 351 | + | ||||
| 352 | + | // GetImageID fails => deploy proceeds normally (graceful degradation) | |||
| 353 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("", errors.New("image inspect failed")) | |||
| 354 | + | ||||
| 355 | + | // Full deploy proceeds despite the error | |||
| 356 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |||
| 357 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(newContainer, nil) | |||
| 358 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |||
| 359 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |||
| 360 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |||
| 361 | + | ID: "new-container", Status: "running", | |||
| 362 | + | }, nil) | |||
| 363 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 364 | + | ||||
| 365 | + | // Old container is finalized | |||
| 366 | + | runtime.EXPECT().StopContainer(mock.Anything, "existing-container").Return(nil) | |||
| 367 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "existing-container", true).Return(nil) | |||
| 368 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |||
| 369 | + | ||||
| 370 | + | result, err := svc.Deploy(ctx, route) | |||
| 371 | + | ||||
| 372 | + | assert.NoError(t, err) | |||
| 373 | + | assert.Equal(t, "new-container", result.ID) | |||
| 321 | } | 374 | } | ||
| 322 | 375 | ||||
| 323 | func TestService_Deploy_WithNetworkIsolation(t *testing.T) { | 376 | func TestService_Deploy_WithNetworkIsolation(t *testing.T) { | ||
| @@ -2003,3 +2056,174 @@ func TestService_Deploy_NilCacheInvalidator(t *testing.T) { | |||||
| 2003 | assert.NoError(t, err) | 2056 | assert.NoError(t, err) | ||
| 2004 | assert.Equal(t, "new-container", result.ID) | 2057 | assert.Equal(t, "new-container", result.ID) | ||
| 2005 | } | 2058 | } | ||
| 2059 | + | ||||
| 2060 | + | func TestService_Deploy_SkipsRedundantDeploy(t *testing.T) { | |||
| 2061 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 2062 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 2063 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 2064 | + | ||||
| 2065 | + | config := Config{ | |||
| 2066 | + | ReadinessDelay: time.Millisecond, | |||
| 2067 | + | DrainDelay: time.Millisecond, | |||
| 2068 | + | } | |||
| 2069 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 2070 | + | ctx := testContext() | |||
| 2071 | + | ||||
| 2072 | + | // Pre-populate with existing container that has an ImageID (set by InspectContainer). | |||
| 2073 | + | // This simulates the first deploy (from image.pushed event) having already completed. | |||
| 2074 | + | existingContainer := &domain.Container{ | |||
| 2075 | + | ID: "existing-container", | |||
| 2076 | + | Name: "gordon-test.example.com", | |||
| 2077 | + | Image: "myapp:latest", | |||
| 2078 | + | ImageID: "sha256:abc123", | |||
| 2079 | + | Status: "running", | |||
| 2080 | + | } | |||
| 2081 | + | svc.containers["test.example.com"] = existingContainer | |||
| 2082 | + | ||||
| 2083 | + | route := domain.Route{ | |||
| 2084 | + | Domain: "test.example.com", | |||
| 2085 | + | Image: "myapp:latest", | |||
| 2086 | + | } | |||
| 2087 | + | ||||
| 2088 | + | // prepareDeployResources will run: orphan cleanup, image pull, etc. | |||
| 2089 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |||
| 2090 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |||
| 2091 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |||
| 2092 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 2093 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |||
| 2094 | + | ||||
| 2095 | + | // skipRedundantDeploy: resolve image ID and compare with existing container | |||
| 2096 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:abc123", nil) | |||
| 2097 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "existing-container").Return(true, nil) | |||
| 2098 | + | ||||
| 2099 | + | // No CreateContainer, StartContainer, or readiness calls should happen | |||
| 2100 | + | ||||
| 2101 | + | result, err := svc.Deploy(ctx, route) | |||
| 2102 | + | ||||
| 2103 | + | assert.NoError(t, err) | |||
| 2104 | + | assert.NotNil(t, result) | |||
| 2105 | + | assert.Equal(t, "existing-container", result.ID) | |||
| 2106 | + | ||||
| 2107 | + | // Container should still be tracked | |||
| 2108 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |||
| 2109 | + | assert.True(t, exists) | |||
| 2110 | + | assert.Equal(t, "existing-container", tracked.ID) | |||
| 2111 | + | } | |||
| 2112 | + | ||||
| 2113 | + | func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) { | |||
| 2114 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 2115 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 2116 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 2117 | + | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | |||
| 2118 | + | ||||
| 2119 | + | config := Config{ | |||
| 2120 | + | ReadinessDelay: time.Millisecond, | |||
| 2121 | + | DrainDelay: time.Millisecond, | |||
| 2122 | + | } | |||
| 2123 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 2124 | + | svc.SetProxyCacheInvalidator(cacheInvalidator) | |||
| 2125 | + | ctx := testContext() | |||
| 2126 | + | ||||
| 2127 | + | // Existing container with a DIFFERENT image ID than what's being deployed | |||
| 2128 | + | existingContainer := &domain.Container{ | |||
| 2129 | + | ID: "old-container", | |||
| 2130 | + | Name: "gordon-test.example.com", | |||
| 2131 | + | Image: "myapp:latest", | |||
| 2132 | + | ImageID: "sha256:old-image", | |||
| 2133 | + | Status: "running", | |||
| 2134 | + | } | |||
| 2135 | + | svc.containers["test.example.com"] = existingContainer | |||
| 2136 | + | ||||
| 2137 | + | route := domain.Route{ | |||
| 2138 | + | Domain: "test.example.com", | |||
| 2139 | + | Image: "myapp:latest", | |||
| 2140 | + | } | |||
| 2141 | + | ||||
| 2142 | + | // prepareDeployResources | |||
| 2143 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |||
| 2144 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |||
| 2145 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |||
| 2146 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 2147 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |||
| 2148 | + | ||||
| 2149 | + | // skipRedundantDeploy: image IDs differ, so deploy proceeds | |||
| 2150 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:new-image", nil) | |||
| 2151 | + | ||||
| 2152 | + | // Full deploy proceeds | |||
| 2153 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |||
| 2154 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(newContainer, nil) | |||
| 2155 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |||
| 2156 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |||
| 2157 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |||
| 2158 | + | ID: "new-container", Status: "running", | |||
| 2159 | + | }, nil) | |||
| 2160 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 2161 | + | cacheInvalidator.EXPECT().InvalidateTarget(mock.Anything, "test.example.com").Return() | |||
| 2162 | + | runtime.EXPECT().StopContainer(mock.Anything, "old-container").Return(nil) | |||
| 2163 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "old-container", true).Return(nil) | |||
| 2164 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |||
| 2165 | + | ||||
| 2166 | + | result, err := svc.Deploy(ctx, route) | |||
| 2167 | + | ||||
| 2168 | + | assert.NoError(t, err) | |||
| 2169 | + | assert.Equal(t, "new-container", result.ID) | |||
| 2170 | + | } | |||
| 2171 | + | ||||
| 2172 | + | func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) { | |||
| 2173 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 2174 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 2175 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 2176 | + | ||||
| 2177 | + | config := Config{ | |||
| 2178 | + | ReadinessDelay: time.Millisecond, | |||
| 2179 | + | DrainDelay: time.Millisecond, | |||
| 2180 | + | } | |||
| 2181 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 2182 | + | ctx := testContext() | |||
| 2183 | + | ||||
| 2184 | + | // Existing container has same image ID but is NOT running (crashed) | |||
| 2185 | + | existingContainer := &domain.Container{ | |||
| 2186 | + | ID: "existing-container", | |||
| 2187 | + | Name: "gordon-test.example.com", | |||
| 2188 | + | Image: "myapp:latest", | |||
| 2189 | + | ImageID: "sha256:abc123", | |||
| 2190 | + | Status: "exited", | |||
| 2191 | + | } | |||
| 2192 | + | svc.containers["test.example.com"] = existingContainer | |||
| 2193 | + | ||||
| 2194 | + | route := domain.Route{ | |||
| 2195 | + | Domain: "test.example.com", | |||
| 2196 | + | Image: "myapp:latest", | |||
| 2197 | + | } | |||
| 2198 | + | ||||
| 2199 | + | // prepareDeployResources | |||
| 2200 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |||
| 2201 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |||
| 2202 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |||
| 2203 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 2204 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |||
| 2205 | + | ||||
| 2206 | + | // skipRedundantDeploy: same image ID but container not running => proceed with deploy | |||
| 2207 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:abc123", nil) | |||
| 2208 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "existing-container").Return(false, nil) | |||
| 2209 | + | ||||
| 2210 | + | // Full deploy proceeds (replaces crashed container) | |||
| 2211 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |||
| 2212 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(newContainer, nil) | |||
| 2213 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |||
| 2214 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |||
| 2215 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |||
| 2216 | + | ID: "new-container", Status: "running", | |||
| 2217 | + | }, nil) | |||
| 2218 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 2219 | + | ||||
| 2220 | + | // Old (exited) container is still finalized | |||
| 2221 | + | runtime.EXPECT().StopContainer(mock.Anything, "existing-container").Return(nil) | |||
| 2222 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "existing-container", true).Return(nil) | |||
| 2223 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |||
| 2224 | + | ||||
| 2225 | + | result, err := svc.Deploy(ctx, route) | |||
| 2226 | + | ||||
| 2227 | + | assert.NoError(t, err) | |||
| 2228 | + | assert.Equal(t, "new-container", result.ID) | |||
| 2229 | + | } | |||