From fa9c31eb5898b924daa2d9491f66f6f3517ca623 Mon Sep 17 00:00:00 2001 From: Rudra Rugge Date: Mon, 6 Oct 2014 17:16:13 -0700 Subject: [PATCH] [LBAAS fixes] - Fix message for associated healthmonitor delete - Prevent second vip association to a pool already associated - Update VIP with different pool - Update member with a different pool Change-Id: Ia20d24efb0ae9d6505036ca63f9b10f8abf38e61 --- .../opencontrail/loadbalancer/driver.py | 8 ++-- .../loadbalancer_healthmonitor.py | 13 +++++- .../loadbalancer/loadbalancer_member.py | 42 +++++++++++++++++++ .../loadbalancer/loadbalancer_pool.py | 3 ++ .../loadbalancer/resource_manager.py | 9 +++- .../opencontrail/loadbalancer/virtual_ip.py | 33 +++++++++++---- 6 files changed, 92 insertions(+), 16 deletions(-) diff --git a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/driver.py b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/driver.py index ff630c1..6b57707 100644 --- a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/driver.py +++ b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/driver.py @@ -161,7 +161,6 @@ def _update_loadbalancer_instance(self, pool_id, vip_id): try: si_obj = self._api.service_instance_read(fq_name=fq_name) update = self._service_instance_update_props(si_obj, props) - # TODO: update template if necessary if update: self._api.service_instance_update(si_obj) @@ -220,12 +219,13 @@ def update_vip(self, context, old_vip, vip): """Driver may call the code below in order to update the status. self.plugin.update_status(context, Vip, id, constants.ACTIVE) """ - if vip['pool_id']: - self._update_loadbalancer_instance(vip['pool_id'], vip['id']) - elif old_vip['pool_id']: + if old_vip['pool_id'] != vip['pool_id']: self._clear_loadbalancer_instance( old_vip['tenant_id'], old_vip['pool_id']) + if vip['pool_id']: + self._update_loadbalancer_instance(vip['pool_id'], vip['id']) + def delete_vip(self, context, vip): """A real driver would invoke a call to his backend and try to delete the Vip. diff --git a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_healthmonitor.py b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_healthmonitor.py index a387b38..f3a18d2 100644 --- a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_healthmonitor.py +++ b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_healthmonitor.py @@ -47,6 +47,14 @@ def make_dict(self, health_monitor, fields=None): if value is not None: res[mapping] = value + pool_ids = [] + pool_back_refs = health_monitor.get_loadbalancer_pool_back_refs() + for pool_back_ref in pool_back_refs or []: + pool_id = {} + pool_id['pool_id'] = pool_back_ref['uuid'] + pool_ids.append(pool_id) + res['pools'] = pool_ids + return self._fields(res, fields) def resource_read(self, id): @@ -66,7 +74,10 @@ def resource_delete(self, id): return self._api.loadbalancer_healthmonitor_delete(id=id) def get_exception_notfound(self, id=None): - return loadbalancer.HealthMonitorNotFound(pool_id=id) + return loadbalancer.HealthMonitorNotFound(monitor_id=id) + + def get_exception_inuse(self, id=None): + return loadbalancer.HealthMonitorInUse(monitor_id=id) @property def neutron_name(self): diff --git a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_member.py b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_member.py index 8b8265a..e5c5f58 100644 --- a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_member.py +++ b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_member.py @@ -32,6 +32,10 @@ def make_properties(self, member): setattr(props, key, member[mapping]) return props + def _get_member_pool_id(self, member): + pool_uuid = member.parent_uuid + return pool_uuid + def make_dict(self, member, fields=None): res = {'id': member.uuid, 'pool_id': member.parent_uuid, @@ -106,6 +110,9 @@ def resource_delete(self, id): def get_exception_notfound(self, id=None): return loadbalancer.MemberNotFound(member_id=id) + def get_exception_inuse(self, id=None): + pass + @property def neutron_name(self): return "member" @@ -146,3 +153,38 @@ def update_properties(self, member_db, id, m): member_db.set_loadbalancer_member_properties(props) return True return False + + def update_object(self, member_db, id, m): + if 'pool_id' in m and self._get_member_pool_id(member_db) != m['pool_id']: + try: + pool = self._api.loadbalancer_pool_read(id=m['pool_id']) + except NoIdError: + raise loadbalancer.PoolNotFound(pool_id=m['pool_id']) + + db_props = member_db.get_loadbalancer_member_properties() + members = pool.get_loadbalancer_members() + for member in members or []: + member_obj = self._api.loadbalancer_member_read( + id=member['uuid']) + props = member_obj.get_loadbalancer_member_properties() + if ((props.get_address() == db_props.get_address()) and + (props.get_protocol_port() == db_props.get_protocol_port())): + raise loadbalancer.MemberExists( + address=props.get_address(), + port=props.get_protocol_port(), + pool=m['pool_id']) + + # delete member from old pool + props = member_db.get_loadbalancer_member_properties() + obj_uuid = member_db.uuid + self._api.loadbalancer_member_delete(id=member_db.uuid) + + # create member for the new pool with same uuid and props + id_perms = IdPermsType(enable=True) + member_obj = LoadbalancerMember( + obj_uuid, pool, loadbalancer_member_properties=props, + id_perms=id_perms) + member_obj.uuid = obj_uuid + self._api.loadbalancer_member_create(member_obj) + + return True diff --git a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_pool.py b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_pool.py index 2c467c7..0c4200c 100644 --- a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_pool.py +++ b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/loadbalancer_pool.py @@ -93,6 +93,9 @@ def resource_delete(self, id): def get_exception_notfound(self, id=None): return loadbalancer.PoolNotFound(pool_id=id) + def get_exception_inuse(self, id=None): + return loadbalancer.PoolInUse(pool_id=id) + @property def neutron_name(self): return "pool" diff --git a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/resource_manager.py b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/resource_manager.py index 5679a4c..f72eb69 100644 --- a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/resource_manager.py +++ b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/resource_manager.py @@ -71,6 +71,12 @@ def get_exception_notfound(self, id): """ pass + @abstractproperty + def get_exception_inuse(self, id): + """ Returns the correct NotFound exception. + """ + pass + @abstractproperty def neutron_name(self): """ Resource name in a request from neutron. @@ -235,13 +241,12 @@ def delete(self, context, id): if tenant_id != project_id: raise n_exc.NotAuthorized() - # TODO: possible exceptions: RefsExistError try: self.resource_delete(id=id) except NoIdError: raise self.get_exception_notfound(id=id) except RefsExistError: - raise loadbalancer.PoolInUse(pool_id=id) + raise self.get_exception_inuse(id=id) def update_properties_subr(self, props, resource): """ Update the DB properties object from the neutron parameters. diff --git a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/virtual_ip.py b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/virtual_ip.py index 971fdb3..88b56e1 100644 --- a/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/virtual_ip.py +++ b/neutron_plugin_contrail/plugins/opencontrail/loadbalancer/virtual_ip.py @@ -122,6 +122,9 @@ def resource_delete(self, id): def get_exception_notfound(self, id=None): return loadbalancer.VipNotFound(vip_id=id) + def get_exception_inuse(self, id=None): + pass + @property def neutron_name(self): return "vip" @@ -192,10 +195,12 @@ def create(self, context, vip): project_id = pool.parent_uuid if str(uuid.UUID(tenant_id)) != project_id: raise n_exc.NotAuthorized() - # TODO: check that the pool has no vip configured - # if pool.protocol != v['protocol']: - # raise loadbalancer.ProtocolMismatch( - # vip_proto=v['protocol'], pool_proto=pool.protocol) + protocol = pool.get_loadbalancer_pool_properties().get_protocol() + if protocol != v['protocol']: + raise loadbalancer.ProtocolMismatch( + vip_proto=v['protocol'], pool_proto=protocol) + if pool.get_virtual_ip_back_refs(): + raise loadbalancer.VipExists(pool_id=v['pool_id']) else: pool = None @@ -276,11 +281,21 @@ def update_object(self, vip_db, id, v): raise loadbalancer.PoolNotFound(pool_id=v['pool_id']) if vip_db.parent_uuid != pool.parent_uuid: raise n_exc.NotAuthorized() - # TODO: check that the pool has no vip configured - # TODO: check that the protocol matches - # TODO: check that the pool is in valid state - # TODO: check that the provider is the same. - vip_db.set_localbalancer_pool(pool) + + # check that the pool has no vip configured + if pool.get_virtual_ip_back_refs(): + raise loadbalancer.VipExists(pool_id=pool_obj.uuid) + + # check that the protocol matches + pool_props = pool.get_loadbalancer_pool_properties() + vip_props = vip_db.get_virtual_ip_properties() + if pool_props.get_protocol() != vip_props.get_protocol(): + raise loadbalancer.ProtocolMismatch( + vip_proto=vip_props.get_protocol(), + pool_proto=pool_props.get_protocol()) + + # update vip + vip_db.set_loadbalancer_pool(pool) return True return False