From 8246a137a7f446d3c601cedbd54d92d30e9a711b Mon Sep 17 00:00:00 2001 From: AleksaMCode Date: Thu, 29 Oct 2026 13:20:57 +0100 Subject: [PATCH] fix(infra): clean up docker networks on partial create and close failures --- internal/infra/network.go | 16 ++++- internal/infra/network_test.go | 114 +++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 internal/infra/network_test.go diff --git a/internal/infra/network.go b/internal/infra/network.go index 16e687b8..870aaefd 100644 --- a/internal/infra/network.go +++ b/internal/infra/network.go @@ -2,7 +2,9 @@ package infra import ( "context" + "errors" "fmt" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" "github.com/moby/moby/pkg/namesgenerator" @@ -18,6 +20,7 @@ type Networks struct { func NewNetworks(ctx context.Context, cli *client.Client) (*Networks, error) { const bridge = "bridge" + var createdNoInternetID string noInternetName := namesgenerator.GetRandomName(1) noInternet, err := cli.NetworkCreate(ctx, noInternetName, network.CreateOptions{ @@ -27,12 +30,16 @@ func NewNetworks(ctx context.Context, cli *client.Client) (*Networks, error) { if err != nil { return nil, fmt.Errorf("failed to create no-internet network: %w", err) } + createdNoInternetID = noInternet.ID internetName := namesgenerator.GetRandomName(1) internet, err := cli.NetworkCreate(ctx, internetName, network.CreateOptions{ Driver: bridge, }) if err != nil { + if removeErr := cli.NetworkRemove(context.Background(), createdNoInternetID); removeErr != nil { + return nil, fmt.Errorf("failed to create internet network: %w (also failed to remove no-internet network %s: %v)", err, createdNoInternetID, removeErr) + } return nil, fmt.Errorf("failed to create internet network: %w", err) } @@ -46,11 +53,14 @@ func NewNetworks(ctx context.Context, cli *client.Client) (*Networks, error) { } func (n *Networks) Close() error { + var closeErr error + if err := n.cli.NetworkRemove(context.Background(), n.NoInternet.ID); err != nil { - return err + closeErr = errors.Join(closeErr, fmt.Errorf("failed to remove no-internet network %s: %w", n.NoInternet.ID, err)) } if err := n.cli.NetworkRemove(context.Background(), n.Internet.ID); err != nil { - return err + closeErr = errors.Join(closeErr, fmt.Errorf("failed to remove internet network %s: %w", n.Internet.ID, err)) } - return nil + + return closeErr } diff --git a/internal/infra/network_test.go b/internal/infra/network_test.go new file mode 100644 index 00000000..3b84e83a --- /dev/null +++ b/internal/infra/network_test.go @@ -0,0 +1,114 @@ +package infra + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/docker/docker/api/types/network" + "github.com/docker/docker/client" +) + +func newDockerClientForNetworkTests(t *testing.T, handler http.Handler) *client.Client { + t.Helper() + + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + + cli, err := client.NewClientWithOpts( + client.WithHost(server.URL), + client.WithVersion("1.44"), + client.WithHTTPClient(server.Client()), + ) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = cli.Close() + }) + + return cli +} + +func TestNewNetworks_CleansUpNoInternetNetworkWhenSecondCreateFails(t *testing.T) { + var mu sync.Mutex + createCalls := 0 + removeCalls := 0 + + cli := newDockerClientForNetworkTests(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + defer mu.Unlock() + + switch { + case r.Method == http.MethodPost && r.URL.Path == "/v1.44/networks/create": + createCalls++ + if createCalls == 1 { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"ID": "no-internet-id"}) + return + } + http.Error(w, "internet create failed", http.StatusInternalServerError) + case r.Method == http.MethodDelete && r.URL.Path == "/v1.44/networks/no-internet-id": + removeCalls++ + w.WriteHeader(http.StatusNoContent) + default: + http.NotFound(w, r) + } + })) + + _, err := NewNetworks(context.Background(), cli) + if err == nil { + t.Fatalf("expected NewNetworks to return an error when second network create fails") + } + + if removeCalls != 1 { + t.Fatalf("expected rollback to remove first-created network once, got %d", removeCalls) + } +} + +func TestNetworksClose_AttemptsRemovingBothNetworksWhenFirstRemoveFails(t *testing.T) { + var mu sync.Mutex + removed := map[string]int{} + + cli := newDockerClientForNetworkTests(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + defer mu.Unlock() + + if r.Method != http.MethodDelete { + http.NotFound(w, r) + return + } + + switch r.URL.Path { + case "/v1.44/networks/no-internet-id": + removed["no-internet-id"]++ + http.Error(w, "cannot remove first network", http.StatusInternalServerError) + case "/v1.44/networks/internet-id": + removed["internet-id"]++ + w.WriteHeader(http.StatusNoContent) + default: + http.NotFound(w, r) + } + })) + + nets := &Networks{ + cli: cli, + NoInternet: network.CreateResponse{ID: "no-internet-id"}, + Internet: network.CreateResponse{ID: "internet-id"}, + } + + err := nets.Close() + if err == nil { + t.Fatalf("expected Close to return an error when one of the removals fails") + } + + if removed["no-internet-id"] != 1 { + t.Fatalf("expected no-internet network removal attempt once, got %d", removed["no-internet-id"]) + } + if removed["internet-id"] != 1 { + t.Fatalf("expected internet network removal to still be attempted once, got %d", removed["internet-id"]) + } +}