diff --git a/README.md b/README.md index 51ce843..bf76989 100644 --- a/README.md +++ b/README.md @@ -271,7 +271,8 @@ statuses are supported: condition. * paused - (Not yet availble) - the unit has been put into the paused state. -The default is for charms to support workload status, and the default installation method sets the status to maintenance with an install message. +The default is for charms to support workload status, and the default +installation method sets the status to maintenance with an install message. If the charm is not going to support workload status, _and this is not recommended_, then the charm author will need to override the `install()` diff --git a/charms_openstack/charm.py b/charms_openstack/charm.py index f38db5c..295cd9d 100644 --- a/charms_openstack/charm.py +++ b/charms_openstack/charm.py @@ -76,6 +76,7 @@ KNOWN_RELEASES = [ 'liberty', 'mitaka', 'newton', + 'ocata', ] VIP_KEY = "vip" @@ -97,6 +98,7 @@ ALLOWED_DEFAULT_HANDLERS = [ 'config.changed', 'charm.default-select-release', 'update-status', + 'upgrade-charm', ] # Where to store the default handler functions for each default state @@ -265,6 +267,17 @@ def make_default_config_changed_handler(): instance.assess_status() +@_map_default_handler('upgrade-charm') +def make_default_upgrade_charm_handler(): + + @reactive.hook('update-charm') + def default_upgrade_charm(): + """Default handler for the 'upgrade-charm' hook. + This calls the charm.singleton.upgrade_charm() function as a default. + """ + OpenStackCharm.singleton.upgrade_charm() + + def default_render_configs(*interfaces): """Default renderer for configurations. Really just a proxy for OpenstackCharm.singleton.render_configs(..) with a call to update the @@ -632,6 +645,7 @@ class OpenStackCharm(object): fetch.apt_install(packages, fatal=True) # AJK: we set this as charms can use it to detect installed state self.set_state('{}-installed'.format(self.name)) + self.update_api_ports() hookenv.status_set('maintenance', 'Installation complete - awaiting next status') @@ -648,7 +662,7 @@ class OpenStackCharm(object): return reactive.bus.get_state(state) def get_adapter(self, state, adapters_instance=None): - """Get the adaptered interface for a state or None if the state doesn't + """Get the adapted interface for a state or None if the state doesn't yet exist. Uses the self.adapters_instance to get the adapter if the passed @@ -678,6 +692,53 @@ class OpenStackCharm(object): """ return self.api_ports[service][endpoint_type] + def update_api_ports(self, ports=None): + """Update the ports list supplied (or the default ports defined in the + classes' api_ports member) using the juju helper. + + It takes the opened-ports from Juju, checks them against the ports + provided. If a port is already open, then it doesn't try to open it, + if it is closed, but should be open, then it opens it, and vice-versa. + + :param ports: List of api port numbers or None. + """ + ports = list(map(int, ( + ports or self._default_port_list(self.api_ports or {})))) + current_ports = list(map(int, self.opened_ports())) + ports_to_open = set(ports).difference(current_ports) + ports_to_close = set(current_ports).difference(ports) + for p in ports_to_open: + hookenv.open_port(p) + for p in ports_to_close: + hookenv.close_port(p) + + @staticmethod + def opened_ports(protocol="tcp"): + """Return a list of ports according to the protocol provided + Open a service network port + + If protocol is intentionally set to None, then the list will be the + list returnted by the Juju opened-ports command. + + :param (OPTIONAL) protocol: the protocol to check, TCP/UDP or None + :returns: List of ports open, according to the protocol + """ + _args = ['opened-ports'] + if protocol: + protocol = protocol.lower() + else: + protocol = '' + lines = [l for l in subprocess.check_output(_args).split('\n') if l] + ports = [] + for line in lines: + p, p_type = line.split('/') + if protocol: + if protocol == p_type.lower(): + ports.append(p) + else: + ports.append(line) + return ports + def configure_source(self): """Configure installation source using the config item 'openstack-origin' @@ -1001,6 +1062,15 @@ class OpenStackCharm(object): services=self.services, ports=self.ports_to_check(self.api_ports)) + def upgrade_charm(self): + """Called (at least) by the default handler (if that is used). This + version just checks that the ports that are open should be open and + that the ports that are closed should be closed. If the charm upgrade + alters the ports then update_api_ports() function will adjust the ports + as needed. + """ + self.update_api_ports() + def ports_to_check(self, ports): """Return a flattened, sorted, unique list of ports from self.api_ports @@ -1010,7 +1080,15 @@ class OpenStackCharm(object): :param ports: {key: {subkey: value}} :returns: [value1, value2, ...] """ - # NB self.api_ports = {key: {space: value}} + return self._default_port_list(ports) + + def _default_port_list(self, ports): + """Return a flattened, sorted, unique list of ports from self.api_ports + + :param ports: {key: {subkey: value}} + :return: [value1, value2, ...] + """ + # NB api_ports = {key: {space: value}} # The chain .. map flattens all the values into a single list return sorted(set(itertools.chain(*map(lambda x: x.values(), ports.values())))) diff --git a/unit_tests/test_charms_openstack_charm.py b/unit_tests/test_charms_openstack_charm.py index 3354191..158196c 100644 --- a/unit_tests/test_charms_openstack_charm.py +++ b/unit_tests/test_charms_openstack_charm.py @@ -495,6 +495,7 @@ class TestOpenStackCharm(BaseOpenStackCharmTest): 'filter_installed_packages', name='fip', return_value=None) + self.patch_object(chm.subprocess, 'check_output', return_value='\n') self.target.install() self.target.set_state.assert_called_once_with('charmname-installed') self.fip.assert_called_once_with([]) @@ -641,6 +642,7 @@ class TestOpenStackAPICharm(BaseOpenStackCharmTest): 'filter_installed_packages', name='fip', return_value=None) + self.patch_object(chm.subprocess, 'check_output', return_value='\n') self.target.install() # self.target.set_state.assert_called_once_with('charmname-installed') self.target.configure_source.assert_called_once_with() @@ -1091,6 +1093,7 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): self.fip.side_effect = lambda x: ['p1', 'p2'] self.patch_object(chm.hookenv, 'status_set') self.patch_object(chm.hookenv, 'apt_install') + self.patch_object(chm.subprocess, 'check_output', return_value='\n') self.target.install() # TODO: remove next commented line as we don't set this state anymore # self.target.set_state.assert_called_once_with('my-charm-installed') @@ -1109,6 +1112,51 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): with self.assertRaises(KeyError): self.target.api_port('service2', chm.os_ip.INTERNAL) + def test_update_api_ports(self): + self.patch_object(chm.hookenv, 'open_port') + self.patch_object(chm.hookenv, 'close_port') + self.patch_object(chm.subprocess, 'check_output', return_value='\n') + self.target.api_ports = { + 'api': { + 'public': 1, + 'internal': 2, + 'admin': 3, + }, + } + test_ports = [4, 5, 6] + self.target.update_api_ports(test_ports) + calls = [mock.call(4), mock.call(5), mock.call(6)] + self.open_port.assert_has_calls(calls) + self.open_port.reset_mock() + self.target.update_api_ports() + calls = [mock.call(1), mock.call(2), mock.call(3)] + self.open_port.assert_has_calls(calls) + self.close_port.assert_not_called() + # now check that it doesn't open ports already open and closes ports + # that should be closed + self.open_port.reset_mock() + self.close_port.reset_mock() + self.check_output.return_value = "1/tcp\n2/tcp\n3/udp\n4/tcp\n" + # port 3 should be opened, port 4 should be closed. + open_calls = [mock.call(3)] + close_calls = [mock.call(4)] + self.target.update_api_ports() + self.open_port.asset_has_calls(open_calls) + self.close_port.assert_has_calls(close_calls) + + def test_opened_ports(self): + self.patch_object(chm.subprocess, 'check_output') + self.check_output.return_value = '\n' + self.assertEqual([], self.target.opened_ports()) + self.check_output.return_value = '1/tcp\n2/tcp\n3/udp\n4/tcp\n5/udp\n' + self.assertEqual(['1', '2', '4'], self.target.opened_ports()) + self.assertEqual(['1', '2', '4'], + self.target.opened_ports(protocol='TCP')) + self.assertEqual(['3', '5'], self.target.opened_ports(protocol='udp')) + self.assertEqual(['1/tcp', '2/tcp', '3/udp', '4/tcp', '5/udp'], + self.target.opened_ports(protocol=None)) + self.assertEqual([], self.target.opened_ports(protocol='other')) + def test_public_url(self): self.patch_object(chm.os_ip, 'canonical_url',