diff --git a/hooks/charmhelpers/__init__.py b/hooks/charmhelpers/__init__.py index 61ef9071..1f57ed2a 100644 --- a/hooks/charmhelpers/__init__.py +++ b/hooks/charmhelpers/__init__.py @@ -49,7 +49,8 @@ __deprecated_functions = {} def deprecate(warning, date=None, log=None): """Add a deprecation warning the first time the function is used. - The date, which is a string in semi-ISO8660 format indicate the year-month + + The date which is a string in semi-ISO8660 format indicates the year-month that the function is officially going to be removed. usage: @@ -62,10 +63,11 @@ def deprecate(warning, date=None, log=None): The reason for passing the logging function (log) is so that hookenv.log can be used for a charm if needed. - :param warning: String to indicat where it has moved ot. - :param date: optional sting, in YYYY-MM format to indicate when the + :param warning: String to indicate what is to be used instead. + :param date: Optional string in YYYY-MM format to indicate when the function will definitely (probably) be removed. - :param log: The log function to call to log. If not, logs to stdout + :param log: The log function to call in order to log. If None, logs to + stdout """ def wrap(f): diff --git a/hooks/charmhelpers/contrib/network/ovs/__init__.py b/hooks/charmhelpers/contrib/network/ovs/__init__.py index fd001bc2..acb503be 100644 --- a/hooks/charmhelpers/contrib/network/ovs/__init__.py +++ b/hooks/charmhelpers/contrib/network/ovs/__init__.py @@ -14,10 +14,12 @@ ''' Helpers for interacting with OpenvSwitch ''' import hashlib -import subprocess import os +import re import six +import subprocess +from charmhelpers import deprecate from charmhelpers.fetch import apt_install @@ -25,6 +27,8 @@ from charmhelpers.core.hookenv import ( log, WARNING, INFO, DEBUG ) from charmhelpers.core.host import ( + CompareHostReleases, + lsb_release, service ) @@ -246,20 +250,23 @@ def add_bridge_port(name, port, promisc=False, ifdata=None, exclusive=False, subprocess.check_call(["ip", "link", "set", port, "promisc", "off"]) -def del_bridge_port(name, port): +def del_bridge_port(name, port, linkdown=True): """Delete a port from the named openvswitch bridge :param name: Name of bridge to remove port from :type name: str :param port: Name of port to remove :type port: str + :param linkdown: Whether to set link down on interface (default: True) + :type linkdown: bool :raises: subprocess.CalledProcessError """ log('Deleting port {} from bridge {}'.format(port, name)) subprocess.check_call(["ovs-vsctl", "--", "--if-exists", "del-port", name, port]) - subprocess.check_call(["ip", "link", "set", port, "down"]) - subprocess.check_call(["ip", "link", "set", port, "promisc", "off"]) + if linkdown: + subprocess.check_call(["ip", "link", "set", port, "down"]) + subprocess.check_call(["ip", "link", "set", port, "promisc", "off"]) def add_bridge_bond(bridge, port, interfaces, portdata=None, ifdatamap=None, @@ -315,7 +322,8 @@ def add_bridge_bond(bridge, port, interfaces, portdata=None, ifdatamap=None, subprocess.check_call(cmd) -def add_ovsbridge_linuxbridge(name, bridge, ifdata=None): +@deprecate('see lp:1877594', '2021-01', log=log) +def add_ovsbridge_linuxbridge(name, bridge, ifdata=None, portdata=None): """Add linux bridge to the named openvswitch bridge :param name: Name of ovs bridge to be added to Linux bridge @@ -343,6 +351,17 @@ def add_ovsbridge_linuxbridge(name, bridge, ifdata=None): 20dac08fdcce4b7fda1d07add3b346aa9751cfbc/ lib/db-ctl-base.c#L189-L215 :type ifdata: Optional[Dict[str,Union[str,Dict[str,str]]]] + :param portdata: Additional data to attach to port. Similar to ifdata. + :type portdata: Optional[Dict[str,Union[str,Dict[str,str]]]] + + WARNINGS: + * The `ifup` command (NetworkManager) must be available on the system for + this to work. Before bionic this was shipped by default. On bionic and + newer you need to install the package `ifupdown`. This might however cause + issues when deploying to LXD, see lp:1877594, which is why this function + isn't supported anymore. + * On focal and newer this function won't even try to run `ifup` and raise + directly. """ try: import netifaces @@ -377,25 +396,42 @@ def add_ovsbridge_linuxbridge(name, bridge, ifdata=None): ovsbridge_port = "cvo{}".format(base) linuxbridge_port = "cvb{}".format(base) + network_interface_already_exists = False interfaces = netifaces.interfaces() for interface in interfaces: if interface == ovsbridge_port or interface == linuxbridge_port: log('Interface {} already exists'.format(interface), level=INFO) - return + network_interface_already_exists = True + break log('Adding linuxbridge {} to ovsbridge {}'.format(bridge, name), level=INFO) - check_for_eni_source() + if not network_interface_already_exists: + setup_eni() # will raise on focal+ - with open('/etc/network/interfaces.d/{}.cfg'.format( - linuxbridge_port), 'w') as config: - config.write(BRIDGE_TEMPLATE.format(linuxbridge_port=linuxbridge_port, - ovsbridge_port=ovsbridge_port, - bridge=bridge)) + with open('/etc/network/interfaces.d/{}.cfg'.format( + linuxbridge_port), 'w') as config: + config.write(BRIDGE_TEMPLATE.format( + linuxbridge_port=linuxbridge_port, + ovsbridge_port=ovsbridge_port, bridge=bridge)) - subprocess.check_call(["ifup", linuxbridge_port]) - add_bridge_port(name, linuxbridge_port, ifdata=ifdata) + try: + # NOTE(lourot): 'ifup ' can't be replaced by + # 'ip link set up' as the latter won't parse + # /etc/network/interfaces* + subprocess.check_call(['ifup', linuxbridge_port]) + except FileNotFoundError: + # NOTE(lourot): on bionic and newer, 'ifup' isn't installed by + # default. It has been replaced by netplan.io but we can't use it + # yet because of lp:1876730. For the time being, charms using this + # have to install 'ifupdown' on bionic and newer. This will however + # cause issues when deploying to LXD, see lp:1877594. + raise RuntimeError('ifup: command not found. Did this charm forget ' + 'to install ifupdown?') + + add_bridge_port(name, linuxbridge_port, ifdata=ifdata, exclusive=False, + portdata=portdata) def is_linuxbridge_interface(port): @@ -455,18 +491,35 @@ def get_certificate(): return None -def check_for_eni_source(): - ''' Juju removes the source line when setting up interfaces, - replace if missing ''' +@deprecate('see lp:1877594', '2021-01', log=log) +def setup_eni(): + """Makes sure /etc/network/interfaces.d/ exists and will be parsed. + When setting up interfaces, Juju removes from + /etc/network/interfaces the line sourcing interfaces.d/ + + WARNING: Not supported on focal and newer anymore. Will raise. + """ + release = CompareHostReleases(lsb_release()['DISTRIB_CODENAME']) + if release >= 'focal': + raise RuntimeError("NetworkManager isn't supported anymore") + + if not os.path.exists('/etc/network/interfaces.d'): + os.makedirs('/etc/network/interfaces.d', mode=0o755) with open('/etc/network/interfaces', 'r') as eni: for line in eni: - if line == 'source /etc/network/interfaces.d/*': + if re.search(r'^\s*source\s+/etc/network/interfaces.d/\*\s*$', + line): return with open('/etc/network/interfaces', 'a') as eni: eni.write('\nsource /etc/network/interfaces.d/*') +@deprecate('use setup_eni() instead', '2021-01', log=log) +def check_for_eni_source(): + setup_eni() + + def full_restart(): ''' Full restart and reload of openvswitch ''' if os.path.exists('/etc/init/openvswitch-force-reload-kmod.conf'): diff --git a/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend b/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend index f614b3fa..530719e9 100644 --- a/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend +++ b/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend @@ -6,8 +6,14 @@ Listen {{ ext_port }} ServerName {{ endpoint }} SSLEngine on - SSLProtocol +TLSv1 +TLSv1.1 +TLSv1.2 - SSLCipherSuite HIGH:!RC4:!MD5:!aNULL:!eNULL:!EXP:!LOW:!MEDIUM + + # This section is based on Mozilla's recommendation + # as the "intermediate" profile as of July 7th, 2020. + # https://wiki.mozilla.org/Security/Server_Side_TLS + SSLProtocol all -SSLv3 -TLSv1 -TLSv1.1 + SSLCipherSuite ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384 + SSLHonorCipherOrder off + SSLCertificateFile /etc/apache2/ssl/{{ namespace }}/cert_{{ endpoint }} # See LP 1484489 - this is to support <= 2.4.7 and >= 2.4.8 SSLCertificateChainFile /etc/apache2/ssl/{{ namespace }}/cert_{{ endpoint }} diff --git a/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend.conf b/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend.conf index f614b3fa..530719e9 100644 --- a/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend.conf +++ b/hooks/charmhelpers/contrib/openstack/templates/openstack_https_frontend.conf @@ -6,8 +6,14 @@ Listen {{ ext_port }} ServerName {{ endpoint }} SSLEngine on - SSLProtocol +TLSv1 +TLSv1.1 +TLSv1.2 - SSLCipherSuite HIGH:!RC4:!MD5:!aNULL:!eNULL:!EXP:!LOW:!MEDIUM + + # This section is based on Mozilla's recommendation + # as the "intermediate" profile as of July 7th, 2020. + # https://wiki.mozilla.org/Security/Server_Side_TLS + SSLProtocol all -SSLv3 -TLSv1 -TLSv1.1 + SSLCipherSuite ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384 + SSLHonorCipherOrder off + SSLCertificateFile /etc/apache2/ssl/{{ namespace }}/cert_{{ endpoint }} # See LP 1484489 - this is to support <= 2.4.7 and >= 2.4.8 SSLCertificateChainFile /etc/apache2/ssl/{{ namespace }}/cert_{{ endpoint }} diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index f21625d3..0aa797c4 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -2241,10 +2241,13 @@ def inform_peers_unit_state(state, relation_name='cluster'): if state not in UNIT_STATES: raise ValueError( "Setting invalid state {} for unit".format(state)) + this_unit = local_unit() for r_id in relation_ids(relation_name): + juju_log('Telling peer behind relation {} that {} is {}'.format( + r_id, this_unit, state), 'DEBUG') relation_set(relation_id=r_id, relation_settings={ - get_peer_key(local_unit()): state}) + get_peer_key(this_unit): state}) def get_peers_unit_state(relation_name='cluster'): @@ -2276,8 +2279,10 @@ def are_peers_ready(relation_name='cluster'): :returns: Whether all units are ready. :rtype: bool """ - unit_states = get_peers_unit_state(relation_name) - return all(v == UNIT_READY for v in unit_states.values()) + unit_states = get_peers_unit_state(relation_name).values() + juju_log('{} peers are in the following states: {}'.format( + relation_name, unit_states), 'DEBUG') + return all(state == UNIT_READY for state in unit_states) def inform_peers_if_ready(check_unit_ready_func, relation_name='cluster'): @@ -2360,7 +2365,9 @@ def get_api_application_status(): app_state, msg = get_api_unit_status() if app_state == WORKLOAD_STATES.ACTIVE: if are_peers_ready(): - return WORKLOAD_STATES.ACTIVE, 'Application Ready' + msg = 'Application Ready' else: - return WORKLOAD_STATES.WAITING, 'Some units are not ready' + app_state = WORKLOAD_STATES.WAITING + msg = 'Some units are not ready' + juju_log(msg, 'DEBUG') return app_state, msg diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index b33ac906..a785efdf 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -193,7 +193,7 @@ def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d", stopped = service_stop(service_name, **kwargs) upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) sysv_file = os.path.join(initd_dir, service_name) - if init_is_systemd(): + if init_is_systemd(service_name=service_name): service('disable', service_name) service('mask', service_name) elif os.path.exists(upstart_file): @@ -227,7 +227,7 @@ def service_resume(service_name, init_dir="/etc/init", """ upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) sysv_file = os.path.join(initd_dir, service_name) - if init_is_systemd(): + if init_is_systemd(service_name=service_name): service('unmask', service_name) service('enable', service_name) elif os.path.exists(upstart_file): @@ -257,7 +257,7 @@ def service(action, service_name, **kwargs): :param **kwargs: additional params to be passed to the service command in the form of key=value. """ - if init_is_systemd(): + if init_is_systemd(service_name=service_name): cmd = ['systemctl', action, service_name] else: cmd = ['service', service_name, action] @@ -281,7 +281,7 @@ def service_running(service_name, **kwargs): units (e.g. service ceph-osd status id=2). The kwargs are ignored in systemd services. """ - if init_is_systemd(): + if init_is_systemd(service_name=service_name): return service('is-active', service_name) else: if os.path.exists(_UPSTART_CONF.format(service_name)): @@ -311,8 +311,14 @@ def service_running(service_name, **kwargs): SYSTEMD_SYSTEM = '/run/systemd/system' -def init_is_systemd(): - """Return True if the host system uses systemd, False otherwise.""" +def init_is_systemd(service_name=None): + """ + Returns whether the host uses systemd for the specified service. + + @param Optional[str] service_name: specific name of service + """ + if str(service_name).startswith("snap."): + return True if lsb_release()['DISTRIB_CODENAME'] == 'trusty': return False return os.path.isdir(SYSTEMD_SYSTEM) diff --git a/requirements.txt b/requirements.txt index 343beed1..2316401b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,5 +13,9 @@ netifaces>=0.10.4 netaddr>=0.7.12,!=0.7.16 Jinja2>=2.6 # BSD License (3 clause) six>=1.9.0 -dnspython>=1.12.0 + +# dnspython 2.0.0 dropped py3.5 support +dnspython<2.0.0; python_version < '3.6' +dnspython; python_version >= '3.6' + psutil>=1.1.1,<2.0.0 diff --git a/test-requirements.txt b/test-requirements.txt index 37854e05..7d9c2587 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,12 +1,18 @@ -# The order of packages is significant, because pip processes them in the order -# of appearance. Changing the order has an impact on the overall integration -# process, which may cause wedges in the gate later. +# This file is managed centrally by release-tools and should not be modified +# within individual charm repos. See the 'global' dir contents for available +# choices of *requirements.txt files for OpenStack Charms: +# https://github.com/openstack-charmers/release-tools +# +# TODO: Distill the func test requirements from the lint/unit test +# requirements. They are intertwined. Also, Zaza itself should specify +# all of its own requirements and if it doesn't, fix it there. +# charm-tools>=2.4.4 -coverage>=3.6 -mock>=1.2 -flake8>=2.2.4 -stestr requests>=2.18.4 -pyudev # for ceph-* charm unit tests (not mocked?) +mock>=1.2 +flake8>=2.2.4,<=2.4.1 +stestr>=2.2.0 +coverage>=4.5.2 +pyudev # for ceph-* charm unit tests (need to fix the ceph-* charm unit tests/mocking) git+https://github.com/openstack-charmers/zaza.git#egg=zaza;python_version>='3.0' git+https://github.com/openstack-charmers/zaza-openstack-tests.git#egg=zaza.openstack diff --git a/tox.ini b/tox.ini index 8080ba6d..b835733a 100644 --- a/tox.ini +++ b/tox.ini @@ -116,5 +116,5 @@ commands = functest-run-suite --keep-model --bundle {posargs} [flake8] -ignore = E402,E226,W504 +ignore = E402,E226 exclude = */charmhelpers