cni: use check command when restoring from restart

When the Nomad client restarts and restores allocations, the network namespace
for an allocation may exist but no longer be correctly configured. For example,
if the host is rebooted and the task was a Docker task using a pause container,
the network namespace may be recreated by the docker daemon.

When we restore an allocation, use the CNI "check" command to verify that any
existing network namespace matches the expected configuration. This requires CNI
plugins of at least version 1.2.0 to avoid a bug in older plugin versions that
would cause the check to fail. If the check fails, fail the restore so that the
allocation can be recreated (rather than silently having networking fail).

This should fix the gap left #24650 for Docker task drivers and any other
drivers with the `MustInitiateNetwork` capability.

Fixes: #24292
Ref: #24650

.changelog/24658.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+networking: check network namespaces on Linux during client restarts and fail the allocation if an existing namespace is invalid
+```

client/allocrunner/network_hook.go

@@ -142,11 +142,14 @@ func (h *networkHook) Prerun() error {
 		h.isolationSetter.SetNetworkIsolation(spec)
 	}
 
-	if created {
-		status, err := h.networkConfigurator.Setup(context.TODO(), h.alloc, spec)
+	if spec != nil {
+		status, err := h.networkConfigurator.Setup(context.TODO(), h.alloc, spec, created)
 		if err != nil {
 			return fmt.Errorf("failed to configure networking for alloc: %v", err)
 		}
+		if status == nil {
+			return nil // netns already existed and was correctly configured
+		}
 
 		// If the driver set the sandbox hostname label, then we will use that
 		// to set the HostsConfig.Hostname. Otherwise, identify the sandbox

client/allocrunner/networking.go

@@ -14,7 +14,7 @@ import (
 // NetworkConfigurator sets up and tears down the interfaces, routes, firewall
 // rules, etc for the configured networking mode of the allocation.
 type NetworkConfigurator interface {
-	Setup(context.Context, *structs.Allocation, *drivers.NetworkIsolationSpec) (*structs.AllocNetworkStatus, error)
+	Setup(context.Context, *structs.Allocation, *drivers.NetworkIsolationSpec, bool) (*structs.AllocNetworkStatus, error)
 	Teardown(context.Context, *structs.Allocation, *drivers.NetworkIsolationSpec) error
 }
 
@@ -23,7 +23,7 @@ type NetworkConfigurator interface {
 // require further configuration
 type hostNetworkConfigurator struct{}
 
-func (h *hostNetworkConfigurator) Setup(context.Context, *structs.Allocation, *drivers.NetworkIsolationSpec) (*structs.AllocNetworkStatus, error) {
+func (h *hostNetworkConfigurator) Setup(context.Context, *structs.Allocation, *drivers.NetworkIsolationSpec, bool) (*structs.AllocNetworkStatus, error) {
 	return nil, nil
 }
 func (h *hostNetworkConfigurator) Teardown(context.Context, *structs.Allocation, *drivers.NetworkIsolationSpec) error {
@@ -41,10 +41,10 @@ type synchronizedNetworkConfigurator struct {
 	nc NetworkConfigurator
 }
 
-func (s *synchronizedNetworkConfigurator) Setup(ctx context.Context, allocation *structs.Allocation, spec *drivers.NetworkIsolationSpec) (*structs.AllocNetworkStatus, error) {
+func (s *synchronizedNetworkConfigurator) Setup(ctx context.Context, allocation *structs.Allocation, spec *drivers.NetworkIsolationSpec, created bool) (*structs.AllocNetworkStatus, error) {
 	networkingGlobalMutex.Lock()
 	defer networkingGlobalMutex.Unlock()
-	return s.nc.Setup(ctx, allocation, spec)
+	return s.nc.Setup(ctx, allocation, spec, created)
 }
 
 func (s *synchronizedNetworkConfigurator) Teardown(ctx context.Context, allocation *structs.Allocation, spec *drivers.NetworkIsolationSpec) error {

client/allocrunner/networking_bridge_linux.go

@@ -120,12 +120,12 @@ func (b *bridgeNetworkConfigurator) ensureForwardingRules() error {
 }
 
 // Setup calls the CNI plugins with the add action
-func (b *bridgeNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Allocation, spec *drivers.NetworkIsolationSpec) (*structs.AllocNetworkStatus, error) {
+func (b *bridgeNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Allocation, spec *drivers.NetworkIsolationSpec, created bool) (*structs.AllocNetworkStatus, error) {
 	if err := b.ensureForwardingRules(); err != nil {
 		return nil, fmt.Errorf("failed to initialize table forwarding rules: %v", err)
 	}
 
-	return b.cni.Setup(ctx, alloc, spec)
+	return b.cni.Setup(ctx, alloc, spec, created)
 }
 
 // Teardown calls the CNI plugins with the delete action

client/allocrunner/networking_cni.go

@@ -139,7 +139,8 @@ func addNomadWorkloadCNIArgs(logger log.Logger, alloc *structs.Allocation, cniAr
 }
 
 // Setup calls the CNI plugins with the add action
-func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Allocation, spec *drivers.NetworkIsolationSpec) (*structs.AllocNetworkStatus, error) {
+func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Allocation, spec *drivers.NetworkIsolationSpec, created bool) (*structs.AllocNetworkStatus, error) {
+
 	if err := c.ensureCNIInitialized(); err != nil {
 		return nil, fmt.Errorf("cni not initialized: %w", err)
 	}
@@ -171,6 +172,16 @@ func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Alloc
 		cniArgs[ConsulIPTablesConfigEnvVar] = string(iptablesCfg)
 	}
 
+	if !created {
+		err := c.cni.Check(ctx, alloc.ID, spec.Path,
+			c.nsOpts.withCapabilityPortMap(portMaps.ports),
+			c.nsOpts.withArgs(cniArgs),
+		)
+		// return early: either the netns is ok or we need to start the alloc
+		// over anyways
+		return nil, err
+	}
+
 	// Depending on the version of bridge cni plugin used, a known race could occure
 	// where two alloc attempt to create the nomad bridge at the same time, resulting
 	// in one of them to fail. This rety attempts to overcome those erroneous failures.

client/allocrunner/networking_cni_test.go

@@ -296,7 +296,7 @@ func TestSetup(t *testing.T) {
 			}
 
 			// method under test
-			result, err := c.Setup(context.Background(), alloc, spec)
+			result, err := c.Setup(context.Background(), alloc, spec, true)
 			if tc.expectErr == "" {
 				must.NoError(t, err)
 				must.Eq(t, tc.expectResult, result)