Reuse container sync snapshot for restart policy migration
- commit
- edf3939934133f4bcdb34c901d2c97aa0ed7d4f6
- author
- brice · · 2026-05-05 12:13 UTC
- parent
- 84bef01
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +46 -42
M internal/usecase/container/service.go +10 -10
| @@ -1405,13 +1405,8 @@ func (s *Service) HealthCheck(ctx context.Context) map[string]bool { | |||
| 1405 | 1405 | return health | |
| 1406 | 1406 | } | |
| 1407 | 1407 | ||
| 1408 | - | func (s *Service) ensureManagedContainerRestartPolicies(ctx context.Context) { | |
| 1408 | + | func (s *Service) ensureManagedContainerRestartPolicies(ctx context.Context, allContainers []*domain.Container) { | |
| 1409 | 1409 | log := zerowrap.FromCtx(ctx) | |
| 1410 | - | allContainers, err := s.runtime.ListContainers(ctx, true) | |
| 1411 | - | if err != nil { | |
| 1412 | - | log.Warn().Err(err).Msg("failed to list containers for restart policy migration") | |
| 1413 | - | return | |
| 1414 | - | } | |
| 1415 | 1410 | ||
| 1416 | 1411 | for _, c := range allContainers { | |
| 1417 | 1412 | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { | |
| @@ -1434,17 +1429,22 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||
| 1434 | 1429 | }) | |
| 1435 | 1430 | log := zerowrap.FromCtx(ctx) | |
| 1436 | 1431 | ||
| 1437 | - | s.ensureManagedContainerRestartPolicies(ctx) | |
| 1438 | - | ||
| 1439 | - | // List containers without holding lock to avoid blocking during Docker API call | |
| 1440 | - | allContainers, err := s.runtime.ListContainers(ctx, false) | |
| 1432 | + | // List containers without holding lock to avoid blocking during Docker API call. | |
| 1433 | + | // Use all=true so the same runtime snapshot can migrate stopped containers and | |
| 1434 | + | // rebuild the running-container maps without a second Docker list call. | |
| 1435 | + | allContainers, err := s.runtime.ListContainers(ctx, true) | |
| 1441 | 1436 | if err != nil { | |
| 1442 | 1437 | return log.WrapErr(err, "failed to list containers") | |
| 1443 | 1438 | } | |
| 1444 | 1439 | ||
| 1440 | + | s.ensureManagedContainerRestartPolicies(ctx, allContainers) | |
| 1441 | + | ||
| 1445 | 1442 | managed := make(map[string]*domain.Container) | |
| 1446 | 1443 | attachments := make(map[string][]string) | |
| 1447 | 1444 | for _, c := range allContainers { | |
| 1445 | + | if c.Status != "" && c.Status != string(domain.ContainerStatusRunning) { | |
| 1446 | + | continue | |
| 1447 | + | } | |
| 1448 | 1448 | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { | |
| 1449 | 1449 | continue | |
| 1450 | 1450 | } | |
| @@ -1405,13 +1405,8 @@ func (s *Service) HealthCheck(ctx context.Context) map[string]bool { | |||||
| 1405 | return health | 1405 | return health | ||
| 1406 | } | 1406 | } | ||
| 1407 | 1407 | ||||
| 1408 | - | func (s *Service) ensureManagedContainerRestartPolicies(ctx context.Context) { | 1408 | + | func (s *Service) ensureManagedContainerRestartPolicies(ctx context.Context, allContainers []*domain.Container) { |
| 1409 | log := zerowrap.FromCtx(ctx) | 1409 | log := zerowrap.FromCtx(ctx) | ||
| 1410 | - | allContainers, err := s.runtime.ListContainers(ctx, true) | |||
| 1411 | - | if err != nil { | |||
| 1412 | - | log.Warn().Err(err).Msg("failed to list containers for restart policy migration") | |||
| 1413 | - | return | |||
| 1414 | - | } | |||
| 1415 | 1410 | ||||
| 1416 | for _, c := range allContainers { | 1411 | for _, c := range allContainers { | ||
| 1417 | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { | 1412 | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { | ||
| @@ -1434,17 +1429,22 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||||
| 1434 | }) | 1429 | }) | ||
| 1435 | log := zerowrap.FromCtx(ctx) | 1430 | log := zerowrap.FromCtx(ctx) | ||
| 1436 | 1431 | ||||
| 1437 | - | s.ensureManagedContainerRestartPolicies(ctx) | 1432 | + | // List containers without holding lock to avoid blocking during Docker API call. |
| 1438 | - | 1433 | + | // Use all=true so the same runtime snapshot can migrate stopped containers and | |
| 1439 | - | // List containers without holding lock to avoid blocking during Docker API call | 1434 | + | // rebuild the running-container maps without a second Docker list call. |
| 1440 | - | allContainers, err := s.runtime.ListContainers(ctx, false) | 1435 | + | allContainers, err := s.runtime.ListContainers(ctx, true) |
| 1441 | if err != nil { | 1436 | if err != nil { | ||
| 1442 | return log.WrapErr(err, "failed to list containers") | 1437 | return log.WrapErr(err, "failed to list containers") | ||
| 1443 | } | 1438 | } | ||
| 1444 | 1439 | ||||
| 1440 | + | s.ensureManagedContainerRestartPolicies(ctx, allContainers) | |||
| 1441 | + | ||||
| 1445 | managed := make(map[string]*domain.Container) | 1442 | managed := make(map[string]*domain.Container) | ||
| 1446 | attachments := make(map[string][]string) | 1443 | attachments := make(map[string][]string) | ||
| 1447 | for _, c := range allContainers { | 1444 | for _, c := range allContainers { | ||
| 1445 | + | if c.Status != "" && c.Status != string(domain.ContainerStatusRunning) { | |||
| 1446 | + | continue | |||
| 1447 | + | } | |||
| 1448 | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { | 1448 | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { | ||
| 1449 | continue | 1449 | continue | ||
| 1450 | } | 1450 | } | ||
M internal/usecase/container/service_test.go +36 -32
| @@ -255,9 +255,9 @@ func TestService_SyncContainers_ManagedContainersMetricTracksDelta(t *testing.T) | |||
| 255 | 255 | ctx := testContext() | |
| 256 | 256 | metrics, reader := setupMetricsTest(t) | |
| 257 | 257 | svc.SetMetrics(metrics) | |
| 258 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |
| 258 | 259 | ||
| 259 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | |
| 260 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 260 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 261 | 261 | { | |
| 262 | 262 | ID: "container-1", | |
| 263 | 263 | Labels: map[string]string{ | |
| @@ -281,8 +281,7 @@ func TestService_SyncContainers_ManagedContainersMetricTracksDelta(t *testing.T) | |||
| 281 | 281 | assert.Equal(t, 0, points[0].Attributes.Len()) | |
| 282 | 282 | ||
| 283 | 283 | // No runtime change: metric should remain stable. | |
| 284 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | |
| 285 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 284 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 286 | 285 | { | |
| 287 | 286 | ID: "container-1", | |
| 288 | 287 | Labels: map[string]string{ | |
| @@ -306,8 +305,7 @@ func TestService_SyncContainers_ManagedContainersMetricTracksDelta(t *testing.T) | |||
| 306 | 305 | assert.Equal(t, 0, points[0].Attributes.Len()) | |
| 307 | 306 | ||
| 308 | 307 | // One container removed in runtime: metric should decrement by one. | |
| 309 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | |
| 310 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 308 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 311 | 309 | { | |
| 312 | 310 | ID: "container-1", | |
| 313 | 311 | Labels: map[string]string{ | |
| @@ -1250,22 +1248,33 @@ func TestService_SyncContainers_EnsuresRestartPolicyForManagedRoutesAndAttachmen | |||
| 1250 | 1248 | ctx := testContext() | |
| 1251 | 1249 | ||
| 1252 | 1250 | routeContainer := &domain.Container{ | |
| 1253 | - | ID: "route-1", | |
| 1254 | - | Name: "gordon-app.example.com", | |
| 1251 | + | ID: "route-1", | |
| 1252 | + | Name: "gordon-app.example.com", | |
| 1253 | + | Status: string(domain.ContainerStatusRunning), | |
| 1255 | 1254 | Labels: map[string]string{ | |
| 1256 | 1255 | domain.LabelManaged: "true", | |
| 1257 | 1256 | domain.LabelDomain: "app.example.com", | |
| 1258 | 1257 | }, | |
| 1259 | 1258 | } | |
| 1260 | 1259 | attachmentContainer := &domain.Container{ | |
| 1261 | - | ID: "attachment-1", | |
| 1262 | - | Name: "gordon-app-example-com-postgres", | |
| 1260 | + | ID: "attachment-1", | |
| 1261 | + | Name: "gordon-app-example-com-postgres", | |
| 1262 | + | Status: string(domain.ContainerStatusRunning), | |
| 1263 | 1263 | Labels: map[string]string{ | |
| 1264 | 1264 | domain.LabelManaged: "true", | |
| 1265 | 1265 | domain.LabelAttachment: "true", | |
| 1266 | 1266 | domain.LabelAttachedTo: "app.example.com", | |
| 1267 | 1267 | }, | |
| 1268 | 1268 | } | |
| 1269 | + | stoppedContainer := &domain.Container{ | |
| 1270 | + | ID: "stopped-1", | |
| 1271 | + | Name: "gordon-stopped.example.com", | |
| 1272 | + | Status: string(domain.ContainerStatusExited), | |
| 1273 | + | Labels: map[string]string{ | |
| 1274 | + | domain.LabelManaged: "true", | |
| 1275 | + | domain.LabelDomain: "stopped.example.com", | |
| 1276 | + | }, | |
| 1277 | + | } | |
| 1269 | 1278 | unmanagedContainer := &domain.Container{ | |
| 1270 | 1279 | ID: "other-1", | |
| 1271 | 1280 | Labels: map[string]string{}, | |
| @@ -1274,15 +1283,12 @@ func TestService_SyncContainers_EnsuresRestartPolicyForManagedRoutesAndAttachmen | |||
| 1274 | 1283 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1275 | 1284 | routeContainer, | |
| 1276 | 1285 | attachmentContainer, | |
| 1286 | + | stoppedContainer, | |
| 1277 | 1287 | unmanagedContainer, | |
| 1278 | 1288 | }, nil).Once() | |
| 1279 | 1289 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "route-1", domain.RestartPolicyAlways).Return(nil).Once() | |
| 1280 | 1290 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "attachment-1", domain.RestartPolicyAlways).Return(nil).Once() | |
| 1281 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 1282 | - | routeContainer, | |
| 1283 | - | attachmentContainer, | |
| 1284 | - | }, nil).Once() | |
| 1285 | - | ||
| 1291 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "stopped-1", domain.RestartPolicyAlways).Return(nil).Once() | |
| 1286 | 1292 | err := svc.SyncContainers(ctx) | |
| 1287 | 1293 | ||
| 1288 | 1294 | require.NoError(t, err) | |
| @@ -1295,7 +1301,8 @@ func TestService_SyncContainers_EnsuresRestartPolicyForManagedRoutesAndAttachmen | |||
| 1295 | 1301 | svc.mu.RUnlock() | |
| 1296 | 1302 | assert.Equal(t, []string{"attachment-1"}, attachmentIDs) | |
| 1297 | 1303 | assert.NotContains(t, svc.containers, "other-1") | |
| 1298 | - | runtime.AssertNumberOfCalls(t, "EnsureContainerRestartPolicy", 2) | |
| 1304 | + | assert.NotContains(t, svc.containers, "stopped.example.com") | |
| 1305 | + | runtime.AssertNumberOfCalls(t, "EnsureContainerRestartPolicy", 3) | |
| 1299 | 1306 | } | |
| 1300 | 1307 | ||
| 1301 | 1308 | func TestService_SyncContainers_RestartPolicyMigrationErrorIsNonFatal(t *testing.T) { | |
| @@ -1317,8 +1324,6 @@ func TestService_SyncContainers_RestartPolicyMigrationErrorIsNonFatal(t *testing | |||
| 1317 | 1324 | ||
| 1318 | 1325 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{routeContainer}, nil).Once() | |
| 1319 | 1326 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "route-1", domain.RestartPolicyAlways).Return(errors.New("update failed")).Once() | |
| 1320 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{routeContainer}, nil).Once() | |
| 1321 | - | ||
| 1322 | 1327 | err := svc.SyncContainers(ctx) | |
| 1323 | 1328 | ||
| 1324 | 1329 | require.NoError(t, err) | |
| @@ -1335,8 +1340,8 @@ func TestService_SyncContainers(t *testing.T) { | |||
| 1335 | 1340 | svc := NewService(runtime, envLoader, eventBus, nil, Config{}, nil) | |
| 1336 | 1341 | ctx := testContext() | |
| 1337 | 1342 | ||
| 1338 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | |
| 1339 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 1343 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |
| 1344 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1340 | 1345 | { | |
| 1341 | 1346 | ID: "container-1", | |
| 1342 | 1347 | Name: "gordon-app1.example.com", | |
| @@ -1462,8 +1467,7 @@ func TestService_Restart_ReconcilesStaleContainerID(t *testing.T) { | |||
| 1462 | 1467 | runtime.EXPECT().RestartContainer(mock.Anything, "stale-container-id"). | |
| 1463 | 1468 | Return(errors.New("no container with name or ID \"stale-container-id\" found")). | |
| 1464 | 1469 | Once() | |
| 1465 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | |
| 1466 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 1470 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1467 | 1471 | { | |
| 1468 | 1472 | ID: "fresh-container-id", | |
| 1469 | 1473 | Name: "gordon-test.example.com", | |
| @@ -1473,6 +1477,7 @@ func TestService_Restart_ReconcilesStaleContainerID(t *testing.T) { | |||
| 1473 | 1477 | }, | |
| 1474 | 1478 | }, | |
| 1475 | 1479 | }, nil).Once() | |
| 1480 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "fresh-container-id", domain.RestartPolicyAlways).Return(nil).Once() | |
| 1476 | 1481 | runtime.EXPECT().RestartContainer(mock.Anything, "fresh-container-id").Return(nil).Once() | |
| 1477 | 1482 | ||
| 1478 | 1483 | err := svc.Restart(ctx, "test.example.com", false) | |
| @@ -1498,7 +1503,6 @@ func TestService_Restart_ReconcilesStaleContainerID_NotFoundAfterSync(t *testing | |||
| 1498 | 1503 | Return(errors.New("no such container")). | |
| 1499 | 1504 | Once() | |
| 1500 | 1505 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | |
| 1501 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil).Once() | |
| 1502 | 1506 | ||
| 1503 | 1507 | err := svc.Restart(ctx, "test.example.com", false) | |
| 1504 | 1508 | ||
| @@ -1520,8 +1524,7 @@ func TestService_Restart_WithAttachments(t *testing.T) { | |||
| 1520 | 1524 | Status: "running", | |
| 1521 | 1525 | } | |
| 1522 | 1526 | svc.attachments["test.example.com"] = []string{"container-attach-1", "container-attach-2"} | |
| 1523 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | |
| 1524 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 1527 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1525 | 1528 | { | |
| 1526 | 1529 | ID: "container-123", | |
| 1527 | 1530 | Labels: map[string]string{ | |
| @@ -1546,6 +1549,7 @@ func TestService_Restart_WithAttachments(t *testing.T) { | |||
| 1546 | 1549 | }, | |
| 1547 | 1550 | }, | |
| 1548 | 1551 | }, nil).Maybe() | |
| 1552 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |
| 1549 | 1553 | ||
| 1550 | 1554 | runtime.EXPECT().RestartContainer(mock.Anything, "container-123").Return(nil) | |
| 1551 | 1555 | runtime.EXPECT().RestartContainer(mock.Anything, "container-attach-1").Return(nil) | |
| @@ -1576,7 +1580,6 @@ func TestService_Restart_WithAttachments_ErrorsWhenConfiguredButNotDeployed(t *t | |||
| 1576 | 1580 | } | |
| 1577 | 1581 | // No attachment containers tracked — s.attachments is empty | |
| 1578 | 1582 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | |
| 1579 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil).Maybe() | |
| 1580 | 1583 | ||
| 1581 | 1584 | err := svc.Restart(testContext(), "test.example.com", true) | |
| 1582 | 1585 | ||
| @@ -1606,8 +1609,7 @@ func TestService_Restart_WithAttachments_ErrorsWhenPartiallyDeployed(t *testing. | |||
| 1606 | 1609 | } | |
| 1607 | 1610 | // Only 1 of 2 attachments deployed | |
| 1608 | 1611 | svc.attachments["test.example.com"] = []string{"attach-postgres"} | |
| 1609 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | |
| 1610 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 1612 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1611 | 1613 | { | |
| 1612 | 1614 | ID: "container-123", | |
| 1613 | 1615 | Labels: map[string]string{ | |
| @@ -1624,6 +1626,7 @@ func TestService_Restart_WithAttachments_ErrorsWhenPartiallyDeployed(t *testing. | |||
| 1624 | 1626 | }, | |
| 1625 | 1627 | }, | |
| 1626 | 1628 | }, nil).Maybe() | |
| 1629 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |
| 1627 | 1630 | ||
| 1628 | 1631 | err := svc.Restart(testContext(), "test.example.com", true) | |
| 1629 | 1632 | ||
| @@ -1653,8 +1656,7 @@ func TestService_Restart_WithAttachments_SucceedsWhenAllDeployed(t *testing.T) { | |||
| 1653 | 1656 | Status: "running", | |
| 1654 | 1657 | } | |
| 1655 | 1658 | svc.attachments["test.example.com"] = []string{"attach-456"} | |
| 1656 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | |
| 1657 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 1659 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1658 | 1660 | { | |
| 1659 | 1661 | ID: "container-123", | |
| 1660 | 1662 | Labels: map[string]string{ | |
| @@ -1671,6 +1673,7 @@ func TestService_Restart_WithAttachments_SucceedsWhenAllDeployed(t *testing.T) { | |||
| 1671 | 1673 | }, | |
| 1672 | 1674 | }, | |
| 1673 | 1675 | }, nil).Maybe() | |
| 1676 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |
| 1674 | 1677 | ||
| 1675 | 1678 | runtime.EXPECT().RestartContainer(mock.Anything, "container-123").Return(nil) | |
| 1676 | 1679 | runtime.EXPECT().RestartContainer(mock.Anything, "attach-456").Return(nil) | |
| @@ -3198,8 +3201,7 @@ func TestSyncContainersRebuildsAttachmentMap(t *testing.T) { | |||
| 3198 | 3201 | ctx := testContext() | |
| 3199 | 3202 | ||
| 3200 | 3203 | // ListContainers returns one main container and one attachment container | |
| 3201 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | |
| 3202 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 3204 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 3203 | 3205 | { | |
| 3204 | 3206 | ID: "main-container-1", | |
| 3205 | 3207 | Labels: map[string]string{ | |
| @@ -3216,6 +3218,8 @@ func TestSyncContainersRebuildsAttachmentMap(t *testing.T) { | |||
| 3216 | 3218 | }, | |
| 3217 | 3219 | }, | |
| 3218 | 3220 | }, nil).Once() | |
| 3221 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "main-container-1", domain.RestartPolicyAlways).Return(nil).Once() | |
| 3222 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "attachment-container-1", domain.RestartPolicyAlways).Return(nil).Once() | |
| 3219 | 3223 | ||
| 3220 | 3224 | require.NoError(t, svc.SyncContainers(ctx)) | |
| 3221 | 3225 | ||
| @@ -255,9 +255,9 @@ func TestService_SyncContainers_ManagedContainersMetricTracksDelta(t *testing.T) | |||||
| 255 | ctx := testContext() | 255 | ctx := testContext() | ||
| 256 | metrics, reader := setupMetricsTest(t) | 256 | metrics, reader := setupMetricsTest(t) | ||
| 257 | svc.SetMetrics(metrics) | 257 | svc.SetMetrics(metrics) | ||
| 258 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |||
| 258 | 259 | ||||
| 259 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | 260 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 260 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 261 | { | 261 | { | ||
| 262 | ID: "container-1", | 262 | ID: "container-1", | ||
| 263 | Labels: map[string]string{ | 263 | Labels: map[string]string{ | ||
| @@ -281,8 +281,7 @@ func TestService_SyncContainers_ManagedContainersMetricTracksDelta(t *testing.T) | |||||
| 281 | assert.Equal(t, 0, points[0].Attributes.Len()) | 281 | assert.Equal(t, 0, points[0].Attributes.Len()) | ||
| 282 | 282 | ||||
| 283 | // No runtime change: metric should remain stable. | 283 | // No runtime change: metric should remain stable. | ||
| 284 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | 284 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 285 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 286 | { | 285 | { | ||
| 287 | ID: "container-1", | 286 | ID: "container-1", | ||
| 288 | Labels: map[string]string{ | 287 | Labels: map[string]string{ | ||
| @@ -306,8 +305,7 @@ func TestService_SyncContainers_ManagedContainersMetricTracksDelta(t *testing.T) | |||||
| 306 | assert.Equal(t, 0, points[0].Attributes.Len()) | 305 | assert.Equal(t, 0, points[0].Attributes.Len()) | ||
| 307 | 306 | ||||
| 308 | // One container removed in runtime: metric should decrement by one. | 307 | // One container removed in runtime: metric should decrement by one. | ||
| 309 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | 308 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 310 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 311 | { | 309 | { | ||
| 312 | ID: "container-1", | 310 | ID: "container-1", | ||
| 313 | Labels: map[string]string{ | 311 | Labels: map[string]string{ | ||
| @@ -1250,22 +1248,33 @@ func TestService_SyncContainers_EnsuresRestartPolicyForManagedRoutesAndAttachmen | |||||
| 1250 | ctx := testContext() | 1248 | ctx := testContext() | ||
| 1251 | 1249 | ||||
| 1252 | routeContainer := &domain.Container{ | 1250 | routeContainer := &domain.Container{ | ||
| 1253 | - | ID: "route-1", | 1251 | + | ID: "route-1", |
| 1254 | - | Name: "gordon-app.example.com", | 1252 | + | Name: "gordon-app.example.com", |
| 1253 | + | Status: string(domain.ContainerStatusRunning), | |||
| 1255 | Labels: map[string]string{ | 1254 | Labels: map[string]string{ | ||
| 1256 | domain.LabelManaged: "true", | 1255 | domain.LabelManaged: "true", | ||
| 1257 | domain.LabelDomain: "app.example.com", | 1256 | domain.LabelDomain: "app.example.com", | ||
| 1258 | }, | 1257 | }, | ||
| 1259 | } | 1258 | } | ||
| 1260 | attachmentContainer := &domain.Container{ | 1259 | attachmentContainer := &domain.Container{ | ||
| 1261 | - | ID: "attachment-1", | 1260 | + | ID: "attachment-1", |
| 1262 | - | Name: "gordon-app-example-com-postgres", | 1261 | + | Name: "gordon-app-example-com-postgres", |
| 1262 | + | Status: string(domain.ContainerStatusRunning), | |||
| 1263 | Labels: map[string]string{ | 1263 | Labels: map[string]string{ | ||
| 1264 | domain.LabelManaged: "true", | 1264 | domain.LabelManaged: "true", | ||
| 1265 | domain.LabelAttachment: "true", | 1265 | domain.LabelAttachment: "true", | ||
| 1266 | domain.LabelAttachedTo: "app.example.com", | 1266 | domain.LabelAttachedTo: "app.example.com", | ||
| 1267 | }, | 1267 | }, | ||
| 1268 | } | 1268 | } | ||
| 1269 | + | stoppedContainer := &domain.Container{ | |||
| 1270 | + | ID: "stopped-1", | |||
| 1271 | + | Name: "gordon-stopped.example.com", | |||
| 1272 | + | Status: string(domain.ContainerStatusExited), | |||
| 1273 | + | Labels: map[string]string{ | |||
| 1274 | + | domain.LabelManaged: "true", | |||
| 1275 | + | domain.LabelDomain: "stopped.example.com", | |||
| 1276 | + | }, | |||
| 1277 | + | } | |||
| 1269 | unmanagedContainer := &domain.Container{ | 1278 | unmanagedContainer := &domain.Container{ | ||
| 1270 | ID: "other-1", | 1279 | ID: "other-1", | ||
| 1271 | Labels: map[string]string{}, | 1280 | Labels: map[string]string{}, | ||
| @@ -1274,15 +1283,12 @@ func TestService_SyncContainers_EnsuresRestartPolicyForManagedRoutesAndAttachmen | |||||
| 1274 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | 1283 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | ||
| 1275 | routeContainer, | 1284 | routeContainer, | ||
| 1276 | attachmentContainer, | 1285 | attachmentContainer, | ||
| 1286 | + | stoppedContainer, | |||
| 1277 | unmanagedContainer, | 1287 | unmanagedContainer, | ||
| 1278 | }, nil).Once() | 1288 | }, nil).Once() | ||
| 1279 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "route-1", domain.RestartPolicyAlways).Return(nil).Once() | 1289 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "route-1", domain.RestartPolicyAlways).Return(nil).Once() | ||
| 1280 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "attachment-1", domain.RestartPolicyAlways).Return(nil).Once() | 1290 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "attachment-1", domain.RestartPolicyAlways).Return(nil).Once() | ||
| 1281 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | 1291 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "stopped-1", domain.RestartPolicyAlways).Return(nil).Once() |
| 1282 | - | routeContainer, | |||
| 1283 | - | attachmentContainer, | |||
| 1284 | - | }, nil).Once() | |||
| 1285 | - | ||||
| 1286 | err := svc.SyncContainers(ctx) | 1292 | err := svc.SyncContainers(ctx) | ||
| 1287 | 1293 | ||||
| 1288 | require.NoError(t, err) | 1294 | require.NoError(t, err) | ||
| @@ -1295,7 +1301,8 @@ func TestService_SyncContainers_EnsuresRestartPolicyForManagedRoutesAndAttachmen | |||||
| 1295 | svc.mu.RUnlock() | 1301 | svc.mu.RUnlock() | ||
| 1296 | assert.Equal(t, []string{"attachment-1"}, attachmentIDs) | 1302 | assert.Equal(t, []string{"attachment-1"}, attachmentIDs) | ||
| 1297 | assert.NotContains(t, svc.containers, "other-1") | 1303 | assert.NotContains(t, svc.containers, "other-1") | ||
| 1298 | - | runtime.AssertNumberOfCalls(t, "EnsureContainerRestartPolicy", 2) | 1304 | + | assert.NotContains(t, svc.containers, "stopped.example.com") |
| 1305 | + | runtime.AssertNumberOfCalls(t, "EnsureContainerRestartPolicy", 3) | |||
| 1299 | } | 1306 | } | ||
| 1300 | 1307 | ||||
| 1301 | func TestService_SyncContainers_RestartPolicyMigrationErrorIsNonFatal(t *testing.T) { | 1308 | func TestService_SyncContainers_RestartPolicyMigrationErrorIsNonFatal(t *testing.T) { | ||
| @@ -1317,8 +1324,6 @@ func TestService_SyncContainers_RestartPolicyMigrationErrorIsNonFatal(t *testing | |||||
| 1317 | 1324 | ||||
| 1318 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{routeContainer}, nil).Once() | 1325 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{routeContainer}, nil).Once() | ||
| 1319 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "route-1", domain.RestartPolicyAlways).Return(errors.New("update failed")).Once() | 1326 | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "route-1", domain.RestartPolicyAlways).Return(errors.New("update failed")).Once() | ||
| 1320 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{routeContainer}, nil).Once() | |||
| 1321 | - | ||||
| 1322 | err := svc.SyncContainers(ctx) | 1327 | err := svc.SyncContainers(ctx) | ||
| 1323 | 1328 | ||||
| 1324 | require.NoError(t, err) | 1329 | require.NoError(t, err) | ||
| @@ -1335,8 +1340,8 @@ func TestService_SyncContainers(t *testing.T) { | |||||
| 1335 | svc := NewService(runtime, envLoader, eventBus, nil, Config{}, nil) | 1340 | svc := NewService(runtime, envLoader, eventBus, nil, Config{}, nil) | ||
| 1336 | ctx := testContext() | 1341 | ctx := testContext() | ||
| 1337 | 1342 | ||||
| 1338 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | 1343 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() |
| 1339 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | 1344 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 1340 | { | 1345 | { | ||
| 1341 | ID: "container-1", | 1346 | ID: "container-1", | ||
| 1342 | Name: "gordon-app1.example.com", | 1347 | Name: "gordon-app1.example.com", | ||
| @@ -1462,8 +1467,7 @@ func TestService_Restart_ReconcilesStaleContainerID(t *testing.T) { | |||||
| 1462 | runtime.EXPECT().RestartContainer(mock.Anything, "stale-container-id"). | 1467 | runtime.EXPECT().RestartContainer(mock.Anything, "stale-container-id"). | ||
| 1463 | Return(errors.New("no container with name or ID \"stale-container-id\" found")). | 1468 | Return(errors.New("no container with name or ID \"stale-container-id\" found")). | ||
| 1464 | Once() | 1469 | Once() | ||
| 1465 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | 1470 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 1466 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 1467 | { | 1471 | { | ||
| 1468 | ID: "fresh-container-id", | 1472 | ID: "fresh-container-id", | ||
| 1469 | Name: "gordon-test.example.com", | 1473 | Name: "gordon-test.example.com", | ||
| @@ -1473,6 +1477,7 @@ func TestService_Restart_ReconcilesStaleContainerID(t *testing.T) { | |||||
| 1473 | }, | 1477 | }, | ||
| 1474 | }, | 1478 | }, | ||
| 1475 | }, nil).Once() | 1479 | }, nil).Once() | ||
| 1480 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "fresh-container-id", domain.RestartPolicyAlways).Return(nil).Once() | |||
| 1476 | runtime.EXPECT().RestartContainer(mock.Anything, "fresh-container-id").Return(nil).Once() | 1481 | runtime.EXPECT().RestartContainer(mock.Anything, "fresh-container-id").Return(nil).Once() | ||
| 1477 | 1482 | ||||
| 1478 | err := svc.Restart(ctx, "test.example.com", false) | 1483 | err := svc.Restart(ctx, "test.example.com", false) | ||
| @@ -1498,7 +1503,6 @@ func TestService_Restart_ReconcilesStaleContainerID_NotFoundAfterSync(t *testing | |||||
| 1498 | Return(errors.New("no such container")). | 1503 | Return(errors.New("no such container")). | ||
| 1499 | Once() | 1504 | Once() | ||
| 1500 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | 1505 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | ||
| 1501 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil).Once() | |||
| 1502 | 1506 | ||||
| 1503 | err := svc.Restart(ctx, "test.example.com", false) | 1507 | err := svc.Restart(ctx, "test.example.com", false) | ||
| 1504 | 1508 | ||||
| @@ -1520,8 +1524,7 @@ func TestService_Restart_WithAttachments(t *testing.T) { | |||||
| 1520 | Status: "running", | 1524 | Status: "running", | ||
| 1521 | } | 1525 | } | ||
| 1522 | svc.attachments["test.example.com"] = []string{"container-attach-1", "container-attach-2"} | 1526 | svc.attachments["test.example.com"] = []string{"container-attach-1", "container-attach-2"} | ||
| 1523 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | 1527 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 1524 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 1525 | { | 1528 | { | ||
| 1526 | ID: "container-123", | 1529 | ID: "container-123", | ||
| 1527 | Labels: map[string]string{ | 1530 | Labels: map[string]string{ | ||
| @@ -1546,6 +1549,7 @@ func TestService_Restart_WithAttachments(t *testing.T) { | |||||
| 1546 | }, | 1549 | }, | ||
| 1547 | }, | 1550 | }, | ||
| 1548 | }, nil).Maybe() | 1551 | }, nil).Maybe() | ||
| 1552 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |||
| 1549 | 1553 | ||||
| 1550 | runtime.EXPECT().RestartContainer(mock.Anything, "container-123").Return(nil) | 1554 | runtime.EXPECT().RestartContainer(mock.Anything, "container-123").Return(nil) | ||
| 1551 | runtime.EXPECT().RestartContainer(mock.Anything, "container-attach-1").Return(nil) | 1555 | runtime.EXPECT().RestartContainer(mock.Anything, "container-attach-1").Return(nil) | ||
| @@ -1576,7 +1580,6 @@ func TestService_Restart_WithAttachments_ErrorsWhenConfiguredButNotDeployed(t *t | |||||
| 1576 | } | 1580 | } | ||
| 1577 | // No attachment containers tracked — s.attachments is empty | 1581 | // No attachment containers tracked — s.attachments is empty | ||
| 1578 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | 1582 | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | ||
| 1579 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil).Maybe() | |||
| 1580 | 1583 | ||||
| 1581 | err := svc.Restart(testContext(), "test.example.com", true) | 1584 | err := svc.Restart(testContext(), "test.example.com", true) | ||
| 1582 | 1585 | ||||
| @@ -1606,8 +1609,7 @@ func TestService_Restart_WithAttachments_ErrorsWhenPartiallyDeployed(t *testing. | |||||
| 1606 | } | 1609 | } | ||
| 1607 | // Only 1 of 2 attachments deployed | 1610 | // Only 1 of 2 attachments deployed | ||
| 1608 | svc.attachments["test.example.com"] = []string{"attach-postgres"} | 1611 | svc.attachments["test.example.com"] = []string{"attach-postgres"} | ||
| 1609 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | 1612 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 1610 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 1611 | { | 1613 | { | ||
| 1612 | ID: "container-123", | 1614 | ID: "container-123", | ||
| 1613 | Labels: map[string]string{ | 1615 | Labels: map[string]string{ | ||
| @@ -1624,6 +1626,7 @@ func TestService_Restart_WithAttachments_ErrorsWhenPartiallyDeployed(t *testing. | |||||
| 1624 | }, | 1626 | }, | ||
| 1625 | }, | 1627 | }, | ||
| 1626 | }, nil).Maybe() | 1628 | }, nil).Maybe() | ||
| 1629 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |||
| 1627 | 1630 | ||||
| 1628 | err := svc.Restart(testContext(), "test.example.com", true) | 1631 | err := svc.Restart(testContext(), "test.example.com", true) | ||
| 1629 | 1632 | ||||
| @@ -1653,8 +1656,7 @@ func TestService_Restart_WithAttachments_SucceedsWhenAllDeployed(t *testing.T) { | |||||
| 1653 | Status: "running", | 1656 | Status: "running", | ||
| 1654 | } | 1657 | } | ||
| 1655 | svc.attachments["test.example.com"] = []string{"attach-456"} | 1658 | svc.attachments["test.example.com"] = []string{"attach-456"} | ||
| 1656 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Maybe() | 1659 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 1657 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 1658 | { | 1660 | { | ||
| 1659 | ID: "container-123", | 1661 | ID: "container-123", | ||
| 1660 | Labels: map[string]string{ | 1662 | Labels: map[string]string{ | ||
| @@ -1671,6 +1673,7 @@ func TestService_Restart_WithAttachments_SucceedsWhenAllDeployed(t *testing.T) { | |||||
| 1671 | }, | 1673 | }, | ||
| 1672 | }, | 1674 | }, | ||
| 1673 | }, nil).Maybe() | 1675 | }, nil).Maybe() | ||
| 1676 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, mock.Anything, domain.RestartPolicyAlways).Return(nil).Maybe() | |||
| 1674 | 1677 | ||||
| 1675 | runtime.EXPECT().RestartContainer(mock.Anything, "container-123").Return(nil) | 1678 | runtime.EXPECT().RestartContainer(mock.Anything, "container-123").Return(nil) | ||
| 1676 | runtime.EXPECT().RestartContainer(mock.Anything, "attach-456").Return(nil) | 1679 | runtime.EXPECT().RestartContainer(mock.Anything, "attach-456").Return(nil) | ||
| @@ -3198,8 +3201,7 @@ func TestSyncContainersRebuildsAttachmentMap(t *testing.T) { | |||||
| 3198 | ctx := testContext() | 3201 | ctx := testContext() | ||
| 3199 | 3202 | ||||
| 3200 | // ListContainers returns one main container and one attachment container | 3203 | // ListContainers returns one main container and one attachment container | ||
| 3201 | - | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil).Once() | 3204 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ |
| 3202 | - | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 3203 | { | 3205 | { | ||
| 3204 | ID: "main-container-1", | 3206 | ID: "main-container-1", | ||
| 3205 | Labels: map[string]string{ | 3207 | Labels: map[string]string{ | ||
| @@ -3216,6 +3218,8 @@ func TestSyncContainersRebuildsAttachmentMap(t *testing.T) { | |||||
| 3216 | }, | 3218 | }, | ||
| 3217 | }, | 3219 | }, | ||
| 3218 | }, nil).Once() | 3220 | }, nil).Once() | ||
| 3221 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "main-container-1", domain.RestartPolicyAlways).Return(nil).Once() | |||
| 3222 | + | runtime.EXPECT().EnsureContainerRestartPolicy(mock.Anything, "attachment-container-1", domain.RestartPolicyAlways).Return(nil).Once() | |||
| 3219 | 3223 | ||||
| 3220 | require.NoError(t, svc.SyncContainers(ctx)) | 3224 | require.NoError(t, svc.SyncContainers(ctx)) | ||
| 3221 | 3225 | ||||