make network setup order deterministic

src/commands/setup.rs

@@ -66,11 +66,17 @@ impl Setup {
         let config_dir = get_config_dir(config_dir, "setup")?;
         let mut drivers = Vec::with_capacity(network_options.network_info.len());
 
-        // Perform per-network setup
-        for (net_name, network) in network_options.network_info.iter() {
-            let per_network_opts = network_options.networks.get(net_name).ok_or_else(|| {
-                NetavarkError::Message(format!("network options for network {net_name} not found"))
-            })?;
+        for named_network_opts in &network_options.networks {
+            let per_network_opts = &named_network_opts.opts;
+            let network = network_options
+                .network_info
+                .get(&named_network_opts.name)
+                .ok_or_else(|| {
+                    NetavarkError::Message(format!(
+                        "network info for network {} not found",
+                        &named_network_opts.name
+                    ))
+                })?;
 
             let mut driver = get_network_driver(
                 DriverInfo {

src/commands/teardown.rs

@@ -95,12 +95,14 @@ impl Teardown {
         let (mut hostns, mut netns) =
             core_utils::open_netlink_sockets(&self.network_namespace_path)?;
 
-        for (net_name, network) in network_options.network_info.iter() {
-            let per_network_opts = match network_options.networks.get(net_name) {
+        for named_network_opts in &network_options.networks {
+            let per_network_opts = &named_network_opts.opts;
+            let network = match network_options.network_info.get(&named_network_opts.name) {
                 Some(opts) => opts,
                 None => {
                     error_list.push(NetavarkError::Message(format!(
-                        "network options for network {net_name} not found"
+                        "network info for network {} not found",
+                        &named_network_opts.name
                     )));
                     continue;
                 }

src/network/types.rs

@@ -1,11 +1,14 @@
 // Crate contains the types which are accepted by netavark.
 
 use ipnet::IpNet;
+use serde::de;
 use std::collections::HashMap;
+use std::fmt::Formatter;
+use std::marker::PhantomData;
 use std::net::IpAddr;
 
 // Network describes the Network attributes.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct Network {
     /// Set up dns for this network
     #[serde(rename = "dns_enabled")]
@@ -60,7 +63,7 @@ pub struct Network {
 }
 
 /// NetworkOptions for a given container.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct NetworkOptions {
     /// The container id, used for ipam allocation.
     #[serde(rename = "container_id")]
@@ -78,7 +81,8 @@ pub struct NetworkOptions {
     /// The networks listed in "network_info" have to match this,
     /// both use the network name as key for the map.
     #[serde(rename = "networks")]
-    pub networks: HashMap<String, PerNetworkOptions>,
+    #[serde(deserialize_with = "deserialize_network_options_map_or_vec")]
+    pub networks: Vec<NamedPerNetworkOptions>,
 
     /// The networks which are needed to run this.
     /// It has to match the networks listed in "networks",
@@ -95,8 +99,18 @@ pub struct NetworkOptions {
     pub dns_servers: Option<Vec<IpAddr>>,
 }
 
+/// NamedPerNetworkOptions are the container specific network setup options.
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
+pub struct NamedPerNetworkOptions {
+    /// The name of the network.
+    pub name: String,
+
+    #[serde(flatten)]
+    pub opts: PerNetworkOptions,
+}
+
 /// PerNetworkOptions are options which should be set on a per network basis
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct PerNetworkOptions {
     /// Aliases contains a list of names which the dns server should resolve
     /// to this container. Should only be set when DNSEnabled is true on the Network.
@@ -123,7 +137,7 @@ pub struct PerNetworkOptions {
 }
 
 /// PortMapping is one or more ports that will be mapped into the container.
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct PortMapping {
     /// ContainerPort is the port number that will be exposed from the
     /// container.
@@ -206,7 +220,7 @@ pub struct NetAddress {
 }
 
 /// Subnet for a network.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct Subnet {
     /// Gateway IP for this Network.
     #[serde(rename = "gateway")]
@@ -222,7 +236,7 @@ pub struct Subnet {
 }
 
 /// Static routes for a network.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct Route {
     /// Gateway IP for this route.
     #[serde(rename = "gateway")]
@@ -238,7 +252,7 @@ pub struct Route {
 }
 
 /// LeaseRange contains the range where IP are leased.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct LeaseRange {
     /// EndIP last IP in the subnet which should be used to assign ips.
     #[serde(rename = "end_ip")]
@@ -268,3 +282,43 @@ pub struct NetworkPluginExec {
     /// The special network options for this specific container
     pub network_options: PerNetworkOptions,
 }
+
+pub fn deserialize_network_options_map_or_vec<'a, 'de, D>(
+    deserializer: D,
+) -> Result<Vec<NamedPerNetworkOptions>, D::Error>
+where
+    D: de::Deserializer<'de>,
+{
+    struct MapOrVec(PhantomData<Vec<NamedPerNetworkOptions>>);
+    impl<'de> de::Visitor<'de> for MapOrVec {
+        type Value = Vec<NamedPerNetworkOptions>;
+
+        fn expecting(&self, formatter: &mut Formatter) -> std::fmt::Result {
+            formatter.write_str("array of network options or map")
+        }
+
+        fn visit_map<M>(self, mut map: M) -> Result<Self::Value, M::Error>
+        where
+            M: de::MapAccess<'de>,
+        {
+            let mut arr = Vec::with_capacity(map.size_hint().unwrap_or_default());
+
+            while let Some((key, value)) = map.next_entry()? {
+                arr.push(NamedPerNetworkOptions {
+                    name: key,
+                    opts: value,
+                });
+            }
+            Ok(arr)
+        }
+
+        fn visit_seq<S>(self, visitor: S) -> Result<Self::Value, S::Error>
+        where
+            S: de::SeqAccess<'de>,
+        {
+            serde::Deserialize::deserialize(de::value::SeqAccessDeserializer::new(visitor))
+        }
+    }
+
+    deserializer.deserialize_any(MapOrVec(PhantomData))
+}

src/test/config/setupopts2.test.json

@@ -1,14 +1,15 @@
 {
    "container_id": "someID",
    "container_name": "someName",
-   "networks": {
-      "podman": {
+   "networks": [
+      {
+         "name": "podman",
          "static_ips": [
             "10.88.0.2"
          ],
          "interface_name": "ethsomething0"
       }
-   },
+   ],
    "network_info": {
       "podman": {
          "name": "podman",

src/test/config/twoNetworks-array.json

@@ -0,0 +1,60 @@
+{
+    "container_id": "2d0fe608dc86d7ba65dcec724a335b618328f08daa62afd2ee7fa9d41f74e5a9",
+    "container_name": "",
+    "networks": [
+        {
+            "name": "podman1",
+            "static_ips": [
+                "10.0.0.2"
+            ],
+            "interface_name": "eth0"
+        },
+        {
+            "name": "podman2",
+            "static_ips": [
+                "10.1.0.2"
+            ],
+            "interface_name": "eth1"
+        }
+    ],
+    "network_info": {
+        "podman1": {
+            "name": "podman1",
+            "id": "4937f73ac0df011d4f2848d5f83f5c20b707e71a8d98789bbe80d8f64a815e79",
+            "driver": "bridge",
+            "network_interface": "podman1",
+            "created": "2021-11-04T19:08:39.124321192+01:00",
+            "subnets": [
+                {
+                    "subnet": "10.0.0.0/24",
+                    "gateway": "10.0.0.1"
+                }
+            ],
+            "ipv6_enabled": false,
+            "internal": false,
+            "dns_enabled": false,
+            "ipam_options": {
+                "driver": "host-local"
+            }
+        },
+        "podman2": {
+            "name": "podman2",
+            "id": "488a7d9be4fa72a5b80811bd847aac1d99d1a09060739b4e08687949c957cda8",
+            "driver": "bridge",
+            "network_interface": "podman2",
+            "created": "2021-11-04T19:08:39.124800596+01:00",
+            "subnets": [
+                {
+                    "subnet": "10.1.0.0/24",
+                    "gateway": "10.1.0.1"
+                }
+            ],
+            "ipv6_enabled": false,
+            "internal": false,
+            "dns_enabled": false,
+            "ipam_options": {
+                "driver": "host-local"
+            }
+        }
+    }
+}

src/test/test.rs

@@ -7,13 +7,43 @@ mod tests {
     use netavark::network;
     #[test]
     // Test setup options loader
-    fn test_setup_opts_load() {
-        match network::types::NetworkOptions::load(Some(OsString::from(
+    fn test_setup_opts_load_map() {
+        let a = network::types::NetworkOptions::load(Some(OsString::from(
             "src/test/config/setupopts.test.json",
-        ))) {
-            Ok(_) => {}
-            Err(e) => panic!("{}", e),
-        }
+        )))
+        .unwrap();
+        assert_eq!(a.networks.len(), 1);
+    }
+
+    #[test]
+    fn test_setup_opts_load_vec() {
+        let a = network::types::NetworkOptions::load(Some(OsString::from(
+            "src/test/config/setupopts2.test.json",
+        )))
+        .unwrap();
+        assert_eq!(a.networks.len(), 1);
+    }
+
+    #[test]
+    // Test setup options loader
+    fn test_load_two_networks() {
+        let array = network::types::NetworkOptions::load(Some(OsString::from(
+            "src/test/config/twoNetworks-array.json",
+        )))
+        .unwrap();
+        assert_eq!(array.networks.len(), 2);
+        assert_eq!(array.networks[0].name, "podman1");
+        assert_eq!(array.networks[0].opts.interface_name, "eth0");
+        assert_eq!(array.networks[1].name, "podman2");
+        assert_eq!(array.networks[1].opts.interface_name, "eth1");
+
+        let map = network::types::NetworkOptions::load(Some(OsString::from(
+            "src/test/config/twoNetworks-map.json",
+        )))
+        .unwrap();
+
+        // both the parsed array or map version should result in the same value
+        assert_eq!(array, map);
     }
 
     // Test if we can deserialize values correctly

test/250-bridge-nftables.bats

@@ -1014,6 +1014,17 @@ net/ipv4/conf/podman1/rp_filter = 2"
     assert_json "$result" 'has("t1")' == "true" "t1 object key exists"
     assert_json "$result" 'has("t2")' == "true" "t2 object key exists"
 
+    # verify the setup order of the contianer interfaces by checking the interface index
+    run_in_container_netns ip -j addr show eth0
+    result="$output"
+    assert_json "$result" '.[0].ifindex' == "2" "eth0 interface must have index 2"
+    assert_json "$result" '.[0].addr_info.[0].local' == "10.89.1.2" "first ip adddress on eth0"
+
+    run_in_container_netns ip -j addr show eth1
+    result="$output"
+    assert_json "$result" '.[0].ifindex' == "3" "eth1 interface must have index 3"
+    assert_json "$result" '.[0].addr_info.[0].local' == "10.89.2.2" "first ip adddress on eth1"
+
     run_in_container_netns ip link del eth0
     run_in_container_netns ip link del eth1
 
@@ -1034,6 +1045,23 @@ net/ipv4/conf/podman1/rp_filter = 2"
     assert "$output" !~ "jump nv_fae505bb_10_89_1_0_nm24_dnat" "network 2 fw rule should not exist"
 }
 
+@test "$fw_driver - two networks reversed" {
+    # reverse the two networks to ensure the order is indeed depended on the array
+    run_netavark --file <(jq  ".networks |= reverse" ${TESTSDIR}/testfiles/two-networks.json) setup $(get_container_netns_path)
+
+    # verify the setup order of the contianer interfaces by checking the interface index
+    # it should be reversed now compared to the test above
+    run_in_container_netns ip -j addr show eth1
+    result="$output"
+    assert_json "$result" '.[0].ifindex' == "2" "eth1 interface must have index 2"
+    assert_json "$result" '.[0].addr_info.[0].local' == "10.89.2.2" "first ip adddress on eth1"
+
+    run_in_container_netns ip -j addr show eth0
+    result="$output"
+    assert_json "$result" '.[0].ifindex' == "3" "eth0 interface must have index 3"
+    assert_json "$result" '.[0].addr_info.[0].local' == "10.89.1.2" "first ip adddress on eth0"
+}
+
 @test "$fw_driver - ipv6 disabled error message" {
     # disable ipv6 in the netns
      run_in_host_netns sysctl net.ipv6.conf.all.disable_ipv6=1
@@ -1227,7 +1255,7 @@ function check_simple_bridge_nftables() {
     assert "${lines[3]}" =~ "ip6 daddr != ff00::/8 snat ip6 to fd20:100:200::1"
 
     run_netavark --file ${TESTSDIR}/testfiles/bridge-outbound-addr6.json teardown $(get_container_netns_path)
- 
+
     # Check that the chain is removed
     expected_rc=1 run_in_host_netns nft list chain inet netavark nv_2f259bab_fd10-88-a--_nm64
 }

test/testfiles/two-networks.json

@@ -3,27 +3,29 @@
     "container_name": "heuristic_archimedes",
     "port_mappings": [
         {
-          "host_ip": "",
-          "container_port": 8080,
-          "host_port": 8080,
-          "range": 1,
-          "protocol": "tcp"
+            "host_ip": "",
+            "container_port": 8080,
+            "host_port": 8080,
+            "range": 1,
+            "protocol": "tcp"
         }
-      ],
-    "networks": {
-        "t1": {
+    ],
+    "networks": [
+        {
+            "name": "t1",
             "static_ips": [
                 "10.89.1.2"
             ],
             "interface_name": "eth0"
         },
-        "t2": {
+        {
+            "name": "t2",
             "static_ips": [
                 "10.89.2.2"
             ],
             "interface_name": "eth1"
         }
-    },
+    ],
     "network_info": {
         "t1": {
             "name": "t1",