bug: Kill aborts cleanup when OCI spec omits network namespace

[Nsanjayboruds]

Description

Kill() aborts before reaching the VMM and network cleanup when the OCI specification does not contain a network namespace.

When linux.namespaces omits specs.NetworkNamespace, findNS() returns:

namespace network was not found

This error propagates through joinSandboxNetNs() to Kill(), which returns immediately instead of continuing with the VMM and network cleanup.

This means the cleanup path is skipped for this OCI configuration.

System info

• urunc version/commit: b752cf9
• Architecture: amd64
• OS: Linux
• Go: 1.26.4
• VMM: Not exercised in the reproduction environment
• Unikernel: Not exercised in the reproduction environment

Steps to reproduce

1. Create an OCI specification whose linux.namespaces contains namespaces such as pid, ipc, uts, and mount, but omits the network namespace.
2. Construct a Unikontainer using this OCI specification.
3. Invoke Unikontainer.Kill().
4. Observe the returned error and whether the cleanup functions are reached.

A minimal regression-style test reproduces the behavior:

=== RUN   TestKill_MissingNetworkNamespace
--- PASS: TestKill_MissingNetworkNamespace (0.00s)
PASS
ok      github.com/urunc-dev/urunc/pkg/unikontainers    0.003s

The test verifies that Kill() returns:

failed to join sandbox netns: namespace network was not found

and that the VMM stop function is not invoked.

Actual behavior

Kill() returns:

failed to join sandbox netns: namespace network was not found

before reaching the subsequent cleanup operations.

The relevant control flow is:

Kill()
  -> joinSandboxNetNs()
      -> findNS(..., specs.NetworkNamespace)
          -> namespace network was not found
  -> return error

Therefore the subsequent cleanup calls are not reached:

vmm.Stop(u.State.Pid)
network.CleanupAllUruncTaps()

The reproduction confirms:

Kill() return value: error
VMM.Stop() called: NO
Network cleanup called: NO

Expected behavior

If an OCI specification legitimately omits the network namespace, Kill() should still perform the required process/VMM cleanup instead of aborting before cleanup.

At minimum, absence of the namespace should not prevent the termination/cleanup path from being executed.

Root cause

findNS() returns an error when the requested namespace type is absent from the OCI namespace list:

namespace network was not found

joinSandboxNetNs() does not distinguish between an intentionally absent network namespace and other namespace lookup failures, so the error propagates to Kill().

Kill() then returns immediately and does not reach the VMM/network cleanup operations.

Testing

A temporary regression-style test was used to verify the behavior.

The test does not claim to demonstrate an actual orphaned QEMU/Firecracker process because a complete VMM/container lifecycle could not be exercised in the available environment.

The confirmed behavior is that Kill() returns before vmm.Stop() and network.CleanupAllUruncTaps() are invoked.

Relevance to robustness testing

This is an OCI edge case that can be exercised by structured testing/fuzzing by mutating the namespace configuration.

A robustness test suite should verify that lifecycle operations such as Kill() continue to perform required cleanup across valid and edge-case OCI configurations.

Possible fix direction

Investigate handling an absent network namespace separately from an invalid/unusable namespace.

The fix should avoid relying on matching an error string and should preserve genuinely fatal namespace errors.

A regression test should verify that a Unikontainer without a network namespace does not abort the cleanup path.

Related issues

I searched the existing urunc issues and pull requests for:

• Kill
• NetworkNamespace
• network namespace
• joinSandboxNetNs
• namespace was not found
• cleanup

Issue #240 is related to Kill() cleanup behavior but covers a different failure mode and does not address the missing network namespace case.

[kunalKumar-13]

This reproduces on main as far as I can tell from the source, and the interesting part is that the code already tries to tolerate a missing network namespace — it just cannot recognise this particular flavour of missing.

findNS has three outcomes, and only two of them are distinguishable.

case	returns
type present, path exists	(path, nil)
type present, path empty or gone	("", ErrNotExistingNS) via checkValidNsPath (utils.go:217)
type absent from the spec	("", fmt.Errorf("namespace %s was not found", ...)) (utils.go:311)

The third is a plain error with no %w and no sentinel, so errors.Is(err, ErrNotExistingNS) is false for it. That is deliberate — utils_test.go:259 asserts exactly this:

assert.False(t, errors.Is(err, ErrNotExistingNS),
    "a namespace type absent from the spec must not be reported as ErrNotExistingNS")

So the tolerance that exists never triggers. joinSandboxNetNs opens with:

netNsPath, err := findNS(u.Spec.Linux.Namespaces, specs.NetworkNamespace)
if err != nil && !errors.Is(err, ErrNotExistingNS) {
    return err
}

and the comment just below — "In case no path was specified for the network namespace it means that we had to create a new one and therefore we can join it by using the pid of the monitor process" — describes the fallback that case 3 ought to reach. It does not, because the guard only forgives ErrNotExistingNS. The plain error returns immediately.

Kill() then takes the other branch (unikontainers.go:957):

if errors.Is(err, ErrNotExistingNS) {
    return nil          // not taken
}
return fmt.Errorf("failed to join sandbox netns: %v", err)

so it never reaches vmm.Stop() or network.CleanupAllUruncTaps() — which is the skipped cleanup described in the issue.

The pattern for fixing it already exists in the file. The mount-namespace caller at unikontainers.go:690 separates the same three cases cleanly:

_, err = findNS(u.Spec.Linux.Namespaces, specs.MountNamespace)
// Only pivot if a mount namespace entry is actually present in the
// spec, either to join (err is nil) or to create (err is
// ErrNotExistingNS). If the entry is missing entirely, ...
withPivot := err == nil || errors.Is(err, ErrNotExistingNS)

That reads "present in the spec" as nil || ErrNotExistingNS, and "absent" as everything else. Kill() needs the same distinction but has no sentinel to test against.

What I would suggest, if it is welcome:

1. Give case 3 its own sentinel — something like ErrNSTypeNotInSpec — wrapped with %w in findNS. The existing test keeps passing, because it asserts the error is not ErrNotExistingNS, which stays true. withPivot at :696 is unaffected, since it never matched case 3 anyway.
2. In Kill(), treat that sentinel as "no namespace to join" and continue to the VMM stop and tap cleanup, rather than the current return nil used for ErrNotExistingNS. The two are not the same: a destroyed namespace suggests the sandbox is already gone, whereas a spec that never declared one can still have a live monitor and tap devices to clean up.

One caveat on my side: pkg/unikontainers is Linux-only, and I am on Windows, so I have not run these paths — the above is read from the source and from the assertions in utils_test.go. GOOS=linux go vet ./pkg/unikontainers/ is clean.

Happy to open a PR for (1) and (2) with unit tests over findNS and the Kill branch selection if that shape sounds right. I did not want to change kill-path behaviour in a container runtime without checking the intent first, particularly the return nil versus continue question, which is a judgement call about what a missing netns implies.