Merge ~harlowja/cloud-init:newer-configobj into cloud-init:master

[Joshua Harlow]

[Scott Moser (smoser)]

if we declare this as minimum version, i think there is some imrpvoement to ConfigObj usage in
  cloudinit/config/cc_mcollective.py

we had to do this:
    try:
        mcollective_config = ConfigObj(server_cfg, file_error=True)
        existed = True
    except IOError:
        LOG.debug("Did not find file %s", server_cfg)
        mcollective_config = ConfigObj()
        existed = False

it'd be nicer if we had use caught the error with util.load_file and then done ConfigObj with the contents. but an older version of ConfigObj did not support that (i think ancient version, but i wasn't sure what versions were in what distro).

could you make that change also ?

[Joshua Harlow (harlowja)]

Sure.

[Scott Moser (smoser)]

This is failing tox for me in the mcollective tests.

[Joshua Harlow (harlowja)]

Shouldn't be a problem anymore.

diff --git a/cloudinit/config/cc_mcollective.py b/cloudinit/config/cc_mcollective.py
index ada535f..b3089f3 100644
--- a/cloudinit/config/cc_mcollective.py
+++ b/cloudinit/config/cc_mcollective.py
@@ -19,6 +19,8 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import errno
+
 import six
 from six import BytesIO
 
@@ -38,16 +40,18 @@ LOG = logging.getLogger(__name__)
 
 def configure(config, server_cfg=SERVER_CFG,
               pubcert_file=PUBCERT_FILE, pricert_file=PRICERT_FILE):
-    # Read server.cfg values from the
-    # original file in order to be able to mix the rest up
+    # Read server.cfg (if it exists) values from the
+    # original file in order to be able to mix the rest up.
     try:
-        mcollective_config = ConfigObj(server_cfg, file_error=True)
-        existed = True
-    except IOError:
-        LOG.debug("Did not find file %s", server_cfg)
-        mcollective_config = ConfigObj()
-        existed = False
-
+        old_contents = util.load_file(server_cfg, quiet=False, decode=False)
+        mcollective_config = ConfigObj(BytesIO(old_contents))
+    except IOError as e:
+        if e.errno != errno.ENOENT:
+            raise
+        else:
+            LOG.debug("Did not find file %s (starting with an empty"
+                      " config)", server_cfg)
+            mcollective_config = ConfigObj()
     for (cfg_name, cfg) in config.items():
         if cfg_name == 'public-cert':
             util.write_file(pubcert_file, cfg, mode=0o644)
@@ -74,12 +78,18 @@ def configure(config, server_cfg=SERVER_CFG,
                 # Otherwise just try to convert it to a string
                 mcollective_config[cfg_name] = str(cfg)
 
-    if existed:
-        # We got all our config as wanted we'll rename
-        # the previous server.cfg and create our new one
-        util.rename(server_cfg, "%s.old" % (server_cfg))
+    try:
+        # We got all our config as wanted we'll copy
+        # the previous server.cfg and overwrite the old with our new one
+        util.copy(server_cfg, "%s.old" % (server_cfg))
+    except IOError as e:
+        if e.errno == errno.ENOENT:
+            # Doesn't exist to copy...
+            pass
+        else:
+            raise
 
-    # Now we got the whole file, write to disk...
+    # Now we got the whole (new) file, write to disk...
     contents = BytesIO()
     mcollective_config.write(contents)
     util.write_file(server_cfg, contents.getvalue(), mode=0o644)
diff --git a/requirements.txt b/requirements.txt
index cc1dc05..0c4951f 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -22,7 +22,7 @@ oauthlib
 # that the built-in config parser is not sufficent (ie
 # when we need to preserve comments, or do not have a top-level
 # section)...
-configobj
+configobj>=5.0.2
 
 # All new style configurations are in the yaml format
 pyyaml
diff --git a/tests/unittests/test_handler/test_handler_mcollective.py b/tests/unittests/test_handler/test_handler_mcollective.py
index 8fa0147..c3a5a63 100644
--- a/tests/unittests/test_handler/test_handler_mcollective.py
+++ b/tests/unittests/test_handler/test_handler_mcollective.py
@@ -138,6 +138,7 @@ class TestHandler(t_help.TestCase):
     def test_mcollective_install(self, mock_util):
         cc = self._get_cloud('ubuntu')
         cc.distro = t_help.mock.MagicMock()
+        mock_util.load_file.return_value = b""
         mycfg = {'mcollective': {'conf': {'loglevel': 'debug'}}}
         cc_mcollective.handle('cc_mcollective', mycfg, cc, LOG, [])
         self.assertTrue(cc.distro.install_packages.called)