openstack/charm-neutron-gateway
Code Issues Proposed changes

Make ovs_use_veth a config option

Browse Source 
This change uses a common DHCPAgentContext and takes care to check for a
pre-existing setting in the dhcp_agent.ini. Only allowing a config
change if there is no pre-existing setting.

Please review and merge charm-helpers PR:
https://github.com/juju/charm-helpers/pull/422

Partial-Bug: #1831935

func-test-pr: https://github.com/openstack-charmers/zaza-openstack-tests/pull/157
Change-Id: Ia01c637b0837a4e594d16f6565c605460ad3f922
This commit is contained in:
David Ames 2020-01-27 14:54:42 -08:00
parent 2ada961a0b
commit a03fe36fa6
18 changed files with 201 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
config.yaml
View File

@ -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.

158
hooks/charmhelpers/contrib/openstack/context.py
View File

@ -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

16
hooks/neutron_contexts.py
View File

@ -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')
)

11
hooks/neutron_utils.py
View File

@ -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):

2
templates/icehouse/dhcp_agent.ini
View File

@ -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 %}

2
templates/mitaka/dhcp_agent.ini
View File

@ -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 %}

2
templates/ocata/dhcp_agent.ini
View File

@ -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 %}

2
tests/bundles/bionic-queens.yaml
View File

@ -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'

2
tests/bundles/bionic-rocky.yaml
View File

@ -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'

2
tests/bundles/bionic-stein.yaml
View File

@ -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'

2
tests/bundles/bionic-train.yaml
View File

@ -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'

2
tests/bundles/eoan-train.yaml
View File

@ -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'

2
tests/bundles/trusty-mitaka.yaml
View File

@ -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'

2
tests/bundles/xenial-mitaka.yaml
View File

@ -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'

2
tests/bundles/xenial-ocata.yaml
View File

@ -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'

2
tests/bundles/xenial-pike.yaml
View File

@ -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'

2
tests/bundles/xenial-queens.yaml
View File

@ -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'

47
unit_tests/test_neutron_contexts.py
View File

@ -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')