charms.openstack charms don't open ports
This patchset adds a new method to the OpenStackCharm() class called 'update_api_ports()' which, by default, uses the api_ports static member of the same class to expose the ports using the Juju 'open_port' and 'close-port' commands. This method will open any ports that aren't already open and close any ports that shouldn't be open according to the Juju 'opened-ports' command. The 'api_ports' static member can be overriden by providing a simple list (or other iterable) to the method call. By default, the default 'install' method on the OpenStackCharm() class now calls update_api_ports() after it has installed the packages. This is normally what a charm would want to do: install the software, which usually results in it starting, and then expose its ports. In order to disable the default behaviour, it is necessary to completely override the default install() method. This is intentional, as the charm should normally expose its api_ports. Also, the update_api_ports() is called from the (new) update_charm() method which can be accessed using a defaults 'update-charm'. This is so that ports can be changed when a charm is upgraded. Change-Id: I4c61179cc6217f4407340a9205619c04b35661a9 Closes-Bug: 1646089
This commit is contained in:
parent
9360d9d942
commit
85424086df
|
@ -264,7 +264,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()`
|
||||
|
|
|
@ -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()))))
|
||||
|
|
|
@ -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',
|
||||
|
|
Loading…
Reference in New Issue