curtin-dev team mailing list archive

[Lucas Albuquerque Medeiros de Moura <lucas.moura@xxxxxxxxxxxxx>]

Lucas Albuquerque Medeiros de Moura has proposed merging ~lamoura/curtin:disable-networking-config into curtin:master.

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~lamoura/curtin/+git/curtin/+merge/383785

Currently, there is no easy way for curtin to not write any network_config on state/network_config. This file is then used by the curthooks to apply network parameters. The easiest way is to simply remove the 'network' stage from the config file.

This PR proposes a {"config": "disabled"} option for networking. If that is passed, curtin will not create the state/network_config file and the curthooks will not use it apply network configurations (such as creating a cloudinit network file with the provided configuration).

Right now, I am only doing the VMTest for focal, since I just want to validate if my proposed solution works. If that is the case, I can add the remaining versions to the test.

Also, this test is only feasible for images with cloudinit already installed. But I don't know right now if there is a right way to perform this test for those images.
-- 
Your team curtin developers is requested to review the proposed merge of ~lamoura/curtin:disable-networking-config into curtin:master.

diff --git a/curtin/commands/apply_net.py b/curtin/commands/apply_net.py
index ddc5056..8e5596b 100644
--- a/curtin/commands/apply_net.py
+++ b/curtin/commands/apply_net.py
@@ -82,22 +82,23 @@ def apply_net(target, network_state=None, network_config=None):
     elif network_config:
         netcfg = config.load_config(network_config)
 
-        # curtin will pass-through the netconfig into the target
-        # for rendering at runtime unless the target OS does not
-        # support NETWORK_CONFIG_V2 feature.
-        LOG.info('Checking cloud-init in target [%s] for network '
-                 'configuration passthrough support.', target)
-        try:
-            passthrough = net.netconfig_passthrough_available(target)
-        except util.ProcessExecutionError:
-            LOG.warning('Failed to determine if passthrough is available')
-
-        if passthrough:
-            LOG.info('Passing network configuration through to target: %s',
-                     target)
-            net.render_netconfig_passthrough(target, netconfig=netcfg)
-        else:
-            ns = net.parse_net_config_data(netcfg.get('network', {}))
+        if netcfg:
+            # curtin will pass-through the netconfig into the target
+            # for rendering at runtime unless the target OS does not
+            # support NETWORK_CONFIG_V2 feature.
+            LOG.info('Checking cloud-init in target [%s] for network '
+                     'configuration passthrough support.', target)
+            try:
+                passthrough = net.netconfig_passthrough_available(target)
+            except util.ProcessExecutionError:
+                LOG.warning('Failed to determine if passthrough is available')
+
+            if passthrough:
+                LOG.info('Passing network configuration through to target: %s',
+                         target)
+                net.render_netconfig_passthrough(target, netconfig=netcfg)
+            else:
+                ns = net.parse_net_config_data(netcfg.get('network', {}))
 
     if not passthrough:
         LOG.info('Rendering network configuration in target')
diff --git a/curtin/commands/net_meta.py b/curtin/commands/net_meta.py
index fdb909e..f80f75b 100644
--- a/curtin/commands/net_meta.py
+++ b/curtin/commands/net_meta.py
@@ -110,6 +110,7 @@ def net_meta(args):
     LOG.debug("net-meta mode is '%s'.  devices=%s", args.mode, devices)
 
     output_network_config = os.environ.get("OUTPUT_NETWORK_CONFIG", "")
+    should_write_network_config = True
     if args.mode == "copy":
         if not args.target:
             raise argparse.ArgumentTypeError("mode 'copy' requires --target")
@@ -126,23 +127,28 @@ def net_meta(args):
         content = config.dump_config(interfaces_basic_dhcp(devices))
 
     elif args.mode == 'custom':
-        target = output_network_config
-        content = config.dump_config(interfaces_custom(args))
+        if cfg["network"].get("config", "") != "disabled":
+            target = output_network_config
+            content = config.dump_config(interfaces_custom(args))
+        else:
+            should_write_network_config = False
 
     else:
         raise Exception("Unexpected network config mode '%s'." % args.mode)
 
