From 96a558b5d02ce4efdb0ffb575f87992331bcb1f8 Mon Sep 17 00:00:00 2001 From: James Arendt Date: Thu, 10 Sep 2015 09:36:20 -0700 Subject: [PATCH] Add flavor option to loadbalancerv2 creation Builds on the Neutron services flavor framework in the referenced Depends-On change id. This solution uses a flavor of service type LOADBALANCERV2 associated with a flavor profile containing a driver. If a flavor_id is passed during loadbalancerv2 creation, the flavor is used to find the provider for the driver and populate the existing lbaas provider field. This allows a user to specify a flavor like 'Gold' with the operator dynamically controlling which provider is associated with that flavor for subsequent creates. The flavor_id is persisted in the DB. The selected provider controls subsequent control operations against the created loadbalancer. Also ensure everything works gracefully if flavors plugin is not loaded. Adding api and doc impact tags as the new optional flavor_id parameter should be described to consumers. Co-Authored-By: James Arendt Co-Authored-By: Madhusudhan Kandadai ApiImpact DocImpact Change-Id: Ic3b459692d092658df93d4c855079311d9bc7ace Depends-On: I5c22ab655a8e2a2e586c10eae9de9b72db49755f Partially implements: blueprint neutron-flavor-framework --- neutron_lbaas/db/loadbalancer/models.py | 2 + .../expand/3426acbc12de_add_flavor_id.py | 39 +++++++++++++++ neutron_lbaas/extensions/loadbalancerv2.py | 14 +++++- .../services/loadbalancer/data_models.py | 7 ++- neutron_lbaas/services/loadbalancer/plugin.py | 47 +++++++++++++++++++ .../v2/api/test_load_balancers_non_admin.py | 17 +++++++ .../loadbalancer/test_loadbalancer_plugin.py | 31 +++++++++++- .../tests/unit/test_agent_scheduler.py | 2 + 8 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 neutron_lbaas/db/migration/alembic_migrations/versions/mitaka/expand/3426acbc12de_add_flavor_id.py diff --git a/neutron_lbaas/db/loadbalancer/models.py b/neutron_lbaas/db/loadbalancer/models.py index c687b356f..d7166eb4e 100644 --- a/neutron_lbaas/db/loadbalancer/models.py +++ b/neutron_lbaas/db/loadbalancer/models.py @@ -194,6 +194,8 @@ class LoadBalancer(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant): # balancer ID and should not be cleared out in this table viewonly=True ) + flavor_id = sa.Column(sa.String(36), sa.ForeignKey( + 'flavors.id', name='fk_lbaas_loadbalancers_flavors_id')) @property def root_loadbalancer(self): diff --git a/neutron_lbaas/db/migration/alembic_migrations/versions/mitaka/expand/3426acbc12de_add_flavor_id.py b/neutron_lbaas/db/migration/alembic_migrations/versions/mitaka/expand/3426acbc12de_add_flavor_id.py new file mode 100644 index 000000000..b4dd217a2 --- /dev/null +++ b/neutron_lbaas/db/migration/alembic_migrations/versions/mitaka/expand/3426acbc12de_add_flavor_id.py @@ -0,0 +1,39 @@ +# Copyright 2015 Hewlett-Packard Development Company, L.P. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""Add flavor id + +Revision ID: 3426acbc12de +Revises: 4a408dd491c2 +Create Date: 2015-12-02 15:24:35.775474 + +""" + +# revision identifiers, used by Alembic. +revision = '3426acbc12de' +down_revision = '4a408dd491c2' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('lbaas_loadbalancers', + sa.Column(u'flavor_id', sa.String(36), nullable=True)) + op.create_foreign_key(u'fk_lbaas_loadbalancers_flavors_id', + u'lbaas_loadbalancers', + u'flavors', + [u'flavor_id'], + [u'id']) diff --git a/neutron_lbaas/extensions/loadbalancerv2.py b/neutron_lbaas/extensions/loadbalancerv2.py index 244e8773a..d3d084a25 100644 --- a/neutron_lbaas/extensions/loadbalancerv2.py +++ b/neutron_lbaas/extensions/loadbalancerv2.py @@ -124,6 +124,14 @@ class CertManagerError(nexception.NeutronException): message = _("Could not process TLS container %(ref)s, %(reason)s") +class ProviderFlavorConflict(nexception.Conflict): + message = _("Cannot specify both a flavor and a provider") + + +class FlavorsPluginNotLoaded(nexception.NotFound): + message = _("Flavors plugin not found") + + RESOURCE_ATTRIBUTE_MAP = { 'loadbalancers': { 'id': {'allow_post': False, 'allow_put': False, @@ -162,7 +170,11 @@ RESOURCE_ATTRIBUTE_MAP = { 'provisioning_status': {'allow_post': False, 'allow_put': False, 'is_visible': True}, 'operating_status': {'allow_post': False, 'allow_put': False, - 'is_visible': True} + 'is_visible': True}, + 'flavor_id': {'allow_post': True, 'allow_put': False, + 'is_visible': True, + 'validate': {'type:string': attr.NAME_MAX_LEN}, + 'default': attr.ATTR_NOT_SPECIFIED} }, 'listeners': { 'id': {'allow_post': False, 'allow_put': False, diff --git a/neutron_lbaas/services/loadbalancer/data_models.py b/neutron_lbaas/services/loadbalancer/data_models.py index ef972515e..08fddd8d1 100644 --- a/neutron_lbaas/services/loadbalancer/data_models.py +++ b/neutron_lbaas/services/loadbalancer/data_models.py @@ -504,7 +504,7 @@ class LoadBalancer(BaseDataModel): vip_subnet_id=None, vip_port_id=None, vip_address=None, provisioning_status=None, operating_status=None, admin_state_up=None, vip_port=None, stats=None, - provider=None, listeners=None): + provider=None, listeners=None, flavor_id=None): self.id = id self.tenant_id = tenant_id self.name = name @@ -519,6 +519,7 @@ class LoadBalancer(BaseDataModel): self.stats = stats self.provider = provider self.listeners = listeners or [] + self.flavor_id = flavor_id def attached_to_loadbalancer(self): return True @@ -530,6 +531,10 @@ class LoadBalancer(BaseDataModel): for listener in self.listeners] if self.provider: ret_dict['provider'] = self.provider.provider_name + + if not self.flavor_id: + del ret_dict['flavor_id'] + return ret_dict @classmethod diff --git a/neutron_lbaas/services/loadbalancer/plugin.py b/neutron_lbaas/services/loadbalancer/plugin.py index b18fac489..0594b2469 100644 --- a/neutron_lbaas/services/loadbalancer/plugin.py +++ b/neutron_lbaas/services/loadbalancer/plugin.py @@ -19,7 +19,10 @@ from neutron.api.v2 import attributes as attrs from neutron.common import exceptions as n_exc from neutron import context as ncontext from neutron.db import servicetype_db as st_db +from neutron.extensions import flavors +from neutron import manager from neutron.plugins.common import constants +from neutron.services.flavors import flavors_plugin from neutron.services import provider_configuration as pconf from neutron.services import service_base from oslo_config import cfg @@ -507,8 +510,52 @@ class LoadBalancerPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2): def get_plugin_description(self): return "Neutron LoadBalancer Service Plugin v2" + def _insert_provider_name_from_flavor(self, context, loadbalancer): + """Select provider based on flavor.""" + + # TODO(jwarendt) Support passing flavor metainfo from the + # selected flavor profile into the provider, not just selecting + # the provider, when flavor templating arrives. + + if ('provider' in loadbalancer and + loadbalancer['provider'] != attrs.ATTR_NOT_SPECIFIED): + raise loadbalancerv2.ProviderFlavorConflict() + + plugin = manager.NeutronManager.get_service_plugins().get( + constants.FLAVORS) + if not plugin: + raise loadbalancerv2.FlavorsPluginNotLoaded() + + # Will raise FlavorNotFound if doesn't exist + fl_db = flavors_plugin.FlavorsPlugin.get_flavor( + plugin, + context, + loadbalancer['flavor_id']) + + if fl_db['service_type'] != constants.LOADBALANCERV2: + raise flavors.InvalidFlavorServiceType( + service_type=fl_db['service_type']) + + if not fl_db['enabled']: + raise flavors.FlavorDisabled() + + providers = flavors_plugin.FlavorsPlugin.get_flavor_next_provider( + plugin, + context, + fl_db['id']) + + provider = providers[0].get('provider') + + LOG.debug("Selected provider %s" % provider) + + loadbalancer['provider'] = provider + def create_loadbalancer(self, context, loadbalancer): loadbalancer = loadbalancer.get('loadbalancer') + if loadbalancer['flavor_id'] != attrs.ATTR_NOT_SPECIFIED: + self._insert_provider_name_from_flavor(context, loadbalancer) + else: + del loadbalancer['flavor_id'] provider_name = self._get_provider_name(loadbalancer) driver = self.drivers[provider_name] lb_db = self.db.create_loadbalancer( diff --git a/neutron_lbaas/tests/tempest/v2/api/test_load_balancers_non_admin.py b/neutron_lbaas/tests/tempest/v2/api/test_load_balancers_non_admin.py index 8c008721e..7b431c249 100644 --- a/neutron_lbaas/tests/tempest/v2/api/test_load_balancers_non_admin.py +++ b/neutron_lbaas/tests/tempest/v2/api/test_load_balancers_non_admin.py @@ -290,6 +290,23 @@ class LoadBalancersTestJSON(base.BaseTestCase): vip_subnet_id=self.subnet['id'], tenant_id=tenant) + @test.attr(type='negative') + def test_create_load_balancer_invalid_flavor_field(self): + """Test create load balancer with an invalid flavor field""" + self.assertRaises(exceptions.NotFound, + self.load_balancers_client.create_load_balancer, + vip_subnet_id=self.subnet['id'], + flavor_id="NO_SUCH_FLAVOR") + + @test.attr(type='negative') + def test_create_load_balancer_provider_flavor_conflict(self): + """Test create load balancer with both a provider and a flavor""" + self.assertRaises(exceptions.Conflict, + self.load_balancers_client.create_load_balancer, + vip_subnet_id=self.subnet['id'], + flavor_id="NO_SUCH_FLAVOR", + provider="NO_SUCH_PROVIDER") + @test.attr(type='smoke') def test_update_load_balancer(self): """Test update load balancer""" diff --git a/neutron_lbaas/tests/unit/services/loadbalancer/test_loadbalancer_plugin.py b/neutron_lbaas/tests/unit/services/loadbalancer/test_loadbalancer_plugin.py index d3d43e657..098b2cd00 100644 --- a/neutron_lbaas/tests/unit/services/loadbalancer/test_loadbalancer_plugin.py +++ b/neutron_lbaas/tests/unit/services/loadbalancer/test_loadbalancer_plugin.py @@ -486,7 +486,8 @@ class LoadBalancerExtensionV2TestCase(base.ExtensionTestCase): res = self.api.post(_get_path('lbaas/loadbalancers', fmt=self.fmt), self.serialize(data), content_type='application/{0}'.format(self.fmt)) - data['loadbalancer'].update({'provider': attr.ATTR_NOT_SPECIFIED}) + data['loadbalancer'].update({'provider': attr.ATTR_NOT_SPECIFIED, + 'flavor_id': attr.ATTR_NOT_SPECIFIED}) instance.create_loadbalancer.assert_called_with(mock.ANY, loadbalancer=data) @@ -495,6 +496,34 @@ class LoadBalancerExtensionV2TestCase(base.ExtensionTestCase): self.assertIn('loadbalancer', res) self.assertEqual(return_value, res['loadbalancer']) + def test_loadbalancer_create_invalid_flavor(self): + data = {'loadbalancer': {'name': 'lb1', + 'description': 'descr_lb1', + 'tenant_id': _uuid(), + 'vip_subnet_id': _uuid(), + 'admin_state_up': True, + 'flavor_id': 123, + 'vip_address': '127.0.0.1'}} + res = self.api.post(_get_path('lbaas/loadbalancers', fmt=self.fmt), + self.serialize(data), + content_type='application/{0}'.format(self.fmt), + expect_errors=True) + self.assertEqual(400, res.status_int) + + def test_loadbalancer_create_valid_flavor(self): + data = {'loadbalancer': {'name': 'lb1', + 'description': 'descr_lb1', + 'tenant_id': _uuid(), + 'vip_subnet_id': _uuid(), + 'admin_state_up': True, + 'flavor_id': _uuid(), + 'vip_address': '127.0.0.1'}} + res = self.api.post(_get_path('lbaas/loadbalancers', fmt=self.fmt), + self.serialize(data), + content_type='application/{0}'.format(self.fmt), + expect_errors=True) + self.assertEqual(201, res.status_int) + def test_loadbalancer_list(self): lb_id = _uuid() return_value = [{'name': 'lb1', diff --git a/neutron_lbaas/tests/unit/test_agent_scheduler.py b/neutron_lbaas/tests/unit/test_agent_scheduler.py index a89a93126..f1b83cb4f 100644 --- a/neutron_lbaas/tests/unit/test_agent_scheduler.py +++ b/neutron_lbaas/tests/unit/test_agent_scheduler.py @@ -172,6 +172,7 @@ class LBaaSAgentSchedulerTestCase(test_agent.AgentDBTestMixIn, 'loadbalancer': { 'vip_subnet_id': subnet['id'], 'provider': 'lbaas', + 'flavor_id': attributes.ATTR_NOT_SPECIFIED, 'vip_address': attributes.ATTR_NOT_SPECIFIED, 'admin_state_up': True, 'tenant_id': self._tenant_id}} @@ -206,6 +207,7 @@ class LBaaSAgentSchedulerTestCase(test_agent.AgentDBTestMixIn, 'loadbalancer': { 'vip_subnet_id': subnet['id'], 'provider': 'lbaas', + 'flavor_id': attributes.ATTR_NOT_SPECIFIED, 'vip_address': attributes.ATTR_NOT_SPECIFIED, 'admin_state_up': True, 'tenant_id': self._tenant_id}}