openstack/charm-neutron-openvswitch
Code Issues Proposed changes

Make ovs_use_veth a config option

Browse Source 
This was originally fixed in commit 7578326 but this caused problems. It
was subsequently reverted in commit 6d2e9ee.

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: I4848a3246d3450540acb8d2f479dfa2e7767be60
This commit is contained in:
David Ames 2020-01-07 15:21:55 -08:00
parent b3930738fb
commit 4075af6a11
11 changed files with 184 additions and 262 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
config.yaml
View File

@ -353,3 +353,12 @@ options:
Can be used to avoid excessive memory consumption.
WARNING: Should be NOT LESS than 25.
(Available from Stein)
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.

166
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,
@ -730,6 +734,10 @@ class AMQPContext(OSContextGenerator):
if notification_format:
ctxt['notification_format'] = notification_format
notification_topics = conf.get('notification-topics', None)
if notification_topics:
ctxt['notification_topics'] = notification_topics
send_notifications_to_logs = conf.get('send-notifications-to-logs', None)
if send_notifications_to_logs:
ctxt['send_notifications_to_logs'] = send_notifications_to_logs
@ -1940,7 +1948,7 @@ class VolumeAPIContext(InternalEndpointContext):
as well as the catalog_info string that would be supplied. Returns
a dict containing the volume_api_version and the volume_catalog_info.
"""
rel = os_release(self.pkg, base='icehouse')
rel = os_release(self.pkg)
version = '2'
if CompareOpenStackReleases(rel) >= 'pike':
version = '3'
@ -2140,7 +2148,7 @@ class VersionsContext(OSContextGenerator):
self.pkg = pkg
def __call__(self):
ostack = os_release(self.pkg, base='icehouse')
ostack = os_release(self.pkg)
osystem = lsb_release()['DISTRIB_CODENAME'].lower()
return {
'openstack_release': ostack,
@ -2240,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

27
hooks/neutron_ovs_context.py
View File

@ -33,7 +33,6 @@ from charmhelpers.core.host import (
)
from charmhelpers.contrib.openstack import context
from charmhelpers.contrib.openstack.utils import (
config_flags_parser,
get_host_ip,
)
from charmhelpers.contrib.network.ip import (
@ -288,32 +287,6 @@ class ZoneContext(OSContextGenerator):
return ctxt
class DHCPAgentContext(ZoneContext):
def __call__(self):
"""Return the 'default_availability_zone' from the principal that this
ovs unit is attached to (as a subordinate) and the 'dns_domain' from
the neutron-plugin-api relations (if one is set).
:returns: {} if no relation set, or
{'availability_zone': availability_zone from principal relation}
"""
ctxt = super(DHCPAgentContext, self).__call__()
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()()
if neutron_api_settings.get('dns_domain'):
ctxt['dns_domain'] = neutron_api_settings.get('dns_domain')
ctxt['instance_mtu'] = config('instance-mtu')
return ctxt
class L3AgentContext(OSContextGenerator):
def __call__(self):

11
hooks/neutron_ovs_utils.py
View File

@ -47,9 +47,9 @@ from charmhelpers.contrib.network.ovs import (
)
from charmhelpers.core.hookenv import (
config,
status_set,
log,
DEBUG,
log,
status_set,
)
from charmhelpers.contrib.openstack.neutron import (
parse_bridge_mappings,
@ -61,6 +61,8 @@ from charmhelpers.contrib.openstack.context import (
DataPortContext,
WorkerConfigContext,
parse_data_port_mappings,
DHCPAgentContext,
validate_ovs_use_veth,
)
from charmhelpers.core.host import (
lsb_release,
@ -214,11 +216,11 @@ METADATA_RESOURCE_MAP = OrderedDict([
DHCP_RESOURCE_MAP = OrderedDict([
(NEUTRON_DHCP_AGENT_CONF, {
'services': ['neutron-dhcp-agent'],
'contexts': [neutron_ovs_context.DHCPAgentContext()],
'contexts': [DHCPAgentContext()],
}),
(NEUTRON_DNSMASQ_CONF, {
'services': ['neutron-dhcp-agent'],
'contexts': [neutron_ovs_context.DHCPAgentContext()],
'contexts': [DHCPAgentContext()],
}),
])
DVR_RESOURCE_MAP = OrderedDict([
@ -945,6 +947,7 @@ def assess_status_func(configs):
required_interfaces['neutron-plugin-api'] = ['neutron-plugin-api']
return make_assess_status_func(
configs, required_interfaces,
charm_func=validate_ovs_use_veth,
services=services(), ports=None)

2
templates/icehouse/dhcp_agent.ini
View File

@ -17,4 +17,4 @@ dnsmasq_config_file = /etc/neutron/dnsmasq.conf
enable_metadata_network = True
enable_isolated_metadata = True
ovs_use_veth = True
ovs_use_veth = {{ ovs_use_veth }}

2
templates/mitaka/dhcp_agent.ini
View File

@ -30,4 +30,4 @@ dhcp_domain = {{ dns_domain }}
enable_metadata_network = True
enable_isolated_metadata = True
ovs_use_veth = True
ovs_use_veth = {{ ovs_use_veth }}

2
templates/ocata/dhcp_agent.ini
View File

@ -31,4 +31,4 @@ enable_metadata_network = True
force_metadata = True
enable_isolated_metadata = True
ovs_use_veth = True
ovs_use_veth = {{ ovs_use_veth }}

89
tests/bundles/disco_stein.yaml
View File

@ -1,89 +0,0 @@
series: disco
machines:
'0':
constraints: mem=3072M
'1':
'2':
'3':
'4':
'5':
'6':
applications:
nova-compute:
charm: cs:~openstack-charmers-next/nova-compute
num_units: 1
to:
- '1'
nova-cloud-controller:
charm: cs:~openstack-charmers-next/nova-cloud-controller
num_units: 1
options:
network-manager: Neutron
to:
- '2'
rabbitmq-server:
charm: cs:~openstack-charmers-next/rabbitmq-server
num_units: 1
to:
- '3'
keystone:
num_units: 1
charm: cs:~openstack-charmers-next/keystone
num_units: 1
to:
- '4'
glance:
charm: cs:~openstack-charmers-next/glance
num_units: 1
to:
- '5'
neutron-api:
charm: cs:~openstack-charmers-next/neutron-api
num_units: 1
to:
- '6'
percona-cluster:
charm: cs:~openstack-charmers-next/percona-cluster
num_units: 1
to:
- '0'
neutron-openvswitch:
charm: ../../../neutron-openvswitch
relations:
- - 'neutron-openvswitch:amqp'
- 'rabbitmq-server:amqp'
- - 'neutron-openvswitch:neutron-plugin'
- 'nova-compute:neutron-plugin'
- - 'neutron-openvswitch:neutron-plugin-api'
- 'neutron-api:neutron-plugin-api'
- - 'neutron-api:identity-service'
- 'keystone:identity-service'
- - 'neutron-api:shared-db'
- 'percona-cluster:shared-db'
- - 'neutron-api:amqp'
- 'rabbitmq-server:amqp'
- - 'nova-compute:amqp'
- 'rabbitmq-server:amqp'
- - 'nova-compute:image-service'
- 'glance:image-service'
- - 'glance:identity-service'
- 'keystone:identity-service'
- - 'glance:shared-db'
- 'percona-cluster:shared-db'
- - 'glance:amqp'
- 'rabbitmq-server:amqp'
- - 'keystone:shared-db'
- 'percona-cluster:shared-db'
- - 'nova-cloud-controller:shared-db'
- 'percona-cluster:shared-db'
- - 'nova-cloud-controller:amqp'
- 'rabbitmq-server:amqp'
- - 'nova-cloud-controller:identity-service'
- 'keystone:identity-service'
- - 'nova-cloud-controller:cloud-compute'
- 'nova-compute:cloud-compute'
- - 'nova-cloud-controller:image-service'
- 'glance:image-service'

1
tests/tests.yaml
View File

@ -12,7 +12,6 @@ gate_bundles:
- bionic_rocky
- bionic_stein
- bionic_train
- disco_stein
dev_bundles:
- bionic_train

136
unit_tests/test_neutron_ovs_context.py
View File

@ -314,142 +314,6 @@ class ZoneContextTest(CharmTestCase):
unit='nova-compute/0')
class DHCPAgentContextTest(CharmTestCase):
def setUp(self):
super(DHCPAgentContextTest, self).setUp(context, TO_PATCH)
self.config.side_effect = self.test_config.get
def tearDown(self):
super(DHCPAgentContextTest, self).tearDown()
@patch.object(charmhelpers.contrib.openstack.context, 'relation_get')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_ids')
@patch.object(charmhelpers.contrib.openstack.context, 'related_units')
def test_default_availability_zone_not_provided(self, _runits, _rids,
_rget):
_runits.return_value = ['neutron-api/0']
_rids.return_value = ['rid2']
rdata = {
'neutron-security-groups': 'True',
'enable-dvr': 'True',
'l2-population': 'True',
'overlay-netweork-type': 'vxlan',
'network-device-mtu': 1500,
'dns-domain': 'openstack.example.'
}
_rget.side_effect = lambda *args, **kwargs: rdata
self.relation_ids.return_value = ['rid1']
self.related_units.return_value = ['nova-compute/0']
self.relation_get.return_value = None
self.assertEqual(
context.DHCPAgentContext()(),
{'dns_domain': 'openstack.example.',
'instance_mtu': None,
'dns_servers': None}
)
self.relation_ids.assert_called_with('neutron-plugin')
self.relation_get.assert_called_once_with(
'default_availability_zone',
rid='rid1',
unit='nova-compute/0')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_get')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_ids')
@patch.object(charmhelpers.contrib.openstack.context, 'related_units')
def test_default_availability_zone_provided(self, _runits, _rids, _rget):
_runits.return_value = ['neutron-api/0']
_rids.return_value = ['rid2']
rdata = {
'neutron-security-groups': 'True',
'enable-dvr': 'True',
'l2-population': 'True',
'overlay-netweork-type': 'vxlan',
'network-device-mtu': 1500,
'dns-domain': 'openstack.example.'
}
_rget.side_effect = lambda *args, **kwargs: rdata
self.test_config.set('dns-servers', '8.8.8.8,4.4.4.4')
self.relation_ids.return_value = ['rid1']
self.related_units.return_value = ['nova-compute/0']
self.relation_get.return_value = 'nova'
self.assertEqual(
context.DHCPAgentContext()(),
{'availability_zone': 'nova',
'dns_domain': 'openstack.example.',
'instance_mtu': None,
'dns_servers': '8.8.8.8,4.4.4.4'}
)
self.relation_ids.assert_called_with('neutron-plugin')
self.relation_get.assert_called_once_with(
'default_availability_zone',
rid='rid1',
unit='nova-compute/0')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_get')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_ids')
@patch.object(charmhelpers.contrib.openstack.context, 'related_units')
def test_no_dns_domain(self, _runits, _rids, _rget):
_runits.return_value = ['neutron-api/0']
_rids.return_value = ['rid2']
rdata = {
'neutron-security-groups': 'True',
'enable-dvr': 'True',
'l2-population': 'True',
'overlay-netweork-type': 'vxlan',
'network-device-mtu': 1500,
}
_rget.side_effect = lambda *args, **kwargs: rdata
self.test_config.set('dns-servers', '8.8.8.8')
self.relation_ids.return_value = ['rid1']
self.related_units.return_value = ['nova-compute/0']
self.relation_get.return_value = 'nova'
self.assertEqual(
context.DHCPAgentContext()(),
{'availability_zone': 'nova',
'instance_mtu': None,
'dns_servers': '8.8.8.8'}
)
self.relation_ids.assert_called_with('neutron-plugin')
self.relation_get.assert_called_once_with(
'default_availability_zone',
rid='rid1',
unit='nova-compute/0')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_get')
@patch.object(charmhelpers.contrib.openstack.context, 'relation_ids')
@patch.object(charmhelpers.contrib.openstack.context, 'related_units')
def test_dnsmasq_flags(self, _runits, _rids, _rget):
_runits.return_value = ['neutron-api/0']
_rids.return_value = ['rid2']
rdata = {
'neutron-security-groups': 'True',
'enable-dvr': 'True',
'l2-population': 'True',
'overlay-netweork-type': 'vxlan',
'network-device-mtu': 1500,
}
_rget.side_effect = lambda *args, **kwargs: rdata
self.relation_ids.return_value = ['rid1']
self.related_units.return_value = ['nova-compute/0']
self.relation_get.return_value = None
self.test_config.set('dnsmasq-flags', 'dhcp-userclass=set:ipxe,iPXE,'
'dhcp-match=set:ipxe,175,'
'server=1.2.3.4')
self.assertEqual(
context.DHCPAgentContext()(),
{
'dnsmasq_flags': {
'dhcp-userclass': 'set:ipxe,iPXE',
'dhcp-match': 'set:ipxe,175',
'server': '1.2.3.4',
},
'instance_mtu': None,
'dns_servers': None,
}
)
class L3AgentContextTest(CharmTestCase):
def setUp(self):

1
unit_tests/test_neutron_ovs_utils.py
View File

@ -860,6 +860,7 @@ class TestNeutronOVSUtils(CharmTestCase):
make_assess_status_func.assert_called_once_with(
'test-config',
{'Test': True},
charm_func=nutils.validate_ovs_use_veth,
services='s1',
ports=None)