-    if not target:
-        raise Exception(
-            "No target given for mode = '%s'.  No where to write content: %s" %
-            (args.mode, content))
+    if should_write_network_config:
+        if not target:
+            raise Exception(
+                "No target given for mode = %s. Nowhere to write content: %s" %
+                (args.mode, content))
 
-    LOG.debug("writing to file %s with network config: %s", target, content)
-    if target == "-":
-        sys.stdout.write(content)
-    else:
-        with open(target, "w") as fp:
-            fp.write(content)
+        LOG.debug(
+            "writing to file %s with network config: %s", target, content)
+        if target == "-":
+            sys.stdout.write(content)
+        else:
+            with open(target, "w") as fp:
+                fp.write(content)
 
     sys.exit(0)
 
diff --git a/curtin/net/deps.py b/curtin/net/deps.py
index fd9e3c0..96d3cb2 100644
--- a/curtin/net/deps.py
+++ b/curtin/net/deps.py
@@ -21,8 +21,10 @@ def network_config_required_packages(network_config, mapping=None):
     if 'network' in network_config:
         network_config = network_config.get('network')
 
+    if network_config.get('config', '') == 'disabled':
+        dev_configs = set()
     # v1 has 'config' key and uses type: devtype elements
-    if 'config' in network_config:
+    elif 'config' in network_config:
         dev_configs = set(device['type']
                           for device in network_config['config'])
     else:
diff --git a/curtin/net/network_state.py b/curtin/net/network_state.py
index ab0f277..70c7fa6 100644
--- a/curtin/net/network_state.py
+++ b/curtin/net/network_state.py
@@ -73,8 +73,10 @@ class NetworkState:
     def parse_config(self):
         # rebuild network state
         for command in self.config:
-            handler = self.command_handlers.get(command['type'])
-            handler(command)
+            # If network is disabled, there is no command to be executed
+            if type(command) is dict:
+                handler = self.command_handlers.get(command['type'])
+                handler(command)
 
     def valid_command(self, command, required_keys):
         if not required_keys:
