diff --git a/config.yaml b/config.yaml index 0e182d52..09287484 100644 --- a/config.yaml +++ b/config.yaml @@ -340,3 +340,12 @@ options: description: | Timeout in seconds for ovsdb commands. (Available from Queens) + ovs-use-veth: + type: string + default: + description: | + "True" or "False" string value. It is safe to leave this option unset. + This option allows the DHCP agent to use a veth interface for OVS in + order to support kernels with limited namespace support. i.e. Trusty. + Changing the value after neutron DHCP agents are created will break + access. The charm will go into a blocked state if this is attempted. diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index e99aba47..8ab0f2cd 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -25,6 +25,10 @@ from subprocess import check_call, CalledProcessError import six +from charmhelpers.contrib.openstack.audits.openstack_security_guide import ( + _config_ini as config_ini +) + from charmhelpers.fetch import ( apt_install, filter_installed_packages, @@ -2244,3 +2248,157 @@ class HostInfoContext(OSContextGenerator): self.use_fqdn_hint_cb() if self.use_fqdn_hint_cb else False) } return ctxt + + +def validate_ovs_use_veth(*args, **kwargs): + """Validate OVS use veth setting for dhcp agents + + The ovs_use_veth setting is considered immutable as it will break existing + deployments. Historically, we set ovs_use_veth=True in dhcp_agent.ini. It + turns out this is no longer necessary. Ideally, all new deployments would + have this set to False. + + This function validates that the config value does not conflict with + previously deployed settings in dhcp_agent.ini. + + See LP Bug#1831935 for details. + + :returns: Status state and message + :rtype: Union[(None, None), (string, string)] + """ + existing_ovs_use_veth = ( + DHCPAgentContext.get_existing_ovs_use_veth()) + config_ovs_use_veth = DHCPAgentContext.parse_ovs_use_veth() + + # Check settings are set and not None + if existing_ovs_use_veth is not None and config_ovs_use_veth is not None: + # Check for mismatch between existing config ini and juju config + if existing_ovs_use_veth != config_ovs_use_veth: + # Stop the line to avoid breakage + msg = ( + "The existing setting for dhcp_agent.ini ovs_use_veth, {}, " + "does not match the juju config setting, {}. This may lead to " + "VMs being unable to receive a DHCP IP. Either change the " + "juju config setting or dhcp agents may need to be recreated." + .format(existing_ovs_use_veth, config_ovs_use_veth)) + log(msg, ERROR) + return ( + "blocked", + "Mismatched existing and configured ovs-use-veth. See log.") + + # Everything is OK + return None, None + + +class DHCPAgentContext(OSContextGenerator): + + def __call__(self): + """Return the DHCPAGentContext. + + Return all DHCP Agent INI related configuration. + ovs unit is attached to (as a subordinate) and the 'dns_domain' from + the neutron-plugin-api relations (if one is set). + + :returns: Dictionary context + :rtype: Dict + """ + + ctxt = {} + dnsmasq_flags = config('dnsmasq-flags') + if dnsmasq_flags: + ctxt['dnsmasq_flags'] = config_flags_parser(dnsmasq_flags) + ctxt['dns_servers'] = config('dns-servers') + + neutron_api_settings = NeutronAPIContext()() + + ctxt['debug'] = config('debug') + ctxt['instance_mtu'] = config('instance-mtu') + ctxt['ovs_use_veth'] = self.get_ovs_use_veth() + + ctxt['enable_metadata_network'] = config('enable-metadata-network') + ctxt['enable_isolated_metadata'] = config('enable-isolated-metadata') + + if neutron_api_settings.get('dns_domain'): + ctxt['dns_domain'] = neutron_api_settings.get('dns_domain') + + # Override user supplied config for these plugins as these settings are + # mandatory + if config('plugin') in ['nvp', 'nsx', 'n1kv']: + ctxt['enable_metadata_network'] = True + ctxt['enable_isolated_metadata'] = True + + return ctxt + + @staticmethod + def get_existing_ovs_use_veth(): + """Return existing ovs_use_veth setting from dhcp_agent.ini. + + :returns: Boolean value of existing ovs_use_veth setting or None + :rtype: Optional[Bool] + """ + DHCP_AGENT_INI = "/etc/neutron/dhcp_agent.ini" + existing_ovs_use_veth = None + # If there is a dhcp_agent.ini file read the current setting + if os.path.isfile(DHCP_AGENT_INI): + # config_ini does the right thing and returns None if the setting is + # commented. + existing_ovs_use_veth = ( + config_ini(DHCP_AGENT_INI)["DEFAULT"].get("ovs_use_veth")) + # Convert to Bool if necessary + if isinstance(existing_ovs_use_veth, six.string_types): + return bool_from_string(existing_ovs_use_veth) + return existing_ovs_use_veth + + @staticmethod + def parse_ovs_use_veth(): + """Parse the ovs-use-veth config setting. + + Parse the string config setting for ovs-use-veth and return a boolean + or None. + + bool_from_string will raise a ValueError if the string is not falsy or + truthy. + + :raises: ValueError for invalid input + :returns: Boolean value of ovs-use-veth or None + :rtype: Optional[Bool] + """ + _config = config("ovs-use-veth") + # An unset parameter returns None. Just in case we will also check for + # an empty string: "". Ironically, (the problem we are trying to avoid) + # "False" returns True and "" returns False. + if _config is None or not _config: + # Return None + return + # bool_from_string handles many variations of true and false strings + # as well as upper and lowercases including: + # ['y', 'yes', 'true', 't', 'on', 'n', 'no', 'false', 'f', 'off'] + return bool_from_string(_config) + + def get_ovs_use_veth(self): + """Return correct ovs_use_veth setting for use in dhcp_agent.ini. + + Get the right value from config or existing dhcp_agent.ini file. + Existing has precedence. Attempt to default to "False" without + disrupting existing deployments. Handle existing deployments and + upgrades safely. See LP Bug#1831935 + + :returns: Value to use for ovs_use_veth setting + :rtype: Bool + """ + # If this is Trust return True + release = lsb_release()['DISTRIB_CODENAME'].lower() + if CompareHostReleases(release) <= 'trusty': + return True + # If there is an existing setting return that + _existing = self.get_existing_ovs_use_veth() + if _existing is not None: + return _existing + # If the config value is unset return False + _config = self.parse_ovs_use_veth() + if _config is None: + # New better default + return False + # If the config value is set return it + else: + return _config diff --git a/hooks/neutron_contexts.py b/hooks/neutron_contexts.py index b963cb19..7aa7c821 100644 --- a/hooks/neutron_contexts.py +++ b/hooks/neutron_contexts.py @@ -13,7 +13,6 @@ from charmhelpers.core.hookenv import ( from charmhelpers.contrib.openstack.context import ( OSContextGenerator, NeutronAPIContext, - config_flags_parser, NovaVendorMetadataContext, NovaVendorMetadataJSONContext, ) @@ -165,19 +164,14 @@ class NeutronGatewayContext(NeutronAPIContext): 'plugin': config('plugin'), 'debug': config('debug'), 'verbose': config('verbose'), - 'instance_mtu': config('instance-mtu'), - 'dns_servers': config('dns-servers'), 'l2_population': api_settings['l2_population'], 'enable_dvr': api_settings['enable_dvr'], 'enable_l3ha': api_settings['enable_l3ha'], 'extension_drivers': api_settings['extension_drivers'], - 'dns_domain': api_settings['dns_domain'], 'overlay_network_type': api_settings['overlay_network_type'], 'rpc_response_timeout': api_settings['rpc_response_timeout'], 'report_interval': api_settings['report_interval'], - 'enable_metadata_network': config('enable-metadata-network'), - 'enable_isolated_metadata': config('enable-isolated-metadata'), 'availability_zone': get_availability_zone(), 'enable_nfg_logging': api_settings['enable_nfg_logging'], 'ovsdb_timeout': config('ovsdb-timeout'), @@ -196,21 +190,11 @@ class NeutronGatewayContext(NeutronAPIContext): if vlan_ranges: ctxt['vlan_ranges'] = ','.join(vlan_ranges.split()) - dnsmasq_flags = config('dnsmasq-flags') - if dnsmasq_flags: - ctxt['dnsmasq_flags'] = config_flags_parser(dnsmasq_flags) - net_dev_mtu = api_settings['network_device_mtu'] if net_dev_mtu: ctxt['network_device_mtu'] = net_dev_mtu ctxt['veth_mtu'] = net_dev_mtu - # Override user supplied config for these plugins as these settings are - # mandatory - if ctxt['plugin'] in ['nvp', 'nsx', 'n1kv']: - ctxt['enable_metadata_network'] = True - ctxt['enable_isolated_metadata'] = True - ctxt['nfg_log_output_base'] = validate_nfg_log_path( config('firewall-group-log-output-base') ) diff --git a/hooks/neutron_utils.py b/hooks/neutron_utils.py index 35aeae7c..2242dbc5 100644 --- a/hooks/neutron_utils.py +++ b/hooks/neutron_utils.py @@ -67,6 +67,8 @@ from charmhelpers.contrib.openstack.context import ( ExternalPortContext, PhyNICMTUContext, DataPortContext, + validate_ovs_use_veth, + DHCPAgentContext, ) import charmhelpers.contrib.openstack.templating as templating from charmhelpers.contrib.openstack.neutron import headers_package @@ -398,15 +400,16 @@ def get_config_files(): NEUTRON_SHARED_CONFIG_FILES = { NEUTRON_DHCP_AGENT_CONF: { - 'hook_contexts': [NeutronGatewayContext()], + 'hook_contexts': [DHCPAgentContext()], 'services': ['neutron-dhcp-agent'] }, NEUTRON_DNSMASQ_CONF: { - 'hook_contexts': [NeutronGatewayContext()], + 'hook_contexts': [DHCPAgentContext()], 'services': ['neutron-dhcp-agent'] }, NEUTRON_METADATA_AGENT_CONF: { 'hook_contexts': [NetworkServiceContext(), + DHCPAgentContext(), context.WorkerConfigContext(), NeutronGatewayContext(), NovaMetadataContext()], @@ -1026,9 +1029,7 @@ def check_optional_relations(configs): 'hacluster missing configuration: ' 'vip, vip_iface, vip_cidr') - # return 'unknown' as the lowest priority to not clobber an existing - # status. - return 'unknown', '' + return validate_ovs_use_veth() def assess_status(configs): diff --git a/templates/icehouse/dhcp_agent.ini b/templates/icehouse/dhcp_agent.ini index ff1ff906..a2707ff9 100644 --- a/templates/icehouse/dhcp_agent.ini +++ b/templates/icehouse/dhcp_agent.ini @@ -24,5 +24,5 @@ resync_interval = 30 use_namespaces = True dhcp_lease_time=3600 {% else %} -ovs_use_veth = True +ovs_use_veth = {{ ovs_use_veth }} {% endif %} diff --git a/templates/mitaka/dhcp_agent.ini b/templates/mitaka/dhcp_agent.ini index b70d74c5..eb7457c3 100644 --- a/templates/mitaka/dhcp_agent.ini +++ b/templates/mitaka/dhcp_agent.ini @@ -33,5 +33,5 @@ resync_interval = 30 use_namespaces = True dhcp_lease_time=3600 {% else %} -ovs_use_veth = True +ovs_use_veth = {{ ovs_use_veth }} {% endif %} diff --git a/templates/ocata/dhcp_agent.ini b/templates/ocata/dhcp_agent.ini index 5fa7501a..bfcff3f5 100644 --- a/templates/ocata/dhcp_agent.ini +++ b/templates/ocata/dhcp_agent.ini @@ -36,5 +36,5 @@ resync_interval = 30 use_namespaces = True dhcp_lease_time=3600 {% else %} -ovs_use_veth = True +ovs_use_veth = {{ ovs_use_veth }} {% endif %} diff --git a/tests/bundles/bionic-queens.yaml b/tests/bundles/bionic-queens.yaml index d4fa30d3..2d6c40fc 100644 --- a/tests/bundles/bionic-queens.yaml +++ b/tests/bundles/bionic-queens.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' diff --git a/tests/bundles/bionic-rocky.yaml b/tests/bundles/bionic-rocky.yaml index 528fcabe..d4f8f1f9 100644 --- a/tests/bundles/bionic-rocky.yaml +++ b/tests/bundles/bionic-rocky.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' @@ -124,4 +126,4 @@ relations: - - 'nova-cloud-controller:image-service' - 'glance:image-service' - - 'nova-cloud-controller:quantum-network-service' - - 'neutron-gateway:quantum-network-service' \ No newline at end of file + - 'neutron-gateway:quantum-network-service' diff --git a/tests/bundles/bionic-stein.yaml b/tests/bundles/bionic-stein.yaml index 5600a68a..2bfe57d6 100644 --- a/tests/bundles/bionic-stein.yaml +++ b/tests/bundles/bionic-stein.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' diff --git a/tests/bundles/bionic-train.yaml b/tests/bundles/bionic-train.yaml index c9ebc57f..84dd23df 100644 --- a/tests/bundles/bionic-train.yaml +++ b/tests/bundles/bionic-train.yaml @@ -116,6 +116,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' diff --git a/tests/bundles/eoan-train.yaml b/tests/bundles/eoan-train.yaml index e3cc20a9..2d7702da 100644 --- a/tests/bundles/eoan-train.yaml +++ b/tests/bundles/eoan-train.yaml @@ -116,6 +116,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' diff --git a/tests/bundles/trusty-mitaka.yaml b/tests/bundles/trusty-mitaka.yaml index 6883cfac..e8d92eaa 100644 --- a/tests/bundles/trusty-mitaka.yaml +++ b/tests/bundles/trusty-mitaka.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' diff --git a/tests/bundles/xenial-mitaka.yaml b/tests/bundles/xenial-mitaka.yaml index 1c7c690f..d0cdefd1 100644 --- a/tests/bundles/xenial-mitaka.yaml +++ b/tests/bundles/xenial-mitaka.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' @@ -124,4 +126,4 @@ relations: - - 'nova-cloud-controller:image-service' - 'glance:image-service' - - 'nova-cloud-controller:quantum-network-service' - - 'neutron-gateway:quantum-network-service' \ No newline at end of file + - 'neutron-gateway:quantum-network-service' diff --git a/tests/bundles/xenial-ocata.yaml b/tests/bundles/xenial-ocata.yaml index 9228802a..c2616147 100644 --- a/tests/bundles/xenial-ocata.yaml +++ b/tests/bundles/xenial-ocata.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' @@ -124,4 +126,4 @@ relations: - - 'nova-cloud-controller:image-service' - 'glance:image-service' - - 'nova-cloud-controller:quantum-network-service' - - 'neutron-gateway:quantum-network-service' \ No newline at end of file + - 'neutron-gateway:quantum-network-service' diff --git a/tests/bundles/xenial-pike.yaml b/tests/bundles/xenial-pike.yaml index cbcc20c0..3d68818e 100644 --- a/tests/bundles/xenial-pike.yaml +++ b/tests/bundles/xenial-pike.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' @@ -124,4 +126,4 @@ relations: - - 'nova-cloud-controller:image-service' - 'glance:image-service' - - 'nova-cloud-controller:quantum-network-service' - - 'neutron-gateway:quantum-network-service' \ No newline at end of file + - 'neutron-gateway:quantum-network-service' diff --git a/tests/bundles/xenial-queens.yaml b/tests/bundles/xenial-queens.yaml index 43da709d..e32f7d26 100644 --- a/tests/bundles/xenial-queens.yaml +++ b/tests/bundles/xenial-queens.yaml @@ -109,6 +109,8 @@ relations: - 'rabbitmq-server:amqp' - - 'neutron-openvswitch:amqp' - 'rabbitmq-server:amqp' + - - 'neutron-openvswitch:neutron-plugin-api' + - 'neutron-api:neutron-plugin-api' - - 'nova-cloud-controller:identity-service' - 'keystone:identity-service' - - 'nova-cloud-controller:cloud-compute' @@ -124,4 +126,4 @@ relations: - - 'nova-cloud-controller:image-service' - 'glance:image-service' - - 'nova-cloud-controller:quantum-network-service' - - 'neutron-gateway:quantum-network-service' \ No newline at end of file + - 'neutron-gateway:quantum-network-service' diff --git a/unit_tests/test_neutron_contexts.py b/unit_tests/test_neutron_contexts.py index db3c35fe..48b7a6b7 100644 --- a/unit_tests/test_neutron_contexts.py +++ b/unit_tests/test_neutron_contexts.py @@ -216,11 +216,8 @@ class TestNeutronGatewayContext(CharmTestCase): 'shared_secret': 'testsecret', 'enable_dvr': True, 'enable_l3ha': True, - 'dns_servers': '8.8.8.8,4.4.4.4', 'extension_drivers': 'qos', - 'dns_domain': 'openstack.example.', 'local_ip': '10.5.0.1', - 'instance_mtu': 1420, 'core_plugin': "ml2", 'plugin': 'ovs', 'debug': False, @@ -234,12 +231,6 @@ class TestNeutronGatewayContext(CharmTestCase): 'vlan_ranges': 'physnet1:1000:2000,physnet2:2001:3000', 'network_device_mtu': 9000, 'veth_mtu': 9000, - 'enable_isolated_metadata': False, - 'enable_metadata_network': False, - 'dnsmasq_flags': { - 'dhcp-userclass': 'set:ipxe,iPXE', - 'dhcp-match': 'set:ipxe,175' - }, 'availability_zone': 'nova', 'enable_nfg_logging': True, 'nfg_log_burst_limit': 50, @@ -287,11 +278,8 @@ class TestNeutronGatewayContext(CharmTestCase): 'shared_secret': 'testsecret', 'enable_dvr': True, 'enable_l3ha': True, - 'dns_servers': None, 'extension_drivers': 'qos', - 'dns_domain': 'openstack.example.', 'local_ip': '192.168.20.2', - 'instance_mtu': 1420, 'core_plugin': "ml2", 'plugin': 'ovs', 'debug': False, @@ -305,12 +293,6 @@ class TestNeutronGatewayContext(CharmTestCase): 'vlan_ranges': 'physnet1:1000:2000,physnet2:2001:3000', 'network_device_mtu': 9000, 'veth_mtu': 9000, - 'enable_isolated_metadata': False, - 'enable_metadata_network': False, - 'dnsmasq_flags': { - 'dhcp-userclass': 'set:ipxe,iPXE', - 'dhcp-match': 'set:ipxe,175' - }, 'availability_zone': 'nova', 'enable_nfg_logging': False, 'nfg_log_burst_limit': 25, @@ -319,35 +301,6 @@ class TestNeutronGatewayContext(CharmTestCase): 'ovsdb_timeout': 60, }) - @patch('charmhelpers.contrib.openstack.context.relation_get') - @patch('charmhelpers.contrib.openstack.context.related_units') - @patch('charmhelpers.contrib.openstack.context.relation_ids') - @patch.object(neutron_contexts, 'get_shared_secret') - def test_dhcp_settings(self, _secret, _rids, _runits, _rget): - self.os_release.return_value = 'icehouse' - self.test_config.set('enable-isolated-metadata', True) - self.test_config.set('enable-metadata-network', True) - self.network_get_primary_address.return_value = '192.168.20.2' - self.unit_get.return_value = '10.5.0.1' - ctxt = neutron_contexts.NeutronGatewayContext()() - self.assertTrue(ctxt['enable_isolated_metadata']) - self.assertTrue(ctxt['enable_metadata_network']) - - @patch('charmhelpers.contrib.openstack.context.relation_get') - @patch('charmhelpers.contrib.openstack.context.related_units') - @patch('charmhelpers.contrib.openstack.context.relation_ids') - @patch.object(neutron_contexts, 'get_shared_secret') - def test_dhcp_setting_plug_override(self, _secret, _rids, _runits, _rget): - self.os_release.return_value = 'icehouse' - self.test_config.set('plugin', 'nsx') - self.test_config.set('enable-isolated-metadata', False) - self.test_config.set('enable-metadata-network', False) - self.network_get_primary_address.return_value = '192.168.20.2' - self.unit_get.return_value = '10.5.0.1' - ctxt = neutron_contexts.NeutronGatewayContext()() - self.assertTrue(ctxt['enable_isolated_metadata']) - self.assertTrue(ctxt['enable_metadata_network']) - @patch('os.environ.get') @patch('charmhelpers.contrib.openstack.context.relation_get') @patch('charmhelpers.contrib.openstack.context.related_units')