a10networks / a10-neutron-lbaas

A10 Networks, Openstack Neutron LBaaS Driver
Apache License 2.0
9 stars 19 forks source link

Pool delete fails in lbaasv2 liberty when pool has members #285

Open Cedev opened 8 years ago

Cedev commented 8 years ago

Deleting a pool fails in lbaasv2 liberty when the pool still has members

[req-7bb30608-1423-4ed3-a6ca-409ce743b8c7 admin dc7b05045d1346bf9a70d70a461b7d8f] There was an error in the driver
Traceback (most recent call last):
  File "/opt/stack/neutron-lbaas/neutron_lbaas/services/loadbalancer/plugin.py", line 471, in _call_driver_operation
    driver_method(context, db_entity)
  File "/opt/stack/neutron-lbaas/neutron_lbaas/drivers/a10networks/driver_v2.py", line 80, in delete
    self.driver.a10.pool.delete(context, pool)
  File "/usr/local/src/a10-neutron-lbaas/a10_neutron_lbaas/v2/handler_pool.py", line 68, in delete
    self.a10_driver.member._delete(c, context, member)
  File "/usr/local/src/a10-neutron-lbaas/a10_neutron_lbaas/v2/handler_member.py", line 95, in _delete
    self._pool_name(context, pool=member.pool),
  File "/usr/local/src/a10-neutron-lbaas/a10_neutron_lbaas/handler_base.py", line 39, in _pool_name
    pool = self.neutron.pool_get(context, pool_id)
  File "/usr/local/src/a10-neutron-lbaas/a10_neutron_lbaas/v2/neutron_ops.py", line 47, in pool_get
    return self.plugin.db.get_pool(context, pool_id)
  File "/opt/stack/neutron-lbaas/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py", line 479, in get_pool
    pool_db = self._get_resource(context, models.PoolV2, id)
  File "/opt/stack/neutron-lbaas/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py", line 70, in _get_resource
    raise loadbalancerv2.EntityNotFound(name=model.NAME, id=id)
EntityNotFound: pool None could not be found

Workaround: delete the members first

Cedev commented 8 years ago

When the pool delete calls the member's delete the member's .pool isn't populated

    def delete(self, context, pool):
        with a10.A10DeleteContext(self, context, pool) as c:
            for member in pool.members:
                self.a10_driver.member._delete(c, context, member)

The member tries to get the _pool_name from the .pool, which isn't populated:

    def _delete(self, c, context, member):
        server_ip = self.neutron.member_get_ip(
            context, member, c.device_cfg['use_float'])
        server_name = self._meta_name(member, server_ip)

        try:
            if self.neutron.member_count(context, member) > 1:
                c.client.slb.service_group.member.delete(
                    self._pool_name(context, pool=member.pool),
                    server_name,
                    member.protocol_port)
            else:
                c.client.slb.server.delete(server_name)
        except acos_errors.NotFound:
            pass

_pool_name also accepts a pool_id and looks up the pool when the pool isn't specified.

There are two possible fixes. Set the .pool on each member in the pool handler:

    def delete(self, context, pool):
        with a10.A10DeleteContext(self, context, pool) as c:
            for member in pool.members:
                member.pool = pool
                self.a10_driver.member._delete(c, context, member)

Or pass the pool_id to _pool_name in the member handler delete:

                c.client.slb.service_group.member.delete(
                    self._pool_name(context, pool_id = member.pool_id, pool=member.pool),
                    server_name,
                    member.protocol_port)
dougwig commented 8 years ago

Wait, if the member is still associated to the pool, why isn't pool populated?

Cedev commented 8 years ago

Because the neutron lbaas data_models don't populate the backreferences.

In liberty:

    @classmethod
    def from_sqlalchemy_model(cls, sa_model, calling_class=None):
        ...
        for attr_name in vars(instance):
            ...
            # Handles M:1 or 1:1 relationships
            if isinstance(attr, model_base.BASEV2):
                if hasattr(instance, attr_name):
                    data_class = SA_MODEL_TO_DATA_MODEL_MAP[attr.__class__]
                    if calling_class != data_class and data_class:
                        setattr(instance, attr_name,
                                data_class.from_sqlalchemy_model(
                                    attr, calling_class=cls))

In master it keeps populating them until it's seen the same class twice, but doesn't reuse instances. Which means that pool.members[0].pool is not pool. Warning, the linked code will make you physcially ill.

Cedev commented 8 years ago

Seriously, why doesn't the code in master reuse the existing instances and tie them together?

    @classmethod
    def from_sqlalchemy_model(cls, sa_model, translated=None):
        translated = translated or {}
        # Reuse already translated instances 
        try:
            return translated[id(sa_model)]
        except KeyError as e:
            pass
        attr_mapping = vars(cls).get("attr_mapping")
        instance = cls()
        translated[id(sa_model)] = instance
        ...
ghost commented 8 years ago

See PR #316

Cedev commented 8 years ago

Leaving this bug open until a fix gets merged into master.