diff --git a/examples/tests/network_disabled.yaml b/examples/tests/network_disabled.yaml
new file mode 100644
index 0000000..1d6590b
--- /dev/null
+++ b/examples/tests/network_disabled.yaml
@@ -0,0 +1,5 @@
+# example with network config disabled
+# showtrace: true
+network:
+    version: 1
+    config: disabled
diff --git a/tests/unittests/test_commands_net_meta.py b/tests/unittests/test_commands_net_meta.py
new file mode 100644
index 0000000..b4c9936
--- /dev/null
+++ b/tests/unittests/test_commands_net_meta.py
@@ -0,0 +1,116 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+import os
+import mock
+
+from .helpers import CiTestCase
+
+from curtin.commands.net_meta import net_meta
+
+
+class NetMetaTarget:
+    def __init__(self, target, mode=None, devices=None):
+        self.target = target
+        self.mode = mode
+        self.devices = devices
+
+
+class TestNetMeta(CiTestCase):
+
+    def setUp(self):
+        super(TestNetMeta, self).setUp()
+
+        patches = [
+            ('curtin.util.run_hook_if_exists', 'm_run_hook'),
+            ('curtin.util.load_command_environment', 'm_command_env'),
+            ('curtin.config.load_command_config', 'm_command_config'),
+            ('os.environ', 'm_os_environ'),
+            ('curtin.config.dump_config', 'm_dump_config')
+        ]
+
+        for (tgt, attr) in patches:
+            self.add_patch(tgt, attr)
+
+        self.args = NetMetaTarget(
+            target='net-meta-target'
+        )
+
+    def test_net_meta_with_disabled_network(self):
+        output_netwotk_path = '/tmp/my-network-config'
+        expected_exit_code = 0
+        self.m_run_hook.return_value = False
+        self.m_command_env.return_value = {}
+        self.m_command_config.return_value = {
+            'network': {
+                'version': 1,
+                'config': 'disabled'
+            }
+        }
+        self.m_os_environ.get.return_value = output_netwotk_path
+
+        with self.assertRaises(SystemExit) as cm:
+            net_meta(self.args)
+
+        self.assertEqual(expected_exit_code, cm.exception.code)
+        self.m_run_hook.assert_called_with(
+            target=self.args.target,
+            hook='network-config'
+        )
+        self.m_command_env.assert_called_once()
+        self.m_command_config.assert_called_with(
+            args=self.args,
+            state={}
+        )
+
+        self.assertEquals(self.args.mode, 'custom')
+        self.m_os_environ.get.assert_called_with(
+            'OUTPUT_NETWORK_CONFIG', '')
+        self.m_dump_config.assert_not_called()
+        self.assertFalse(os.path.isfile(output_netwotk_path))
+
+    @mock.patch('curtin.commands.net_meta.open')
+    def test_net_meta_with_config_network(self, m_open):
+        output_netwotk_path = '/tmp/my-network-config'
+        network_config = {
+            'network': {
+                'version': 1,
+                'config': {
+                    'type': 'physical',
+                    'name': 'interface0',
+                    'mac_address': '52:54:00:12:34:00',
+                    'subnets': {
+                        'type': 'dhcp4'
+                    }
+                }
+            }
+        }
+        dump_content = 'yaml-format-network-config'
+        expected_m_command_env_calls = 2
+        expected_m_command_config_calls = 2
+        expected_exit_code = 0
+        m_file = mock.MagicMock()
+
+        self.m_run_hook.return_value = False
+        self.m_command_env.return_value = {}
+        self.m_command_config.return_value = network_config
+        self.m_os_environ.get.return_value = output_netwotk_path
+        self.m_dump_config.return_value = dump_content
+        m_open.return_value = m_file
+
+        with self.assertRaises(SystemExit) as cm:
+            net_meta(self.args)
+
+        self.assertEqual(expected_exit_code, cm.exception.code)
+        self.m_run_hook.assert_called_with(
+            target=self.args.target,
+            hook='network-config'
+        )
+        self.assertEquals(self.args.mode, 'custom')
+        self.assertEqual(
+            expected_m_command_env_calls, self.m_command_env.call_count)
+        self.assertEqual(
+            expected_m_command_config_calls, self.m_command_env.call_count)
+        self.m_dump_config.assert_called_with(network_config)
+        m_open.assert_called_with(output_netwotk_path, 'w')
+        m_file.__enter__.return_value.write.assert_called_with(
+            dump_content)
diff --git a/tests/vmtests/test_net_config_disabled.py b/tests/vmtests/test_net_config_disabled.py
new file mode 100644
index 0000000..0d30377
--- /dev/null
+++ b/tests/vmtests/test_net_config_disabled.py
@@ -0,0 +1,38 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+from . import VMBaseClass
+from .releases import base_vm_classes as relbase
+
+import os
+import textwrap
+
+
+class TestNetworkDisabledBaseTestsAbs(VMBaseClass):
+    """ Test disabled network config """
+
+    interactive = False
+    test_type = 'network'
+
+    extra_disks = []
+    extra_nics = []
+
+    conf_file = "examples/tests/network_disabled.yaml"
+
+    extra_collect_scripts = [textwrap.dedent("""
+        cd OUTPUT_COLLECT_D
+        cp -av /etc/cloud ./etc_cloud
+
+        exit 0
+        """)]
+
+    def test_cloudinit_network_not_created(self):
+        cc_passthrough = "cloud.cfg.d/50-curtin-networking.cfg"
+
+        pt_file = os.path.join(self.td.collect, 'etc_cloud',
+                               cc_passthrough)
+        self.assertFalse(os.path.exists(pt_file))
+
+
+class FocalTestNetworkDisabled(relbase.focal,
+                               TestNetworkDisabledBaseTestsAbs):
+    __test__ = True