[PATCH installer v4 16/30] install: network: set up vlan on management interface if requested

[Christoph Heiss <c.heiss@proxmox.com>]

Adds the necessary configuration to /etc/network/interfaces when a VLAN
ID was configured by the user in the installation config.

Depending on the rest of the network configuration (bridged, bonded), a
separated (dotted) VLAN interface will be created and integrated in the
rest of the configuration as needed.

This follows our recommended approach for setting up such a
network configuration.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Had a discussion with @Stefan about this, and this way of configuring
this was the best / most user-friendly we plotted out.

Changes v3 -> v4:
  * no changes

Changes v2 -> v3:
  * new patch

 Proxmox/Install.pm                |  45 +++++++--
 test/generate-ifupdown2-config.pl | 159 ++++++++++++++++++++++++++----
 2 files changed, 175 insertions(+), 29 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 9e167d8..b39df6f 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -889,14 +889,40 @@ sub generate_ifupdown2_config {
             && ($_->{protocol} // '') eq 'ra';
     } $gateways6->@*;
 
-    if ($iso_env->{cfg}->{bridged_network}) {
-        if ($bond_enabled) {
-            $ifaces .= "auto $ethdev\n" #
-                . "iface $ethdev $ntype manual\n" #
-                . $bond_config;
-        } else {
-            $ifaces .= "iface $ethdev $ntype manual\n";
-        }
+    my $vlan_enabled = Proxmox::Install::Config::get_mngmt_vlan_enabled();
+    my $vlan_id = Proxmox::Install::Config::get_mngmt_vlan_opt('vid');
+
+    # the live environment is not on the management VLAN, so its routes say nothing about it
+    $is_gateway6_from_ra = 0 if $vlan_enabled;
+
+    my $link_config =
+        $bond_enabled
+        ? "auto $ethdev\niface $ethdev $ntype manual\n$bond_config"
+        : "iface $ethdev $ntype manual\n";
+
+    my $get_vlan_config = sub {
+        my ($raw_device) = @_;
+
+        return
+            "\nauto $raw_device.$vlan_id\niface $raw_device.$vlan_id $ntype static\n"
+            . "\taddress $cidr\n"
+            . "\tgateway $gateway\n";
+    };
+
+    if ($iso_env->{cfg}->{bridged_network} && $vlan_enabled) {
+        $ifaces .= $link_config;
+
+        $ifaces .=
+            "\nauto vmbr0\niface vmbr0 $ntype manual\n"
+            . "\tbridge-ports $ethdev\n"
+            . "\tbridge-stp off\n"
+            . "\tbridge-fd 0\n"
+            . "\tbridge-vlan-aware yes\n"
+            . "\tbridge-vids 2-4094\n";
+
+        $ifaces .= $get_vlan_config->('vmbr0');
+    } elsif ($iso_env->{cfg}->{bridged_network}) {
+        $ifaces .= $link_config;
 
         $ifaces .= "\nauto vmbr0\niface vmbr0 $ntype static\n";
 
@@ -908,6 +934,9 @@ sub generate_ifupdown2_config {
         $ifaces .= "\tbridge-ports $ethdev\n" #
             . "\tbridge-stp off\n" #
             . "\tbridge-fd 0\n";
+    } elsif ($vlan_enabled) {
+        $ifaces .= $bond_enabled ? $link_config : "auto $ethdev\n$link_config";
+        $ifaces .= $get_vlan_config->($ethdev);
     } else {
         # if bonding is enabled, this becomes the bond interface stanza
         # otherwise, it's the physical interface stanza
diff --git a/test/generate-ifupdown2-config.pl b/test/generate-ifupdown2-config.pl
index f40689c..9ac9b3d 100755
--- a/test/generate-ifupdown2-config.pl
+++ b/test/generate-ifupdown2-config.pl
@@ -34,6 +34,10 @@ my sub set_config {
 
     Proxmox::Install::Config::merge({
         mngmt_nic => $cfg{bond} ? 'bond0' : 'eno1',
+        mngmt_vlan => {
+            enabled => $cfg{vlan},
+            vid => $cfg{vlan} ? 5 : 0,
+        },
         mngmt_bond => {
             members => $cfg{bond} ? ['12:34:56:ab:cd:ef', 'ab:cd:ef:12:34:56'] : [],
             mode => 'active-backup',
@@ -54,7 +58,7 @@ $expected
 
 source /etc/network/interfaces.d/*
 EOT
-        "$msg (bridge:$current_cfg->{bridge} bond:$current_cfg->{bond})",
+        "$msg (bridge:$current_cfg->{bridge} bond:$current_cfg->{bond} vlan:$current_cfg->{vlan})",
     );
 }
 
@@ -84,7 +88,7 @@ $proxmox_install_runenv->redefine(
         };
     },
 );
-set_config(bridge => 0, bond => 0);
+set_config(bridge => 0, bond => 0, vlan => 0);
 Proxmox::Install::Config::merge({
     cidr => '192.168.0.10/24',
     gateway => '192.168.0.1',
@@ -93,18 +97,22 @@ Proxmox::Install::Config::merge({
 ###
 # There's essentially a matrix to test w.r.t the overall networking setup:
 #
-# +-------+--------+------+
-# | test# | bridge | bond |
-# +=======+========+======+
-# |   1   |    0   |  0   |
-# |   2   |    0   |  1   |
-# |   3   |    1   |  0   |
-# |   4   |    1   |  1   |
-# +-------+--------+------+
+# +-------+--------+------+------+
+# | test# | bridge | bond | vlan |
+# +=======+========+======+======+
+# |   1   |    0   |  0   |  0   |
+# |   2   |    0   |  0   |  1   |
+# |   3   |    0   |  1   |  0   |
+# |   4   |    0   |  1   |  1   |
+# |   5   |    1   |  0   |  0   |
+# |   6   |    1   |  0   |  1   |
+# |   7   |    1   |  1   |  0   |
+# |   8   |    1   |  1   |  1   |
+# +-------+--------+------+------+
 ###
 
 # 1.
-set_config(bridge => 0, bond => 0);
+set_config(bridge => 0, bond => 0, vlan => 0);
 is_generated(
     <<EOT
 auto eno1
@@ -118,7 +126,24 @@ EOT
 );
 
 # 2.
-set_config(bridge => 0, bond => 1);
+set_config(bridge => 0, bond => 0, vlan => 1);
+is_generated(
+    <<EOT
+auto eno1
+iface eno1 inet manual
+
+auto eno1.5
+iface eno1.5 inet static
+	address 192.168.0.10/24
+	gateway 192.168.0.1
+
+iface eno2 inet manual
+EOT
+    , 'host network setup',
+);
+
+# 3.
+set_config(bridge => 0, bond => 1, vlan => 0);
 is_generated(
     <<EOT
 auto bond0
@@ -138,8 +163,32 @@ EOT
     , 'host network setup',
 );
 
-# 3.
-set_config(bridge => 1, bond => 0);
+# 4.
+set_config(bridge => 0, bond => 1, vlan => 1);
+is_generated(
+    <<EOT
+auto bond0
+iface bond0 inet manual
+	bond-slaves eno1 eno2
+	bond-miimon 100
+	bond-mode active-backup
+
+auto bond0.5
+iface bond0.5 inet static
+	address 192.168.0.10/24
+	gateway 192.168.0.1
+
+auto eno1
+iface eno1 inet manual
+
+auto eno2
+iface eno2 inet manual
+EOT
+    , 'host network setup',
+);
+
+# 5.
+set_config(bridge => 1, bond => 0, vlan => 0);
 is_generated(
     <<EOT
 iface eno1 inet manual
@@ -157,8 +206,32 @@ EOT
     , 'host network setup',
 );
 
-# 4.
-set_config(bridge => 1, bond => 1);
+# 6.
+set_config(bridge => 1, bond => 0, vlan => 1);
+is_generated(
+    <<EOT
+iface eno1 inet manual
+
+auto vmbr0
+iface vmbr0 inet manual
+	bridge-ports eno1
+	bridge-stp off
+	bridge-fd 0
+	bridge-vlan-aware yes
+	bridge-vids 2-4094
+
+auto vmbr0.5
+iface vmbr0.5 inet static
+	address 192.168.0.10/24
+	gateway 192.168.0.1
+
+iface eno2 inet manual
+EOT
+    , 'host network setup',
+);
+
+# 7.
+set_config(bridge => 1, bond => 1, vlan => 0);
 is_generated(
     <<EOT
 auto bond0
@@ -184,12 +257,44 @@ EOT
     , 'host network setup',
 );
 
+# 8.
+set_config(bridge => 1, bond => 1, vlan => 1);
+is_generated(
+    <<EOT
+auto bond0
+iface bond0 inet manual
+	bond-slaves eno1 eno2
+	bond-miimon 100
+	bond-mode active-backup
+
+auto vmbr0
+iface vmbr0 inet manual
+	bridge-ports bond0
+	bridge-stp off
+	bridge-fd 0
+	bridge-vlan-aware yes
+	bridge-vids 2-4094
+
+auto vmbr0.5
+iface vmbr0.5 inet static
+	address 192.168.0.10/24
+	gateway 192.168.0.1
+
+auto eno1
+iface eno1 inet manual
+
+auto eno2
+iface eno2 inet manual
+EOT
+    , 'host network setup',
+);
+
 # interface name pinning test
 Proxmox::Install::Config::merge({
     mngmt_nic => 'eno1',
     network_interface_pin_map => $pin_map,
 });
-set_config(bridge => 1, bond => 0);
+set_config(bridge => 1, bond => 0, vlan => 0);
 is_generated(
     <<EOT
 iface nic0 inet manual
@@ -239,7 +344,7 @@ $proxmox_install_runenv->redefine(
         };
     },
 );
-set_config(bridge => 1, bond => 0);
+set_config(bridge => 1, bond => 0, vlan => 0);
 
 is_generated(
     <<EOT
@@ -263,7 +368,7 @@ Proxmox::Install::Config::merge({
     cidr => '2001:db8::1/64',
     gateway => 'fe80::1',
 });
-set_config(bridge => 0, bond => 1);
+set_config(bridge => 0, bond => 1, vlan => 0);
 
 is_generated(
     <<EOT
@@ -289,7 +394,7 @@ Proxmox::Install::Config::merge({
     cidr => '192.168.0.10/24',
     gateway => '192.168.0.1',
 });
-set_config(bridge => 0, bond => 0);
+set_config(bridge => 0, bond => 0, vlan => 0);
 $proxmox_install_runenv->redefine(
     get => sub {
         return {
@@ -330,7 +435,7 @@ EOT
     , 'unpinnable names keep their kernel name',
 );
 
-set_config(bridge => 1, bond => 1);
+set_config(bridge => 1, bond => 1, vlan => 0);
 Proxmox::Install::Config::merge({
     mngmt_nic => 'bond0',
     mngmt_bond => {
@@ -421,4 +526,16 @@ is(
     'unknown bond member is an error',
 );
 
+for my $id (0, 1, 4095, 'abc', '1.5') {
+    eval { Proxmox::Install::Config::set_mngmt_vlan_opt('vid', $id) };
+    like($@, qr/^invalid VLAN tag '\Q$id\E'/, "VLAN tag '$id' is rejected");
+}
+
+Proxmox::Install::Config::set_mngmt_vlan_opt('vid', '0005');
+is(
+    Proxmox::Install::Config::get_mngmt_vlan_opt('vid'),
+    5,
+    'VLAN tag with leading zeros is normalized',
+);
+
 done_testing();
-- 
2.55.0