Merge lp:~smoser/curtin/trunk-lp1731709-curthooks-write-files into lp:~raharper/curtin/trunk-lp1731709-curthooks-write-files

[Scott Moser]

suggested changes

a.) put only the import into curthooks.write_files, not a definition.
    move the legacy definition to futil.
b.) drop the vmtests
c.) drop the mocks in the added unit tests, actually read the files
    that are written and verify they have expected content.
d.) raise a DeprecatedWarning if the function is called.

=== modified file 'curtin/commands/curthooks.py'
--- curtin/commands/curthooks.py	2017-11-13 22:48:29 +0000
+++ curtin/commands/curthooks.py	2017-11-17 21:11:09 +0000
@@ -36,6 +36,8 @@
 
 from . import populate_one_subcmd
 
+write_files = futil._legacy_write_files  # LP: #1731709
+
 CMD_ARGUMENTS = (
     ((('-t', '--target'),
       {'help': 'operate on target. default is env[TARGET_MOUNT_POINT]',
@@ -907,26 +909,6 @@
     return False
 
 
-def write_files(cfg, base_dir=None):
-    """Backwards compatibility for curthooks.write_files (LP: #1731709)
-    It needs to work like:
-        curthooks.write_files(cfg, target)
-    where cfg is a write_files dictionary and target is the
-    base directory under which the files will be written.
-    """
-    # this takes 'write_files' entry in config and writes files in the target
-    # config entry example:
-    # f1:
-    #  path: /file1
-    #  content: !!binary |
-    #    f0VMRgIBAQAAAAAAAAAAAAIAPgABAAAAwARAAAAAAABAAAAAAAAAAJAVAAAAAAA
-    # f2: {path: /file2, content: "foobar", permissions: '0666'}
-    if 'write_files' not in cfg:
-        return
-
-    futil.write_files(cfg.get('write_files'), base_dir=base_dir)
-
-
 def curthooks(args):
     state = util.load_command_environment()
 

=== modified file 'curtin/futil.py'
--- curtin/futil.py	2017-07-28 19:01:38 +0000
+++ curtin/futil.py	2017-11-17 21:11:09 +0000
@@ -18,6 +18,7 @@
 import grp
 import pwd
 import os
+import warnings
 
 from .util import write_file, target_path
 from .log import LOG
@@ -101,3 +102,16 @@
                     content=info.get('content', ''),
                     owner=info.get('owner', "-1:-1"),
                     perms=info.get('permissions', info.get('perms', "0644")))
+
+
+def _legacy_write_files(cfg, base_dir=None):
+    """Backwards compatibility for curthooks.write_files (LP: #1731709)
+    It needs to work like:
+        curthooks.write_files(cfg, target)
+    cfg is a 'cfg' dictionary with a 'write_files' entry in it.
+    """
+    warnings.warn(
+        "write_files use from curtin.util is deprecated. "
+        "Please use futil.write_files.", DeprecationWarning)
+
+    return write_files(cfg.get('write_files', {}), base_dir=base_dir)

=== removed file 'examples/tests/curthooks_writefiles.yaml'
--- examples/tests/curthooks_writefiles.yaml	2017-11-14 22:24:40 +0000
+++ examples/tests/curthooks_writefiles.yaml	1970-01-01 00:00:00 +0000
@@ -1,40 +0,0 @@
-# LP: #1731709
-# during install use write_files to write out a python program
-# that calls curtin.curthooks.write_files and feeds it yaml 
-# config to simulate older maas images which may have called
-# curthooks.write_files(cfg, target) pattern.
-write_files:
-    test_curthooks_input:
-      path: /tmp/writefiles.yaml
-      content: |
-        # this is my input yaml file
-        write_files:
-          testfile1:
-            path: /root/testfile1
-            content: "This is testfile1\n"
-    test_curthooks:
-      path: /tmp/test_curthooks.py
-      permissions: '0755'
-      content: |
-        #!/usr/bin/python3
-        import sys, yaml, os
-        from curtin.commands import curthooks
-        target_path = os.environ['TARGET_MOUNT_POINT']
-        print('Found TARGET_MOUNT_POINT=%s' % target_path)
-        ypath = target_path + '/tmp/writefiles.yaml'
-        cfg = yaml.load(open(ypath))
-        curthooks.write_files(cfg, target_path)
-        print('Wrote %s to %s' % (cfg, target_path))
-
-
-# call python3 in ephemeral to import curtin code use files written to target
-run_curthooks:
-  - &call_python3 |
-    echo $@
-    TP=${TARGET_MOUNT_POINT};
-    FILE=${TP}${1};
-    echo "running $FILE";
-    exec python3 $FILE
-
-late_commands:
-  01_runcurthooks: [sh, -c, *call_python3, 'curthook', '/tmp/test_curthooks.py']

=== modified file 'tests/unittests/helpers.py'
--- tests/unittests/helpers.py	2017-08-03 19:31:49 +0000
+++ tests/unittests/helpers.py	2017-11-17 21:11:09 +0000
@@ -79,3 +79,16 @@
         if _dir is None:
             _dir = self.tmp_dir()
         return os.path.normpath(os.path.abspath(os.path.join(_dir, path)))
+
+
+def dir2dict(startdir, prefix=None):
+    flist = {}
+    if prefix is None:
+        prefix = startdir
+    for root, dirs, files in os.walk(startdir):
+        for fname in files:
+            fpath = os.path.join(root, fname)
+            key = fpath[len(prefix):]
+            with open(fpath, "r") as fp:
+                flist[key] = fp.read()
+    return flist

=== modified file 'tests/unittests/test_curthooks.py'
--- tests/unittests/test_curthooks.py	2017-11-13 22:48:29 +0000
+++ tests/unittests/test_curthooks.py	2017-11-17 21:11:09 +0000
@@ -5,7 +5,7 @@
 from curtin import util
 from curtin import config
 from curtin.reporter import events
-from .helpers import CiTestCase
+from .helpers import CiTestCase, dir2dict
 
 
 class TestGetFlashKernelPkgs(CiTestCase):
@@ -762,38 +762,23 @@
 
 
 class TestCurthooksWriteFiles(CiTestCase):
-    @patch('curtin.commands.curthooks.futil.write_files')
-    def test_handle_write_files_empty(self, mock_write_files):
+    def test_handle_write_files_empty(self):
         """ Test curthooks.write_files returns for empty config """
-        ret = curthooks.write_files({}, "/my/target")
+        tmpd = self.tmp_dir()
+        ret = curthooks.write_files({}, tmpd)
+        self.assertEqual({}, dir2dict(tmpd, prefix=tmpd))
         self.assertIsNone(ret)
-        self.assertEqual(0, len(mock_write_files.call_args_list))
 
-    @patch('curtin.commands.curthooks.futil.write_files')
-    def test_handle_write_files(self, mock_write_files):
+    def test_handle_write_files(self):
         """ Test curthooks.write_files works as it used to """
-        cc_target = "tmpXXXX/random/dir/used/by/maas"
-        cfg = {
-            'file1': {
-                'path': '/etc/default/hello.txt',
-                'content': "Hello World!\n",
-            },
-            'foobar': {
-                'path': '/sys/wark',
-                'content': "Engauge!\n",
-            }
-        }
-
-        expected_cfg = {
-            'file1': {
-                'path': '/etc/default/hello.txt',
-                'content': cfg['file1']['content']},
-            'foobar': {
-                'path': '/sys/wark',
-                'content': cfg['foobar']['content']}
-        }
-        curthooks.write_files({'write_files': cfg}, cc_target)
-        mock_write_files.assert_called_with(expected_cfg, base_dir=cc_target)
+        tmpd = self.tmp_dir()
+        cfg = {'file1': {'path': '/etc/default/hello.txt',
+                         'content': "Hello World!\n"},
+               'foobar': {'path': '/sys/wark', 'content': "Engauge!\n"}}
+        ret = curthooks.write_files({'write_files': cfg}, tmpd)
+        self.assertEqual(
+            dict((cfg[i]['path'], cfg[i]['content']) for i in cfg.keys()),
+            dir2dict(tmpd, prefix=tmpd))
 
     @patch('curtin.commands.curthooks.futil.target_path')
     @patch('curtin.commands.curthooks.futil.write_finfo')

=== removed file 'tests/vmtests/test_curthooks_write_files.py'
--- tests/vmtests/test_curthooks_write_files.py	2017-11-14 22:24:40 +0000
+++ tests/vmtests/test_curthooks_write_files.py	1970-01-01 00:00:00 +0000
@@ -1,36 +0,0 @@
-from . import VMBaseClass
-from .releases import base_vm_classes as relbase
-
-import textwrap
-
-
-class TestCurthooksWriteFiles(VMBaseClass):
-    """ Test curthooks.write_files() legacy call works """
-    conf_file = "examples/tests/curthooks_writefiles.yaml"
-    extra_disks = []
-    extra_nics = []
-    collect_scripts = [textwrap.dedent("""
-        cd OUTPUT_COLLECT_D
-        cp /etc/network/interfaces interfaces
-        if [ -f /var/log/cloud-init-output.log ]; then
-           cp /var/log/cloud-init-output.log .
-        fi
-        cp /var/log/cloud-init.log .
-        find /etc/network/interfaces.d > find_interfacesd
-        """)]
-
-    def test_output_files_exist(self):
-        self.output_files_exist(["interfaces", "cloud-init.log"])
-
-    def test_curthooks_write_files(self):
-        self.output_files_exist(["root/testfile1"])
-        content = self.load_collect_file("root/testfile1")
-        self.assertEqual("This is testfile1", content.strip())
-
-
-class XenialTestCurthooksWriteFiles(relbase.xenial, TestCurthooksWriteFiles):
-    __test__ = True
-
-
-class ArtfulTestCurthooksWriteFiles(relbase.artful, TestCurthooksWriteFiles):
-    __test__ = True