diff --git a/backend/pkg/docker/client.go b/backend/pkg/docker/client.go index 6a114561..af5604d4 100644 --- a/backend/pkg/docker/client.go +++ b/backend/pkg/docker/client.go @@ -600,7 +600,11 @@ func (dc *dockerClient) Cleanup(ctx context.Context) error { func (dc *dockerClient) IsContainerRunning(ctx context.Context, containerID string) (bool, error) { inspection, err := dc.client.ContainerInspect(ctx, containerID) if err != nil { - return false, fmt.Errorf("container inspection failed: %w", err) + if !client.IsErrNotFound(err) { + return false, fmt.Errorf("container inspection failed: %w", err) + } + // a removed container is missing, not an inspection failure + return false, nil } // Check both Running state and health status if available diff --git a/backend/pkg/docker/client_test.go b/backend/pkg/docker/client_test.go index f7ae3a23..62de6f6e 100644 --- a/backend/pkg/docker/client_test.go +++ b/backend/pkg/docker/client_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/client" ) func TestStatContainerEntries_AllSucceed(t *testing.T) { @@ -260,3 +261,79 @@ func failNames(failures []statFailure) map[string]bool { } return m } + +const probeImage = "alpine:3.20" + +// newDaemonClient binds a client to the local daemon, skipping the test when +// none is reachable. +func newDaemonClient(t *testing.T) *dockerClient { + t.Helper() + + cli, err := client.NewClientWithOpts(client.FromEnv) + if err != nil { + t.Skipf("docker daemon unavailable: %v", err) + } + + ctx := context.Background() + cli.NegotiateAPIVersion(ctx) + if _, err := cli.Ping(ctx); err != nil { + t.Skipf("docker daemon unavailable: %v", err) + } + + return &dockerClient{client: cli} +} + +// A flow keeps the id of its primary container in the database. When that +// container is removed behind pentagi's back the id has to read as not running, +// otherwise the flow can never rebuild it. +func TestIsContainerRunningRemovedContainer(t *testing.T) { + dc := newDaemonClient(t) + ctx := context.Background() + + created, err := dc.client.ContainerCreate(ctx, &container.Config{ + Image: probeImage, + Entrypoint: []string{"tail", "-f", "/dev/null"}, + }, nil, nil, nil, "") + if client.IsErrNotFound(err) { + t.Skipf("%s is not present locally", probeImage) + } + if err != nil { + t.Fatalf("create probe container: %v", err) + } + t.Cleanup(func() { + dc.client.ContainerRemove(context.Background(), created.ID, container.RemoveOptions{Force: true}) + }) + + if err := dc.client.ContainerStart(ctx, created.ID, container.StartOptions{}); err != nil { + t.Fatalf("start probe container: %v", err) + } + + running, err := dc.IsContainerRunning(ctx, created.ID) + if err != nil || !running { + t.Fatalf("got running=%v err=%v, want true and no error", running, err) + } + + if err := dc.client.ContainerRemove(ctx, created.ID, container.RemoveOptions{Force: true}); err != nil { + t.Fatalf("remove probe container: %v", err) + } + + running, err = dc.IsContainerRunning(ctx, created.ID) + if err != nil { + t.Fatalf("removed container: got error %v, want none", err) + } + if running { + t.Fatal("removed container reported as running") + } +} + +func TestIsContainerRunningUnknownContainer(t *testing.T) { + dc := newDaemonClient(t) + + running, err := dc.IsContainerRunning(context.Background(), "pentagi-container-that-does-not-exist") + if err != nil { + t.Fatalf("unknown container: got error %v, want none", err) + } + if running { + t.Fatal("unknown container reported as running") + } +} diff --git a/backend/pkg/tools/tools.go b/backend/pkg/tools/tools.go index a460e15d..c3d8b107 100644 --- a/backend/pkg/tools/tools.go +++ b/backend/pkg/tools/tools.go @@ -481,18 +481,24 @@ func (fte *flowToolsExecutor) SetGraphitiClient(client *graphiti.Client) { func (fte *flowToolsExecutor) Prepare(ctx context.Context) error { if cnt, err := fte.db.GetFlowPrimaryContainer(ctx, fte.flowID); err == nil { - switch cnt.Status { - case database.ContainerStatusRunning: - fte.primaryID = cnt.ID - fte.primaryLID = cnt.LocalID.String - if err := fte.syncMissingFiles(ctx); err != nil { - containerName := PrimaryTerminalName(fte.cfg.TenantPrefix(), fte.flowID) - return fmt.Errorf("failed to sync missing files to container '%s': %w", containerName, err) + // the stored status goes stale when the container is removed outside pentagi + if cnt.Status == database.ContainerStatusRunning { + containerName := PrimaryTerminalName(fte.cfg.TenantPrefix(), fte.flowID) + running, err := fte.docker.IsContainerRunning(ctx, cnt.LocalID.String) + if err != nil { + return fmt.Errorf("failed to inspect container '%s': %w", containerName, err) + } + if running { + fte.primaryID = cnt.ID + fte.primaryLID = cnt.LocalID.String + if err := fte.syncMissingFiles(ctx); err != nil { + return fmt.Errorf("failed to sync missing files to container '%s': %w", containerName, err) + } + return nil } - return nil - default: - fte.docker.RemoveContainer(ctx, cnt.LocalID.String, cnt.ID) } + + fte.docker.RemoveContainer(ctx, cnt.LocalID.String, cnt.ID) } // Explicit capability allow-list (CapDrop: ALL below): Docker's default 14