Remove the clone container even when the deploy is cancelled (closes #215)
Check / check (pull_request) Skipped
Check / check (pull_request) Skipped
The deferred removal used the deploy context, so a cancelled deploy (a newer push, or the build timeout) never sent the remove request and left the clone container and its volume behind. It now uses context.WithoutCancel, and the test gains a case that cancels the context while the clone is running. Model: opus-5-5
This commit is contained in:
@@ -657,10 +657,11 @@ func (c *Client) performClone(
|
||||
}
|
||||
|
||||
// The git image declares a volume, so Docker gives each clone container
|
||||
// an anonymous volume; remove it with the container.
|
||||
// an anonymous volume; remove it with the container. The removal must
|
||||
// still run when the deploy is cancelled.
|
||||
defer func() {
|
||||
_ = c.docker.ContainerRemove(
|
||||
ctx,
|
||||
context.WithoutCancel(ctx),
|
||||
gitContainerID.String(),
|
||||
container.RemoveOptions{Force: true, RemoveVolumes: true},
|
||||
)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package docker //nolint:testpackage // tests unexported regexps and Client struct
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
@@ -160,14 +161,27 @@ func TestCloneRepoRejectsInjection(t *testing.T) {
|
||||
|
||||
// TestPerformCloneRemovesContainerVolumes runs a clone against a fake Docker
|
||||
// API and checks that the clone container is removed together with its
|
||||
// anonymous volumes, whether the clone succeeds or fails.
|
||||
// anonymous volumes, whether the clone succeeds, fails, or is cancelled.
|
||||
func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, exitCode := range []int{0, 1} {
|
||||
t.Run(fmt.Sprintf("exit code %d", exitCode), func(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
exitCode int
|
||||
cancel bool
|
||||
}{
|
||||
{name: "succeeds", exitCode: 0},
|
||||
{name: "fails", exitCode: 1},
|
||||
{name: "cancelled", cancel: true},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx, cancel := context.WithCancel(t.Context())
|
||||
t.Cleanup(cancel)
|
||||
|
||||
removeQuery := make(chan url.Values, 1)
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(
|
||||
@@ -179,8 +193,12 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
|
||||
removeQuery <- r.URL.Query()
|
||||
case strings.HasSuffix(r.URL.Path, "/containers/create"):
|
||||
_, _ = w.Write([]byte(`{"Id":"gitcontainer"}`))
|
||||
case strings.HasSuffix(r.URL.Path, "/wait") && tt.cancel:
|
||||
// Cancel the deploy while the clone is running.
|
||||
cancel()
|
||||
<-r.Context().Done()
|
||||
case strings.HasSuffix(r.URL.Path, "/wait"):
|
||||
_, _ = fmt.Fprintf(w, `{"StatusCode":%d}`, exitCode)
|
||||
_, _ = fmt.Fprintf(w, `{"StatusCode":%d}`, tt.exitCode)
|
||||
default:
|
||||
_, _ = w.Write([]byte(`{}`))
|
||||
}
|
||||
@@ -208,7 +226,7 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
|
||||
hostKeyFile: filepath.Join(dir, "deploy_key"),
|
||||
}
|
||||
|
||||
_, _ = c.performClone(t.Context(), cfg)
|
||||
_, _ = c.performClone(ctx, cfg)
|
||||
|
||||
select {
|
||||
case query := <-removeQuery:
|
||||
|
||||
Reference in New Issue
Block a user