fix(secrets): recover orphaned pass keys for env injection
- commit
- cdca39f9973b85f585e398d94a56fc58bce8a592
- author
- brice · · 2026-02-06 16:28 UTC
- committer
- brice · · 2026-02-06 19:34 UTC
- parent
- 0bd5d71
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
10 files · +647 -35
M docs/cli/restart.md +3 -1
| @@ -44,7 +44,9 @@ gordon restart myapp.example.com --with-attachments | |||
| 44 | 44 | ||
| 45 | 45 | ### Notes | |
| 46 | 46 | ||
| 47 | - | - Remote mode required. See [CLI Overview](./index.md) for targeting options. | |
| 47 | + | - Local mode works when run on the Gordon host. It uses the same deploy-signal path as `gordon deploy <domain>`. | |
| 48 | + | - `--with-attachments` is only supported in remote mode. | |
| 49 | + | - In remote mode, target your Gordon admin endpoint (for example `https://gordon.example.com`), not the registry host (for example `https://reg.example.com`). | |
| 48 | 50 | ||
| 49 | 51 | ## Related | |
| 50 | 52 | ||
| @@ -44,7 +44,9 @@ gordon restart myapp.example.com --with-attachments | |||||
| 44 | 44 | ||||
| 45 | ### Notes | 45 | ### Notes | ||
| 46 | 46 | ||||
| 47 | - | - Remote mode required. See [CLI Overview](./index.md) for targeting options. | 47 | + | - Local mode works when run on the Gordon host. It uses the same deploy-signal path as `gordon deploy <domain>`. |
| 48 | + | - `--with-attachments` is only supported in remote mode. | |||
| 49 | + | - In remote mode, target your Gordon admin endpoint (for example `https://gordon.example.com`), not the registry host (for example `https://reg.example.com`). | |||
| 48 | 50 | ||||
| 49 | ## Related | 51 | ## Related | ||
| 50 | 52 | ||||
M internal/adapters/in/cli/remote/client.go +76 -10
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "bytes" | |
| 6 | 6 | "context" | |
| 7 | 7 | "encoding/json" | |
| 8 | + | "errors" | |
| 8 | 9 | "fmt" | |
| 9 | 10 | "io" | |
| 10 | 11 | "net/http" | |
| @@ -23,6 +24,11 @@ type Client struct { | |||
| 23 | 24 | httpClient *http.Client | |
| 24 | 25 | } | |
| 25 | 26 | ||
| 27 | + | var ( | |
| 28 | + | retryMaxAttempts = 4 | |
| 29 | + | retryBaseDelay = 250 * time.Millisecond | |
| 30 | + | ) | |
| 31 | + | ||
| 26 | 32 | // ClientOption configures the Client. | |
| 27 | 33 | type ClientOption func(*Client) | |
| 28 | 34 | ||
| @@ -100,13 +106,7 @@ func parseResponse(resp *http.Response, target any) error { | |||
| 100 | 106 | ||
| 101 | 107 | if resp.StatusCode >= 400 { | |
| 102 | 108 | body, _ := io.ReadAll(resp.Body) | |
| 103 | - | var errResp struct { | |
| 104 | - | Error string `json:"error"` | |
| 105 | - | } | |
| 106 | - | if err := json.Unmarshal(body, &errResp); err == nil && errResp.Error != "" { | |
| 107 | - | return fmt.Errorf("%s: %s", resp.Status, errResp.Error) | |
| 108 | - | } | |
| 109 | - | return fmt.Errorf("%s: %s", resp.Status, string(body)) | |
| 109 | + | return parseErrorResponse(resp, body) | |
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | 112 | if target != nil { | |
| @@ -118,6 +118,72 @@ func parseResponse(resp *http.Response, target any) error { | |||
| 118 | 118 | return nil | |
| 119 | 119 | } | |
| 120 | 120 | ||
| 121 | + | func parseErrorResponse(resp *http.Response, body []byte) error { | |
| 122 | + | var errResp struct { | |
| 123 | + | Error string `json:"error"` | |
| 124 | + | } | |
| 125 | + | if err := json.Unmarshal(body, &errResp); err == nil && errResp.Error != "" { | |
| 126 | + | return fmt.Errorf("%s: %s", resp.Status, errResp.Error) | |
| 127 | + | } | |
| 128 | + | return fmt.Errorf("%s: %s", resp.Status, string(body)) | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | func isRetryableRequestError(err error) bool { | |
| 132 | + | return !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) | |
| 133 | + | } | |
| 134 | + | ||
| 135 | + | func isRetryableStatus(status int) bool { | |
| 136 | + | return status >= 500 && status <= 599 | |
| 137 | + | } | |
| 138 | + | ||
| 139 | + | func retryDelay(attempt int) time.Duration { | |
| 140 | + | if attempt <= 1 { | |
| 141 | + | return retryBaseDelay | |
| 142 | + | } | |
| 143 | + | return retryBaseDelay * time.Duration(1<<(attempt-1)) | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | // requestWithRetry performs an HTTP request and retries transient failures. | |
| 147 | + | // Retries occur on transport errors and any 5xx response. | |
| 148 | + | func (c *Client) requestWithRetry(ctx context.Context, method, path string, body any) (*http.Response, error) { | |
| 149 | + | var lastErr error | |
| 150 | + | ||
| 151 | + | for attempt := 1; attempt <= retryMaxAttempts; attempt++ { | |
| 152 | + | resp, err := c.request(ctx, method, path, body) | |
| 153 | + | if err != nil { | |
| 154 | + | lastErr = err | |
| 155 | + | if attempt == retryMaxAttempts || !isRetryableRequestError(err) { | |
| 156 | + | return nil, err | |
| 157 | + | } | |
| 158 | + | } else { | |
| 159 | + | if !isRetryableStatus(resp.StatusCode) { | |
| 160 | + | return resp, nil | |
| 161 | + | } | |
| 162 | + | ||
| 163 | + | respBody, _ := io.ReadAll(resp.Body) | |
| 164 | + | _ = resp.Body.Close() | |
| 165 | + | lastErr = parseErrorResponse(resp, respBody) | |
| 166 | + | if attempt == retryMaxAttempts { | |
| 167 | + | return nil, lastErr | |
| 168 | + | } | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | delay := retryDelay(attempt) | |
| 172 | + | timer := time.NewTimer(delay) | |
| 173 | + | select { | |
| 174 | + | case <-ctx.Done(): | |
| 175 | + | timer.Stop() | |
| 176 | + | return nil, ctx.Err() | |
| 177 | + | case <-timer.C: | |
| 178 | + | } | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | if lastErr != nil { | |
| 182 | + | return nil, lastErr | |
| 183 | + | } | |
| 184 | + | return nil, fmt.Errorf("request failed after retries") | |
| 185 | + | } | |
| 186 | + | ||
| 121 | 187 | // Routes API | |
| 122 | 188 | ||
| 123 | 189 | // Type aliases for API types using shared DTO package. | |
| @@ -371,7 +437,7 @@ func (c *Client) GetHealth(ctx context.Context) (map[string]*RouteHealth, error) | |||
| 371 | 437 | ||
| 372 | 438 | // Reload triggers a configuration reload. | |
| 373 | 439 | func (c *Client) Reload(ctx context.Context) error { | |
| 374 | - | resp, err := c.request(ctx, http.MethodPost, "/reload", nil) | |
| 440 | + | resp, err := c.requestWithRetry(ctx, http.MethodPost, "/reload", nil) | |
| 375 | 441 | if err != nil { | |
| 376 | 442 | return err | |
| 377 | 443 | } | |
| @@ -431,7 +497,7 @@ type DeployResult struct { | |||
| 431 | 497 | ||
| 432 | 498 | // Deploy triggers a deployment for the specified domain. | |
| 433 | 499 | func (c *Client) Deploy(ctx context.Context, deployDomain string) (*DeployResult, error) { | |
| 434 | - | resp, err := c.request(ctx, http.MethodPost, "/deploy/"+url.PathEscape(deployDomain), nil) | |
| 500 | + | resp, err := c.requestWithRetry(ctx, http.MethodPost, "/deploy/"+url.PathEscape(deployDomain), nil) | |
| 435 | 501 | if err != nil { | |
| 436 | 502 | return nil, err | |
| 437 | 503 | } | |
| @@ -461,7 +527,7 @@ func (c *Client) Restart(ctx context.Context, restartDomain string, withAttachme | |||
| 461 | 527 | if withAttachments { | |
| 462 | 528 | path += "?attachments=true" | |
| 463 | 529 | } | |
| 464 | - | resp, err := c.request(ctx, http.MethodPost, path, nil) | |
| 530 | + | resp, err := c.requestWithRetry(ctx, http.MethodPost, path, nil) | |
| 465 | 531 | if err != nil { | |
| 466 | 532 | return nil, err | |
| 467 | 533 | } | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "bytes" | 5 | "bytes" | ||
| 6 | "context" | 6 | "context" | ||
| 7 | "encoding/json" | 7 | "encoding/json" | ||
| 8 | + | "errors" | |||
| 8 | "fmt" | 9 | "fmt" | ||
| 9 | "io" | 10 | "io" | ||
| 10 | "net/http" | 11 | "net/http" | ||
| @@ -23,6 +24,11 @@ type Client struct { | |||||
| 23 | httpClient *http.Client | 24 | httpClient *http.Client | ||
| 24 | } | 25 | } | ||
| 25 | 26 | ||||
| 27 | + | var ( | |||
| 28 | + | retryMaxAttempts = 4 | |||
| 29 | + | retryBaseDelay = 250 * time.Millisecond | |||
| 30 | + | ) | |||
| 31 | + | ||||
| 26 | // ClientOption configures the Client. | 32 | // ClientOption configures the Client. | ||
| 27 | type ClientOption func(*Client) | 33 | type ClientOption func(*Client) | ||
| 28 | 34 | ||||
| @@ -100,13 +106,7 @@ func parseResponse(resp *http.Response, target any) error { | |||||
| 100 | 106 | ||||
| 101 | if resp.StatusCode >= 400 { | 107 | if resp.StatusCode >= 400 { | ||
| 102 | body, _ := io.ReadAll(resp.Body) | 108 | body, _ := io.ReadAll(resp.Body) | ||
| 103 | - | var errResp struct { | 109 | + | return parseErrorResponse(resp, body) |
| 104 | - | Error string `json:"error"` | |||
| 105 | - | } | |||
| 106 | - | if err := json.Unmarshal(body, &errResp); err == nil && errResp.Error != "" { | |||
| 107 | - | return fmt.Errorf("%s: %s", resp.Status, errResp.Error) | |||
| 108 | - | } | |||
| 109 | - | return fmt.Errorf("%s: %s", resp.Status, string(body)) | |||
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | if target != nil { | 112 | if target != nil { | ||
| @@ -118,6 +118,72 @@ func parseResponse(resp *http.Response, target any) error { | |||||
| 118 | return nil | 118 | return nil | ||
| 119 | } | 119 | } | ||
| 120 | 120 | ||||
| 121 | + | func parseErrorResponse(resp *http.Response, body []byte) error { | |||
| 122 | + | var errResp struct { | |||
| 123 | + | Error string `json:"error"` | |||
| 124 | + | } | |||
| 125 | + | if err := json.Unmarshal(body, &errResp); err == nil && errResp.Error != "" { | |||
| 126 | + | return fmt.Errorf("%s: %s", resp.Status, errResp.Error) | |||
| 127 | + | } | |||
| 128 | + | return fmt.Errorf("%s: %s", resp.Status, string(body)) | |||
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | func isRetryableRequestError(err error) bool { | |||
| 132 | + | return !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) | |||
| 133 | + | } | |||
| 134 | + | ||||
| 135 | + | func isRetryableStatus(status int) bool { | |||
| 136 | + | return status >= 500 && status <= 599 | |||
| 137 | + | } | |||
| 138 | + | ||||
| 139 | + | func retryDelay(attempt int) time.Duration { | |||
| 140 | + | if attempt <= 1 { | |||
| 141 | + | return retryBaseDelay | |||
| 142 | + | } | |||
| 143 | + | return retryBaseDelay * time.Duration(1<<(attempt-1)) | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | // requestWithRetry performs an HTTP request and retries transient failures. | |||
| 147 | + | // Retries occur on transport errors and any 5xx response. | |||
| 148 | + | func (c *Client) requestWithRetry(ctx context.Context, method, path string, body any) (*http.Response, error) { | |||
| 149 | + | var lastErr error | |||
| 150 | + | ||||
| 151 | + | for attempt := 1; attempt <= retryMaxAttempts; attempt++ { | |||
| 152 | + | resp, err := c.request(ctx, method, path, body) | |||
| 153 | + | if err != nil { | |||
| 154 | + | lastErr = err | |||
| 155 | + | if attempt == retryMaxAttempts || !isRetryableRequestError(err) { | |||
| 156 | + | return nil, err | |||
| 157 | + | } | |||
| 158 | + | } else { | |||
| 159 | + | if !isRetryableStatus(resp.StatusCode) { | |||
| 160 | + | return resp, nil | |||
| 161 | + | } | |||
| 162 | + | ||||
| 163 | + | respBody, _ := io.ReadAll(resp.Body) | |||
| 164 | + | _ = resp.Body.Close() | |||
| 165 | + | lastErr = parseErrorResponse(resp, respBody) | |||
| 166 | + | if attempt == retryMaxAttempts { | |||
| 167 | + | return nil, lastErr | |||
| 168 | + | } | |||
| 169 | + | } | |||
| 170 | + | ||||
| 171 | + | delay := retryDelay(attempt) | |||
| 172 | + | timer := time.NewTimer(delay) | |||
| 173 | + | select { | |||
| 174 | + | case <-ctx.Done(): | |||
| 175 | + | timer.Stop() | |||
| 176 | + | return nil, ctx.Err() | |||
| 177 | + | case <-timer.C: | |||
| 178 | + | } | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | if lastErr != nil { | |||
| 182 | + | return nil, lastErr | |||
| 183 | + | } | |||
| 184 | + | return nil, fmt.Errorf("request failed after retries") | |||
| 185 | + | } | |||
| 186 | + | ||||
| 121 | // Routes API | 187 | // Routes API | ||
| 122 | 188 | ||||
| 123 | // Type aliases for API types using shared DTO package. | 189 | // Type aliases for API types using shared DTO package. | ||
| @@ -371,7 +437,7 @@ func (c *Client) GetHealth(ctx context.Context) (map[string]*RouteHealth, error) | |||||
| 371 | 437 | ||||
| 372 | // Reload triggers a configuration reload. | 438 | // Reload triggers a configuration reload. | ||
| 373 | func (c *Client) Reload(ctx context.Context) error { | 439 | func (c *Client) Reload(ctx context.Context) error { | ||
| 374 | - | resp, err := c.request(ctx, http.MethodPost, "/reload", nil) | 440 | + | resp, err := c.requestWithRetry(ctx, http.MethodPost, "/reload", nil) |
| 375 | if err != nil { | 441 | if err != nil { | ||
| 376 | return err | 442 | return err | ||
| 377 | } | 443 | } | ||
| @@ -431,7 +497,7 @@ type DeployResult struct { | |||||
| 431 | 497 | ||||
| 432 | // Deploy triggers a deployment for the specified domain. | 498 | // Deploy triggers a deployment for the specified domain. | ||
| 433 | func (c *Client) Deploy(ctx context.Context, deployDomain string) (*DeployResult, error) { | 499 | func (c *Client) Deploy(ctx context.Context, deployDomain string) (*DeployResult, error) { | ||
| 434 | - | resp, err := c.request(ctx, http.MethodPost, "/deploy/"+url.PathEscape(deployDomain), nil) | 500 | + | resp, err := c.requestWithRetry(ctx, http.MethodPost, "/deploy/"+url.PathEscape(deployDomain), nil) |
| 435 | if err != nil { | 501 | if err != nil { | ||
| 436 | return nil, err | 502 | return nil, err | ||
| 437 | } | 503 | } | ||
| @@ -461,7 +527,7 @@ func (c *Client) Restart(ctx context.Context, restartDomain string, withAttachme | |||||
| 461 | if withAttachments { | 527 | if withAttachments { | ||
| 462 | path += "?attachments=true" | 528 | path += "?attachments=true" | ||
| 463 | } | 529 | } | ||
| 464 | - | resp, err := c.request(ctx, http.MethodPost, path, nil) | 530 | + | resp, err := c.requestWithRetry(ctx, http.MethodPost, path, nil) |
| 465 | if err != nil { | 531 | if err != nil { | ||
| 466 | return nil, err | 532 | return nil, err | ||
| 467 | } | 533 | } | ||
A internal/adapters/in/cli/remote/client_test.go +95 -0
| @@ -0,0 +1,95 @@ | |||
| 1 | + | package remote | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "net/http" | |
| 6 | + | "net/http/httptest" | |
| 7 | + | "sync/atomic" | |
| 8 | + | "testing" | |
| 9 | + | "time" | |
| 10 | + | ||
| 11 | + | "github.com/stretchr/testify/assert" | |
| 12 | + | "github.com/stretchr/testify/require" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func withFastRetry(t *testing.T) { | |
| 16 | + | t.Helper() | |
| 17 | + | prevAttempts := retryMaxAttempts | |
| 18 | + | prevDelay := retryBaseDelay | |
| 19 | + | retryMaxAttempts = 3 | |
| 20 | + | retryBaseDelay = 10 * time.Millisecond | |
| 21 | + | t.Cleanup(func() { | |
| 22 | + | retryMaxAttempts = prevAttempts | |
| 23 | + | retryBaseDelay = prevDelay | |
| 24 | + | }) | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | func TestClientRestartRetriesOn5xx(t *testing.T) { | |
| 28 | + | withFastRetry(t) | |
| 29 | + | ||
| 30 | + | var attempts int32 | |
| 31 | + | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 32 | + | require.Equal(t, "/admin/restart/test.example.com", r.URL.Path) | |
| 33 | + | require.Equal(t, http.MethodPost, r.Method) | |
| 34 | + | current := atomic.AddInt32(&attempts, 1) | |
| 35 | + | if current < 3 { | |
| 36 | + | w.WriteHeader(http.StatusServiceUnavailable) | |
| 37 | + | _, _ = w.Write([]byte(`{"error":"temporary outage"}`)) | |
| 38 | + | return | |
| 39 | + | } | |
| 40 | + | w.Header().Set("Content-Type", "application/json") | |
| 41 | + | _, _ = w.Write([]byte(`{"status":"restarted","domain":"test.example.com"}`)) | |
| 42 | + | })) | |
| 43 | + | defer srv.Close() | |
| 44 | + | ||
| 45 | + | client := NewClient(srv.URL) | |
| 46 | + | result, err := client.Restart(context.Background(), "test.example.com", false) | |
| 47 | + | require.NoError(t, err) | |
| 48 | + | require.NotNil(t, result) | |
| 49 | + | assert.Equal(t, "test.example.com", result.Domain) | |
| 50 | + | assert.EqualValues(t, 3, atomic.LoadInt32(&attempts)) | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | func TestClientReloadRetriesOn5xx(t *testing.T) { | |
| 54 | + | withFastRetry(t) | |
| 55 | + | ||
| 56 | + | var attempts int32 | |
| 57 | + | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 58 | + | require.Equal(t, "/admin/reload", r.URL.Path) | |
| 59 | + | require.Equal(t, http.MethodPost, r.Method) | |
| 60 | + | current := atomic.AddInt32(&attempts, 1) | |
| 61 | + | if current < 2 { | |
| 62 | + | w.WriteHeader(http.StatusBadGateway) | |
| 63 | + | _, _ = w.Write([]byte(`upstream down`)) | |
| 64 | + | return | |
| 65 | + | } | |
| 66 | + | w.Header().Set("Content-Type", "application/json") | |
| 67 | + | _, _ = w.Write([]byte(`{"status":"reloaded"}`)) | |
| 68 | + | })) | |
| 69 | + | defer srv.Close() | |
| 70 | + | ||
| 71 | + | client := NewClient(srv.URL) | |
| 72 | + | err := client.Reload(context.Background()) | |
| 73 | + | require.NoError(t, err) | |
| 74 | + | assert.EqualValues(t, 2, atomic.LoadInt32(&attempts)) | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) { | |
| 78 | + | withFastRetry(t) | |
| 79 | + | ||
| 80 | + | var attempts int32 | |
| 81 | + | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 82 | + | require.Equal(t, "/admin/deploy/test.example.com", r.URL.Path) | |
| 83 | + | require.Equal(t, http.MethodPost, r.Method) | |
| 84 | + | atomic.AddInt32(&attempts, 1) | |
| 85 | + | w.WriteHeader(http.StatusInternalServerError) | |
| 86 | + | _, _ = w.Write([]byte(`{"error":"failed to deploy container"}`)) | |
| 87 | + | })) | |
| 88 | + | defer srv.Close() | |
| 89 | + | ||
| 90 | + | client := NewClient(srv.URL) | |
| 91 | + | _, err := client.Deploy(context.Background(), "test.example.com") | |
| 92 | + | require.Error(t, err) | |
| 93 | + | assert.Contains(t, err.Error(), "500 Internal Server Error: failed to deploy container") | |
| 94 | + | assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts)) | |
| 95 | + | } | |
| @@ -0,0 +1,95 @@ | |||||
| 1 | + | package remote | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "net/http" | |||
| 6 | + | "net/http/httptest" | |||
| 7 | + | "sync/atomic" | |||
| 8 | + | "testing" | |||
| 9 | + | "time" | |||
| 10 | + | ||||
| 11 | + | "github.com/stretchr/testify/assert" | |||
| 12 | + | "github.com/stretchr/testify/require" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func withFastRetry(t *testing.T) { | |||
| 16 | + | t.Helper() | |||
| 17 | + | prevAttempts := retryMaxAttempts | |||
| 18 | + | prevDelay := retryBaseDelay | |||
| 19 | + | retryMaxAttempts = 3 | |||
| 20 | + | retryBaseDelay = 10 * time.Millisecond | |||
| 21 | + | t.Cleanup(func() { | |||
| 22 | + | retryMaxAttempts = prevAttempts | |||
| 23 | + | retryBaseDelay = prevDelay | |||
| 24 | + | }) | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | func TestClientRestartRetriesOn5xx(t *testing.T) { | |||
| 28 | + | withFastRetry(t) | |||
| 29 | + | ||||
| 30 | + | var attempts int32 | |||
| 31 | + | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 32 | + | require.Equal(t, "/admin/restart/test.example.com", r.URL.Path) | |||
| 33 | + | require.Equal(t, http.MethodPost, r.Method) | |||
| 34 | + | current := atomic.AddInt32(&attempts, 1) | |||
| 35 | + | if current < 3 { | |||
| 36 | + | w.WriteHeader(http.StatusServiceUnavailable) | |||
| 37 | + | _, _ = w.Write([]byte(`{"error":"temporary outage"}`)) | |||
| 38 | + | return | |||
| 39 | + | } | |||
| 40 | + | w.Header().Set("Content-Type", "application/json") | |||
| 41 | + | _, _ = w.Write([]byte(`{"status":"restarted","domain":"test.example.com"}`)) | |||
| 42 | + | })) | |||
| 43 | + | defer srv.Close() | |||
| 44 | + | ||||
| 45 | + | client := NewClient(srv.URL) | |||
| 46 | + | result, err := client.Restart(context.Background(), "test.example.com", false) | |||
| 47 | + | require.NoError(t, err) | |||
| 48 | + | require.NotNil(t, result) | |||
| 49 | + | assert.Equal(t, "test.example.com", result.Domain) | |||
| 50 | + | assert.EqualValues(t, 3, atomic.LoadInt32(&attempts)) | |||
| 51 | + | } | |||
| 52 | + | ||||
| 53 | + | func TestClientReloadRetriesOn5xx(t *testing.T) { | |||
| 54 | + | withFastRetry(t) | |||
| 55 | + | ||||
| 56 | + | var attempts int32 | |||
| 57 | + | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 58 | + | require.Equal(t, "/admin/reload", r.URL.Path) | |||
| 59 | + | require.Equal(t, http.MethodPost, r.Method) | |||
| 60 | + | current := atomic.AddInt32(&attempts, 1) | |||
| 61 | + | if current < 2 { | |||
| 62 | + | w.WriteHeader(http.StatusBadGateway) | |||
| 63 | + | _, _ = w.Write([]byte(`upstream down`)) | |||
| 64 | + | return | |||
| 65 | + | } | |||
| 66 | + | w.Header().Set("Content-Type", "application/json") | |||
| 67 | + | _, _ = w.Write([]byte(`{"status":"reloaded"}`)) | |||
| 68 | + | })) | |||
| 69 | + | defer srv.Close() | |||
| 70 | + | ||||
| 71 | + | client := NewClient(srv.URL) | |||
| 72 | + | err := client.Reload(context.Background()) | |||
| 73 | + | require.NoError(t, err) | |||
| 74 | + | assert.EqualValues(t, 2, atomic.LoadInt32(&attempts)) | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) { | |||
| 78 | + | withFastRetry(t) | |||
| 79 | + | ||||
| 80 | + | var attempts int32 | |||
| 81 | + | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 82 | + | require.Equal(t, "/admin/deploy/test.example.com", r.URL.Path) | |||
| 83 | + | require.Equal(t, http.MethodPost, r.Method) | |||
| 84 | + | atomic.AddInt32(&attempts, 1) | |||
| 85 | + | w.WriteHeader(http.StatusInternalServerError) | |||
| 86 | + | _, _ = w.Write([]byte(`{"error":"failed to deploy container"}`)) | |||
| 87 | + | })) | |||
| 88 | + | defer srv.Close() | |||
| 89 | + | ||||
| 90 | + | client := NewClient(srv.URL) | |||
| 91 | + | _, err := client.Deploy(context.Background(), "test.example.com") | |||
| 92 | + | require.Error(t, err) | |||
| 93 | + | assert.Contains(t, err.Error(), "500 Internal Server Error: failed to deploy container") | |||
| 94 | + | assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts)) | |||
| 95 | + | } | |||
M internal/adapters/in/cli/restart.go +10 -1
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "github.com/spf13/cobra" | |
| 7 | 7 | ||
| 8 | 8 | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | |
| 9 | + | "github.com/bnema/gordon/internal/app" | |
| 9 | 10 | ) | |
| 10 | 11 | ||
| 11 | 12 | func newRestartCmd() *cobra.Command { | |
| @@ -29,7 +30,15 @@ Examples: | |||
| 29 | 30 | ||
| 30 | 31 | client, isRemote := GetRemoteClient() | |
| 31 | 32 | if !isRemote { | |
| 32 | - | return fmt.Errorf("restart requires remote mode") | |
| 33 | + | if withAttachments { | |
| 34 | + | return fmt.Errorf("local restart does not support --with-attachments; use --remote") | |
| 35 | + | } | |
| 36 | + | domain, err := app.SendDeploySignal(restartDomain) | |
| 37 | + | if err != nil { | |
| 38 | + | return fmt.Errorf("failed to trigger local restart via deploy signal: %w", err) | |
| 39 | + | } | |
| 40 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Restart signal sent for %s (local deploy path)", domain))) | |
| 41 | + | return nil | |
| 33 | 42 | } | |
| 34 | 43 | ||
| 35 | 44 | result, err := client.Restart(ctx, restartDomain, withAttachments) | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "github.com/spf13/cobra" | 6 | "github.com/spf13/cobra" | ||
| 7 | 7 | ||||
| 8 | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | 8 | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | ||
| 9 | + | "github.com/bnema/gordon/internal/app" | |||
| 9 | ) | 10 | ) | ||
| 10 | 11 | ||||
| 11 | func newRestartCmd() *cobra.Command { | 12 | func newRestartCmd() *cobra.Command { | ||
| @@ -29,7 +30,15 @@ Examples: | |||||
| 29 | 30 | ||||
| 30 | client, isRemote := GetRemoteClient() | 31 | client, isRemote := GetRemoteClient() | ||
| 31 | if !isRemote { | 32 | if !isRemote { | ||
| 32 | - | return fmt.Errorf("restart requires remote mode") | 33 | + | if withAttachments { |
| 34 | + | return fmt.Errorf("local restart does not support --with-attachments; use --remote") | |||
| 35 | + | } | |||
| 36 | + | domain, err := app.SendDeploySignal(restartDomain) | |||
| 37 | + | if err != nil { | |||
| 38 | + | return fmt.Errorf("failed to trigger local restart via deploy signal: %w", err) | |||
| 39 | + | } | |||
| 40 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Restart signal sent for %s (local deploy path)", domain))) | |||
| 41 | + | return nil | |||
| 33 | } | 42 | } | ||
| 34 | 43 | ||||
| 35 | result, err := client.Restart(ctx, restartDomain, withAttachments) | 44 | result, err := client.Restart(ctx, restartDomain, withAttachments) | ||
M internal/adapters/in/http/admin/handler.go +2 -2
| @@ -837,8 +837,8 @@ func (h *Handler) handleDeploy(w http.ResponseWriter, r *http.Request, path stri | |||
| 837 | 837 | return | |
| 838 | 838 | } | |
| 839 | 839 | ||
| 840 | - | // Deploy the container | |
| 841 | - | container, err := h.containerSvc.Deploy(ctx, *route) | |
| 840 | + | // Deploy is an internal server-side action: pull from local registry path. | |
| 841 | + | container, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), *route) | |
| 842 | 842 | if err != nil { | |
| 843 | 843 | log.Error().Err(err).Str("domain", deployDomain).Msg("failed to deploy container") | |
| 844 | 844 | h.sendError(w, http.StatusInternalServerError, "failed to deploy container") | |
| @@ -837,8 +837,8 @@ func (h *Handler) handleDeploy(w http.ResponseWriter, r *http.Request, path stri | |||||
| 837 | return | 837 | return | ||
| 838 | } | 838 | } | ||
| 839 | 839 | ||||
| 840 | - | // Deploy the container | 840 | + | // Deploy is an internal server-side action: pull from local registry path. |
| 841 | - | container, err := h.containerSvc.Deploy(ctx, *route) | 841 | + | container, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), *route) |
| 842 | if err != nil { | 842 | if err != nil { | ||
| 843 | log.Error().Err(err).Str("domain", deployDomain).Msg("failed to deploy container") | 843 | log.Error().Err(err).Str("domain", deployDomain).Msg("failed to deploy container") | ||
| 844 | h.sendError(w, http.StatusInternalServerError, "failed to deploy container") | 844 | h.sendError(w, http.StatusInternalServerError, "failed to deploy container") | ||
M internal/adapters/out/domainsecrets/pass_store.go +87 -9
| @@ -63,20 +63,41 @@ func (s *PassStore) ListKeys(domainName string) ([]string, error) { | |||
| 63 | 63 | if err != nil { | |
| 64 | 64 | return nil, err | |
| 65 | 65 | } | |
| 66 | - | if !exists { | |
| 67 | - | return []string{}, nil | |
| 66 | + | keys := []string{} | |
| 67 | + | if exists { | |
| 68 | + | for _, line := range strings.Split(content, "\n") { | |
| 69 | + | key := strings.TrimSpace(line) | |
| 70 | + | if key == "" { | |
| 71 | + | continue | |
| 72 | + | } | |
| 73 | + | keys = append(keys, key) | |
| 74 | + | } | |
| 68 | 75 | } | |
| 76 | + | sort.Strings(keys) | |
| 69 | 77 | ||
| 70 | - | keys := []string{} | |
| 71 | - | for _, line := range strings.Split(content, "\n") { | |
| 72 | - | key := strings.TrimSpace(line) | |
| 73 | - | if key == "" { | |
| 74 | - | continue | |
| 78 | + | // Recover keys that may exist in pass but are missing from the manifest. | |
| 79 | + | discovered, err := s.listDomainKeys(domainName) | |
| 80 | + | if err != nil { | |
| 81 | + | return nil, err | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | merged, changed := mergeUniqueKeys(keys, discovered) | |
| 85 | + | if changed { | |
| 86 | + | // Best-effort self-heal of stale manifest. | |
| 87 | + | writeCtx, writeCancel := context.WithTimeout(context.Background(), s.timeout) | |
| 88 | + | writeErr := s.passInsert(writeCtx, manifestPath, strings.Join(merged, "\n")) | |
| 89 | + | writeCancel() | |
| 90 | + | if writeErr != nil { | |
| 91 | + | s.log.Warn(). | |
| 92 | + | Str(zerowrap.FieldLayer, "adapter"). | |
| 93 | + | Str(zerowrap.FieldAdapter, "domainsecrets"). | |
| 94 | + | Str("domain", domainName). | |
| 95 | + | Err(writeErr). | |
| 96 | + | Msg("failed to self-heal pass manifest, continuing with recovered keys") | |
| 75 | 97 | } | |
| 76 | - | keys = append(keys, key) | |
| 77 | 98 | } | |
| 78 | 99 | ||
| 79 | - | return keys, nil | |
| 100 | + | return merged, nil | |
| 80 | 101 | } | |
| 81 | 102 | ||
| 82 | 103 | // GetAll returns all secrets for a domain as a key-value map. | |
| @@ -496,6 +517,35 @@ func (s *PassStore) manifestPath(domainName string) (string, error) { | |||
| 496 | 517 | return path, nil | |
| 497 | 518 | } | |
| 498 | 519 | ||
| 520 | + | func (s *PassStore) listDomainKeys(domainName string) ([]string, error) { | |
| 521 | + | basePath, err := s.domainPath(domainName) | |
| 522 | + | if err != nil { | |
| 523 | + | return nil, err | |
| 524 | + | } | |
| 525 | + | ||
| 526 | + | ctx, cancel := context.WithTimeout(context.Background(), s.timeout) | |
| 527 | + | defer cancel() | |
| 528 | + | ||
| 529 | + | entries, err := s.listTopLevelEntries(ctx, basePath) | |
| 530 | + | if err != nil { | |
| 531 | + | return nil, err | |
| 532 | + | } | |
| 533 | + | ||
| 534 | + | keys := make([]string, 0, len(entries)) | |
| 535 | + | for _, entry := range entries { | |
| 536 | + | if entry == ".keys" { | |
| 537 | + | continue | |
| 538 | + | } | |
| 539 | + | if err := domain.ValidateEnvKey(entry); err != nil { | |
| 540 | + | continue | |
| 541 | + | } | |
| 542 | + | keys = append(keys, entry) | |
| 543 | + | } | |
| 544 | + | ||
| 545 | + | sort.Strings(keys) | |
| 546 | + | return keys, nil | |
| 547 | + | } | |
| 548 | + | ||
| 499 | 549 | func (s *PassStore) attachmentPath(containerName string) (string, error) { | |
| 500 | 550 | if err := domain.ValidateContainerName(containerName); err != nil { | |
| 501 | 551 | return "", err | |
| @@ -725,6 +775,34 @@ func passEntryMissing(output string) bool { | |||
| 725 | 775 | strings.Contains(lower, "password store is empty") | |
| 726 | 776 | } | |
| 727 | 777 | ||
| 778 | + | func mergeUniqueKeys(primary, secondary []string) ([]string, bool) { | |
| 779 | + | mergedSet := make(map[string]struct{}, len(primary)+len(secondary)) | |
| 780 | + | for _, key := range primary { | |
| 781 | + | mergedSet[key] = struct{}{} | |
| 782 | + | } | |
| 783 | + | for _, key := range secondary { | |
| 784 | + | mergedSet[key] = struct{}{} | |
| 785 | + | } | |
| 786 | + | ||
| 787 | + | merged := make([]string, 0, len(mergedSet)) | |
| 788 | + | for key := range mergedSet { | |
| 789 | + | merged = append(merged, key) | |
| 790 | + | } | |
| 791 | + | sort.Strings(merged) | |
| 792 | + | ||
| 793 | + | if len(merged) != len(primary) { | |
| 794 | + | return merged, true | |
| 795 | + | } | |
| 796 | + | ||
| 797 | + | for i := range primary { | |
| 798 | + | if primary[i] != merged[i] { | |
| 799 | + | return merged, true | |
| 800 | + | } | |
| 801 | + | } | |
| 802 | + | ||
| 803 | + | return merged, false | |
| 804 | + | } | |
| 805 | + | ||
| 728 | 806 | func (s *PassStore) cleanupInsertedPaths(paths []string) { | |
| 729 | 807 | if len(paths) == 0 { | |
| 730 | 808 | return | |
| @@ -63,20 +63,41 @@ func (s *PassStore) ListKeys(domainName string) ([]string, error) { | |||||
| 63 | if err != nil { | 63 | if err != nil { | ||
| 64 | return nil, err | 64 | return nil, err | ||
| 65 | } | 65 | } | ||
| 66 | - | if !exists { | 66 | + | keys := []string{} |
| 67 | - | return []string{}, nil | 67 | + | if exists { |
| 68 | + | for _, line := range strings.Split(content, "\n") { | |||
| 69 | + | key := strings.TrimSpace(line) | |||
| 70 | + | if key == "" { | |||
| 71 | + | continue | |||
| 72 | + | } | |||
| 73 | + | keys = append(keys, key) | |||
| 74 | + | } | |||
| 68 | } | 75 | } | ||
| 76 | + | sort.Strings(keys) | |||
| 69 | 77 | ||||
| 70 | - | keys := []string{} | 78 | + | // Recover keys that may exist in pass but are missing from the manifest. |
| 71 | - | for _, line := range strings.Split(content, "\n") { | 79 | + | discovered, err := s.listDomainKeys(domainName) |
| 72 | - | key := strings.TrimSpace(line) | 80 | + | if err != nil { |
| 73 | - | if key == "" { | 81 | + | return nil, err |
| 74 | - | continue | 82 | + | } |
| 83 | + | ||||
| 84 | + | merged, changed := mergeUniqueKeys(keys, discovered) | |||
| 85 | + | if changed { | |||
| 86 | + | // Best-effort self-heal of stale manifest. | |||
| 87 | + | writeCtx, writeCancel := context.WithTimeout(context.Background(), s.timeout) | |||
| 88 | + | writeErr := s.passInsert(writeCtx, manifestPath, strings.Join(merged, "\n")) | |||
| 89 | + | writeCancel() | |||
| 90 | + | if writeErr != nil { | |||
| 91 | + | s.log.Warn(). | |||
| 92 | + | Str(zerowrap.FieldLayer, "adapter"). | |||
| 93 | + | Str(zerowrap.FieldAdapter, "domainsecrets"). | |||
| 94 | + | Str("domain", domainName). | |||
| 95 | + | Err(writeErr). | |||
| 96 | + | Msg("failed to self-heal pass manifest, continuing with recovered keys") | |||
| 75 | } | 97 | } | ||
| 76 | - | keys = append(keys, key) | |||
| 77 | } | 98 | } | ||
| 78 | 99 | ||||
| 79 | - | return keys, nil | 100 | + | return merged, nil |
| 80 | } | 101 | } | ||
| 81 | 102 | ||||
| 82 | // GetAll returns all secrets for a domain as a key-value map. | 103 | // GetAll returns all secrets for a domain as a key-value map. | ||
| @@ -496,6 +517,35 @@ func (s *PassStore) manifestPath(domainName string) (string, error) { | |||||
| 496 | return path, nil | 517 | return path, nil | ||
| 497 | } | 518 | } | ||
| 498 | 519 | ||||
| 520 | + | func (s *PassStore) listDomainKeys(domainName string) ([]string, error) { | |||
| 521 | + | basePath, err := s.domainPath(domainName) | |||
| 522 | + | if err != nil { | |||
| 523 | + | return nil, err | |||
| 524 | + | } | |||
| 525 | + | ||||
| 526 | + | ctx, cancel := context.WithTimeout(context.Background(), s.timeout) | |||
| 527 | + | defer cancel() | |||
| 528 | + | ||||
| 529 | + | entries, err := s.listTopLevelEntries(ctx, basePath) | |||
| 530 | + | if err != nil { | |||
| 531 | + | return nil, err | |||
| 532 | + | } | |||
| 533 | + | ||||
| 534 | + | keys := make([]string, 0, len(entries)) | |||
| 535 | + | for _, entry := range entries { | |||
| 536 | + | if entry == ".keys" { | |||
| 537 | + | continue | |||
| 538 | + | } | |||
| 539 | + | if err := domain.ValidateEnvKey(entry); err != nil { | |||
| 540 | + | continue | |||
| 541 | + | } | |||
| 542 | + | keys = append(keys, entry) | |||
| 543 | + | } | |||
| 544 | + | ||||
| 545 | + | sort.Strings(keys) | |||
| 546 | + | return keys, nil | |||
| 547 | + | } | |||
| 548 | + | ||||
| 499 | func (s *PassStore) attachmentPath(containerName string) (string, error) { | 549 | func (s *PassStore) attachmentPath(containerName string) (string, error) { | ||
| 500 | if err := domain.ValidateContainerName(containerName); err != nil { | 550 | if err := domain.ValidateContainerName(containerName); err != nil { | ||
| 501 | return "", err | 551 | return "", err | ||
| @@ -725,6 +775,34 @@ func passEntryMissing(output string) bool { | |||||
| 725 | strings.Contains(lower, "password store is empty") | 775 | strings.Contains(lower, "password store is empty") | ||
| 726 | } | 776 | } | ||
| 727 | 777 | ||||
| 778 | + | func mergeUniqueKeys(primary, secondary []string) ([]string, bool) { | |||
| 779 | + | mergedSet := make(map[string]struct{}, len(primary)+len(secondary)) | |||
| 780 | + | for _, key := range primary { | |||
| 781 | + | mergedSet[key] = struct{}{} | |||
| 782 | + | } | |||
| 783 | + | for _, key := range secondary { | |||
| 784 | + | mergedSet[key] = struct{}{} | |||
| 785 | + | } | |||
| 786 | + | ||||
| 787 | + | merged := make([]string, 0, len(mergedSet)) | |||
| 788 | + | for key := range mergedSet { | |||
| 789 | + | merged = append(merged, key) | |||
| 790 | + | } | |||
| 791 | + | sort.Strings(merged) | |||
| 792 | + | ||||
| 793 | + | if len(merged) != len(primary) { | |||
| 794 | + | return merged, true | |||
| 795 | + | } | |||
| 796 | + | ||||
| 797 | + | for i := range primary { | |||
| 798 | + | if primary[i] != merged[i] { | |||
| 799 | + | return merged, true | |||
| 800 | + | } | |||
| 801 | + | } | |||
| 802 | + | ||||
| 803 | + | return merged, false | |||
| 804 | + | } | |||
| 805 | + | ||||
| 728 | func (s *PassStore) cleanupInsertedPaths(paths []string) { | 806 | func (s *PassStore) cleanupInsertedPaths(paths []string) { | ||
| 729 | if len(paths) == 0 { | 807 | if len(paths) == 0 { | ||
| 730 | return | 808 | return | ||
M internal/adapters/out/domainsecrets/pass_store_test.go +59 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "os/exec" | |
| 7 | + | "strings" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "time" | |
| 9 | 10 | ||
| @@ -19,6 +20,25 @@ func passCmd(args ...string) error { | |||
| 19 | 20 | return exec.CommandContext(ctx, "pass", args...).Run() | |
| 20 | 21 | } | |
| 21 | 22 | ||
| 23 | + | func passInsertValue(path, value string) error { | |
| 24 | + | ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) | |
| 25 | + | defer cancel() | |
| 26 | + | cmd := exec.CommandContext(ctx, "pass", "insert", "-m", "-f", path) | |
| 27 | + | cmd.Stdin = strings.NewReader(value) | |
| 28 | + | _, err := cmd.CombinedOutput() | |
| 29 | + | return err | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | func passShow(path string) (string, error) { | |
| 33 | + | ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) | |
| 34 | + | defer cancel() | |
| 35 | + | out, err := exec.CommandContext(ctx, "pass", "show", path).CombinedOutput() | |
| 36 | + | if err != nil { | |
| 37 | + | return "", err | |
| 38 | + | } | |
| 39 | + | return strings.TrimSpace(string(out)), nil | |
| 40 | + | } | |
| 41 | + | ||
| 22 | 42 | func requirePass(t *testing.T) { | |
| 23 | 43 | if err := passCmd("version"); err != nil { | |
| 24 | 44 | t.Skip("pass not available") | |
| @@ -157,6 +177,45 @@ func TestPassStore_DeleteAttachment(t *testing.T) { | |||
| 157 | 177 | assert.False(t, exists) | |
| 158 | 178 | } | |
| 159 | 179 | ||
| 180 | + | func TestPassStore_ListKeys_RecoversOrphanedEntries(t *testing.T) { | |
| 181 | + | requirePass(t) | |
| 182 | + | ||
| 183 | + | store, err := NewPassStore(testLogger()) | |
| 184 | + | require.NoError(t, err) | |
| 185 | + | ||
| 186 | + | domainName := fmt.Sprintf("orphan.%d.example.com", time.Now().UnixNano()) | |
| 187 | + | keys := []string{"EXISTING", "ORIGIN"} | |
| 188 | + | defer cleanupPassDomain(t, domainName, keys) | |
| 189 | + | ||
| 190 | + | err = store.Set(domainName, map[string]string{"EXISTING": "present"}) | |
| 191 | + | require.NoError(t, err) | |
| 192 | + | ||
| 193 | + | safeDomain, err := domain.SanitizeDomainForEnvFile(domainName) | |
| 194 | + | require.NoError(t, err) | |
| 195 | + | ||
| 196 | + | orphanPath := fmt.Sprintf("%s/%s/ORIGIN", PassDomainSecretsPath, safeDomain) | |
| 197 | + | err = passInsertValue(orphanPath, "https://example.com") | |
| 198 | + | require.NoError(t, err) | |
| 199 | + | ||
| 200 | + | manifestPath := fmt.Sprintf("%s/%s/.keys", PassDomainSecretsPath, safeDomain) | |
| 201 | + | err = passInsertValue(manifestPath, "EXISTING\n") | |
| 202 | + | require.NoError(t, err) | |
| 203 | + | ||
| 204 | + | listed, err := store.ListKeys(domainName) | |
| 205 | + | require.NoError(t, err) | |
| 206 | + | assert.ElementsMatch(t, []string{"EXISTING", "ORIGIN"}, listed) | |
| 207 | + | ||
| 208 | + | values, err := store.GetAll(domainName) | |
| 209 | + | require.NoError(t, err) | |
| 210 | + | assert.Equal(t, "present", values["EXISTING"]) | |
| 211 | + | assert.Equal(t, "https://example.com", values["ORIGIN"]) | |
| 212 | + | ||
| 213 | + | manifest, err := passShow(manifestPath) | |
| 214 | + | require.NoError(t, err) | |
| 215 | + | assert.Contains(t, manifest, "EXISTING") | |
| 216 | + | assert.Contains(t, manifest, "ORIGIN") | |
| 217 | + | } | |
| 218 | + | ||
| 160 | 219 | func TestParsePassListOutput(t *testing.T) { | |
| 161 | 220 | tests := []struct { | |
| 162 | 221 | name string | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "os/exec" | 6 | "os/exec" | ||
| 7 | + | "strings" | |||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "time" | 9 | "time" | ||
| 9 | 10 | ||||
| @@ -19,6 +20,25 @@ func passCmd(args ...string) error { | |||||
| 19 | return exec.CommandContext(ctx, "pass", args...).Run() | 20 | return exec.CommandContext(ctx, "pass", args...).Run() | ||
| 20 | } | 21 | } | ||
| 21 | 22 | ||||
| 23 | + | func passInsertValue(path, value string) error { | |||
| 24 | + | ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) | |||
| 25 | + | defer cancel() | |||
| 26 | + | cmd := exec.CommandContext(ctx, "pass", "insert", "-m", "-f", path) | |||
| 27 | + | cmd.Stdin = strings.NewReader(value) | |||
| 28 | + | _, err := cmd.CombinedOutput() | |||
| 29 | + | return err | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | func passShow(path string) (string, error) { | |||
| 33 | + | ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) | |||
| 34 | + | defer cancel() | |||
| 35 | + | out, err := exec.CommandContext(ctx, "pass", "show", path).CombinedOutput() | |||
| 36 | + | if err != nil { | |||
| 37 | + | return "", err | |||
| 38 | + | } | |||
| 39 | + | return strings.TrimSpace(string(out)), nil | |||
| 40 | + | } | |||
| 41 | + | ||||
| 22 | func requirePass(t *testing.T) { | 42 | func requirePass(t *testing.T) { | ||
| 23 | if err := passCmd("version"); err != nil { | 43 | if err := passCmd("version"); err != nil { | ||
| 24 | t.Skip("pass not available") | 44 | t.Skip("pass not available") | ||
| @@ -157,6 +177,45 @@ func TestPassStore_DeleteAttachment(t *testing.T) { | |||||
| 157 | assert.False(t, exists) | 177 | assert.False(t, exists) | ||
| 158 | } | 178 | } | ||
| 159 | 179 | ||||
| 180 | + | func TestPassStore_ListKeys_RecoversOrphanedEntries(t *testing.T) { | |||
| 181 | + | requirePass(t) | |||
| 182 | + | ||||
| 183 | + | store, err := NewPassStore(testLogger()) | |||
| 184 | + | require.NoError(t, err) | |||
| 185 | + | ||||
| 186 | + | domainName := fmt.Sprintf("orphan.%d.example.com", time.Now().UnixNano()) | |||
| 187 | + | keys := []string{"EXISTING", "ORIGIN"} | |||
| 188 | + | defer cleanupPassDomain(t, domainName, keys) | |||
| 189 | + | ||||
| 190 | + | err = store.Set(domainName, map[string]string{"EXISTING": "present"}) | |||
| 191 | + | require.NoError(t, err) | |||
| 192 | + | ||||
| 193 | + | safeDomain, err := domain.SanitizeDomainForEnvFile(domainName) | |||
| 194 | + | require.NoError(t, err) | |||
| 195 | + | ||||
| 196 | + | orphanPath := fmt.Sprintf("%s/%s/ORIGIN", PassDomainSecretsPath, safeDomain) | |||
| 197 | + | err = passInsertValue(orphanPath, "https://example.com") | |||
| 198 | + | require.NoError(t, err) | |||
| 199 | + | ||||
| 200 | + | manifestPath := fmt.Sprintf("%s/%s/.keys", PassDomainSecretsPath, safeDomain) | |||
| 201 | + | err = passInsertValue(manifestPath, "EXISTING\n") | |||
| 202 | + | require.NoError(t, err) | |||
| 203 | + | ||||
| 204 | + | listed, err := store.ListKeys(domainName) | |||
| 205 | + | require.NoError(t, err) | |||
| 206 | + | assert.ElementsMatch(t, []string{"EXISTING", "ORIGIN"}, listed) | |||
| 207 | + | ||||
| 208 | + | values, err := store.GetAll(domainName) | |||
| 209 | + | require.NoError(t, err) | |||
| 210 | + | assert.Equal(t, "present", values["EXISTING"]) | |||
| 211 | + | assert.Equal(t, "https://example.com", values["ORIGIN"]) | |||
| 212 | + | ||||
| 213 | + | manifest, err := passShow(manifestPath) | |||
| 214 | + | require.NoError(t, err) | |||
| 215 | + | assert.Contains(t, manifest, "EXISTING") | |||
| 216 | + | assert.Contains(t, manifest, "ORIGIN") | |||
| 217 | + | } | |||
| 218 | + | ||||
| 160 | func TestParsePassListOutput(t *testing.T) { | 219 | func TestParsePassListOutput(t *testing.T) { | ||
| 161 | tests := []struct { | 220 | tests := []struct { | ||
| 162 | name string | 221 | name string | ||
M internal/usecase/container/events.go +2 -2
| @@ -332,8 +332,8 @@ func (h *ManualDeployHandler) Handle(ctx context.Context, event domain.Event) er | |||
| 332 | 332 | return fmt.Errorf("route not found for domain: %s", payload.Domain) | |
| 333 | 333 | } | |
| 334 | 334 | ||
| 335 | - | // Deploy the container (this will replace any existing container) | |
| 336 | - | if _, err := h.containerSvc.Deploy(ctx, *targetRoute); err != nil { | |
| 335 | + | // Manual deploy is an internal trigger, so use internal deploy context. | |
| 336 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), *targetRoute); err != nil { | |
| 337 | 337 | return log.WrapErr(err, "failed to deploy container") | |
| 338 | 338 | } | |
| 339 | 339 | ||
| @@ -332,8 +332,8 @@ func (h *ManualDeployHandler) Handle(ctx context.Context, event domain.Event) er | |||||
| 332 | return fmt.Errorf("route not found for domain: %s", payload.Domain) | 332 | return fmt.Errorf("route not found for domain: %s", payload.Domain) | ||
| 333 | } | 333 | } | ||
| 334 | 334 | ||||
| 335 | - | // Deploy the container (this will replace any existing container) | 335 | + | // Manual deploy is an internal trigger, so use internal deploy context. |
| 336 | - | if _, err := h.containerSvc.Deploy(ctx, *targetRoute); err != nil { | 336 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), *targetRoute); err != nil { |
| 337 | return log.WrapErr(err, "failed to deploy container") | 337 | return log.WrapErr(err, "failed to deploy container") | ||
| 338 | } | 338 | } | ||
| 339 | 339 | ||||
M internal/usecase/container/service.go +105 -10
| @@ -46,6 +46,7 @@ const ( | |||
| 46 | 46 | // is briefly not running at the end of readiness delay. This avoids false | |
| 47 | 47 | // negatives during short startup flaps. | |
| 48 | 48 | readinessRecoveryWindow = 30 * time.Second | |
| 49 | + | internalPullMaxAttempts = 3 | |
| 49 | 50 | ) | |
| 50 | 51 | ||
| 51 | 52 | // Service implements the ContainerService interface. | |
| @@ -143,12 +144,28 @@ func (s *Service) acquireDomainDeployLock(ctx context.Context, domain string) (f | |||
| 143 | 144 | } | |
| 144 | 145 | ||
| 145 | 146 | // buildContainerConfig constructs the container configuration for deployment. | |
| 146 | - | func (s *Service) buildContainerConfig(containerDomain, image, actualImageRef string, exposedPorts []int, envVars []string, volumes map[string]string, networkName string, hasExisting bool) *domain.ContainerConfig { | |
| 147 | - | // Determine container name (use temp suffix for zero-downtime if existing) | |
| 148 | - | containerName := fmt.Sprintf("gordon-%s", containerDomain) | |
| 149 | - | if hasExisting { | |
| 150 | - | containerName = fmt.Sprintf("gordon-%s-new", containerDomain) | |
| 147 | + | func (s *Service) deploymentContainerName(containerDomain string, existing *domain.Container) string { | |
| 148 | + | canonicalName := fmt.Sprintf("gordon-%s", containerDomain) | |
| 149 | + | if existing == nil { | |
| 150 | + | return canonicalName | |
| 151 | + | } | |
| 152 | + | ||
| 153 | + | // Alternate temporary names if the tracked container was left with a temp suffix. | |
| 154 | + | // This prevents name collisions after interrupted zero-downtime deploys. | |
| 155 | + | newName := canonicalName + "-new" | |
| 156 | + | nextName := canonicalName + "-next" | |
| 157 | + | switch existing.Name { | |
| 158 | + | case newName: | |
| 159 | + | return nextName | |
| 160 | + | case nextName: | |
| 161 | + | return newName | |
| 162 | + | default: | |
| 163 | + | return newName | |
| 151 | 164 | } | |
| 165 | + | } | |
| 166 | + | ||
| 167 | + | func (s *Service) buildContainerConfig(containerDomain, image, actualImageRef string, exposedPorts []int, envVars []string, volumes map[string]string, networkName string, existing *domain.Container) *domain.ContainerConfig { | |
| 168 | + | containerName := s.deploymentContainerName(containerDomain, existing) | |
| 152 | 169 | ||
| 153 | 170 | return &domain.ContainerConfig{ | |
| 154 | 171 | Image: actualImageRef, | |
| @@ -241,7 +258,7 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||
| 241 | 258 | } | |
| 242 | 259 | ||
| 243 | 260 | // Build container configuration | |
| 244 | - | containerConfig := s.buildContainerConfig(route.Domain, route.Image, actualImageRef, exposedPorts, envVars, volumes, networkName, hasExisting) | |
| 261 | + | containerConfig := s.buildContainerConfig(route.Domain, route.Image, actualImageRef, exposedPorts, envVars, volumes, networkName, existing) | |
| 245 | 262 | ||
| 246 | 263 | newContainer, err := s.runtime.CreateContainer(ctx, containerConfig) | |
| 247 | 264 | if err != nil { | |
| @@ -318,7 +335,33 @@ func (s *Service) Restart(ctx context.Context, domainName string, withAttachment | |||
| 318 | 335 | ||
| 319 | 336 | // Restart the main container | |
| 320 | 337 | if err := s.runtime.RestartContainer(ctx, container.ID); err != nil { | |
| 321 | - | return log.WrapErr(err, "failed to restart container") | |
| 338 | + | if !isContainerNotFoundError(err) { | |
| 339 | + | return log.WrapErr(err, "failed to restart container") | |
| 340 | + | } | |
| 341 | + | ||
| 342 | + | // In-memory container ID can become stale after external runtime changes. | |
| 343 | + | // Re-sync and retry once with the latest tracked container. | |
| 344 | + | log.Warn(). | |
| 345 | + | Err(err). | |
| 346 | + | Str(zerowrap.FieldEntityID, container.ID). | |
| 347 | + | Msg("tracked container missing during restart, attempting state reconciliation") | |
| 348 | + | ||
| 349 | + | if syncErr := s.SyncContainers(ctx); syncErr != nil { | |
| 350 | + | log.Warn().Err(syncErr).Msg("failed to sync container state during restart recovery") | |
| 351 | + | } | |
| 352 | + | ||
| 353 | + | s.mu.RLock() | |
| 354 | + | refreshed, refreshedExists := s.containers[domainName] | |
| 355 | + | attachmentIDs = append([]string{}, s.attachments[domainName]...) | |
| 356 | + | s.mu.RUnlock() | |
| 357 | + | ||
| 358 | + | if !refreshedExists || refreshed == nil { | |
| 359 | + | return domain.ErrContainerNotFound | |
| 360 | + | } | |
| 361 | + | if err := s.runtime.RestartContainer(ctx, refreshed.ID); err != nil { | |
| 362 | + | return log.WrapErr(err, "failed to restart container after state reconciliation") | |
| 363 | + | } | |
| 364 | + | container = refreshed | |
| 322 | 365 | } | |
| 323 | 366 | log.Info().Str(zerowrap.FieldEntityID, container.ID).Msg("container restarted") | |
| 324 | 367 | ||
| @@ -1008,11 +1051,15 @@ func (s *Service) pullImage(ctx context.Context, pullRef string, isInternal bool | |||
| 1008 | 1051 | if cfg.InternalRegistryUsername == "" || cfg.InternalRegistryPassword == "" { | |
| 1009 | 1052 | return log.WrapErr(fmt.Errorf("internal registry auth not configured"), "failed to pull image for internal deploy") | |
| 1010 | 1053 | } | |
| 1011 | - | if err := s.runtime.PullImageWithAuth(ctx, pullRef, cfg.InternalRegistryUsername, cfg.InternalRegistryPassword); err != nil { | |
| 1054 | + | if err := s.pullImageWithRetry(ctx, func(ctx context.Context) error { | |
| 1055 | + | return s.runtime.PullImageWithAuth(ctx, pullRef, cfg.InternalRegistryUsername, cfg.InternalRegistryPassword) | |
| 1056 | + | }, isInternal); err != nil { | |
| 1012 | 1057 | return log.WrapErr(err, "failed to pull image with internal auth") | |
| 1013 | 1058 | } | |
| 1014 | 1059 | case isInternal: | |
| 1015 | - | if err := s.runtime.PullImage(ctx, pullRef); err != nil { | |
| 1060 | + | if err := s.pullImageWithRetry(ctx, func(ctx context.Context) error { | |
| 1061 | + | return s.runtime.PullImage(ctx, pullRef) | |
| 1062 | + | }, isInternal); err != nil { | |
| 1016 | 1063 | return log.WrapErr(err, "failed to pull image") | |
| 1017 | 1064 | } | |
| 1018 | 1065 | case cfg.RegistryAuthEnabled: | |
| @@ -1031,6 +1078,35 @@ func (s *Service) pullImage(ctx context.Context, pullRef string, isInternal bool | |||
| 1031 | 1078 | return nil | |
| 1032 | 1079 | } | |
| 1033 | 1080 | ||
| 1081 | + | func (s *Service) pullImageWithRetry(ctx context.Context, pullFn func(context.Context) error, isInternal bool) error { | |
| 1082 | + | log := zerowrap.FromCtx(ctx) | |
| 1083 | + | ||
| 1084 | + | for attempt := 1; attempt <= internalPullMaxAttempts; attempt++ { | |
| 1085 | + | err := pullFn(ctx) | |
| 1086 | + | if err == nil { | |
| 1087 | + | return nil | |
| 1088 | + | } | |
| 1089 | + | if !isInternal || !isConnectionRefusedError(err) || attempt == internalPullMaxAttempts { | |
| 1090 | + | return err | |
| 1091 | + | } | |
| 1092 | + | ||
| 1093 | + | backoff := time.Duration(attempt) * time.Second | |
| 1094 | + | log.Warn(). | |
| 1095 | + | Err(err). | |
| 1096 | + | Int("attempt", attempt). | |
| 1097 | + | Dur("backoff", backoff). | |
| 1098 | + | Msg("internal image pull failed with connection refused, retrying") | |
| 1099 | + | ||
| 1100 | + | select { | |
| 1101 | + | case <-time.After(backoff): | |
| 1102 | + | case <-ctx.Done(): | |
| 1103 | + | return ctx.Err() | |
| 1104 | + | } | |
| 1105 | + | } | |
| 1106 | + | ||
| 1107 | + | return fmt.Errorf("internal image pull failed after %d attempts", internalPullMaxAttempts) | |
| 1108 | + | } | |
| 1109 | + | ||
| 1034 | 1110 | func (s *Service) tagImageIfNeeded(ctx context.Context, sourceRef, targetRef string) error { | |
| 1035 | 1111 | if sourceRef == targetRef { | |
| 1036 | 1112 | return nil | |
| @@ -1160,6 +1236,8 @@ func (s *Service) createNetworkIfNeeded(ctx context.Context, networkName string) | |||
| 1160 | 1236 | func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName string, skipContainerID string) error { | |
| 1161 | 1237 | log := zerowrap.FromCtx(ctx) | |
| 1162 | 1238 | expectedName := fmt.Sprintf("gordon-%s", domainName) | |
| 1239 | + | expectedNewName := expectedName + "-new" | |
| 1240 | + | expectedNextName := expectedName + "-next" | |
| 1163 | 1241 | ||
| 1164 | 1242 | allContainers, err := s.runtime.ListContainers(ctx, true) | |
| 1165 | 1243 | if err != nil { | |
| @@ -1167,7 +1245,7 @@ func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName stri | |||
| 1167 | 1245 | } | |
| 1168 | 1246 | ||
| 1169 | 1247 | for _, c := range allContainers { | |
| 1170 | - | if c.Name == expectedName && c.ID != skipContainerID { | |
| 1248 | + | if (c.Name == expectedName || c.Name == expectedNewName || c.Name == expectedNextName) && c.ID != skipContainerID { | |
| 1171 | 1249 | log.Info().Str(zerowrap.FieldEntityID, c.ID).Str(zerowrap.FieldStatus, c.Status).Msg("found orphaned container, removing") | |
| 1172 | 1250 | ||
| 1173 | 1251 | if err := s.runtime.StopContainer(ctx, c.ID); err != nil { | |
| @@ -1649,6 +1727,23 @@ func sanitizeNameLegacy(d string) string { | |||
| 1649 | 1727 | return domain.SanitizeDomainForContainerLegacy(d) | |
| 1650 | 1728 | } | |
| 1651 | 1729 | ||
| 1730 | + | func isContainerNotFoundError(err error) bool { | |
| 1731 | + | if err == nil { | |
| 1732 | + | return false | |
| 1733 | + | } | |
| 1734 | + | msg := strings.ToLower(err.Error()) | |
| 1735 | + | return strings.Contains(msg, "no such container") || | |
| 1736 | + | strings.Contains(msg, "no container with name or id") || | |
| 1737 | + | strings.Contains(msg, "container not found") | |
| 1738 | + | } | |
| 1739 | + | ||
| 1740 | + | func isConnectionRefusedError(err error) bool { | |
| 1741 | + | if err == nil { | |
| 1742 | + | return false | |
| 1743 | + | } | |
| 1744 | + | return strings.Contains(strings.ToLower(err.Error()), "connection refused") | |
| 1745 | + | } | |
| 1746 | + | ||
| 1652 | 1747 | // findContainerByName finds a container by its name. | |
| 1653 | 1748 | func (s *Service) findContainerByName(ctx context.Context, name string) *domain.Container { | |
| 1654 | 1749 | containers, err := s.runtime.ListContainers(ctx, true) | |
| @@ -46,6 +46,7 @@ const ( | |||||
| 46 | // is briefly not running at the end of readiness delay. This avoids false | 46 | // is briefly not running at the end of readiness delay. This avoids false | ||
| 47 | // negatives during short startup flaps. | 47 | // negatives during short startup flaps. | ||
| 48 | readinessRecoveryWindow = 30 * time.Second | 48 | readinessRecoveryWindow = 30 * time.Second | ||
| 49 | + | internalPullMaxAttempts = 3 | |||
| 49 | ) | 50 | ) | ||
| 50 | 51 | ||||
| 51 | // Service implements the ContainerService interface. | 52 | // Service implements the ContainerService interface. | ||
| @@ -143,12 +144,28 @@ func (s *Service) acquireDomainDeployLock(ctx context.Context, domain string) (f | |||||
| 143 | } | 144 | } | ||
| 144 | 145 | ||||
| 145 | // buildContainerConfig constructs the container configuration for deployment. | 146 | // buildContainerConfig constructs the container configuration for deployment. | ||
| 146 | - | func (s *Service) buildContainerConfig(containerDomain, image, actualImageRef string, exposedPorts []int, envVars []string, volumes map[string]string, networkName string, hasExisting bool) *domain.ContainerConfig { | 147 | + | func (s *Service) deploymentContainerName(containerDomain string, existing *domain.Container) string { |
| 147 | - | // Determine container name (use temp suffix for zero-downtime if existing) | 148 | + | canonicalName := fmt.Sprintf("gordon-%s", containerDomain) |
| 148 | - | containerName := fmt.Sprintf("gordon-%s", containerDomain) | 149 | + | if existing == nil { |
| 149 | - | if hasExisting { | 150 | + | return canonicalName |
| 150 | - | containerName = fmt.Sprintf("gordon-%s-new", containerDomain) | 151 | + | } |
| 152 | + | ||||
| 153 | + | // Alternate temporary names if the tracked container was left with a temp suffix. | |||
| 154 | + | // This prevents name collisions after interrupted zero-downtime deploys. | |||
| 155 | + | newName := canonicalName + "-new" | |||
| 156 | + | nextName := canonicalName + "-next" | |||
| 157 | + | switch existing.Name { | |||
| 158 | + | case newName: | |||
| 159 | + | return nextName | |||
| 160 | + | case nextName: | |||
| 161 | + | return newName | |||
| 162 | + | default: | |||
| 163 | + | return newName | |||
| 151 | } | 164 | } | ||
| 165 | + | } | |||
| 166 | + | ||||
| 167 | + | func (s *Service) buildContainerConfig(containerDomain, image, actualImageRef string, exposedPorts []int, envVars []string, volumes map[string]string, networkName string, existing *domain.Container) *domain.ContainerConfig { | |||
| 168 | + | containerName := s.deploymentContainerName(containerDomain, existing) | |||
| 152 | 169 | ||||
| 153 | return &domain.ContainerConfig{ | 170 | return &domain.ContainerConfig{ | ||
| 154 | Image: actualImageRef, | 171 | Image: actualImageRef, | ||
| @@ -241,7 +258,7 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||||
| 241 | } | 258 | } | ||
| 242 | 259 | ||||
| 243 | // Build container configuration | 260 | // Build container configuration | ||
| 244 | - | containerConfig := s.buildContainerConfig(route.Domain, route.Image, actualImageRef, exposedPorts, envVars, volumes, networkName, hasExisting) | 261 | + | containerConfig := s.buildContainerConfig(route.Domain, route.Image, actualImageRef, exposedPorts, envVars, volumes, networkName, existing) |
| 245 | 262 | ||||
| 246 | newContainer, err := s.runtime.CreateContainer(ctx, containerConfig) | 263 | newContainer, err := s.runtime.CreateContainer(ctx, containerConfig) | ||
| 247 | if err != nil { | 264 | if err != nil { | ||
| @@ -318,7 +335,33 @@ func (s *Service) Restart(ctx context.Context, domainName string, withAttachment | |||||
| 318 | 335 | ||||
| 319 | // Restart the main container | 336 | // Restart the main container | ||
| 320 | if err := s.runtime.RestartContainer(ctx, container.ID); err != nil { | 337 | if err := s.runtime.RestartContainer(ctx, container.ID); err != nil { | ||
| 321 | - | return log.WrapErr(err, "failed to restart container") | 338 | + | if !isContainerNotFoundError(err) { |
| 339 | + | return log.WrapErr(err, "failed to restart container") | |||
| 340 | + | } | |||
| 341 | + | ||||
| 342 | + | // In-memory container ID can become stale after external runtime changes. | |||
| 343 | + | // Re-sync and retry once with the latest tracked container. | |||
| 344 | + | log.Warn(). | |||
| 345 | + | Err(err). | |||
| 346 | + | Str(zerowrap.FieldEntityID, container.ID). | |||
| 347 | + | Msg("tracked container missing during restart, attempting state reconciliation") | |||
| 348 | + | ||||
| 349 | + | if syncErr := s.SyncContainers(ctx); syncErr != nil { | |||
| 350 | + | log.Warn().Err(syncErr).Msg("failed to sync container state during restart recovery") | |||
| 351 | + | } | |||
| 352 | + | ||||
| 353 | + | s.mu.RLock() | |||
| 354 | + | refreshed, refreshedExists := s.containers[domainName] | |||
| 355 | + | attachmentIDs = append([]string{}, s.attachments[domainName]...) | |||
| 356 | + | s.mu.RUnlock() | |||
| 357 | + | ||||
| 358 | + | if !refreshedExists || refreshed == nil { | |||
| 359 | + | return domain.ErrContainerNotFound | |||
| 360 | + | } | |||
| 361 | + | if err := s.runtime.RestartContainer(ctx, refreshed.ID); err != nil { | |||
| 362 | + | return log.WrapErr(err, "failed to restart container after state reconciliation") | |||
| 363 | + | } | |||
| 364 | + | container = refreshed | |||
| 322 | } | 365 | } | ||
| 323 | log.Info().Str(zerowrap.FieldEntityID, container.ID).Msg("container restarted") | 366 | log.Info().Str(zerowrap.FieldEntityID, container.ID).Msg("container restarted") | ||
| 324 | 367 | ||||
| @@ -1008,11 +1051,15 @@ func (s *Service) pullImage(ctx context.Context, pullRef string, isInternal bool | |||||
| 1008 | if cfg.InternalRegistryUsername == "" || cfg.InternalRegistryPassword == "" { | 1051 | if cfg.InternalRegistryUsername == "" || cfg.InternalRegistryPassword == "" { | ||
| 1009 | return log.WrapErr(fmt.Errorf("internal registry auth not configured"), "failed to pull image for internal deploy") | 1052 | return log.WrapErr(fmt.Errorf("internal registry auth not configured"), "failed to pull image for internal deploy") | ||
| 1010 | } | 1053 | } | ||
| 1011 | - | if err := s.runtime.PullImageWithAuth(ctx, pullRef, cfg.InternalRegistryUsername, cfg.InternalRegistryPassword); err != nil { | 1054 | + | if err := s.pullImageWithRetry(ctx, func(ctx context.Context) error { |
| 1055 | + | return s.runtime.PullImageWithAuth(ctx, pullRef, cfg.InternalRegistryUsername, cfg.InternalRegistryPassword) | |||
| 1056 | + | }, isInternal); err != nil { | |||
| 1012 | return log.WrapErr(err, "failed to pull image with internal auth") | 1057 | return log.WrapErr(err, "failed to pull image with internal auth") | ||
| 1013 | } | 1058 | } | ||
| 1014 | case isInternal: | 1059 | case isInternal: | ||
| 1015 | - | if err := s.runtime.PullImage(ctx, pullRef); err != nil { | 1060 | + | if err := s.pullImageWithRetry(ctx, func(ctx context.Context) error { |
| 1061 | + | return s.runtime.PullImage(ctx, pullRef) | |||
| 1062 | + | }, isInternal); err != nil { | |||
| 1016 | return log.WrapErr(err, "failed to pull image") | 1063 | return log.WrapErr(err, "failed to pull image") | ||
| 1017 | } | 1064 | } | ||
| 1018 | case cfg.RegistryAuthEnabled: | 1065 | case cfg.RegistryAuthEnabled: | ||
| @@ -1031,6 +1078,35 @@ func (s *Service) pullImage(ctx context.Context, pullRef string, isInternal bool | |||||
| 1031 | return nil | 1078 | return nil | ||
| 1032 | } | 1079 | } | ||
| 1033 | 1080 | ||||
| 1081 | + | func (s *Service) pullImageWithRetry(ctx context.Context, pullFn func(context.Context) error, isInternal bool) error { | |||
| 1082 | + | log := zerowrap.FromCtx(ctx) | |||
| 1083 | + | ||||
| 1084 | + | for attempt := 1; attempt <= internalPullMaxAttempts; attempt++ { | |||
| 1085 | + | err := pullFn(ctx) | |||
| 1086 | + | if err == nil { | |||
| 1087 | + | return nil | |||
| 1088 | + | } | |||
| 1089 | + | if !isInternal || !isConnectionRefusedError(err) || attempt == internalPullMaxAttempts { | |||
| 1090 | + | return err | |||
| 1091 | + | } | |||
| 1092 | + | ||||
| 1093 | + | backoff := time.Duration(attempt) * time.Second | |||
| 1094 | + | log.Warn(). | |||
| 1095 | + | Err(err). | |||
| 1096 | + | Int("attempt", attempt). | |||
| 1097 | + | Dur("backoff", backoff). | |||
| 1098 | + | Msg("internal image pull failed with connection refused, retrying") | |||
| 1099 | + | ||||
| 1100 | + | select { | |||
| 1101 | + | case <-time.After(backoff): | |||
| 1102 | + | case <-ctx.Done(): | |||
| 1103 | + | return ctx.Err() | |||
| 1104 | + | } | |||
| 1105 | + | } | |||
| 1106 | + | ||||
| 1107 | + | return fmt.Errorf("internal image pull failed after %d attempts", internalPullMaxAttempts) | |||
| 1108 | + | } | |||
| 1109 | + | ||||
| 1034 | func (s *Service) tagImageIfNeeded(ctx context.Context, sourceRef, targetRef string) error { | 1110 | func (s *Service) tagImageIfNeeded(ctx context.Context, sourceRef, targetRef string) error { | ||
| 1035 | if sourceRef == targetRef { | 1111 | if sourceRef == targetRef { | ||
| 1036 | return nil | 1112 | return nil | ||
| @@ -1160,6 +1236,8 @@ func (s *Service) createNetworkIfNeeded(ctx context.Context, networkName string) | |||||
| 1160 | func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName string, skipContainerID string) error { | 1236 | func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName string, skipContainerID string) error { | ||
| 1161 | log := zerowrap.FromCtx(ctx) | 1237 | log := zerowrap.FromCtx(ctx) | ||
| 1162 | expectedName := fmt.Sprintf("gordon-%s", domainName) | 1238 | expectedName := fmt.Sprintf("gordon-%s", domainName) | ||
| 1239 | + | expectedNewName := expectedName + "-new" | |||
| 1240 | + | expectedNextName := expectedName + "-next" | |||
| 1163 | 1241 | ||||
| 1164 | allContainers, err := s.runtime.ListContainers(ctx, true) | 1242 | allContainers, err := s.runtime.ListContainers(ctx, true) | ||
| 1165 | if err != nil { | 1243 | if err != nil { | ||
| @@ -1167,7 +1245,7 @@ func (s *Service) cleanupOrphanedContainers(ctx context.Context, domainName stri | |||||
| 1167 | } | 1245 | } | ||
| 1168 | 1246 | ||||
| 1169 | for _, c := range allContainers { | 1247 | for _, c := range allContainers { | ||
| 1170 | - | if c.Name == expectedName && c.ID != skipContainerID { | 1248 | + | if (c.Name == expectedName || c.Name == expectedNewName || c.Name == expectedNextName) && c.ID != skipContainerID { |
| 1171 | log.Info().Str(zerowrap.FieldEntityID, c.ID).Str(zerowrap.FieldStatus, c.Status).Msg("found orphaned container, removing") | 1249 | log.Info().Str(zerowrap.FieldEntityID, c.ID).Str(zerowrap.FieldStatus, c.Status).Msg("found orphaned container, removing") | ||
| 1172 | 1250 | ||||
| 1173 | if err := s.runtime.StopContainer(ctx, c.ID); err != nil { | 1251 | if err := s.runtime.StopContainer(ctx, c.ID); err != nil { | ||
| @@ -1649,6 +1727,23 @@ func sanitizeNameLegacy(d string) string { | |||||
| 1649 | return domain.SanitizeDomainForContainerLegacy(d) | 1727 | return domain.SanitizeDomainForContainerLegacy(d) | ||
| 1650 | } | 1728 | } | ||
| 1651 | 1729 | ||||
| 1730 | + | func isContainerNotFoundError(err error) bool { | |||
| 1731 | + | if err == nil { | |||
| 1732 | + | return false | |||
| 1733 | + | } | |||
| 1734 | + | msg := strings.ToLower(err.Error()) | |||
| 1735 | + | return strings.Contains(msg, "no such container") || | |||
| 1736 | + | strings.Contains(msg, "no container with name or id") || | |||
| 1737 | + | strings.Contains(msg, "container not found") | |||
| 1738 | + | } | |||
| 1739 | + | ||||
| 1740 | + | func isConnectionRefusedError(err error) bool { | |||
| 1741 | + | if err == nil { | |||
| 1742 | + | return false | |||
| 1743 | + | } | |||
| 1744 | + | return strings.Contains(strings.ToLower(err.Error()), "connection refused") | |||
| 1745 | + | } | |||
| 1746 | + | ||||
| 1652 | // findContainerByName finds a container by its name. | 1747 | // findContainerByName finds a container by its name. | ||
| 1653 | func (s *Service) findContainerByName(ctx context.Context, name string) *domain.Container { | 1748 | func (s *Service) findContainerByName(ctx context.Context, name string) *domain.Container { | ||
| 1654 | containers, err := s.runtime.ListContainers(ctx, true) | 1749 | containers, err := s.runtime.ListContainers(ctx, true) | ||
M internal/usecase/container/service_test.go +208 -0
| @@ -215,6 +215,30 @@ func TestService_PullImage_RequiresServiceTokenForExternalPull(t *testing.T) { | |||
| 215 | 215 | assert.Contains(t, err.Error(), "registry service token not configured") | |
| 216 | 216 | } | |
| 217 | 217 | ||
| 218 | + | func TestService_PullImage_InternalRetriesOnConnectionRefused(t *testing.T) { | |
| 219 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 220 | + | ||
| 221 | + | config := Config{ | |
| 222 | + | RegistryAuthEnabled: true, | |
| 223 | + | InternalRegistryUsername: "internal", | |
| 224 | + | InternalRegistryPassword: "secret", | |
| 225 | + | } | |
| 226 | + | ||
| 227 | + | svc := NewService(runtime, nil, nil, nil, config) | |
| 228 | + | ctx := testContext() | |
| 229 | + | ||
| 230 | + | runtime.EXPECT().PullImageWithAuth(mock.Anything, "localhost:5000/myapp:latest", "internal", "secret"). | |
| 231 | + | Return(errors.New("connection refused")). | |
| 232 | + | Once() | |
| 233 | + | runtime.EXPECT().PullImageWithAuth(mock.Anything, "localhost:5000/myapp:latest", "internal", "secret"). | |
| 234 | + | Return(nil). | |
| 235 | + | Once() | |
| 236 | + | ||
| 237 | + | err := svc.pullImage(ctx, "localhost:5000/myapp:latest", true) | |
| 238 | + | ||
| 239 | + | assert.NoError(t, err) | |
| 240 | + | } | |
| 241 | + | ||
| 218 | 242 | func TestService_Deploy_ReplacesExistingContainer(t *testing.T) { | |
| 219 | 243 | runtime := mocks.NewMockContainerRuntime(t) | |
| 220 | 244 | envLoader := mocks.NewMockEnvLoader(t) | |
| @@ -684,6 +708,65 @@ func TestService_Restart_Error(t *testing.T) { | |||
| 684 | 708 | assert.Contains(t, err.Error(), "failed to restart container") | |
| 685 | 709 | } | |
| 686 | 710 | ||
| 711 | + | func TestService_Restart_ReconcilesStaleContainerID(t *testing.T) { | |
| 712 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 713 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 714 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 715 | + | ||
| 716 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{}) | |
| 717 | + | ctx := testContext() | |
| 718 | + | ||
| 719 | + | svc.containers["test.example.com"] = &domain.Container{ | |
| 720 | + | ID: "stale-container-id", | |
| 721 | + | Name: "gordon-test.example.com", | |
| 722 | + | Status: "running", | |
| 723 | + | } | |
| 724 | + | ||
| 725 | + | runtime.EXPECT().RestartContainer(mock.Anything, "stale-container-id"). | |
| 726 | + | Return(errors.New("no container with name or ID \"stale-container-id\" found")). | |
| 727 | + | Once() | |
| 728 | + | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 729 | + | { | |
| 730 | + | ID: "fresh-container-id", | |
| 731 | + | Name: "gordon-test.example.com", | |
| 732 | + | Labels: map[string]string{ | |
| 733 | + | "gordon.domain": "test.example.com", | |
| 734 | + | "gordon.managed": "true", | |
| 735 | + | }, | |
| 736 | + | }, | |
| 737 | + | }, nil).Once() | |
| 738 | + | runtime.EXPECT().RestartContainer(mock.Anything, "fresh-container-id").Return(nil).Once() | |
| 739 | + | ||
| 740 | + | err := svc.Restart(ctx, "test.example.com", false) | |
| 741 | + | ||
| 742 | + | assert.NoError(t, err) | |
| 743 | + | } | |
| 744 | + | ||
| 745 | + | func TestService_Restart_ReconcilesStaleContainerID_NotFoundAfterSync(t *testing.T) { | |
| 746 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 747 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 748 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 749 | + | ||
| 750 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{}) | |
| 751 | + | ctx := testContext() | |
| 752 | + | ||
| 753 | + | svc.containers["test.example.com"] = &domain.Container{ | |
| 754 | + | ID: "stale-container-id", | |
| 755 | + | Name: "gordon-test.example.com", | |
| 756 | + | Status: "running", | |
| 757 | + | } | |
| 758 | + | ||
| 759 | + | runtime.EXPECT().RestartContainer(mock.Anything, "stale-container-id"). | |
| 760 | + | Return(errors.New("no such container")). | |
| 761 | + | Once() | |
| 762 | + | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil).Once() | |
| 763 | + | ||
| 764 | + | err := svc.Restart(ctx, "test.example.com", false) | |
| 765 | + | ||
| 766 | + | assert.Error(t, err) | |
| 767 | + | assert.True(t, errors.Is(err, domain.ErrContainerNotFound)) | |
| 768 | + | } | |
| 769 | + | ||
| 687 | 770 | func TestService_Restart_WithAttachments(t *testing.T) { | |
| 688 | 771 | runtime := mocks.NewMockContainerRuntime(t) | |
| 689 | 772 | envLoader := mocks.NewMockEnvLoader(t) | |
| @@ -1398,6 +1481,131 @@ func TestService_Deploy_OrphanCleanupRemovesTrueOrphans(t *testing.T) { | |||
| 1398 | 1481 | assert.Equal(t, "new-container", tracked.ID) | |
| 1399 | 1482 | } | |
| 1400 | 1483 | ||
| 1484 | + | func TestService_Deploy_OrphanCleanupRemovesStaleNewContainer(t *testing.T) { | |
| 1485 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 1486 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 1487 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 1488 | + | ||
| 1489 | + | config := Config{ | |
| 1490 | + | ReadinessDelay: time.Millisecond, | |
| 1491 | + | } | |
| 1492 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 1493 | + | ctx := testContext() | |
| 1494 | + | ||
| 1495 | + | // Tracked canonical container exists, but stale -new container should be removed. | |
| 1496 | + | svc.containers["test.example.com"] = &domain.Container{ | |
| 1497 | + | ID: "tracked-container-123", | |
| 1498 | + | Name: "gordon-test.example.com", | |
| 1499 | + | Status: "running", | |
| 1500 | + | } | |
| 1501 | + | ||
| 1502 | + | route := domain.Route{ | |
| 1503 | + | Domain: "test.example.com", | |
| 1504 | + | Image: "myapp:v2", | |
| 1505 | + | } | |
| 1506 | + | ||
| 1507 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1508 | + | { | |
| 1509 | + | ID: "tracked-container-123", | |
| 1510 | + | Name: "gordon-test.example.com", | |
| 1511 | + | Status: "running", | |
| 1512 | + | }, | |
| 1513 | + | { | |
| 1514 | + | ID: "stale-new-container", | |
| 1515 | + | Name: "gordon-test.example.com-new", | |
| 1516 | + | Status: "exited", | |
| 1517 | + | }, | |
| 1518 | + | }, nil) | |
| 1519 | + | runtime.EXPECT().StopContainer(mock.Anything, "stale-new-container").Return(nil).Once() | |
| 1520 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "stale-new-container", true).Return(nil).Once() | |
| 1521 | + | ||
| 1522 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |
| 1523 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil) | |
| 1524 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil) | |
| 1525 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 1526 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil) | |
| 1527 | + | ||
| 1528 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |
| 1529 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |
| 1530 | + | return cfg.Name == "gordon-test.example.com-new" | |
| 1531 | + | })).Return(newContainer, nil) | |
| 1532 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |
| 1533 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |
| 1534 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |
| 1535 | + | ID: "new-container", | |
| 1536 | + | Status: "running", | |
| 1537 | + | }, nil) | |
| 1538 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 1539 | + | ||
| 1540 | + | runtime.EXPECT().StopContainer(mock.Anything, "tracked-container-123").Return(nil) | |
| 1541 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "tracked-container-123", true).Return(nil) | |
| 1542 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |
| 1543 | + | ||
| 1544 | + | result, err := svc.Deploy(ctx, route) | |
| 1545 | + | assert.NoError(t, err) | |
| 1546 | + | assert.NotNil(t, result) | |
| 1547 | + | assert.Equal(t, "new-container", result.ID) | |
| 1548 | + | } | |
| 1549 | + | ||
| 1550 | + | func TestService_Deploy_TrackedTempContainerUsesAlternateTempName(t *testing.T) { | |
| 1551 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 1552 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 1553 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 1554 | + | ||
| 1555 | + | config := Config{ | |
| 1556 | + | ReadinessDelay: time.Millisecond, | |
| 1557 | + | } | |
| 1558 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 1559 | + | ctx := testContext() | |
| 1560 | + | ||
| 1561 | + | // Simulate an interrupted zero-downtime deploy that left "-new" as the tracked container name. | |
| 1562 | + | svc.containers["test.example.com"] = &domain.Container{ | |
| 1563 | + | ID: "tracked-temp-container", | |
| 1564 | + | Name: "gordon-test.example.com-new", | |
| 1565 | + | Status: "running", | |
| 1566 | + | } | |
| 1567 | + | ||
| 1568 | + | route := domain.Route{ | |
| 1569 | + | Domain: "test.example.com", | |
| 1570 | + | Image: "myapp:v2", | |
| 1571 | + | } | |
| 1572 | + | ||
| 1573 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |
| 1574 | + | { | |
| 1575 | + | ID: "tracked-temp-container", | |
| 1576 | + | Name: "gordon-test.example.com-new", | |
| 1577 | + | Status: "running", | |
| 1578 | + | }, | |
| 1579 | + | }, nil) | |
| 1580 | + | ||
| 1581 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |
| 1582 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil) | |
| 1583 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil) | |
| 1584 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 1585 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil) | |
| 1586 | + | ||
| 1587 | + | created := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-next", Status: "created"} | |
| 1588 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |
| 1589 | + | return cfg.Name == "gordon-test.example.com-next" | |
| 1590 | + | })).Return(created, nil) | |
| 1591 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |
| 1592 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |
| 1593 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |
| 1594 | + | ID: "new-container", | |
| 1595 | + | Status: "running", | |
| 1596 | + | }, nil) | |
| 1597 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 1598 | + | ||
| 1599 | + | runtime.EXPECT().StopContainer(mock.Anything, "tracked-temp-container").Return(nil) | |
| 1600 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "tracked-temp-container", true).Return(nil) | |
| 1601 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |
| 1602 | + | ||
| 1603 | + | result, err := svc.Deploy(ctx, route) | |
| 1604 | + | assert.NoError(t, err) | |
| 1605 | + | assert.NotNil(t, result) | |
| 1606 | + | assert.Equal(t, "new-container", result.ID) | |
| 1607 | + | } | |
| 1608 | + | ||
| 1401 | 1609 | func TestService_StripRegistryPrefix(t *testing.T) { | |
| 1402 | 1610 | tests := []struct { | |
| 1403 | 1611 | name string | |
| @@ -215,6 +215,30 @@ func TestService_PullImage_RequiresServiceTokenForExternalPull(t *testing.T) { | |||||
| 215 | assert.Contains(t, err.Error(), "registry service token not configured") | 215 | assert.Contains(t, err.Error(), "registry service token not configured") | ||
| 216 | } | 216 | } | ||
| 217 | 217 | ||||
| 218 | + | func TestService_PullImage_InternalRetriesOnConnectionRefused(t *testing.T) { | |||
| 219 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 220 | + | ||||
| 221 | + | config := Config{ | |||
| 222 | + | RegistryAuthEnabled: true, | |||
| 223 | + | InternalRegistryUsername: "internal", | |||
| 224 | + | InternalRegistryPassword: "secret", | |||
| 225 | + | } | |||
| 226 | + | ||||
| 227 | + | svc := NewService(runtime, nil, nil, nil, config) | |||
| 228 | + | ctx := testContext() | |||
| 229 | + | ||||
| 230 | + | runtime.EXPECT().PullImageWithAuth(mock.Anything, "localhost:5000/myapp:latest", "internal", "secret"). | |||
| 231 | + | Return(errors.New("connection refused")). | |||
| 232 | + | Once() | |||
| 233 | + | runtime.EXPECT().PullImageWithAuth(mock.Anything, "localhost:5000/myapp:latest", "internal", "secret"). | |||
| 234 | + | Return(nil). | |||
| 235 | + | Once() | |||
| 236 | + | ||||
| 237 | + | err := svc.pullImage(ctx, "localhost:5000/myapp:latest", true) | |||
| 238 | + | ||||
| 239 | + | assert.NoError(t, err) | |||
| 240 | + | } | |||
| 241 | + | ||||
| 218 | func TestService_Deploy_ReplacesExistingContainer(t *testing.T) { | 242 | func TestService_Deploy_ReplacesExistingContainer(t *testing.T) { | ||
| 219 | runtime := mocks.NewMockContainerRuntime(t) | 243 | runtime := mocks.NewMockContainerRuntime(t) | ||
| 220 | envLoader := mocks.NewMockEnvLoader(t) | 244 | envLoader := mocks.NewMockEnvLoader(t) | ||
| @@ -684,6 +708,65 @@ func TestService_Restart_Error(t *testing.T) { | |||||
| 684 | assert.Contains(t, err.Error(), "failed to restart container") | 708 | assert.Contains(t, err.Error(), "failed to restart container") | ||
| 685 | } | 709 | } | ||
| 686 | 710 | ||||
| 711 | + | func TestService_Restart_ReconcilesStaleContainerID(t *testing.T) { | |||
| 712 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 713 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 714 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 715 | + | ||||
| 716 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{}) | |||
| 717 | + | ctx := testContext() | |||
| 718 | + | ||||
| 719 | + | svc.containers["test.example.com"] = &domain.Container{ | |||
| 720 | + | ID: "stale-container-id", | |||
| 721 | + | Name: "gordon-test.example.com", | |||
| 722 | + | Status: "running", | |||
| 723 | + | } | |||
| 724 | + | ||||
| 725 | + | runtime.EXPECT().RestartContainer(mock.Anything, "stale-container-id"). | |||
| 726 | + | Return(errors.New("no container with name or ID \"stale-container-id\" found")). | |||
| 727 | + | Once() | |||
| 728 | + | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 729 | + | { | |||
| 730 | + | ID: "fresh-container-id", | |||
| 731 | + | Name: "gordon-test.example.com", | |||
| 732 | + | Labels: map[string]string{ | |||
| 733 | + | "gordon.domain": "test.example.com", | |||
| 734 | + | "gordon.managed": "true", | |||
| 735 | + | }, | |||
| 736 | + | }, | |||
| 737 | + | }, nil).Once() | |||
| 738 | + | runtime.EXPECT().RestartContainer(mock.Anything, "fresh-container-id").Return(nil).Once() | |||
| 739 | + | ||||
| 740 | + | err := svc.Restart(ctx, "test.example.com", false) | |||
| 741 | + | ||||
| 742 | + | assert.NoError(t, err) | |||
| 743 | + | } | |||
| 744 | + | ||||
| 745 | + | func TestService_Restart_ReconcilesStaleContainerID_NotFoundAfterSync(t *testing.T) { | |||
| 746 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 747 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 748 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 749 | + | ||||
| 750 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{}) | |||
| 751 | + | ctx := testContext() | |||
| 752 | + | ||||
| 753 | + | svc.containers["test.example.com"] = &domain.Container{ | |||
| 754 | + | ID: "stale-container-id", | |||
| 755 | + | Name: "gordon-test.example.com", | |||
| 756 | + | Status: "running", | |||
| 757 | + | } | |||
| 758 | + | ||||
| 759 | + | runtime.EXPECT().RestartContainer(mock.Anything, "stale-container-id"). | |||
| 760 | + | Return(errors.New("no such container")). | |||
| 761 | + | Once() | |||
| 762 | + | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil).Once() | |||
| 763 | + | ||||
| 764 | + | err := svc.Restart(ctx, "test.example.com", false) | |||
| 765 | + | ||||
| 766 | + | assert.Error(t, err) | |||
| 767 | + | assert.True(t, errors.Is(err, domain.ErrContainerNotFound)) | |||
| 768 | + | } | |||
| 769 | + | ||||
| 687 | func TestService_Restart_WithAttachments(t *testing.T) { | 770 | func TestService_Restart_WithAttachments(t *testing.T) { | ||
| 688 | runtime := mocks.NewMockContainerRuntime(t) | 771 | runtime := mocks.NewMockContainerRuntime(t) | ||
| 689 | envLoader := mocks.NewMockEnvLoader(t) | 772 | envLoader := mocks.NewMockEnvLoader(t) | ||
| @@ -1398,6 +1481,131 @@ func TestService_Deploy_OrphanCleanupRemovesTrueOrphans(t *testing.T) { | |||||
| 1398 | assert.Equal(t, "new-container", tracked.ID) | 1481 | assert.Equal(t, "new-container", tracked.ID) | ||
| 1399 | } | 1482 | } | ||
| 1400 | 1483 | ||||
| 1484 | + | func TestService_Deploy_OrphanCleanupRemovesStaleNewContainer(t *testing.T) { | |||
| 1485 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 1486 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 1487 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 1488 | + | ||||
| 1489 | + | config := Config{ | |||
| 1490 | + | ReadinessDelay: time.Millisecond, | |||
| 1491 | + | } | |||
| 1492 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 1493 | + | ctx := testContext() | |||
| 1494 | + | ||||
| 1495 | + | // Tracked canonical container exists, but stale -new container should be removed. | |||
| 1496 | + | svc.containers["test.example.com"] = &domain.Container{ | |||
| 1497 | + | ID: "tracked-container-123", | |||
| 1498 | + | Name: "gordon-test.example.com", | |||
| 1499 | + | Status: "running", | |||
| 1500 | + | } | |||
| 1501 | + | ||||
| 1502 | + | route := domain.Route{ | |||
| 1503 | + | Domain: "test.example.com", | |||
| 1504 | + | Image: "myapp:v2", | |||
| 1505 | + | } | |||
| 1506 | + | ||||
| 1507 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |||
| 1508 | + | { | |||
| 1509 | + | ID: "tracked-container-123", | |||
| 1510 | + | Name: "gordon-test.example.com", | |||
| 1511 | + | Status: "running", | |||
| 1512 | + | }, | |||
| 1513 | + | { | |||
| 1514 | + | ID: "stale-new-container", | |||
| 1515 | + | Name: "gordon-test.example.com-new", | |||
| 1516 | + | Status: "exited", | |||
| 1517 | + | }, | |||
| 1518 | + | }, nil) | |||
| 1519 | + | runtime.EXPECT().StopContainer(mock.Anything, "stale-new-container").Return(nil).Once() | |||
| 1520 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "stale-new-container", true).Return(nil).Once() | |||
| 1521 | + | ||||
| 1522 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |||
| 1523 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil) | |||
| 1524 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil) | |||
| 1525 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 1526 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil) | |||
| 1527 | + | ||||
| 1528 | + | newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"} | |||
| 1529 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |||
| 1530 | + | return cfg.Name == "gordon-test.example.com-new" | |||
| 1531 | + | })).Return(newContainer, nil) | |||
| 1532 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |||
| 1533 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |||
| 1534 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |||
| 1535 | + | ID: "new-container", | |||
| 1536 | + | Status: "running", | |||
| 1537 | + | }, nil) | |||
| 1538 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 1539 | + | ||||
| 1540 | + | runtime.EXPECT().StopContainer(mock.Anything, "tracked-container-123").Return(nil) | |||
| 1541 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "tracked-container-123", true).Return(nil) | |||
| 1542 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |||
| 1543 | + | ||||
| 1544 | + | result, err := svc.Deploy(ctx, route) | |||
| 1545 | + | assert.NoError(t, err) | |||
| 1546 | + | assert.NotNil(t, result) | |||
| 1547 | + | assert.Equal(t, "new-container", result.ID) | |||
| 1548 | + | } | |||
| 1549 | + | ||||
| 1550 | + | func TestService_Deploy_TrackedTempContainerUsesAlternateTempName(t *testing.T) { | |||
| 1551 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 1552 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 1553 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 1554 | + | ||||
| 1555 | + | config := Config{ | |||
| 1556 | + | ReadinessDelay: time.Millisecond, | |||
| 1557 | + | } | |||
| 1558 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 1559 | + | ctx := testContext() | |||
| 1560 | + | ||||
| 1561 | + | // Simulate an interrupted zero-downtime deploy that left "-new" as the tracked container name. | |||
| 1562 | + | svc.containers["test.example.com"] = &domain.Container{ | |||
| 1563 | + | ID: "tracked-temp-container", | |||
| 1564 | + | Name: "gordon-test.example.com-new", | |||
| 1565 | + | Status: "running", | |||
| 1566 | + | } | |||
| 1567 | + | ||||
| 1568 | + | route := domain.Route{ | |||
| 1569 | + | Domain: "test.example.com", | |||
| 1570 | + | Image: "myapp:v2", | |||
| 1571 | + | } | |||
| 1572 | + | ||||
| 1573 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{ | |||
| 1574 | + | { | |||
| 1575 | + | ID: "tracked-temp-container", | |||
| 1576 | + | Name: "gordon-test.example.com-new", | |||
| 1577 | + | Status: "running", | |||
| 1578 | + | }, | |||
| 1579 | + | }, nil) | |||
| 1580 | + | ||||
| 1581 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil) | |||
| 1582 | + | runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil) | |||
| 1583 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil) | |||
| 1584 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 1585 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil) | |||
| 1586 | + | ||||
| 1587 | + | created := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-next", Status: "created"} | |||
| 1588 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool { | |||
| 1589 | + | return cfg.Name == "gordon-test.example.com-next" | |||
| 1590 | + | })).Return(created, nil) | |||
| 1591 | + | runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil) | |||
| 1592 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2) | |||
| 1593 | + | runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{ | |||
| 1594 | + | ID: "new-container", | |||
| 1595 | + | Status: "running", | |||
| 1596 | + | }, nil) | |||
| 1597 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 1598 | + | ||||
| 1599 | + | runtime.EXPECT().StopContainer(mock.Anything, "tracked-temp-container").Return(nil) | |||
| 1600 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "tracked-temp-container", true).Return(nil) | |||
| 1601 | + | runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil) | |||
| 1602 | + | ||||
| 1603 | + | result, err := svc.Deploy(ctx, route) | |||
| 1604 | + | assert.NoError(t, err) | |||
| 1605 | + | assert.NotNil(t, result) | |||
| 1606 | + | assert.Equal(t, "new-container", result.ID) | |||
| 1607 | + | } | |||
| 1608 | + | ||||
| 1401 | func TestService_StripRegistryPrefix(t *testing.T) { | 1609 | func TestService_StripRegistryPrefix(t *testing.T) { | ||
| 1402 | tests := []struct { | 1610 | tests := []struct { | ||
| 1403 | name string | 1611 | name string | ||