apply: fix is_composite_member

This function was always returning False. It was happening because is
expects a dict of dicts as input but (for a long time now I suppose) it
is actually a dict of NetDefinitions.

The functions was changes to check if the netdef has a link to a parent
NetDefinitions, in which case it will be a member.

Due to this bug, Netplan would not skip renaming interfaces that are
members of bonds and bridges.

netplan_cli/cli/commands/apply.py

@@ -29,6 +29,8 @@
 import filecmp
 import time
 
+from netplan.netdef import NetDefinition
+
 from .. import utils
 from ...configmanager import ConfigManager, ConfigurationError
 from ..sriov import apply_sriov_config
@@ -331,21 +333,19 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state
                 utils.nm_bring_interface_up(loopback_connection)
 
     @staticmethod
-    def is_composite_member(composites, phy):
+    def is_composite_member(netdef: NetDefinition) -> bool:
         """
         Is this physical interface a member of a 'composite' virtual
         interface? (bond, bridge)
         """
-        for composite in composites:
-            for _, settings in composite.items():
-                if not type(settings) is dict:
-                    continue
-                members = settings.get('interfaces', [])
-                for iface in members:
-                    if iface == phy:
-                        return True
 
-        return False
+        # Should this be extended to other types of parent interfaces (vlans, vrfs)
+        bond_link = netdef.links.get('bond')
+        bridge_link = netdef.links.get('bridge')
+
+        is_composite = bool(bond_link) or bool(bridge_link)
+
+        return is_composite
 
     @staticmethod
     def clear_virtual_links(prev_links, curr_links, devices=[]):
@@ -381,7 +381,6 @@ def process_link_changes(interfaces, config_manager: ConfigManager):  # pragma:
         """
 
         changes = {}
-        composite_interfaces = [config_manager.bridges, config_manager.bonds]
 
         # Find physical interfaces which need a rename
         # But do not rename virtual interfaces
@@ -391,7 +390,7 @@ def process_link_changes(interfaces, config_manager: ConfigManager):  # pragma:
                 continue  # Skip if no new name needs to be set
             if not netdef._has_match:
                 continue  # Skip if no match for current name is given
-            if NetplanApply.is_composite_member(composite_interfaces, netdef.id):
+            if NetplanApply.is_composite_member(netdef):
                 logging.debug('Skipping composite member {}'.format(netdef.id))
                 # do not rename members of virtual devices. MAC addresses
                 # may be the same for all interface members.

tests/cli/test_units.py

@@ -25,6 +25,9 @@
 import tempfile
 
 from unittest.mock import patch
+
+from netplan.parser import Parser
+from netplan.state import State
 from netplan_cli.cli.commands.apply import NetplanApply
 from netplan_cli.cli.commands.try_command import NetplanTry
 from netplan_cli.cli.core import Netplan
@@ -43,19 +46,52 @@ def tearDown(self):
         shutil.rmtree(self.tmproot)
 
     def test_is_composite_member(self):
-        res = NetplanApply.is_composite_member([{'br0': {'interfaces': ['eth0']}}], 'eth0')
+        with open(os.path.join(self.tmproot, 'etc/netplan/test.yaml'), 'w') as f:
+            f.write('''network:
+  ethernets:
+    eth0: {}
+    eth1: {}
+    eth2: {}
+  bonds:
+    bn0:
+      interfaces: [eth0]
+  bridges:
+    br0:
+      interfaces: [eth2]
+''')
+
+        parser = Parser()
+        state = State()
+
+        parser.load_yaml(os.path.join(self.tmproot, 'etc/netplan/test.yaml'))
+        state.import_parser_results(parser)
+        netdef = state.ethernets.get('eth0')
+        res = NetplanApply.is_composite_member(netdef)
+        self.assertTrue(res)
+
+        netdef = state.ethernets.get('eth2')
+        res = NetplanApply.is_composite_member(netdef)
         self.assertTrue(res)
 
     def test_is_composite_member_false(self):
-        res = NetplanApply.is_composite_member([
-                  {'br0': {'interfaces': ['eth42']}},
-                  {'bond0': {'interfaces': ['eth1']}}
-              ], 'eth0')
-        self.assertFalse(res)
+        with open(os.path.join(self.tmproot, 'etc/netplan/test.yaml'), 'w') as f:
+            f.write('''network:
+  ethernets:
+    eth0: {}
+    eth1: {}
+  bonds:
+    bn0:
+      interfaces: [eth0]
+''')
 
-    def test_is_composite_member_with_renderer(self):
-        res = NetplanApply.is_composite_member([{'renderer': 'networkd', 'br0': {'interfaces': ['eth0']}}], 'eth0')
-        self.assertTrue(res)
+        parser = Parser()
+        state = State()
+
+        parser.load_yaml(os.path.join(self.tmproot, 'etc/netplan/test.yaml'))
+        state.import_parser_results(parser)
+        netdef = state.ethernets.get('eth1')
+        res = NetplanApply.is_composite_member(netdef)
+        self.assertFalse(res)
 
     @patch('subprocess.check_call')
     def test_clear_virtual_links(self, mock):