Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 18 additions & 19 deletions internal/engine/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,33 +190,32 @@ func (e *Engine) Destroy(ctx context.Context, removeVolumes, removeProxy bool) e
// ownership released with something still holding :80/:443, and
// the compose file deleted so no ob command can reach it.
//
// --remove-orphans is deliberately absent for the same reason
// the sweep is name-anchored: it removes every container carrying
// the project label that the file does not declare, which is the
// seizure described below. The sweep already covers ob's own
// orphan, which is the only container ob created.
// --remove-orphans is deliberately absent because a project label
// alone is not proof of ownership: a user can independently run
// `docker compose -p onebox-proxy up`. A fixed name alone is not
// proof either: after a rename (or on a host installed later), the
// old name is free for an unrelated container to use.
//
// It matches ob's own fixed container names, not the project
// label: `docker compose -p onebox-proxy up` run by a user for their
// own reasons carries that label too, and force-removing their
// containers is the foreign-resource seizure preflight refuses
// everywhere else. The compose file declares these two fixed names,
// so together they are complete for ob.
// The fallback therefore requires the exact generated container
// name AND the Compose project/service labels emitted by ob's
// generated two-service document. This still finds ob containers
// after the file or Compose state disappears without seizing a
// same-named foreign container.
//
// Without `|| exit $?` a failed docker ps yields an empty list,
// the if is skipped, and teardown "succeeds" — the very outcome
// this exists to prevent.
//
// Reaching here implies the proxy is managed: Destroy refuses
// --proxy for an unmanaged one before any of this runs. That is
// what makes the name sweep safe — on an unmanaged host ob never
// created a onebox-proxy container, so anything answering to the
// name would be the operator's.
// Reaching here implies the proxy is managed, but that proves only
// the requested operation's scope. The filters below separately
// prove ownership of every container selected for force-removal.
down := "if [ -f " + q(hp.Compose) + " ]; then docker compose -p " + proxy.Project + " -f " + q(hp.Compose) + " down || exit $?; fi; " +
"proxy_orphans=$(docker ps -aq --filter name=^" + proxy.ContainerName + "$) || exit $?; " +
"discovery_orphans=$(docker ps -aq --filter name=^" + proxy.DiscoveryContainerName + "$) || exit $?; " +
"proxy_orphans=$(docker ps -aq --filter name=^" + proxy.ContainerName + "$ --filter label=com.docker.compose.project=" + proxy.Project + " --filter label=com.docker.compose.service=proxy) || exit $?; " +
"discovery_orphans=$(docker ps -aq --filter name=^" + proxy.DiscoveryContainerName + "$ --filter label=com.docker.compose.project=" + proxy.Project + " --filter label=com.docker.compose.service=discovery) || exit $?; " +
"legacy_discovery_orphans=$(docker ps -aq --filter name=^" + proxy.LegacyDiscoveryContainerName + "$ --filter label=com.docker.compose.project=" + proxy.Project + " --filter label=com.docker.compose.service=discovery) || exit $?; " +
"if [ -n \"$proxy_orphans\" ]; then docker rm -f $proxy_orphans || exit $?; fi; " +
"if [ -n \"$discovery_orphans\" ]; then docker rm -f $discovery_orphans; fi"
"if [ -n \"$discovery_orphans\" ]; then docker rm -f $discovery_orphans || exit $?; fi; " +
"if [ -n \"$legacy_discovery_orphans\" ]; then docker rm -f $legacy_discovery_orphans; fi"
if res, err := e.hostMutate(ctx, down); err != nil {
return err
} else if res.ExitCode != 0 {
Expand Down
10 changes: 7 additions & 3 deletions internal/engine/ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,13 @@ func TestDestroyProxyTeardownForSoleOwner(t *testing.T) {
if !strings.Contains(seq, "docker compose -p onebox-proxy -f '/var/lib/ob/_host/proxy/compose.yaml' down") {
t.Fatalf("sole owner with --proxy must tear the proxy down:\n%s", seq)
}
for _, name := range []string{"name=^onebox-proxy$", "name=^onebox-proxy-discovery$"} {
if !strings.Contains(seq, name) {
t.Fatalf("proxy teardown must sweep orphan %s even when Compose state is missing:\n%s", name, seq)
for _, selector := range []string{
"name=^onebox-proxy$ --filter label=com.docker.compose.project=onebox-proxy --filter label=com.docker.compose.service=proxy",
"name=^onebox-discovery$ --filter label=com.docker.compose.project=onebox-proxy --filter label=com.docker.compose.service=discovery",
"name=^onebox-proxy-discovery$ --filter label=com.docker.compose.project=onebox-proxy --filter label=com.docker.compose.service=discovery",
} {
if !strings.Contains(seq, selector) {
t.Fatalf("proxy teardown must sweep owned orphan %s even when Compose state is missing:\n%s", selector, seq)
}
}
if !strings.Contains(seq, "rm -rf '/var/lib/ob/_host/proxy'") {
Expand Down
7 changes: 4 additions & 3 deletions internal/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ const (
DiscoveryImageRepository = "ghcr.io/labstack/onebox-discovery"
// Project is the compose project name; ContainerName the fixed container
// name — both host-global, which is the point.
Project = app.ProxyProject
ContainerName = app.ProxyProject
DiscoveryContainerName = app.ProxyProject + "-discovery"
Project = app.ProxyProject
ContainerName = app.ProxyProject
DiscoveryContainerName = "onebox-discovery"
LegacyDiscoveryContainerName = app.ProxyProject + "-discovery"
)

var releaseVersion = regexp.MustCompile(`^v[0-9]{4}\.[0-9]{1,2}\.[0-9]+$`)
Expand Down
2 changes: 1 addition & 1 deletion internal/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestRenderCompose(t *testing.T) {
"container_name: onebox-proxy",
"image: traefik:v3.7",
`"80:80"`, `"443:443"`,
"container_name: onebox-proxy-discovery",
"container_name: onebox-discovery",
"image: ghcr.io/labstack/onebox-discovery:edge",
"./config:/etc/traefik:ro",
"./dynamic:/etc/traefik/dynamic:ro",
Expand Down