cloud-init

Merge lp:~smoser/cloud-init/trunk.mcollective-cleanup into lp:~cloud-init-dev/cloud-init/trunk

  1. trunk.mcollective-cleanup
  2. Merge into trunk
Proposed by Scott Moser
Status: Merged
Merge reported by: Scott Moser
Merged at revision: not available
Proposed branch: lp:~smoser/cloud-init/trunk.mcollective-cleanup
Merge into: lp:~cloud-init-dev/cloud-init/trunk
Diff against target: 298 lines (+176/-62)
2 files modified
cloudinit/config/cc_mcollective.py (+52/-44)
tests/unittests/test_handler/test_handler_mcollective.py (+124/-18)
To merge this branch: bzr merge lp:~smoser/cloud-init/trunk.mcollective-cleanup
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Needs Fixing
Sergii Golovatiuk (community) Approve
cloud-init Commiters Pending
Review via email: mp+300394@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Sergii Golovatiuk (sgolovatiuk) wrote :

Refactoring looks good to me. It doesn't break anything. Also, Scott added more unit tests which is really good.

review: Approve
lp:~smoser/cloud-init/trunk.mcollective-cleanup updated
1258. By Scott Moser

move most of handle to apply_config, change tests.

1259. By Scott Moser

test dictionary in config

1260. By Scott Moser

whitespace

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:1260
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~smoser/cloud-init/trunk.mcollective-cleanup/+merge/300394/+edit-commit-message

https://server-team-jenkins.canonical.com/job/cloud-init-ci/27/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/lp-vote-on-merge/1/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/cloud-init-ci/27/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

