Raise on focal+

charmhelpers/contrib/network/ovs/__init__.py

@@ -27,6 +27,8 @@
     log, WARNING, INFO, DEBUG
 )
 from charmhelpers.core.host import (
+    CompareHostReleases,
+    lsb_release,
     service
 )
 
@@ -352,11 +354,14 @@ def add_ovsbridge_linuxbridge(name, bridge, ifdata=None, portdata=None):
     :param portdata: Additional data to attach to port. Similar to ifdata.
     :type portdata: Optional[Dict[str,Union[str,Dict[str,str]]]]
 
-    WARNING: the `ifup` command (NetworkManager) must be available on the system
-    for this to work. Before Ubuntu 18.04 this was shipped by default. On
-    Ubuntu 18.04 and newer you need to install the package `ifupdown`. This
-    might however cause issues when deploying to LXD, see lp:1877594, which is
-    why this isn't supported anymore.
+    WARNINGS:
+    * The `ifup` command (NetworkManager) must be available on the system for
+      this to work. Before bionic this was shipped by default. On bionic and
+      newer you need to install the package `ifupdown`. This might however cause
+      issues when deploying to LXD, see lp:1877594, which is why this function
+      isn't supported anymore.
+    * On focal and newer this function won't even try to run `ifup` and raise
+      directly.
     """
     try:
         import netifaces
@@ -403,7 +408,7 @@ def add_ovsbridge_linuxbridge(name, bridge, ifdata=None, portdata=None):
         level=INFO)
 
     if not network_interface_already_exists:
-        setup_eni()
+        setup_eni()  # will raise on focal+
 
         with open('/etc/network/interfaces.d/{}.cfg'.format(
                 linuxbridge_port), 'w') as config:
@@ -486,12 +491,19 @@ def get_certificate():
         return None
 
 
+@deprecate('see lp:1877594', '2021-01', log=log)
 def setup_eni():
     """Makes sure /etc/network/interfaces.d/ exists and will be parsed.
 
     When setting up interfaces, Juju removes from
     /etc/network/interfaces the line sourcing interfaces.d/
+
+    WARNING: Not supported on focal and newer anymore. Will raise.
     """
+    release = CompareHostReleases(lsb_release()['DISTRIB_CODENAME'])
+    if release >= 'focal':
+        raise RuntimeError("NetworkManager isn't supported anymore")
+
     if not os.path.exists('/etc/network/interfaces.d'):
         os.makedirs('/etc/network/interfaces.d', mode=0o755)
     with open('/etc/network/interfaces', 'r') as eni:

tests/contrib/network/test_ovs.py

@@ -165,12 +165,14 @@ def test_del_bridge(self, check_call):
 
     @patch.object(ovs, 'port_to_br')
     @patch.object(ovs, 'add_bridge_port')
+    @patch.object(ovs, 'lsb_release')
     @patch('os.path.exists')
     @patch('subprocess.check_call')
-    def test_add_ovsbridge_linuxbridge(self, check_call, exists,
+    def test_add_ovsbridge_linuxbridge(self, check_call, exists, lsb_release,
                                        add_bridge_port,
                                        port_to_br):
         exists.return_value = True
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'bionic'}
         port_to_br.return_value = None
         if_and_port_data = {
             'external-ids': {'mycharm': 'br-ex'}
@@ -187,26 +189,28 @@ def test_add_ovsbridge_linuxbridge(self, check_call, exists,
 
     @patch.object(ovs, 'port_to_br')
     @patch.object(ovs, 'add_bridge_port')
+    @patch.object(ovs, 'lsb_release')
     @patch('os.path.exists')
     @patch('subprocess.check_call')
-    def test_add_ovsbridge_linuxbridge_already_direct_wired(self,
-                                                            check_call, exists,
-                                                            add_bridge_port,
-                                                            port_to_br):
+    def test_add_ovsbridge_linuxbridge_already_direct_wired(
+            self, check_call, exists, lsb_release, add_bridge_port, port_to_br):
         exists.return_value = True
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'bionic'}
         port_to_br.return_value = 'br-ex'
         ovs.add_ovsbridge_linuxbridge('br-ex', 'br-eno1')
         check_call.assert_not_called()
         add_bridge_port.assert_not_called()
 
     @patch.object(ovs, 'port_to_br')
     @patch.object(ovs, 'add_bridge_port')
+    @patch.object(ovs, 'lsb_release')
     @patch('os.path.exists')
     @patch('subprocess.check_call')
     def test_add_ovsbridge_linuxbridge_longname(self, check_call, exists,
-                                                add_bridge_port,
+                                                lsb_release, add_bridge_port,
                                                 port_to_br):
         exists.return_value = True
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'bionic'}
         port_to_br.return_value = None
         mock_hasher = MagicMock()
         mock_hasher.hexdigest.return_value = '12345678901234578910'
@@ -328,9 +332,11 @@ def test_disable_ipfix(self, check_call):
             ['ovs-vsctl', 'clear', 'Bridge', 'br-int', 'ipfix']
         )
 
+    @patch.object(ovs, 'lsb_release')
     @patch('os.path.exists')
-    def test_setup_eni_sources_eni_folder(self, exists):
+    def test_setup_eni_sources_eni_folder(self, exists, lsb_release):
         exists.return_value = True
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'bionic'}
         with patch_open() as (_, mock_file):
             # Mocked initial /etc/network/interfaces file content:
             mock_file.__iter__.return_value = [
@@ -341,9 +347,11 @@ def test_setup_eni_sources_eni_folder(self, exists):
             mock_file.write.assert_called_once_with(
                 '\nsource /etc/network/interfaces.d/*')
 
+    @patch.object(ovs, 'lsb_release')
     @patch('os.path.exists')
-    def test_setup_eni_wont_source_eni_folder_twice(self, exists):
+    def test_setup_eni_wont_source_eni_folder_twice(self, exists, lsb_release):
         exists.return_value = True
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'bionic'}
         with patch_open() as (_, mock_file):
             # Mocked initial /etc/network/interfaces file content:
             mock_file.__iter__.return_value = [
@@ -353,3 +361,8 @@ def test_setup_eni_wont_source_eni_folder_twice(self, exists):
 
             ovs.setup_eni()
             self.assertFalse(mock_file.write.called)
+
+    @patch.object(ovs, 'lsb_release')
+    def test_setup_eni_raises_on_focal(self, lsb_release):
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'focal'}
+        self.assertRaises(RuntimeError, ovs.setup_eni)