fix network recreate

[alexcos20]

Closing #1415

Fix: service restart wedged by leaked Docker network (409 "network already exists")

Restarting a Service-on-Demand job could fail permanently with
(HTTP code 409) unexpected - network with name ocean-svc-<serviceId> already exists,
leaving the job stuck in Error (99) with empty containerId/networkId — and every
subsequent restart failing the same way, with no way out short of manual docker network rm
on the host.

Root cause

Each service gets a Docker network with a deterministic name (ocean-svc-<serviceId>),
but every cleanup path (restart teardown, stop, orphan recovery, expiry sweep) removed it
only via the persisted job.networkId field. That field is written only after the
container starts successfully, so a node crash (or error) between createNetwork and that
persist leaked the network with networkId='' in the DB. From then on, restart skipped
teardown (empty id), hit the leftover named network at createNetwork → Docker 409 → job
marked Error — a self-perpetuating dead end. Leaked networks were also never reaped by the
expiry sweep (same gate), permanently consuming the Docker daemon's limited IPAM subnet pool.

What changed

All in src/components/c2d/compute_engine_docker.ts:

• removeServiceNetwork(serviceId, networkId?) (new): removes the service network by
  both the persisted id (when set) and the deterministic name, so cleanup never depends
  solely on the DB field. If the network still has stale attached containers (crash after
  container.start() but before the persist), they are force-removed first — otherwise
  network.remove() fails with "has active endpoints". Benign Docker errors (404/304 =
  already gone) count as success; anything else propagates.
• createServiceNetwork(serviceId) (new): wraps createNetwork; on a 409 name conflict
  it removes the stale network and retries once. This makes already-broken jobs (like the
  reported one) self-heal on the next restart with no manual intervention or DB surgery.
  The stale network is removed and recreated rather than reused: reuse saves nothing (a
  leaked network can still hold a stale container bound to the service's host ports, which
  must be inspected and force-removed either way), a fresh network always reflects the
  current code's configuration instead of inheriting options from whatever node version
  leaked it, and remove+recreate matches restart's tear-down-and-rebuild semantics — the
  container is never reused either.
• All four cleanup paths now go through the helper: restartService pre-teardown,
  stopService, processServiceStart orphan recovery, and cleanupServiceDocker
  (failure-path cleanup — now works even when createNetwork itself threw and no live
  handle exists). Both createNetwork call sites (initial start + restart) use the
  self-healing wrapper.
• Since the expiry sweep funnels Running/Error jobs through stopService, leaked
  networks from abandoned jobs are now reaped automatically too — no schema or sweep changes.
• isBenignDockerError hoisted from stopService to module scope (shared, unchanged).

No handler/API changes: Error-status jobs were already restartable; the bug was entirely
engine-side. markServiceFailed still deliberately leaves Docker resources in place for
restart to reclaim — reclaiming is just reliable now.

Tests

• Unit (src/test/unit/service/serviceNetworkCleanup.test.ts, no Docker daemon):
  removal by deterministic name with empty networkId (the exact leak scenario), stale
  attached containers force-removed before network removal, benign 404/304 vs propagated
  errors, 409 → remove + retry-once (and no retry on other errors), and stopService
  end-to-end with a leaked network ending Stopped.
• Integration (src/test/integration/services.test.ts): new case simulates the
  post-crash DB state (containerId='', networkId='', status=Error while the real
  container + network keep running) and asserts SERVICE_RESTART succeeds (pre-fix: 500
  with the 409 message), the service returns to Running on the same host port, the stale
  container is gone, and exactly one ocean-svc-<serviceId> network remains. The stop test
  now also asserts the network is gone by name.
• Docs: docs/services.md notes the self-healing teardown semantics.

Summary by CodeRabbit

• Bug Fixes

  • Improved service restart and stop reliability after interrupted starts or node crashes.
  • Automatically removes stale containers and Docker networks that could block service recovery.
  • Added recovery for network-name conflicts during service startup.
  • Ensured failed container starts correctly report an error status.

• Documentation

  • Clarified service network cleanup and restart behavior.

• Tests

  • Added coverage for stale network recovery, cleanup, and restart scenarios.

[coderabbitai]

Warning

Review limit reached

@alexcos20, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ed827edb-8deb-41b8-9b7d-5ec3e1c73c72

📥 Commits

Reviewing files that changed from the base of the PR and between 1733e6d and 4b02baf.

📒 Files selected for processing (5)

• docs/services.md
• src/components/c2d/compute_engine_docker.ts
• src/test/integration/services.test.ts
• src/test/unit/compute.test.ts
• src/test/unit/service/serviceNetworkCleanup.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch bug/service_restart_fix_network

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[alexcos20]

/run-security-scan

[alexcos20]

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This PR effectively addresses the Docker network leak issue caused by node crashes mid-start. The self-healing implementation properly removes network instances by both their deterministic name and stored ID, ensuring robust cleanup. Code structure, error handling, and test coverage are excellent.

Comments:
• [INFO][style] Extracting isBenignDockerError to a higher scope is a great refactoring move. It avoids re-defining it inside stopService and ensures consistent error checking across different methods like removeServiceNetwork.
• [INFO][other] Excellent handling of the edge case where a container might be removed by an external process between network.inspect() and the .remove({ force: true }) call. Leveraging isBenignDockerError guarantees teardown stability.
• [INFO][bug] The self-healing logic here effectively catches the 409 conflict and properly recreates the network without halting the process. Well implemented.
• [INFO][other] Great addition of unit tests to mock and assert network operations. The testing of both leak scenarios and non-benign error propagation provides solid assurance for this fix. LGTM!

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

putComment timed out

[alexcos20]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[alexcos20]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

---

Your plan includes PR reviews subject to rate limits. More reviews will be available in 42 minutes.