this got merged at 4264783ae982e48d184e5991a67deab6f63a9054

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cloudinit/config/cc_mcollective.py'
--- cloudinit/config/cc_mcollective.py 2016-07-14 15:28:46 +0000
+++ cloudinit/config/cc_mcollective.py 2016-07-19 15:17:25 +0000
@@ -36,67 +36,75 @@
36LOG = logging.getLogger(__name__)36LOG = logging.getLogger(__name__)
3737
3838
39def configure(config):39def configure(config, server_cfg=SERVER_CFG,
40 pubcert_file=PUBCERT_FILE, pricert_file=PRICERT_FILE):
40 # Read server.cfg values from the41 # Read server.cfg values from the
41 # original file in order to be able to mix the rest up42 # original file in order to be able to mix the rest up
42 try:43 try:
43 mcollective_config = ConfigObj(SERVER_CFG, file_error=True)44 mcollective_config = ConfigObj(server_cfg, file_error=True)
45 existed = True
44 except IOError:46 except IOError:
45 LOG.warn("Did not find file %s", SERVER_CFG)47 LOG.debug("Did not find file %s", server_cfg)
46 mcollective_config = ConfigObj(config)48 mcollective_config = ConfigObj()
47 else:49 existed = False
48 for (cfg_name, cfg) in config.items():50
49 if cfg_name == 'public-cert':51 for (cfg_name, cfg) in config.items():
50 util.write_file(PUBCERT_FILE, cfg, mode=0o644)52 if cfg_name == 'public-cert':
51 mcollective_config[53 util.write_file(pubcert_file, cfg, mode=0o644)
52 'plugin.ssl_server_public'] = PUBCERT_FILE54 mcollective_config[
53 mcollective_config['securityprovider'] = 'ssl'55 'plugin.ssl_server_public'] = pubcert_file
54 elif cfg_name == 'private-cert':56 mcollective_config['securityprovider'] = 'ssl'
55 util.write_file(PRICERT_FILE, cfg, mode=0o600)57 elif cfg_name == 'private-cert':
56 mcollective_config[58 util.write_file(pricert_file, cfg, mode=0o600)
57 'plugin.ssl_server_private'] = PRICERT_FILE59 mcollective_config[
58 mcollective_config['securityprovider'] = 'ssl'60 'plugin.ssl_server_private'] = pricert_file
61 mcollective_config['securityprovider'] = 'ssl'
62 else:
63 if isinstance(cfg, six.string_types):
64 # Just set it in the 'main' section
65 mcollective_config[cfg_name] = cfg
66 elif isinstance(cfg, (dict)):
67 # Iterate through the config items, create a section if
68 # it is needed and then add/or create items as needed
69 if cfg_name not in mcollective_config.sections:
70 mcollective_config[cfg_name] = {}
71 for (o, v) in cfg.items():
72 mcollective_config[cfg_name][o] = v
59 else:73 else:
60 if isinstance(cfg, six.string_types):74 # Otherwise just try to convert it to a string
61 # Just set it in the 'main' section75 mcollective_config[cfg_name] = str(cfg)
62 mcollective_config[cfg_name] = cfg76
63 elif isinstance(cfg, (dict)):77 if existed:
64 # Iterate through the config items, create a section if
65 # it is needed and then add/or create items as needed
66 if cfg_name not in mcollective_config.sections:
67 mcollective_config[cfg_name] = {}
68 for (o, v) in cfg.items():
69 mcollective_config[cfg_name][o] = v
70 else:
71 # Otherwise just try to convert it to a string
72 mcollective_config[cfg_name] = str(cfg)
73 # We got all our config as wanted we'll rename78 # We got all our config as wanted we'll rename
74 # the previous server.cfg and create our new one79 # the previous server.cfg and create our new one
75 util.rename(SERVER_CFG, "%s.old" % (SERVER_CFG))80 util.rename(server_cfg, "%s.old" % (server_cfg))
7681
77 # Now we got the whole file, write to disk...82 # Now we got the whole file, write to disk...
78 contents = BytesIO()83 contents = BytesIO()
79 mcollective_config.write(contents)84 mcollective_config.write(contents)
80 contents = contents.getvalue()85 util.write_file(server_cfg, contents.getvalue(), mode=0o644)
81 util.write_file(SERVER_CFG, contents, mode=0o644)86
87
88def apply_config(config, install_packages):
89 # Start by installing the mcollective package ...
90 install_packages(("mcollective",))
91
92 # ... and then update the mcollective configuration
93 if 'conf' in config:
94 configure(config=config['conf'])
95
96 # restart mcollective to handle updated config
97 util.subp(['service', 'mcollective', 'restart'], capture=False)
98 return True
8299
83100
84def handle(name, cfg, cloud, log, _args):101def handle(name, cfg, cloud, log, _args):
85
86 # If there isn't a mcollective key in the configuration don't do anything102 # If there isn't a mcollective key in the configuration don't do anything
87 if 'mcollective' not in cfg:103 if 'mcollective' not in cfg:
88 log.debug(("Skipping module named %s, "104 log.debug(("Skipping module named %s, "
89 "no 'mcollective' key in configuration"), name)105 "no 'mcollective' key in configuration"), name)
90 return106 return
91107
92 mcollective_cfg = cfg['mcollective']108 return apply_config(
93109 config=cfg['mcollective'],
94 # Start by installing the mcollective package ...110 install_packages=cloud.distro.install_packages)
95 cloud.distro.install_packages(("mcollective",))
96
97 # ... and then update the mcollective configuration
98 if 'conf' in mcollective_cfg:
99 configure(config=mcollective_cfg['conf'])
100
101 # Start mcollective
102 util.subp(['service', 'mcollective', 'start'], capture=False)
103111
=== modified file 'tests/unittests/test_handler/test_handler_mcollective.py'
--- tests/unittests/test_handler/test_handler_mcollective.py 2016-07-14 15:28:46 +0000
+++ tests/unittests/test_handler/test_handler_mcollective.py 2016-07-19 15:17:25 +0000
@@ -1,10 +1,11 @@
1from cloudinit.config import cc_mcollective1from cloudinit.config import cc_mcollective
2from cloudinit import util2from cloudinit import util
33
4from .. import helpers4from ..helpers import mock, FilesystemMockingTestCase, TestCase
55
6import configobj6import configobj
7import logging7import logging
8import os
8import shutil9import shutil
9from six import BytesIO10from six import BytesIO
10import tempfile11import tempfile
@@ -12,11 +13,43 @@
12LOG = logging.getLogger(__name__)13LOG = logging.getLogger(__name__)
1314
1415
15class TestConfig(helpers.FilesystemMockingTestCase):16STOCK_CONFIG = """\
17main_collective = mcollective
18collectives = mcollective
19libdir = /usr/share/mcollective/plugins
20logfile = /var/log/mcollective.log
21loglevel = info
22daemonize = 1
23
24# Plugins
25securityprovider = psk
26plugin.psk = unset
27
28connector = activemq
29plugin.activemq.pool.size = 1
30plugin.activemq.pool.1.host = stomp1
31plugin.activemq.pool.1.port = 61613
32plugin.activemq.pool.1.user = mcollective
33plugin.activemq.pool.1.password = marionette
34
35# Facts
36factsource = yaml
37plugin.yaml = /etc/mcollective/facts.yaml
38"""
39
40
41class TestConfig(FilesystemMockingTestCase):
16 def setUp(self):42 def setUp(self):
17 super(TestConfig, self).setUp()43 super(TestConfig, self).setUp()
18 self.tmp = tempfile.mkdtemp()44 self.tmp = tempfile.mkdtemp()
19 self.addCleanup(shutil.rmtree, self.tmp)45 self.addCleanup(shutil.rmtree, self.tmp)
46 # "./": make os.path.join behave correctly with abs path as second arg
47 self.server_cfg = os.path.join(
48 self.tmp, "./" + cc_mcollective.SERVER_CFG)
49 self.pubcert_file = os.path.join(
50 self.tmp, "./" + cc_mcollective.PUBCERT_FILE)
51 self.pricert_file = os.path.join(
52 self.tmp, self.tmp, "./" + cc_mcollective.PRICERT_FILE)
2053
21 def test_basic_config(self):54 def test_basic_config(self):
22 cfg = {55 cfg = {
@@ -38,23 +71,96 @@
38 },71 },
39 },72 },
40 }73 }
74 expected = cfg['mcollective']['conf']
75
41 self.patchUtils(self.tmp)76 self.patchUtils(self.tmp)
42 cc_mcollective.configure(cfg['mcollective']['conf'])77 cc_mcollective.configure(cfg['mcollective']['conf'])
43 contents = util.load_file("/etc/mcollective/server.cfg", decode=False)78 contents = util.load_file(cc_mcollective.SERVER_CFG, decode=False)
44 contents = configobj.ConfigObj(BytesIO(contents))79 contents = configobj.ConfigObj(BytesIO(contents))
45 expected = {
46 'loglevel': 'debug',
47 'connector': 'rabbitmq',
48 'logfile': '/var/log/mcollective.log',
49 'ttl': '4294957',
50 'collectives': 'mcollective',
51 'main_collective': 'mcollective',
52 'securityprovider': 'psk',
53 'daemonize': '1',
54 'factsource': 'yaml',
55 'direct_addressing': '1',
56 'plugin.psk': 'unset',
57 'libdir': '/usr/share/mcollective/plugins',
58 'identity': '1',
59 }
60 self.assertEqual(expected, dict(contents))80 self.assertEqual(expected, dict(contents))
81
82 def test_existing_config_is_saved(self):
83 cfg = {'loglevel': 'warn'}
84 util.write_file(self.server_cfg, STOCK_CONFIG)
85 cc_mcollective.configure(config=cfg, server_cfg=self.server_cfg)
86 self.assertTrue(os.path.exists(self.server_cfg))
87 self.assertTrue(os.path.exists(self.server_cfg + ".old"))
88 self.assertEqual(util.load_file(self.server_cfg + ".old"),
89 STOCK_CONFIG)
90
91 def test_existing_updated(self):
92 cfg = {'loglevel': 'warn'}
93 util.write_file(self.server_cfg, STOCK_CONFIG)
94 cc_mcollective.configure(config=cfg, server_cfg=self.server_cfg)
95 cfgobj = configobj.ConfigObj(self.server_cfg)
96 self.assertEqual(cfg['loglevel'], cfgobj['loglevel'])
97
98 def test_dictionary_in_config(self):
99 cfg = {'loglevel': 'debug', 'subfoo': {'subkey': 'subval'}}
100 cc_mcollective.configure(config=cfg, server_cfg=self.server_cfg)
101 cfgobj = configobj.ConfigObj(self.server_cfg)
102 self.assertEqual(cfg['loglevel'], cfgobj['loglevel'])
103 contents = util.load_file(self.server_cfg)
104 self.assertIn('[subfoo]', contents)
105 self.assertEqual(cfgobj['subfoo']['subkey'], 'subval')
106
107 def test_certificats_written(self):
108 # check public-cert and private-cert keys in config get written
109 cfg = {'loglevel': 'debug',
110 'public-cert': "this is my public-certificate",
111 'private-cert': "secret private certificate"}
112
113 cc_mcollective.configure(
114 config=cfg, server_cfg=self.server_cfg,
115 pricert_file=self.pricert_file, pubcert_file=self.pubcert_file)
116
117 found = configobj.ConfigObj(self.server_cfg)
118
119 # make sure these didnt get written in
120 self.assertFalse('public-cert' in found)
121 self.assertFalse('private-cert' in found)
122
123 # these need updating to the specified paths
124 self.assertEqual(found['plugin.ssl_server_public'], self.pubcert_file)
125 self.assertEqual(found['plugin.ssl_server_private'], self.pricert_file)
126
127 # and the security provider should be ssl
128 self.assertEqual(found['securityprovider'], 'ssl')
129
130 self.assertEqual(
131 util.load_file(self.pricert_file), cfg['private-cert'])
132 self.assertEqual(
133 util.load_file(self.pubcert_file), cfg['public-cert'])
134
135
136class TestApplyConfig(TestCase):
137 def setUp(self):
138 super(TestApplyConfig, self).setUp()
139 self.installed_packages = []
140
141 def _install_packages(self, pkglist):
142 self.installed_packages.extend(pkglist)
143
144 @mock.patch("cloudinit.config.cc_mcollective.util.subp")
145 @mock.patch("cloudinit.config.cc_mcollective.configure")
146 def test_with_conf_configure_called(self, configure, subp):
147 mycfg = {'conf': {'loglevel': 'warn'}}
148 cc_mcollective.apply_config(
149 mycfg, install_packages=self._install_packages)
150 self.assertTrue(configure.called_once)
151 self.assertEqual(configure.call_args_list,
152 [mock.call(config=mycfg['conf'])])
153 self.assertEqual(subp.call_args_list[0][0][0],
154 ['service', 'mcollective', 'restart'])
155 self.assertEqual(self.installed_packages, ['mcollective'])
156
157 @mock.patch("cloudinit.config.cc_mcollective.util.subp")
158 @mock.patch("cloudinit.config.cc_mcollective.configure")
159 def test_without_conf_configure_not_called(self, configure, subp):
160 mycfg = {'foo': 'bar'}
161 cc_mcollective.apply_config(
162 mycfg, install_packages=self._install_packages)
163 self.assertFalse(configure.called)
164 self.assertEqual(subp.call_args_list[0][0][0],
165 ['service', 'mcollective', 'restart'])
166 self.assertEqual(self.installed_packages, ['mcollective'])