From d0f79d593d279aee9591ae25e19b4ebb075f8cb8 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 14 Jul 2014 14:47:38 +0300 Subject: [PATCH 1/2] Add vlan/flat networking support --- config.yaml | 5 +++ .../charmhelpers/contrib/hahelpers/cluster.py | 1 + .../contrib/network/ovs/__init__.py | 7 +++- .../contrib/openstack/amulet/deployment.py | 29 +++++++++++--- .../contrib/openstack/amulet/utils.py | 10 ++--- .../charmhelpers/contrib/openstack/context.py | 31 +++++++++------ .../contrib/openstack/templating.py | 37 +++++++++--------- .../contrib/storage/linux/ceph.py | 2 +- hooks/charmhelpers/core/hookenv.py | 9 +++-- hooks/charmhelpers/core/host.py | 12 +++--- hooks/charmhelpers/fetch/__init__.py | 38 +++++++++++-------- hooks/neutron_ovs_context.py | 29 +++++++++++++- templates/icehouse/ml2_conf.ini | 13 +++++-- 13 files changed, 150 insertions(+), 73 deletions(-) diff --git a/config.yaml b/config.yaml index 3033d6f0..07a070a4 100644 --- a/config.yaml +++ b/config.yaml @@ -21,3 +21,8 @@ options: default: False type: boolean description: Enable verbose logging + data-port: + type: string + description: | + The data port will be added to br-data and will allow usage of flat or VLAN + network types diff --git a/hooks/charmhelpers/contrib/hahelpers/cluster.py b/hooks/charmhelpers/contrib/hahelpers/cluster.py index bf832f7d..4e1a473d 100644 --- a/hooks/charmhelpers/contrib/hahelpers/cluster.py +++ b/hooks/charmhelpers/contrib/hahelpers/cluster.py @@ -170,6 +170,7 @@ def canonical_url(configs, vip_setting='vip'): :configs : OSTemplateRenderer: A config tempating object to inspect for a complete https context. + :vip_setting: str: Setting in charm config that specifies VIP address. ''' diff --git a/hooks/charmhelpers/contrib/network/ovs/__init__.py b/hooks/charmhelpers/contrib/network/ovs/__init__.py index 5eba8376..8f8a5230 100644 --- a/hooks/charmhelpers/contrib/network/ovs/__init__.py +++ b/hooks/charmhelpers/contrib/network/ovs/__init__.py @@ -21,12 +21,16 @@ def del_bridge(name): subprocess.check_call(["ovs-vsctl", "--", "--if-exists", "del-br", name]) -def add_bridge_port(name, port): +def add_bridge_port(name, port, promisc=False): ''' Add a port to the named openvswitch bridge ''' log('Adding port {} to bridge {}'.format(port, name)) subprocess.check_call(["ovs-vsctl", "--", "--may-exist", "add-port", name, port]) subprocess.check_call(["ip", "link", "set", port, "up"]) + if promisc: + subprocess.check_call(["ip", "link", "set", port, "promisc", "on"]) + else: + subprocess.check_call(["ip", "link", "set", port, "promisc", "off"]) def del_bridge_port(name, port): @@ -35,6 +39,7 @@ def del_bridge_port(name, port): 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"]) def set_manager(manager): diff --git a/hooks/charmhelpers/contrib/openstack/amulet/deployment.py b/hooks/charmhelpers/contrib/openstack/amulet/deployment.py index 9e164821..e476b6f2 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/deployment.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/deployment.py @@ -7,19 +7,36 @@ class OpenStackAmuletDeployment(AmuletDeployment): """This class inherits from AmuletDeployment and has additional support that is specifically for use by OpenStack charms.""" - def __init__(self, series=None, openstack=None): + def __init__(self, series=None, openstack=None, source=None): """Initialize the deployment environment.""" - self.openstack = None super(OpenStackAmuletDeployment, self).__init__(series) + self.openstack = openstack + self.source = source - if openstack: - self.openstack = openstack + def _add_services(self, this_service, other_services): + """Add services to the deployment and set openstack-origin.""" + super(OpenStackAmuletDeployment, self)._add_services(this_service, + other_services) + name = 0 + services = other_services + services.append(this_service) + use_source = ['mysql', 'mongodb', 'rabbitmq-server', 'ceph'] + + if self.openstack: + for svc in services: + if svc[name] not in use_source: + config = {'openstack-origin': self.openstack} + self.d.configure(svc[name], config) + + if self.source: + for svc in services: + if svc[name] in use_source: + config = {'source': self.source} + self.d.configure(svc[name], config) def _configure_services(self, configs): """Configure all of the services.""" for service, config in configs.iteritems(): - if service == self.this_service: - config['openstack-origin'] = self.openstack self.d.configure(service, config) def _get_openstack_release(self): diff --git a/hooks/charmhelpers/contrib/openstack/amulet/utils.py b/hooks/charmhelpers/contrib/openstack/amulet/utils.py index 6515f907..222281e3 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/utils.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/utils.py @@ -74,7 +74,7 @@ class OpenStackAmuletUtils(AmuletUtils): if ret: return "unexpected tenant data - {}".format(ret) if not found: - return "tenant {} does not exist".format(e.name) + return "tenant {} does not exist".format(e['name']) return ret def validate_role_data(self, expected, actual): @@ -91,7 +91,7 @@ class OpenStackAmuletUtils(AmuletUtils): if ret: return "unexpected role data - {}".format(ret) if not found: - return "role {} does not exist".format(e.name) + return "role {} does not exist".format(e['name']) return ret def validate_user_data(self, expected, actual): @@ -110,7 +110,7 @@ class OpenStackAmuletUtils(AmuletUtils): if ret: return "unexpected user data - {}".format(ret) if not found: - return "user {} does not exist".format(e.name) + return "user {} does not exist".format(e['name']) return ret def validate_flavor_data(self, expected, actual): @@ -192,8 +192,8 @@ class OpenStackAmuletUtils(AmuletUtils): count = 1 status = instance.status - while status == 'BUILD' and count < 10: - time.sleep(5) + while status != 'ACTIVE' and count < 60: + time.sleep(3) instance = nova.servers.get(instance.id) status = instance.status self.log.debug('instance status: {}'.format(status)) diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index e0526d89..7d745cdb 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -24,6 +24,7 @@ from charmhelpers.core.hookenv import ( unit_get, unit_private_ip, ERROR, + INFO ) from charmhelpers.contrib.hahelpers.cluster import ( @@ -426,12 +427,13 @@ class ApacheSSLContext(OSContextGenerator): """ Generates a context for an apache vhost configuration that configures HTTPS reverse proxying for one or many endpoints. Generated context - looks something like: - { - 'namespace': 'cinder', - 'private_address': 'iscsi.mycinderhost.com', - 'endpoints': [(8776, 8766), (8777, 8767)] - } + looks something like:: + + { + 'namespace': 'cinder', + 'private_address': 'iscsi.mycinderhost.com', + 'endpoints': [(8776, 8766), (8777, 8767)] + } The endpoints list consists of a tuples mapping external ports to internal ports. @@ -641,7 +643,7 @@ class SubordinateConfigContext(OSContextGenerator): The subordinate interface allows subordinates to export their configuration requirements to the principle for multiple config files and multiple serivces. Ie, a subordinate that has interfaces - to both glance and nova may export to following yaml blob as json: + to both glance and nova may export to following yaml blob as json:: glance: /etc/glance/glance-api.conf: @@ -660,7 +662,8 @@ class SubordinateConfigContext(OSContextGenerator): It is then up to the principle charms to subscribe this context to the service+config file it is interestd in. Configuration data will - be available in the template context, in glance's case, as: + be available in the template context, in glance's case, as:: + ctxt = { ... other context ... 'subordinate_config': { @@ -687,7 +690,7 @@ class SubordinateConfigContext(OSContextGenerator): self.interface = interface def __call__(self): - ctxt = {} + ctxt = {'sections': {}} for rid in relation_ids(self.interface): for unit in related_units(rid): sub_config = relation_get('subordinate_configuration', @@ -713,10 +716,14 @@ class SubordinateConfigContext(OSContextGenerator): sub_config = sub_config[self.config_file] for k, v in sub_config.iteritems(): - ctxt[k] = v + if k == 'sections': + for section, config_dict in v.iteritems(): + log("adding section '%s'" % (section)) + ctxt[k][section] = config_dict + else: + ctxt[k] = v - if not ctxt: - ctxt['sections'] = {} + log("%d section(s) found" % (len(ctxt['sections'])), level=INFO) return ctxt diff --git a/hooks/charmhelpers/contrib/openstack/templating.py b/hooks/charmhelpers/contrib/openstack/templating.py index 4595778c..f5442712 100644 --- a/hooks/charmhelpers/contrib/openstack/templating.py +++ b/hooks/charmhelpers/contrib/openstack/templating.py @@ -30,17 +30,17 @@ def get_loader(templates_dir, os_release): loading dir. A charm may also ship a templates dir with this module - and it will be appended to the bottom of the search list, eg: - hooks/charmhelpers/contrib/openstack/templates. + and it will be appended to the bottom of the search list, eg:: - :param templates_dir: str: Base template directory containing release - sub-directories. - :param os_release : str: OpenStack release codename to construct template - loader. + hooks/charmhelpers/contrib/openstack/templates - :returns : jinja2.ChoiceLoader constructed with a list of - jinja2.FilesystemLoaders, ordered in descending - order by OpenStack release. + :param templates_dir (str): Base template directory containing release + sub-directories. + :param os_release (str): OpenStack release codename to construct template + loader. + :returns: jinja2.ChoiceLoader constructed with a list of + jinja2.FilesystemLoaders, ordered in descending + order by OpenStack release. """ tmpl_dirs = [(rel, os.path.join(templates_dir, rel)) for rel in OPENSTACK_CODENAMES.itervalues()] @@ -111,7 +111,8 @@ class OSConfigRenderer(object): and ease the burden of managing config templates across multiple OpenStack releases. - Basic usage: + Basic usage:: + # import some common context generates from charmhelpers from charmhelpers.contrib.openstack import context @@ -131,21 +132,19 @@ class OSConfigRenderer(object): # write out all registered configs configs.write_all() - Details: + **OpenStack Releases and template loading** - OpenStack Releases and template loading - --------------------------------------- When the object is instantiated, it is associated with a specific OS release. This dictates how the template loader will be constructed. The constructed loader attempts to load the template from several places in the following order: - - from the most recent OS release-specific template dir (if one exists) - - the base templates_dir - - a template directory shipped in the charm with this helper file. + - from the most recent OS release-specific template dir (if one exists) + - the base templates_dir + - a template directory shipped in the charm with this helper file. + For the example above, '/tmp/templates' contains the following structure:: - For the example above, '/tmp/templates' contains the following structure: /tmp/templates/nova.conf /tmp/templates/api-paste.ini /tmp/templates/grizzly/api-paste.ini @@ -169,8 +168,8 @@ class OSConfigRenderer(object): $CHARM/hooks/charmhelpers/contrib/openstack/templates. This allows us to ship common templates (haproxy, apache) with the helpers. - Context generators - --------------------------------------- + **Context generators** + Context generators are used to generate template contexts during hook execution. Doing so may require inspecting service relations, charm config, etc. When registered, a config file is associated with a list diff --git a/hooks/charmhelpers/contrib/storage/linux/ceph.py b/hooks/charmhelpers/contrib/storage/linux/ceph.py index 12417410..768438a4 100644 --- a/hooks/charmhelpers/contrib/storage/linux/ceph.py +++ b/hooks/charmhelpers/contrib/storage/linux/ceph.py @@ -303,7 +303,7 @@ def ensure_ceph_storage(service, pool, rbd_img, sizemb, mount_point, blk_device, fstype, system_services=[]): """ NOTE: This function must only be called from a single service unit for - the same rbd_img otherwise data loss will occur. + the same rbd_img otherwise data loss will occur. Ensures given pool and RBD image exists, is mapped to a block device, and the device is formatted and mounted at the given mount_point. diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index c2e66f66..c9530433 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -25,7 +25,7 @@ cache = {} def cached(func): """Cache return values for multiple executions of func + args - For example: + For example:: @cached def unit_get(attribute): @@ -445,18 +445,19 @@ class UnregisteredHookError(Exception): class Hooks(object): """A convenient handler for hook functions. - Example: + Example:: + hooks = Hooks() # register a hook, taking its name from the function name @hooks.hook() def install(): - ... + pass # your code here # register a hook, providing a custom hook name @hooks.hook("config-changed") def config_changed(): - ... + pass # your code here if __name__ == "__main__": # execute a hook based on the name the program is called by diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index 59f8facc..8b617a42 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -211,13 +211,13 @@ def file_hash(path): def restart_on_change(restart_map, stopstart=False): """Restart services based on configuration files changing - This function is used a decorator, for example + This function is used a decorator, for example:: @restart_on_change({ '/etc/ceph/ceph.conf': [ 'cinder-api', 'cinder-volume' ] }) def ceph_client_changed(): - ... + pass # your code here In this example, the cinder-api and cinder-volume services would be restarted if /etc/ceph/ceph.conf is changed by the @@ -313,9 +313,11 @@ def get_nic_hwaddr(nic): def cmp_pkgrevno(package, revno, pkgcache=None): '''Compare supplied revno with the revno of the installed package - 1 => Installed revno is greater than supplied arg - 0 => Installed revno is the same as supplied arg - -1 => Installed revno is less than supplied arg + + * 1 => Installed revno is greater than supplied arg + * 0 => Installed revno is the same as supplied arg + * -1 => Installed revno is less than supplied arg + ''' import apt_pkg if not pkgcache: diff --git a/hooks/charmhelpers/fetch/__init__.py b/hooks/charmhelpers/fetch/__init__.py index b5cb48ef..5be512ce 100644 --- a/hooks/charmhelpers/fetch/__init__.py +++ b/hooks/charmhelpers/fetch/__init__.py @@ -235,31 +235,39 @@ def configure_sources(update=False, sources_var='install_sources', keys_var='install_keys'): """ - Configure multiple sources from charm configuration + Configure multiple sources from charm configuration. + + The lists are encoded as yaml fragments in the configuration. + The frament needs to be included as a string. Example config: - install_sources: + install_sources: | - "ppa:foo" - "http://example.com/repo precise main" - install_keys: + install_keys: | - null - "a1b2c3d4" Note that 'null' (a.k.a. None) should not be quoted. """ - sources = safe_load(config(sources_var)) - keys = config(keys_var) - if keys is not None: - keys = safe_load(keys) - if isinstance(sources, basestring) and ( - keys is None or isinstance(keys, basestring)): - add_source(sources, keys) + sources = safe_load((config(sources_var) or '').strip()) or [] + keys = safe_load((config(keys_var) or '').strip()) or None + + if isinstance(sources, basestring): + sources = [sources] + + if keys is None: + for source in sources: + add_source(source, None) else: - if not len(sources) == len(keys): - msg = 'Install sources and keys lists are different lengths' - raise SourceConfigError(msg) - for src_num in range(len(sources)): - add_source(sources[src_num], keys[src_num]) + if isinstance(keys, basestring): + keys = [keys] + + if len(sources) != len(keys): + raise SourceConfigError( + 'Install sources and keys lists are different lengths') + for source, key in zip(sources, keys): + add_source(source, key) if update: apt_update(fatal=True) diff --git a/hooks/neutron_ovs_context.py b/hooks/neutron_ovs_context.py index d8b373a1..b0d70ff6 100644 --- a/hooks/neutron_ovs_context.py +++ b/hooks/neutron_ovs_context.py @@ -5,12 +5,16 @@ from charmhelpers.core.hookenv import ( config, unit_get, ) - +from charmhelpers.core.host import list_nics, get_nic_hwaddr from charmhelpers.contrib.openstack import context from charmhelpers.core.host import service_running, service_start -from charmhelpers.contrib.network.ovs import add_bridge +from charmhelpers.contrib.network.ovs import add_bridge, add_bridge_port from charmhelpers.contrib.openstack.utils import get_host_ip + +import re + OVS_BRIDGE = 'br-int' +DATA_BRIDGE = 'br-data' def _neutron_security_groups(): @@ -43,10 +47,31 @@ class OVSPluginContext(context.NeutronContext): def neutron_security_groups(self): return _neutron_security_groups() + def get_data_port(self): + data_ports = config('data-port') + if not data_ports: + return None + hwaddrs = {} + for nic in list_nics(['eth', 'bond']): + hwaddrs[get_nic_hwaddr(nic).lower()] = nic + mac_regex = re.compile(r'([0-9A-F]{2}[:-]){5}([0-9A-F]{2})', re.I) + for entry in data_ports.split(): + entry = entry.strip().lower() + if re.match(mac_regex, entry): + if entry in hwaddrs: + return hwaddrs[entry] + else: + return entry + return None + def _ensure_bridge(self): if not service_running('openvswitch-switch'): service_start('openvswitch-switch') add_bridge(OVS_BRIDGE) + add_bridge(DATA_BRIDGE) + data_port = self.get_data_port() + if data_port: + add_bridge_port(DATA_BRIDGE, data_port, promisc=True) def ovs_ctxt(self): # In addition to generating config context, ensure the OVS service diff --git a/templates/icehouse/ml2_conf.ini b/templates/icehouse/ml2_conf.ini index 08f2ce44..fe260725 100644 --- a/templates/icehouse/ml2_conf.ini +++ b/templates/icehouse/ml2_conf.ini @@ -5,9 +5,9 @@ # Config managed by neutron-openvswitch charm ############################################################################### [ml2] -type_drivers = gre,vxlan -tenant_network_types = gre,vxlan -mechanism_drivers = openvswitch +type_drivers = gre,vxlan,vlan,flat +tenant_network_types = gre,vxlan,vlan,flat +mechanism_drivers = openvswitch,hyperv [ml2_type_gre] tunnel_id_ranges = 1:1000 @@ -15,9 +15,16 @@ tunnel_id_ranges = 1:1000 [ml2_type_vxlan] vni_ranges = 1001:2000 +[ml2_type_vlan] +network_vlan_ranges = physnet1:1000:2000 + +[ml2_type_flat] +flat_networks = physnet1 + [ovs] enable_tunneling = True local_ip = {{ local_ip }} +bridge_mappings = physnet1:br-data [agent] tunnel_types = gre From 1639f6373767be70f657cca4ec8a35f2ae1b22f9 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 15 Jul 2014 09:15:31 +0300 Subject: [PATCH 2/2] added tests for data-port --- unit_tests/test_neutron_ovs_context.py | 35 ++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/unit_tests/test_neutron_ovs_context.py b/unit_tests/test_neutron_ovs_context.py index 2c46b3cf..8363f3e5 100644 --- a/unit_tests/test_neutron_ovs_context.py +++ b/unit_tests/test_neutron_ovs_context.py @@ -10,9 +10,12 @@ TO_PATCH = [ 'config', 'unit_get', 'add_bridge', + 'add_bridge_port', 'service_running', 'service_start', 'get_host_ip', + 'get_nic_hwaddr', + 'list_nics', ] @@ -28,6 +31,38 @@ class OVSPluginContextTest(CharmTestCase): def tearDown(self): super(OVSPluginContextTest, self).tearDown() + + def test_data_port_name(self): + self.test_config.set('data-port', 'em1') + self.assertEquals(context.OVSPluginContext().get_data_port(), 'em1') + + def test_data_port_mac(self): + machine_machs = { + 'em1': 'aa:aa:aa:aa:aa:aa', + 'eth0': 'bb:bb:bb:bb:bb:bb', + } + absent_mac = "cc:cc:cc:cc:cc:cc" + config_macs = "%s %s" % (absent_mac, machine_machs['em1']) + self.test_config.set('data-port', config_macs) + + def get_hwaddr(eth): + return machine_machs[eth] + self.get_nic_hwaddr.side_effect = get_hwaddr + self.list_nics.return_value = machine_machs.keys() + self.assertEquals(context.OVSPluginContext().get_data_port(), 'em1') + + @patch.object(context.OVSPluginContext, 'get_data_port') + def test_ensure_bridge_data_port_present(self, get_data_port): + def add_port(bridge, port, promisc): + if bridge == 'br-data' and port == 'em1' and promisc is True: + self.bridge_added = True + return + self.bridge_added =False + + get_data_port.return_value = 'em1' + self.add_bridge_port.side_effect = add_port + context.OVSPluginContext()._ensure_bridge() + self.assertEquals(self.bridge_added, True) @patch.object(charmhelpers.contrib.openstack.context, 'config') @patch.object(charmhelpers.contrib.openstack.context, 'unit_get')