fix(container): propagate attachment config changes to container service on config reload
- commit
- f5b6a2e54c7e1a64b9bb76d2ff12be7f4b813a70
- author
- bnema · · 2026-02-24 21:57 UTC
- parent
- 7643cd4
- signature
- unsigned
5 files · +104 -0
M internal/boundaries/in/container.go +4 -0
| @@ -45,6 +45,10 @@ type ContainerService interface { | |||
| 45 | 45 | // SyncContainers synchronizes containers with configured routes. | |
| 46 | 46 | SyncContainers(ctx context.Context) error | |
| 47 | 47 | ||
| 48 | + | // UpdateAttachments updates the attachment configuration in the container service. | |
| 49 | + | // This is called after a config reload to propagate attachment changes without restart. | |
| 50 | + | UpdateAttachments(attachments map[string][]string) | |
| 51 | + | ||
| 48 | 52 | // AutoStart starts containers for the provided routes that aren't running. | |
| 49 | 53 | AutoStart(ctx context.Context, routes []domain.Route) error | |
| 50 | 54 | ||
| @@ -45,6 +45,10 @@ type ContainerService interface { | |||||
| 45 | // SyncContainers synchronizes containers with configured routes. | 45 | // SyncContainers synchronizes containers with configured routes. | ||
| 46 | SyncContainers(ctx context.Context) error | 46 | SyncContainers(ctx context.Context) error | ||
| 47 | 47 | ||||
| 48 | + | // UpdateAttachments updates the attachment configuration in the container service. | |||
| 49 | + | // This is called after a config reload to propagate attachment changes without restart. | |||
| 50 | + | UpdateAttachments(attachments map[string][]string) | |||
| 51 | + | ||||
| 48 | // AutoStart starts containers for the provided routes that aren't running. | 52 | // AutoStart starts containers for the provided routes that aren't running. | ||
| 49 | AutoStart(ctx context.Context, routes []domain.Route) error | 53 | AutoStart(ctx context.Context, routes []domain.Route) error | ||
| 50 | 54 | ||||
M internal/boundaries/in/mocks/mock_container_service.go +37 -0
| @@ -795,3 +795,40 @@ func (_c *MockContainerService_SyncContainers_Call) RunAndReturn(run func(ctx co | |||
| 795 | 795 | _c.Call.Return(run) | |
| 796 | 796 | return _c | |
| 797 | 797 | } | |
| 798 | + | ||
| 799 | + | // UpdateAttachments provides a mock function for the type MockContainerService | |
| 800 | + | func (_mock *MockContainerService) UpdateAttachments(attachments map[string][]string) { | |
| 801 | + | _mock.Called(attachments) | |
| 802 | + | } | |
| 803 | + | ||
| 804 | + | // MockContainerService_UpdateAttachments_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'UpdateAttachments' | |
| 805 | + | type MockContainerService_UpdateAttachments_Call struct { | |
| 806 | + | *mock.Call | |
| 807 | + | } | |
| 808 | + | ||
| 809 | + | // UpdateAttachments is a helper method to define mock.On call | |
| 810 | + | // - attachments map[string][]string | |
| 811 | + | func (_e *MockContainerService_Expecter) UpdateAttachments(attachments interface{}) *MockContainerService_UpdateAttachments_Call { | |
| 812 | + | return &MockContainerService_UpdateAttachments_Call{Call: _e.mock.On("UpdateAttachments", attachments)} | |
| 813 | + | } | |
| 814 | + | ||
| 815 | + | func (_c *MockContainerService_UpdateAttachments_Call) Run(run func(attachments map[string][]string)) *MockContainerService_UpdateAttachments_Call { | |
| 816 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 817 | + | var arg0 map[string][]string | |
| 818 | + | if args[0] != nil { | |
| 819 | + | arg0 = args[0].(map[string][]string) | |
| 820 | + | } | |
| 821 | + | run(arg0) | |
| 822 | + | }) | |
| 823 | + | return _c | |
| 824 | + | } | |
| 825 | + | ||
| 826 | + | func (_c *MockContainerService_UpdateAttachments_Call) Return() *MockContainerService_UpdateAttachments_Call { | |
| 827 | + | _c.Call.Return() | |
| 828 | + | return _c | |
| 829 | + | } | |
| 830 | + | ||
| 831 | + | func (_c *MockContainerService_UpdateAttachments_Call) RunAndReturn(run func(attachments map[string][]string)) *MockContainerService_UpdateAttachments_Call { | |
| 832 | + | _c.Run(run) | |
| 833 | + | return _c | |
| 834 | + | } | |
| @@ -795,3 +795,40 @@ func (_c *MockContainerService_SyncContainers_Call) RunAndReturn(run func(ctx co | |||||
| 795 | _c.Call.Return(run) | 795 | _c.Call.Return(run) | ||
| 796 | return _c | 796 | return _c | ||
| 797 | } | 797 | } | ||
| 798 | + | ||||
| 799 | + | // UpdateAttachments provides a mock function for the type MockContainerService | |||
| 800 | + | func (_mock *MockContainerService) UpdateAttachments(attachments map[string][]string) { | |||
| 801 | + | _mock.Called(attachments) | |||
| 802 | + | } | |||
| 803 | + | ||||
| 804 | + | // MockContainerService_UpdateAttachments_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'UpdateAttachments' | |||
| 805 | + | type MockContainerService_UpdateAttachments_Call struct { | |||
| 806 | + | *mock.Call | |||
| 807 | + | } | |||
| 808 | + | ||||
| 809 | + | // UpdateAttachments is a helper method to define mock.On call | |||
| 810 | + | // - attachments map[string][]string | |||
| 811 | + | func (_e *MockContainerService_Expecter) UpdateAttachments(attachments interface{}) *MockContainerService_UpdateAttachments_Call { | |||
| 812 | + | return &MockContainerService_UpdateAttachments_Call{Call: _e.mock.On("UpdateAttachments", attachments)} | |||
| 813 | + | } | |||
| 814 | + | ||||
| 815 | + | func (_c *MockContainerService_UpdateAttachments_Call) Run(run func(attachments map[string][]string)) *MockContainerService_UpdateAttachments_Call { | |||
| 816 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 817 | + | var arg0 map[string][]string | |||
| 818 | + | if args[0] != nil { | |||
| 819 | + | arg0 = args[0].(map[string][]string) | |||
| 820 | + | } | |||
| 821 | + | run(arg0) | |||
| 822 | + | }) | |||
| 823 | + | return _c | |||
| 824 | + | } | |||
| 825 | + | ||||
| 826 | + | func (_c *MockContainerService_UpdateAttachments_Call) Return() *MockContainerService_UpdateAttachments_Call { | |||
| 827 | + | _c.Call.Return() | |||
| 828 | + | return _c | |||
| 829 | + | } | |||
| 830 | + | ||||
| 831 | + | func (_c *MockContainerService_UpdateAttachments_Call) RunAndReturn(run func(attachments map[string][]string)) *MockContainerService_UpdateAttachments_Call { | |||
| 832 | + | _c.Run(run) | |||
| 833 | + | return _c | |||
| 834 | + | } | |||
M internal/usecase/container/events.go +5 -0
| @@ -107,6 +107,11 @@ func (h *ConfigReloadHandler) Handle(ctx context.Context, event domain.Event) er | |||
| 107 | 107 | log.Warn().Err(err).Msg("failed to sync containers before reload, proceeding with current state") | |
| 108 | 108 | } | |
| 109 | 109 | ||
| 110 | + | // Propagate updated attachment configuration so new attachments take effect | |
| 111 | + | // without requiring a Gordon restart (fixes issue #87 part 2). | |
| 112 | + | attachments := h.configSvc.GetAllAttachments(ctx) | |
| 113 | + | h.containerSvc.UpdateAttachments(attachments) | |
| 114 | + | ||
| 110 | 115 | currentContainers := h.containerSvc.List(ctx) | |
| 111 | 116 | ||
| 112 | 117 | activeRoutes := make(map[string]*domain.Container) | |
| @@ -107,6 +107,11 @@ func (h *ConfigReloadHandler) Handle(ctx context.Context, event domain.Event) er | |||||
| 107 | log.Warn().Err(err).Msg("failed to sync containers before reload, proceeding with current state") | 107 | log.Warn().Err(err).Msg("failed to sync containers before reload, proceeding with current state") | ||
| 108 | } | 108 | } | ||
| 109 | 109 | ||||
| 110 | + | // Propagate updated attachment configuration so new attachments take effect | |||
| 111 | + | // without requiring a Gordon restart (fixes issue #87 part 2). | |||
| 112 | + | attachments := h.configSvc.GetAllAttachments(ctx) | |||
| 113 | + | h.containerSvc.UpdateAttachments(attachments) | |||
| 114 | + | ||||
| 110 | currentContainers := h.containerSvc.List(ctx) | 115 | currentContainers := h.containerSvc.List(ctx) | ||
| 111 | 116 | ||||
| 112 | activeRoutes := make(map[string]*domain.Container) | 117 | activeRoutes := make(map[string]*domain.Container) | ||
M internal/usecase/container/events_test.go +50 -0
| @@ -209,6 +209,10 @@ func TestConfigReloadHandler_Handle_DeploysNewRoutes(t *testing.T) { | |||
| 209 | 209 | // SyncContainers is called first | |
| 210 | 210 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | |
| 211 | 211 | ||
| 212 | + | // Attachment config is propagated after sync | |
| 213 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |
| 214 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |
| 215 | + | ||
| 212 | 216 | // No existing containers | |
| 213 | 217 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{}) | |
| 214 | 218 | ||
| @@ -241,6 +245,10 @@ func TestConfigReloadHandler_Handle_StopsRemovedRoutes(t *testing.T) { | |||
| 241 | 245 | // SyncContainers is called first | |
| 242 | 246 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | |
| 243 | 247 | ||
| 248 | + | // Attachment config is propagated after sync | |
| 249 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |
| 250 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |
| 251 | + | ||
| 244 | 252 | // Existing container for a route that's no longer configured | |
| 245 | 253 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | |
| 246 | 254 | "removed.example.com": { | |
| @@ -277,6 +285,10 @@ func TestConfigReloadHandler_Handle_RedeploysChangedImage(t *testing.T) { | |||
| 277 | 285 | // SyncContainers is called first | |
| 278 | 286 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | |
| 279 | 287 | ||
| 288 | + | // Attachment config is propagated after sync | |
| 289 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |
| 290 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |
| 291 | + | ||
| 280 | 292 | // Existing container with old image | |
| 281 | 293 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | |
| 282 | 294 | "app.example.com": { | |
| @@ -317,6 +329,10 @@ func TestConfigReloadHandler_Handle_NoChanges(t *testing.T) { | |||
| 317 | 329 | // SyncContainers is called first | |
| 318 | 330 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | |
| 319 | 331 | ||
| 332 | + | // Attachment config is propagated after sync | |
| 333 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |
| 334 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |
| 335 | + | ||
| 320 | 336 | // Existing container matches config | |
| 321 | 337 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | |
| 322 | 338 | "app.example.com": { | |
| @@ -494,6 +510,40 @@ func TestManualReloadHandler_Handle_DoesNotRestartRunningContainers(t *testing.T | |||
| 494 | 510 | ||
| 495 | 511 | // ManualDeployHandler tests | |
| 496 | 512 | ||
| 513 | + | func TestConfigReloadHandlerUpdatesContainerConfig(t *testing.T) { | |
| 514 | + | containerSvc := inmocks.NewMockContainerService(t) | |
| 515 | + | configSvc := inmocks.NewMockConfigService(t) | |
| 516 | + | ||
| 517 | + | handler := NewConfigReloadHandler(testCtx(), containerSvc, configSvc) | |
| 518 | + | ||
| 519 | + | // SyncContainers is called first | |
| 520 | + | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | |
| 521 | + | ||
| 522 | + | // No existing containers | |
| 523 | + | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{}) | |
| 524 | + | ||
| 525 | + | // New attachments from config | |
| 526 | + | newAttachments := map[string][]string{ | |
| 527 | + | "app.example.com": {"postgres:15", "redis:7"}, | |
| 528 | + | } | |
| 529 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(newAttachments) | |
| 530 | + | ||
| 531 | + | // Expect UpdateAttachments to be called with new attachment config | |
| 532 | + | containerSvc.EXPECT().UpdateAttachments(newAttachments).Return() | |
| 533 | + | ||
| 534 | + | // No routes configured | |
| 535 | + | configSvc.EXPECT().GetRoutes(mock.Anything).Return([]domain.Route{}) | |
| 536 | + | ||
| 537 | + | event := domain.Event{ | |
| 538 | + | ID: "event-123", | |
| 539 | + | Type: domain.EventConfigReload, | |
| 540 | + | } | |
| 541 | + | ||
| 542 | + | err := handler.Handle(context.Background(), event) | |
| 543 | + | ||
| 544 | + | assert.NoError(t, err) | |
| 545 | + | } | |
| 546 | + | ||
| 497 | 547 | func TestManualDeployHandler_CanHandle(t *testing.T) { | |
| 498 | 548 | containerSvc := inmocks.NewMockContainerService(t) | |
| 499 | 549 | configSvc := inmocks.NewMockConfigService(t) | |
| @@ -209,6 +209,10 @@ func TestConfigReloadHandler_Handle_DeploysNewRoutes(t *testing.T) { | |||||
| 209 | // SyncContainers is called first | 209 | // SyncContainers is called first | ||
| 210 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | 210 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | ||
| 211 | 211 | ||||
| 212 | + | // Attachment config is propagated after sync | |||
| 213 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |||
| 214 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |||
| 215 | + | ||||
| 212 | // No existing containers | 216 | // No existing containers | ||
| 213 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{}) | 217 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{}) | ||
| 214 | 218 | ||||
| @@ -241,6 +245,10 @@ func TestConfigReloadHandler_Handle_StopsRemovedRoutes(t *testing.T) { | |||||
| 241 | // SyncContainers is called first | 245 | // SyncContainers is called first | ||
| 242 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | 246 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | ||
| 243 | 247 | ||||
| 248 | + | // Attachment config is propagated after sync | |||
| 249 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |||
| 250 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |||
| 251 | + | ||||
| 244 | // Existing container for a route that's no longer configured | 252 | // Existing container for a route that's no longer configured | ||
| 245 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | 253 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | ||
| 246 | "removed.example.com": { | 254 | "removed.example.com": { | ||
| @@ -277,6 +285,10 @@ func TestConfigReloadHandler_Handle_RedeploysChangedImage(t *testing.T) { | |||||
| 277 | // SyncContainers is called first | 285 | // SyncContainers is called first | ||
| 278 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | 286 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | ||
| 279 | 287 | ||||
| 288 | + | // Attachment config is propagated after sync | |||
| 289 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |||
| 290 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |||
| 291 | + | ||||
| 280 | // Existing container with old image | 292 | // Existing container with old image | ||
| 281 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | 293 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | ||
| 282 | "app.example.com": { | 294 | "app.example.com": { | ||
| @@ -317,6 +329,10 @@ func TestConfigReloadHandler_Handle_NoChanges(t *testing.T) { | |||||
| 317 | // SyncContainers is called first | 329 | // SyncContainers is called first | ||
| 318 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | 330 | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | ||
| 319 | 331 | ||||
| 332 | + | // Attachment config is propagated after sync | |||
| 333 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(map[string][]string{}) | |||
| 334 | + | containerSvc.EXPECT().UpdateAttachments(map[string][]string{}).Return() | |||
| 335 | + | ||||
| 320 | // Existing container matches config | 336 | // Existing container matches config | ||
| 321 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | 337 | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{ | ||
| 322 | "app.example.com": { | 338 | "app.example.com": { | ||
| @@ -494,6 +510,40 @@ func TestManualReloadHandler_Handle_DoesNotRestartRunningContainers(t *testing.T | |||||
| 494 | 510 | ||||
| 495 | // ManualDeployHandler tests | 511 | // ManualDeployHandler tests | ||
| 496 | 512 | ||||
| 513 | + | func TestConfigReloadHandlerUpdatesContainerConfig(t *testing.T) { | |||
| 514 | + | containerSvc := inmocks.NewMockContainerService(t) | |||
| 515 | + | configSvc := inmocks.NewMockConfigService(t) | |||
| 516 | + | ||||
| 517 | + | handler := NewConfigReloadHandler(testCtx(), containerSvc, configSvc) | |||
| 518 | + | ||||
| 519 | + | // SyncContainers is called first | |||
| 520 | + | containerSvc.EXPECT().SyncContainers(mock.Anything).Return(nil) | |||
| 521 | + | ||||
| 522 | + | // No existing containers | |||
| 523 | + | containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{}) | |||
| 524 | + | ||||
| 525 | + | // New attachments from config | |||
| 526 | + | newAttachments := map[string][]string{ | |||
| 527 | + | "app.example.com": {"postgres:15", "redis:7"}, | |||
| 528 | + | } | |||
| 529 | + | configSvc.EXPECT().GetAllAttachments(mock.Anything).Return(newAttachments) | |||
| 530 | + | ||||
| 531 | + | // Expect UpdateAttachments to be called with new attachment config | |||
| 532 | + | containerSvc.EXPECT().UpdateAttachments(newAttachments).Return() | |||
| 533 | + | ||||
| 534 | + | // No routes configured | |||
| 535 | + | configSvc.EXPECT().GetRoutes(mock.Anything).Return([]domain.Route{}) | |||
| 536 | + | ||||
| 537 | + | event := domain.Event{ | |||
| 538 | + | ID: "event-123", | |||
| 539 | + | Type: domain.EventConfigReload, | |||
| 540 | + | } | |||
| 541 | + | ||||
| 542 | + | err := handler.Handle(context.Background(), event) | |||
| 543 | + | ||||
| 544 | + | assert.NoError(t, err) | |||
| 545 | + | } | |||
| 546 | + | ||||
| 497 | func TestManualDeployHandler_CanHandle(t *testing.T) { | 547 | func TestManualDeployHandler_CanHandle(t *testing.T) { | ||
| 498 | containerSvc := inmocks.NewMockContainerService(t) | 548 | containerSvc := inmocks.NewMockContainerService(t) | ||
| 499 | configSvc := inmocks.NewMockConfigService(t) | 549 | configSvc := inmocks.NewMockConfigService(t) | ||
M internal/usecase/container/service.go +8 -0
| @@ -1071,6 +1071,14 @@ func (s *Service) UpdateConfig(config Config) { | |||
| 1071 | 1071 | s.mu.Unlock() | |
| 1072 | 1072 | } | |
| 1073 | 1073 | ||
| 1074 | + | // UpdateAttachments updates only the attachment configuration in the service. | |
| 1075 | + | // This is called after a config reload to propagate attachment changes without restart. | |
| 1076 | + | func (s *Service) UpdateAttachments(attachments map[string][]string) { | |
| 1077 | + | s.mu.Lock() | |
| 1078 | + | s.config.Attachments = attachments | |
| 1079 | + | s.mu.Unlock() | |
| 1080 | + | } | |
| 1081 | + | ||
| 1074 | 1082 | // Helper methods | |
| 1075 | 1083 | ||
| 1076 | 1084 | // cleanupFailedContainer stops and removes a container that failed to start properly. | |
| @@ -1071,6 +1071,14 @@ func (s *Service) UpdateConfig(config Config) { | |||||
| 1071 | s.mu.Unlock() | 1071 | s.mu.Unlock() | ||
| 1072 | } | 1072 | } | ||
| 1073 | 1073 | ||||
| 1074 | + | // UpdateAttachments updates only the attachment configuration in the service. | |||
| 1075 | + | // This is called after a config reload to propagate attachment changes without restart. | |||
| 1076 | + | func (s *Service) UpdateAttachments(attachments map[string][]string) { | |||
| 1077 | + | s.mu.Lock() | |||
| 1078 | + | s.config.Attachments = attachments | |||
| 1079 | + | s.mu.Unlock() | |||
| 1080 | + | } | |||
| 1081 | + | ||||
| 1074 | // Helper methods | 1082 | // Helper methods | ||
| 1075 | 1083 | ||||
| 1076 | // cleanupFailedContainer stops and removes a container that failed to start properly. | 1084 | // cleanupFailedContainer stops and removes a container that failed to start properly. | ||