nm: Activate new virtual interfaces

NMState supports several virtual interfaces, all having a master-slave
relationship (bond, ovs-bridge). The master interface has been activated
so far by its slaves (which automatically perform such an activation
when they are activated).
With the introduction of VLAN interfaces, it is now required for the
virtual interface to be explicitly activated.

Activating a new interface is now supported by executing an activation
command on the (remote) connection profile. Existing interfaces are
still activated using their device object.

Due to the relation between master-slave interfaces, the flow of
activation has been changed, ordering the activation based on their
type: New interfaces, master interfaces and all the rest.

Signed-off-by: Edward Haas <edwardh@redhat.com>

libnmstate/netapplier.py

@@ -273,7 +273,7 @@ def _add_interfaces(ifaces_desired_state, ifaces_current_state):
     ifaces_configs = nm.applier.prepare_new_ifaces_configuration(ifaces2add)
     nm.applier.create_new_ifaces(ifaces_configs)
 
-    nm.applier.set_ifaces_admin_state(ifaces2add)
+    nm.applier.set_ifaces_admin_state(ifaces2add, con_profiles=ifaces_configs)
 
 
 def _edit_interfaces(ifaces_desired_state, ifaces_current_state):

libnmstate/nm/applier.py

@@ -85,23 +85,70 @@ def prepare_edited_ifaces_configuration(ifaces_desired_state):
     return con_profiles
 
 
-def set_ifaces_admin_state(ifaces_desired_state):
+def set_ifaces_admin_state(ifaces_desired_state, con_profiles=()):
+    """
+    Control interface admin state by activating, deactivating and deleting
+    devices connection profiles.
+
+    The `absent` state results in deactivating the device and deleting
+    the connection profile.
+    FIXME: The `down` state is currently handled in the same way.
+
+    For new virtual devices, the `up` state is handled by activating the
+    new connection profile. For existing devices, the device is activated,
+    leaving it to choose the correct profile.
+
+    In order to activate correctly the interfaces, the order is significant:
+    - New interfaces (virtual interfaces).
+    - Master interfaces.
+    - All the rest.
+    """
+    new_ifaces = _get_new_ifaces(con_profiles)
+    new_ifaces_to_activate = set()
+    master_ifaces_to_activate = set()
     devs_actions = {}
+
     for iface_desired_state in ifaces_desired_state:
-        devs = _get_affected_devices(iface_desired_state)
-        if iface_desired_state['state'] == 'up':
-            devs_actions.update({dev: (device.activate,) for dev in devs})
-        elif iface_desired_state['state'] in ('down', 'absent'):
-            devs_actions.update(
-                {dev: (device.deactivate, device.delete) for dev in devs})
+        nmdev = device.get_device_by_name(iface_desired_state['name'])
+        if not nmdev:
+            ifname = iface_desired_state['name']
+            if ifname in new_ifaces and iface_desired_state['state'] == 'up':
+                    new_ifaces_to_activate.add(ifname)
         else:
-            raise UnsupportedIfaceStateError(iface_desired_state)
+            if iface_desired_state['state'] == 'up':
+                master_iface_types = ovs.BRIDGE_TYPE, bond.BOND_TYPE
+                if iface_desired_state['type'] in master_iface_types:
+                    master_ifaces_to_activate.add(nmdev)
+                else:
+                    devs_actions[nmdev] = (device.activate,)
+            elif iface_desired_state['state'] in ('down', 'absent'):
+                nmdevs = _get_affected_devices(iface_desired_state)
+                for nmdev in nmdevs:
+                    devs_actions[nmdev] = (device.deactivate, device.delete)
+            else:
+                raise UnsupportedIfaceStateError(iface_desired_state)
+
+    for ifname in new_ifaces_to_activate:
+        device.activate(dev=None, connection_id=ifname)
+
+    for dev in master_ifaces_to_activate:
+        device.activate(dev)
 
     for dev, actions in six.viewitems(devs_actions):
         for action in actions:
             action(dev)
 
 
+def _get_new_ifaces(con_profiles):
+    ifaces_without_device = set()
+    for con_profile in con_profiles:
+        ifname = con_profile.get_interface_name()
+        nmdev = device.get_device_by_name(ifname)
+        if not nmdev:
+            ifaces_without_device.add(ifname)
+    return ifaces_without_device
+
+
 def _get_affected_devices(iface_state):
     nmdev = device.get_device_by_name(iface_state['name'])
     devs = []

libnmstate/nm/device.py

@@ -20,13 +20,22 @@ import logging
 from . import nmclient
 
 
-def activate(dev, connection=None):
+def activate(dev=None, connection_id=None):
+    """Activate the given device or remote connection profile."""
+    mainloop = nmclient.mainloop()
+    mainloop.push_action(
+        _safe_activate_async, dev, connection_id)
+
+
+def _safe_activate_async(dev, connection_id):
     client = nmclient.client()
     mainloop = nmclient.mainloop()
+    connection = None
+    if connection_id:
+        connection = client.get_connection_by_id(connection_id)
     specific_object = None
     user_data = mainloop, dev
-    mainloop.push_action(
-        client.activate_connection_async,
+    client.activate_connection_async(
         connection,
         dev,
         specific_object,

tests/integration/bond_test.py

@@ -85,8 +85,6 @@ def test_remove_bond_with_minimum_desired_state():
     assert not state[INTERFACES]
 
 
-@pytest.mark.xfail(reason='https://nmstate.atlassian.net/browse/NMSTATE-72',
-                   strict=True)
 def test_add_bond_without_slaves():
     desired_bond_state = {
             INTERFACES: [

tests/integration/nm/vlan_test.py

@@ -67,9 +67,7 @@ def _create_vlan(vlan_desired_state):
         con_profile = nm.connection.create_profile(
             (con_setting, vlan_setting, ipv4_setting, ipv6_setting))
         nm.connection.add_profile(con_profile, save_to_disk=False)
-    with mainloop():
-        remote_profile = nm.nmclient.client().get_connection_by_id(ifname)
-        nm.device.activate(dev=None, connection=remote_profile)
+        nm.device.activate(connection_id=ifname)
 
 
 def _delete_vlan(devname):

tests/lib/netapplier_test.py

@@ -84,9 +84,11 @@ def test_iface_admin_state_change(netinfo_nm_mock, netapplier_nm_mock):
     desired_config[INTERFACES][0]['state'] = 'down'
     netapplier.apply(desired_config, verify_change=False)
 
-    netapplier_nm_mock.applier.set_ifaces_admin_state.assert_has_calls(
+    applier_mock = netapplier_nm_mock.applier
+    ifaces_conf = applier_mock.prepare_new_ifaces_configuration.return_value
+    applier_mock.set_ifaces_admin_state.assert_has_calls(
         [
-            mock.call([]),
+            mock.call([], con_profiles=ifaces_conf),
             mock.call(desired_config[INTERFACES])
         ]
     )

tests/lib/nm/device_test.py

@@ -41,10 +41,11 @@ def mainloop_mock():
 
 def test_activate(client_mock, mainloop_mock):
     dev = 'foodev'
+    mainloop_mock.push_action = lambda func, dev, con_id: func(dev, con_id)
+
     nm.device.activate(dev)
 
-    mainloop_mock.push_action.assert_called_once_with(
-        client_mock.activate_connection_async,
+    client_mock.activate_connection_async.assert_called_once_with(
         None,
         dev,
         None,