← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~paul-meyer/cloud-init:bug-1687712 into cloud-init:master

 

Paul Meyer has proposed merging ~paul-meyer/cloud-init:bug-1687712 into cloud-init:master.

Requested reviews:
  cloud init development team (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~paul-meyer/cloud-init/+git/cloud-init/+merge/323532

Fixes 1687712. Added some tests for mkfs that verify bug and fix
-- 
Your team cloud init development team is requested to review the proposed merge of ~paul-meyer/cloud-init:bug-1687712 into cloud-init:master.
diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py
index f49386e..6588c17 100644
--- a/cloudinit/config/cc_disk_setup.py
+++ b/cloudinit/config/cc_disk_setup.py
@@ -910,12 +910,14 @@ def mkfs(fs_cfg):
                         "must be set.", label)
 
     # Create the commands
+    shell=False
     if fs_cmd:
         fs_cmd = fs_cfg['cmd'] % {
             'label': label,
             'filesystem': fs_type,
             'device': device,
         }
+        shell=True
     else:
         # Find the mkfs command
         mkfs_cmd = util.which("mkfs.%s" % fs_type)
@@ -936,14 +938,14 @@ def mkfs(fs_cfg):
         if overwrite or device_type(device) == "disk":
             fs_cmd.append(lookup_force_flag(fs_type))
 
-    # Add the extends FS options
-    if fs_opts:
-        fs_cmd.extend(fs_opts)
+        # Add the extends FS options
+        if fs_opts:
+            fs_cmd.extend(fs_opts)
 
     LOG.debug("Creating file system %s on %s", label, device)
-    LOG.debug("     Using cmd: %s", " ".join(fs_cmd))
+    LOG.debug("     Using cmd: %s", fs_cmd if shell else " ".join(fs_cmd))
     try:
-        util.subp(fs_cmd)
+        util.subp(fs_cmd, shell=shell)
     except Exception as e:
         raise Exception("Failed to exec of '%s':\n%s" % (fs_cmd, e))
 
diff --git a/doc/examples/cloud-config-disk-setup.txt b/doc/examples/cloud-config-disk-setup.txt
index 3e46a22..a031a52 100644
--- a/doc/examples/cloud-config-disk-setup.txt
+++ b/doc/examples/cloud-config-disk-setup.txt
@@ -159,7 +159,7 @@ fs_setup:
      filesystem: 'ext4'
      device: '/dev/xvda1'
    - special:
-     cmd: mkfs -t %(FILESYSTEM)s -L %(LABEL)s %(DEVICE)s
+     cmd: mkfs -t %(filesystem)s -L %(label)s %(device)s
      filesystem: 'btrfs'
      device: '/dev/xvdh'
 
diff --git a/tests/unittests/test_handler/test_handler_disk_setup.py b/tests/unittests/test_handler/test_handler_disk_setup.py
index 7ff3922..07b158c 100644
--- a/tests/unittests/test_handler/test_handler_disk_setup.py
+++ b/tests/unittests/test_handler/test_handler_disk_setup.py
@@ -147,4 +147,44 @@ class TestUpdateFsSetupDevices(TestCase):
             'filesystem': 'xfs'
         }, fs_setup)
 
+
+class TestMkfsCommandHandling(TestCase):
+
+    def setUp(self):
+        super(TestMkfsCommandHandling, self).setUp()
+        self.patches = ExitStack()
+        mod_name = 'cloudinit.config.cc_disk_setup'
+        self.check_fs = self.patches.enter_context(
+            mock.patch('{0}.check_fs'.format(mod_name)))
+        self.find_device_node = self.patches.enter_context(
+            mock.patch('{0}.find_device_node'.format(mod_name)))
+        self.subp = self.patches.enter_context(
+                mock.patch.object(cc_disk_setup.util, 'subp'))
+
+        def _subp(cmd, *args, **kwargs):
+            if kwargs.get('shell'):
+                self.assertIs(type(cmd), str, msg='Expected shell command to be a string')
+            else:
+                self.assertIs(type(cmd), list, msg='Expected exec command to be a list')
+            return "success", None
+        self.subp.side_effect = _subp
+
+    def test_with_cmd(self):
+        cc_disk_setup.mkfs({
+            'special': '',
+            'cmd': 'mkfs -t %(filesystem)s -L %(label)s %(device)s',
+            'filesystem': 'ext4',
+            'device': '/dev/xdb1',
+            'label': 'repro',
+            'extra_opts': ['should', 'be', 'ignored']
+            })
+
+    def test_without_cmd(self):
+        cc_disk_setup.mkfs({
+            'filesystem': 'ext4',
+            'device': '/dev/xdb1',
+            'label': 'repro',
+            'extra_opts': ['are', 'added']
+            })
+
 # vi: ts=4 expandtab

Follow ups