Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

9.10 fix delete #2058

Open
wants to merge 2 commits into
base: 9.10-stable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion f5_openstack_agent/lbaasv2/drivers/bigip/icontrol_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import json
import os

from requests import HTTPError
from time import strftime

from oslo_config import cfg
Expand Down Expand Up @@ -835,10 +836,12 @@ def annotate_service_members(self, service):
self.network_builder._annotate_service_route_domains(service)
except f5ex.InvalidNetworkType as exc:
LOG.warning(exc.message)
except HTTPError as err:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change improve anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will throw the original HTTPError, rather cover up by f5ex.RouteDomainCreationException.
The caller will not tell what Exception happens exactly.

raise err
except Exception as err:
LOG.exception(err)
raise f5ex.RouteDomainCreationException(
"Route domain annotation error")
"Service Member Route domain annotation error")

def update_service_status(self, service, timed_out=False):
"""Update status of objects in controller."""
Expand Down
12 changes: 7 additions & 5 deletions f5_openstack_agent/lbaasv2/drivers/bigip/network_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,6 @@ def create_rd_by_net(
exists = self.network_helper.route_domain_exists(
bigip, partition=partition, name=name
)

if exists:
LOG.info("route domain: %s, %s exists on bigip: %s"
% (name, route_domain, bigip.hostname))
Expand All @@ -411,13 +410,16 @@ def create_rd_by_net(

LOG.info("create route domain: %s, %s on bigip: %s"
% (name, route_domain, bigip.hostname))
except Exception as ex:
if ex.response.status_code == 409:
except HTTPError as err:
if err.response.status_code == 409:
LOG.info("route domain %s already exists: %s, ignored.." % (
route_domain, ex.message))
route_domain, err.message))
elif err.response.status_code == 400:
zhang-shengping marked this conversation as resolved.
Show resolved Hide resolved
LOG.info("maybe partition is misssing, dirty data.")
raise err
else:
# FIXME(pzhang): what to do with multiple agent race?
LOG.error(ex.message)
LOG.error(err.message)
raise f5_ex.RouteDomainCreationException(
"Failed to create route domain: %s, %s on bigip %s"
% (name, route_domain, bigip.hostname)
Expand Down
27 changes: 24 additions & 3 deletions f5_openstack_agent/lbaasv2/drivers/bigip/resource_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1612,6 +1612,13 @@ def _delete(self, bigip, payload, pool, service):
service['listener'] = listener
""" Unmap listener and pool"""
vs = self.driver.service_adapter.get_virtual_name(service)
vs_exist = mgr.resource_helper.exists(
bigip, name=vs['name'],
partition=vs['partition']
)
if not vs_exist:
continue

vs['pool'] = ""
# Need to remove persist profile from virtual server,
# if its persist profile is configured by its default pool.
Expand Down Expand Up @@ -1641,7 +1648,13 @@ def update(self, old_pool, pool, service, **kwargs):
@serialized('PoolManager.delete')
@log_helpers.log_method_call
def delete(self, pool, service, **kwargs):
self.driver.annotate_service_members(service)
try:
self.driver.annotate_service_members(service)
except HTTPError as err:
if err.response.status_code == 400:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to ignore it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to ignore it?

Olvan delete the whole partition, and nothing is left. When you try to annotate the route domain for members, the partition is disappeared, and the method returns 400 HTTPError. It will break the pool deleting process, it cause the pool cannot be deleted in the Neutron DB at last.

LOG.debug(err)
else:
raise err
super(PoolManager, self).delete(pool, service)


Expand Down Expand Up @@ -1730,8 +1743,16 @@ def _delete(self, bigip, payload, healthmonitor, service):
)
pool_payload['monitor'] = ''
try:
# update the pool
mgr._update(bigip, pool_payload, None, None, service)
# check if pool exist, if not exist,
# we delete healthmonitor directly
exist = mgr.resource_helper.exists(
bigip,
name=pool_payload['name'],
partition=pool_payload['partition']
)
if exist:
# update the pool
mgr._update(bigip, pool_payload, None, None, service)

super(MonitorManager, self)._delete(
bigip, payload, healthmonitor, service
Expand Down