refactor: address CodeRabbit review nitpicks
- Remove unused containerConfig parameter from resolveContainerEndpoint and update both call sites in readinessCascade - Add t.Logf in TestTCPProbe_DelayedListener to surface re-listen failures instead of silently swallowing them - Times(5) for IsContainerRunning in concurrent test is correct: 2x per deploy (pollContainerRunning + waitForReadyByDelay) + 1x stabilization for the second deploy = 5 total; left as-is
- commit
- fe5898575cce58e4f1b6e78402d54d0cdf1292d5
- author
- brice · · 2026-03-03 03:47 UTC
- parent
- 6cba420
- signature
- unsigned
2 files · +4 -3
M internal/usecase/container/readiness_test.go +1 -0
| @@ -63,6 +63,7 @@ func TestTCPProbe_DelayedListener(t *testing.T) { | |||
| 63 | 63 | time.Sleep(time.Second) | |
| 64 | 64 | newLn, err := net.Listen("tcp", addr) | |
| 65 | 65 | if err != nil { | |
| 66 | + | t.Logf("delayed listener failed to bind: %v", err) | |
| 66 | 67 | return | |
| 67 | 68 | } | |
| 68 | 69 | defer newLn.Close() | |
| @@ -63,6 +63,7 @@ func TestTCPProbe_DelayedListener(t *testing.T) { | |||||
| 63 | time.Sleep(time.Second) | 63 | time.Sleep(time.Second) | ||
| 64 | newLn, err := net.Listen("tcp", addr) | 64 | newLn, err := net.Listen("tcp", addr) | ||
| 65 | if err != nil { | 65 | if err != nil { | ||
| 66 | + | t.Logf("delayed listener failed to bind: %v", err) | |||
| 66 | return | 67 | return | ||
| 67 | } | 68 | } | ||
| 68 | defer newLn.Close() | 69 | defer newLn.Close() | ||
M internal/usecase/container/service.go +3 -3
| @@ -2288,7 +2288,7 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont | |||
| 2288 | 2288 | // 2. HTTP probe via gordon.health label | |
| 2289 | 2289 | if containerConfig != nil { | |
| 2290 | 2290 | if healthPath, ok := containerConfig.Labels[domain.LabelHealth]; ok && healthPath != "" { | |
| 2291 | - | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID, containerConfig) | |
| 2291 | + | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID) | |
| 2292 | 2292 | if probeErr == nil && ip != "" && port > 0 { | |
| 2293 | 2293 | url := fmt.Sprintf("http://%s:%d%s", ip, port, healthPath) | |
| 2294 | 2294 | timeout := cfg.HTTPProbeTimeout | |
| @@ -2305,7 +2305,7 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont | |||
| 2305 | 2305 | ||
| 2306 | 2306 | // 3. TCP probe (if port info available) | |
| 2307 | 2307 | if containerConfig != nil && len(containerConfig.Ports) > 0 { | |
| 2308 | - | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID, containerConfig) | |
| 2308 | + | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID) | |
| 2309 | 2309 | if probeErr == nil && ip != "" && port > 0 { | |
| 2310 | 2310 | addr := fmt.Sprintf("%s:%d", ip, port) | |
| 2311 | 2311 | timeout := cfg.TCPProbeTimeout | |
| @@ -2326,7 +2326,7 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont | |||
| 2326 | 2326 | ||
| 2327 | 2327 | // resolveContainerEndpoint retrieves the container's IP and first port via | |
| 2328 | 2328 | // GetContainerNetworkInfo. The result can be used for HTTP or TCP probes. | |
| 2329 | - | func (s *Service) resolveContainerEndpoint(ctx context.Context, containerID string, containerConfig *domain.ContainerConfig) (string, int, error) { | |
| 2329 | + | func (s *Service) resolveContainerEndpoint(ctx context.Context, containerID string) (string, int, error) { | |
| 2330 | 2330 | ip, port, err := s.runtime.GetContainerNetworkInfo(ctx, containerID) | |
| 2331 | 2331 | if err != nil { | |
| 2332 | 2332 | return "", 0, err | |
| @@ -2288,7 +2288,7 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont | |||||
| 2288 | // 2. HTTP probe via gordon.health label | 2288 | // 2. HTTP probe via gordon.health label | ||
| 2289 | if containerConfig != nil { | 2289 | if containerConfig != nil { | ||
| 2290 | if healthPath, ok := containerConfig.Labels[domain.LabelHealth]; ok && healthPath != "" { | 2290 | if healthPath, ok := containerConfig.Labels[domain.LabelHealth]; ok && healthPath != "" { | ||
| 2291 | - | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID, containerConfig) | 2291 | + | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID) |
| 2292 | if probeErr == nil && ip != "" && port > 0 { | 2292 | if probeErr == nil && ip != "" && port > 0 { | ||
| 2293 | url := fmt.Sprintf("http://%s:%d%s", ip, port, healthPath) | 2293 | url := fmt.Sprintf("http://%s:%d%s", ip, port, healthPath) | ||
| 2294 | timeout := cfg.HTTPProbeTimeout | 2294 | timeout := cfg.HTTPProbeTimeout | ||
| @@ -2305,7 +2305,7 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont | |||||
| 2305 | 2305 | ||||
| 2306 | // 3. TCP probe (if port info available) | 2306 | // 3. TCP probe (if port info available) | ||
| 2307 | if containerConfig != nil && len(containerConfig.Ports) > 0 { | 2307 | if containerConfig != nil && len(containerConfig.Ports) > 0 { | ||
| 2308 | - | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID, containerConfig) | 2308 | + | ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID) |
| 2309 | if probeErr == nil && ip != "" && port > 0 { | 2309 | if probeErr == nil && ip != "" && port > 0 { | ||
| 2310 | addr := fmt.Sprintf("%s:%d", ip, port) | 2310 | addr := fmt.Sprintf("%s:%d", ip, port) | ||
| 2311 | timeout := cfg.TCPProbeTimeout | 2311 | timeout := cfg.TCPProbeTimeout | ||
| @@ -2326,7 +2326,7 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont | |||||
| 2326 | 2326 | ||||
| 2327 | // resolveContainerEndpoint retrieves the container's IP and first port via | 2327 | // resolveContainerEndpoint retrieves the container's IP and first port via | ||
| 2328 | // GetContainerNetworkInfo. The result can be used for HTTP or TCP probes. | 2328 | // GetContainerNetworkInfo. The result can be used for HTTP or TCP probes. | ||
| 2329 | - | func (s *Service) resolveContainerEndpoint(ctx context.Context, containerID string, containerConfig *domain.ContainerConfig) (string, int, error) { | 2329 | + | func (s *Service) resolveContainerEndpoint(ctx context.Context, containerID string) (string, int, error) { |
| 2330 | ip, port, err := s.runtime.GetContainerNetworkInfo(ctx, containerID) | 2330 | ip, port, err := s.runtime.GetContainerNetworkInfo(ctx, containerID) | ||
| 2331 | if err != nil { | 2331 | if err != nil { | ||
| 2332 | return "", 0, err | 2332 | return "", 0, err | ||