Merge "charms.openstack charms don't open ports"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
a996530d41
|
|
@ -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()`
|
||||
|
|
|
|||
|
|
@ -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