Fix mcollective module with python3

fixes mcollective when used with python3 and also adds a unit test.

3 files changed, 104 insertions, 29 deletions

diff --git a/ChangeLog b/ChangeLog
index fa5e7df4..bae982e3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -126,6 +126,7 @@
  - support network rendering to sysconfig (for centos and RHEL)
  - write_files: if no permissions are given, just use default without warn.
  - user_data: fix error when user-data is not utf-8 decodable (LP: #1532072)
+ - fix mcollective module with python3 (LP: #1597699) [Sergii Golovatiuk] 
 
 0.7.6:
  - open 0.7.6
diff --git a/cloudinit/config/cc_mcollective.py b/cloudinit/config/cc_mcollective.py
index 425420ae..0c84d600 100644
--- a/cloudinit/config/cc_mcollective.py
+++ b/cloudinit/config/cc_mcollective.py
@@ -20,54 +20,49 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import six
-from six import StringIO
+from six import BytesIO
 
 # Used since this can maintain comments
 # and doesn't need a top level section
 from configobj import ConfigObj
 
+from cloudinit import log as logging
 from cloudinit import util
 
 PUBCERT_FILE = "/etc/mcollective/ssl/server-public.pem"
 PRICERT_FILE = "/etc/mcollective/ssl/server-private.pem"
 SERVER_CFG = '/etc/mcollective/server.cfg'
 
+LOG = logging.getLogger(__name__)
 
-def handle(name, cfg, cloud, log, _args):
-
-    # If there isn't a mcollective key in the configuration don't do anything
-    if 'mcollective' not in cfg:
-        log.debug(("Skipping module named %s, "
-                   "no 'mcollective' key in configuration"), name)
-        return
-
-    mcollective_cfg = cfg['mcollective']
-
-    # Start by installing the mcollective package ...
-    cloud.distro.install_packages(("mcollective",))
 
-    # ... and then update the mcollective configuration
-    if 'conf' in mcollective_cfg:
-        # Read server.cfg values from the
-        # original file in order to be able to mix the rest up
-        mcollective_config = ConfigObj(SERVER_CFG)
-        # See: http://tiny.cc/jh9agw
-        for (cfg_name, cfg) in mcollective_cfg['conf'].items():
+def configure(config):
+    # Read server.cfg values from the
+    # original file in order to be able to mix the rest up
+    try:
+        mcollective_config = ConfigObj(SERVER_CFG, file_error=True)
+    except IOError:
+        LOG.warn("Did not find file %s", SERVER_CFG)
+        mcollective_config = ConfigObj(config)
+    else:
+        for (cfg_name, cfg) in config.items():
             if cfg_name == 'public-cert':
                 util.write_file(PUBCERT_FILE, cfg, mode=0o644)
-                mcollective_config['plugin.ssl_server_public'] = PUBCERT_FILE
+                mcollective_config[
+                    'plugin.ssl_server_public'] = PUBCERT_FILE
                 mcollective_config['securityprovider'] = 'ssl'
             elif cfg_name == 'private-cert':
                 util.write_file(PRICERT_FILE, cfg, mode=0o600)
-                mcollective_config['plugin.ssl_server_private'] = PRICERT_FILE
+                mcollective_config[
+                    'plugin.ssl_server_private'] = PRICERT_FILE
                 mcollective_config['securityprovider'] = 'ssl'
             else:
                 if isinstance(cfg, six.string_types):
                     # Just set it in the 'main' section
                     mcollective_config[cfg_name] = cfg
                 elif isinstance(cfg, (dict)):
-                    # Iterate through the config items, create a section
-                    # if it is needed and then add/or create items as needed
+                    # Iterate through the config items, create a section if
+                    # it is needed and then add/or create items as needed
                     if cfg_name not in mcollective_config.sections:
                         mcollective_config[cfg_name] = {}
                     for (o, v) in cfg.items():
@@ -78,11 +73,30 @@ def handle(name, cfg, cloud, log, _args):
         # 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))
-        # Now we got the whole file, write to disk...
-        contents = StringIO()
-        mcollective_config.write(contents)
-        contents = contents.getvalue()
-        util.write_file(SERVER_CFG, contents, mode=0o644)
+
+    # Now we got the whole file, write to disk...
+    contents = BytesIO()
+    mcollective_config.write(contents)
+    contents = contents.getvalue()
+    util.write_file(SERVER_CFG, contents, mode=0o644)
+
+
+def handle(name, cfg, cloud, log, _args):
+
+    # If there isn't a mcollective key in the configuration don't do anything
+    if 'mcollective' not in cfg:
+        log.debug(("Skipping module named %s, "
+                   "no 'mcollective' key in configuration"), name)
+        return
+
+    mcollective_cfg = cfg['mcollective']
+
+    # Start by installing the mcollective package ...
+    cloud.distro.install_packages(("mcollective",))
+
+    # ... and then update the mcollective configuration
+    if 'conf' in mcollective_cfg:
+        configure(config=mcollective_cfg['conf'])
 
     # Start mcollective
     util.subp(['service', 'mcollective', 'start'], capture=False)
diff --git a/tests/unittests/test_handler/test_handler_mcollective.py b/tests/unittests/test_handler/test_handler_mcollective.py
new file mode 100644
index 00000000..f9448d80
--- /dev/null
+++ b/tests/unittests/test_handler/test_handler_mcollective.py
@@ -0,0 +1,60 @@
+from cloudinit.config import cc_mcollective
+from cloudinit import util
+
+from .. import helpers
+
+import configobj
+import logging
+import shutil
+from six import BytesIO
+import tempfile
+
+LOG = logging.getLogger(__name__)
+
+
+class TestConfig(helpers.FilesystemMockingTestCase):
+    def setUp(self):
+        super(TestConfig, self).setUp()
+        self.tmp = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.tmp)
+
+    def test_basic_config(self):
+        cfg = {
+            'mcollective': {
+                'conf': {
+                    'loglevel': 'debug',
+                    'connector': 'rabbitmq',
+                    'logfile': '/var/log/mcollective.log',
+                    'ttl': '4294957',
+                    'collectives': 'mcollective',
+                    'main_collective': 'mcollective',
+                    'securityprovider': 'psk',
+                    'daemonize': '1',
+                    'factsource': 'yaml',
+                    'direct_addressing': '1',
+                    'plugin.psk': 'unset',
+                    'libdir': '/usr/share/mcollective/plugins',
+                    'identity': '1',
+                },
+            },
+        }
+        self.patchUtils(self.tmp)
+        cc_mcollective.configure(cfg['mcollective']['conf'])
+        contents = util.load_file("/etc/mcollective/server.cfg", decode=False)
+        contents = configobj.ConfigObj(BytesIO(contents))
+        expected = {
+            'loglevel': 'debug',
+            'connector': 'rabbitmq',
+            'logfile': '/var/log/mcollective.log',
+            'ttl': '4294957',
+            'collectives': 'mcollective',
+            'main_collective': 'mcollective',
+            'securityprovider': 'psk',
+            'daemonize': '1',
+            'factsource': 'yaml',
+            'direct_addressing': '1',
+            'plugin.psk': 'unset',
+            'libdir': '/usr/share/mcollective/plugins',
+            'identity': '1',
+        }
+        self.assertEqual(expected, dict(contents))