From 58ea1d895fbc7bf474153274a05cc4fad8fa44f4 Mon Sep 17 00:00:00 2001
From: Gris Ge <fge@redhat.com>
Date: Wed, 11 Mar 2020 10:37:03 +0800
Subject: [PATCH] nm bond: workaround on miimon=100 option

The NM will ignore the `miimon=100`(the default option) when
wired/ethernet setting not included due to bug:
    https://bugzilla.redhat.com/show_bug.cgi?id=1806549

This cause verification error when user try to set `miimon=100` on
existing bond.

To workaround that, always include `miimon` option when querying bond
options.

Integration test case included.

Signed-off-by: Gris Ge <fge@redhat.com>

In addition:

nm applier: Sort interface creation/modification order

The bond or bridge will take its first slave's MAC address, hence
NetworkManager by default will activate the slaves in the order of its
interface name.

When nmstate create or editing bond/bridge, the `autoconnect` feature
will cause NetworkManager activating the slaves in unexpected order
which then generate different MAC address of bond and bridge.

To fix this problem, sort the interface creation and modification order.

Signed-off-by: Gris Ge <fge@redhat.com>
---
 libnmstate/nm/applier.py       | 13 +++++++++++--
 libnmstate/nm/bond.py          |  3 +++
 tests/integration/bond_test.py |  8 ++++++++
 tests/lib/nm/applier_test.py   | 24 +++++++++++++++++-------
 4 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/libnmstate/nm/applier.py b/libnmstate/nm/applier.py
index 8d31e35..1291469 100644
--- a/libnmstate/nm/applier.py
+++ b/libnmstate/nm/applier.py
@@ -20,6 +20,7 @@
 import base64
 import hashlib
 import itertools
+from operator import attrgetter
 
 from libnmstate.error import NmstateValueError
 from libnmstate.schema import Interface
@@ -57,7 +58,11 @@ IFACE_NAME_METADATA = "_iface_name"
 
 
 def create_new_ifaces(con_profiles):
-    for connection_profile in con_profiles:
+    # By default, NetworkManager will activate the slave profiles in the order
+    # of sorted interface name. To make sure user get the same MAC address
+    # after reboot for bond/bridge/etc, sort the interfaces to be created
+    # as autoconnect=True will activate the profile at the creation time.
+    for connection_profile in sorted(con_profiles, key=attrgetter("con_id")):
         connection_profile.add(save_to_disk=True)
 
 
@@ -78,7 +83,11 @@ def prepare_new_ifaces_configuration(ifaces_desired_state):
 
 
 def edit_existing_ifaces(con_profiles):
-    for connection_profile in con_profiles:
+    # By default, NetworkManager will activate the slave profiles in the order
+    # of sorted interface name. To make sure user get the same MAC address
+    # after reboot for bond/bridge/etc, sort the interfaces to be updated
+    # as autoconnect=True will activate the profile at the profile update time.
+    for connection_profile in sorted(con_profiles, key=attrgetter("con_id")):
         devname = connection_profile.devname
         nmdev = device.get_device_by_name(devname)
         cur_con_profile = None
diff --git a/libnmstate/nm/bond.py b/libnmstate/nm/bond.py
index b1291c4..310ef6e 100644
--- a/libnmstate/nm/bond.py
+++ b/libnmstate/nm/bond.py
@@ -86,6 +86,9 @@ def _get_options(nm_device):
                 if option == "arp_ip_target":
                     value = value.replace(" ", ",")
                 options[option] = value
+    # Workaround of https://bugzilla.redhat.com/show_bug.cgi?id=1806549
+    if "miimon" not in options:
+        options["miimon"] = bond_setting.get_option_default("miimon")
     return options
 
 
-- 
2.24.1