Skip to content

Commit 633ea75

Browse files
authored
Prevent deleting proxy containers on restart (#1083)
1 parent 8e88d7c commit 633ea75

File tree

1 file changed

+27
-24
lines changed

1 file changed

+27
-24
lines changed

pkg/container/docker/client.go

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -724,9 +724,15 @@ func (c *Client) RemoveWorkload(ctx context.Context, workloadID string) error {
724724
} else {
725725
labels = make(map[string]string)
726726
}
727-
err = c.removeWorkloadContainers(ctx, containerName, workloadID, labels)
727+
err = c.removeContainer(ctx, workloadID)
728728
if err != nil {
729-
return err // removeWorkloadContainers already wraps the error with context.
729+
return err // removeContainer already wraps the error with context.
730+
}
731+
732+
// Clean up any proxy containers associated with this workload.
733+
err = c.removeProxyContainers(ctx, containerName, labels)
734+
if err != nil {
735+
return err // removeProxyContainers already wraps the error with context.
730736
}
731737

732738
// Clear up any networks associated with this workload.
@@ -1328,16 +1334,9 @@ func (c *Client) handleExistingContainer(
13281334
}
13291335

13301336
// Configurations don't match, we need to recreate the container.
1331-
// Remove the existing container and any of the proxy containers.
1332-
// Note that we do not use the RemoveWorkload method because that
1333-
// will delete networks - which we do not want to do here.
1334-
var labels map[string]string
1335-
if info.Config != nil {
1336-
labels = info.Config.Labels
1337-
} else {
1338-
labels = make(map[string]string)
1339-
}
1340-
if err := c.removeWorkloadContainers(ctx, info.Name, containerID, labels); err != nil {
1337+
// Remove only this container, leave any associated networks and containers intact
1338+
// Any proxy containers (like ingress/egress) will have already recreated themselves at this point
1339+
if err := c.removeContainer(ctx, containerID); err != nil {
13411340
return false, err
13421341
}
13431342

@@ -1399,27 +1398,31 @@ func (c *Client) deleteNetwork(ctx context.Context, name string) error {
13991398
return nil
14001399
}
14011400

1402-
// removeWorkloadContainers removes the MCP server container and any proxy containers.
1403-
func (c *Client) removeWorkloadContainers(
1404-
ctx context.Context,
1405-
containerName string,
1406-
workloadID string,
1407-
workloadLabels map[string]string,
1408-
) error {
1409-
// remove the / if it starts with it
1410-
containerName = strings.TrimPrefix(containerName, "/")
1411-
1412-
err := c.client.ContainerRemove(ctx, workloadID, container.RemoveOptions{
1401+
// removeContainer removes a container by ID, without removing any associated networks or proxy containers.
1402+
func (c *Client) removeContainer(ctx context.Context, containerID string) error {
1403+
err := c.client.ContainerRemove(ctx, containerID, container.RemoveOptions{
14131404
Force: true,
14141405
})
14151406
if err != nil {
14161407
// If the workload doesn't exist, that's fine - it's already removed
14171408
if errdefs.IsNotFound(err) {
14181409
return nil
14191410
}
1420-
return NewContainerError(err, workloadID, fmt.Sprintf("failed to remove workload: %v", err))
1411+
return NewContainerError(err, containerID, fmt.Sprintf("failed to remove container: %v", err))
14211412
}
14221413

1414+
return nil
1415+
}
1416+
1417+
// removeProxyContainers removes the MCP server container and any proxy containers.
1418+
func (c *Client) removeProxyContainers(
1419+
ctx context.Context,
1420+
containerName string,
1421+
workloadLabels map[string]string,
1422+
) error {
1423+
// remove the / if it starts with it
1424+
containerName = strings.TrimPrefix(containerName, "/")
1425+
14231426
// If network isolation is not enabled, then there is nothing else to do.
14241427
// NOTE: This check treats all workloads created before the introduction of
14251428
// this label as having network isolation enabled. This is to ensure that they

0 commit comments

Comments
 (0)