Container private networking broken with hostBridge enabled

[tadfisher]

Issue description

Since #49392 was merged, containers configured with privateNetworking and hostBridge but not hostAddress or localAddress fail to create the veth pair.

It should be pretty obvious by reading through that PR what the cause is; since hostBridge obviates both hostAddress and localAddress, we still need to pass --network-bridge= and allow the container to bind to an address via DHCP, even when neither hostAddress nor localAddress are configured.

Steps to reproduce

Minimal configuration:

containers.brtest = {
  privateNetwork = true;
  hostBridge = "br0";
  config = { config, pkgs, ... }: {
    networking.interfaces."eth0".useDHCP = true;
  };
};

Technical details

• system: "x86_64-linux"
• host os: Linux 4.19.9, NixOS, 19.03pre162671.44b02b52ea6 (Koi)
• multi-user?: yes
• sandbox: yes
• version: nix-env (Nix) 2.1.3
• channels(root): "nixos-19.03pre162671.44b02b52ea6"
• channels(tad): ""
• nixpkgs: /nix/var/nix/profiles/per-user/root/channels/nixos

[arianvp]

This seems like not to hard to fix right? Do you want to take a stab at it yourself, or shall I take a look at it?

[arianvp]

@tadfisher do you have interest in taking a stab at this?
Also Ping @uvNikita as you were the original author of that PR. perhaps you could have a look at this?

Otherwise i'll put it on my todo list

[qolii]

I hope someone can look at this soon, it really bites me! I don't understand it well enough now to look myself.

What might be a workaround in the mean time? Should I specify the hostAddress and localAddress by hand for each container, and then deal with adding them to a bridge separately on the host?

[uvNikita]

I think just moving the host bridge specification part out of the if statement should do the trick, the port part can probably be moved out as well. Here is a patch, but I haven't tested it yet:

--- a/nixos/modules/virtualisation/containers.nix
+++ b/nixos/modules/virtualisation/containers.nix
@@ -90,18 +90,20 @@ let
 
       if [ -n "$HOST_ADDRESS" ] || [ -n "$LOCAL_ADDRESS" ]; then
         extraFlags+=" --network-veth"
-        if [ -n "$HOST_BRIDGE" ]; then
-          extraFlags+=" --network-bridge=$HOST_BRIDGE"
-        fi
-        if [ -n "$HOST_PORT" ]; then
-          OIFS=$IFS
-          IFS=","
-          for i in $HOST_PORT
-          do
-              extraFlags+=" --port=$i"
-          done
-          IFS=$OIFS
-        fi
       fi
+
+      if [ -n "$HOST_PORT" ]; then
+        OIFS=$IFS
+        IFS=","
+        for i in $HOST_PORT
+        do
+            extraFlags+=" --port=$i"
+        done
+        IFS=$OIFS
+      fi
+
+      if [ -n "$HOST_BRIDGE" ]; then
+        extraFlags+=" --network-bridge=$HOST_BRIDGE"
+      fi
 
       extraFlags+=" ${concatStringsSep " " (mapAttrsToList nspawnExtraVethArgs cfg.extraVeths)}"

[qolii]

@uvNikita, thanks for the fast reply! Have you been able to test it yet yourself? I think I am seeing the same behavior with this patch applied (that is, my container still comes up with only a lo interface)

[uvNikita]

@qolii I changed the patch a bit and it's working on my tests now. Can you double check that it fixes the problem for you as well?

[arianvp]

Could we write a VM Test that checks the behaviour? To stop these regressions in the future?

[uvNikita]

@arianvp good point! I now added a test the PR.

[qolii]

Hi @uvNikita, it works perfectly now! Thanks!