apply: don't skip members of bonds on renaming

Skipping members of bonds when searching for interfaces that need to
be renamed was done as a workaround for LP: #1802322.

The actual problem was that members of bonds will have their MAC
addresses changed to a common address when they are added as members.
The renaming process looked for interfaces based on their transient MAC
addresses not the permanent one. Because of that, it would confuse one
interface by another and try to rename it.

The process was changed to look for the permanent MAC address so this
problem shouldn't happen. It also searches only for physical interfaces,
which should have a permanent MAC address.

Skipping the renaming of members of bonds will cause the backend to
"forget" about it. Once the configuration is generated with a different
name, the backend will look for the new name. As the interface will
continue to have the previous name, it will not be managed anymore.

Lets fix this by allowing the renaming these interfaces. If the system
administrator changed the "set-name" setting, they probably want to get
the interface renamed. This might cause a quick loss of connectivity on
the interface being renamed.

Drop is_composite_member(). This method is not needed anymore.

netplan_cli/cli/commands/apply.py

@@ -330,23 +330,6 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state
             if 'lo' in nm_interfaces and loopback_connection:
                 utils.nm_bring_interface_up(loopback_connection)
 
-    @staticmethod
-    def is_composite_member(composites, phy):
-        """
-        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
-
     @staticmethod
     def clear_virtual_links(prev_links, curr_links, devices=[]):
         """
@@ -381,7 +364,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,11 +373,6 @@ 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):
-                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.
-                continue
             # Find current name of the interface, according to match conditions and globs (name, mac, driver)
             current_iface_name = utils.find_matching_iface(interfaces, netdef)
             if not current_iface_name:

tests/cli/test_units.py

@@ -42,21 +42,6 @@ def setUp(self):
     def tearDown(self):
         shutil.rmtree(self.tmproot)
 
-    def test_is_composite_member(self):
-        res = NetplanApply.is_composite_member([{'br0': {'interfaces': ['eth0']}}], 'eth0')
-        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)
-
-    def test_is_composite_member_with_renderer(self):
-        res = NetplanApply.is_composite_member([{'renderer': 'networkd', 'br0': {'interfaces': ['eth0']}}], 'eth0')
-        self.assertTrue(res)
-
     @patch('subprocess.check_call')
     def test_clear_virtual_links(self, mock):
         # simulate as if 'tun3' would have already been delete another way,