Merge "Fix a lifecycle bug with child objects"

This commit is contained in:
Zuul 2019-03-21 00:24:58 +00:00 committed by Gerrit Code Review
commit 68b9d06dae
8 changed files with 111 additions and 0 deletions

View File

@ -92,6 +92,9 @@ class HealthMonitorController(base.BaseController):
# mutable # mutable
pool = self.repositories.pool.get(session, id=hm.pool_id) pool = self.repositories.pool.get(session, id=hm.pool_id)
load_balancer_id = pool.load_balancer_id load_balancer_id = pool.load_balancer_id
# Check the parent is not locked for some reason (ERROR, etc.)
if pool.provisioning_status not in consts.MUTABLE_STATUSES:
raise exceptions.ImmutableObject(resource='Pool', id=hm.pool_id)
if not self.repositories.test_and_set_lb_and_listeners_prov_status( if not self.repositories.test_and_set_lb_and_listeners_prov_status(
session, load_balancer_id, session, load_balancer_id,
consts.PENDING_UPDATE, consts.PENDING_UPDATE, consts.PENDING_UPDATE, consts.PENDING_UPDATE,

View File

@ -87,6 +87,10 @@ class L7RuleController(base.BaseController):
l7policy = self._get_db_l7policy(session, self.l7policy_id) l7policy = self._get_db_l7policy(session, self.l7policy_id)
listener_id = l7policy.listener_id listener_id = l7policy.listener_id
load_balancer_id = l7policy.listener.load_balancer_id load_balancer_id = l7policy.listener.load_balancer_id
# Check the parent is not locked for some reason (ERROR, etc.)
if l7policy.provisioning_status not in constants.MUTABLE_STATUSES:
raise exceptions.ImmutableObject(resource='L7Policy',
id=self.l7policy_id)
if not self.repositories.test_and_set_lb_and_listeners_prov_status( if not self.repositories.test_and_set_lb_and_listeners_prov_status(
session, load_balancer_id, session, load_balancer_id,
constants.PENDING_UPDATE, constants.PENDING_UPDATE, constants.PENDING_UPDATE, constants.PENDING_UPDATE,

View File

@ -54,6 +54,8 @@ class MemberController(base.BaseController):
self._auth_validate_action(context, db_member.project_id, self._auth_validate_action(context, db_member.project_id,
constants.RBAC_GET_ONE) constants.RBAC_GET_ONE)
self._validate_pool_id(id, db_member.pool_id)
result = self._convert_db_to_type( result = self._convert_db_to_type(
db_member, member_types.MemberResponse) db_member, member_types.MemberResponse)
if fields is not None: if fields is not None:
@ -98,6 +100,9 @@ class MemberController(base.BaseController):
# We need to verify that any listeners referencing this member's # We need to verify that any listeners referencing this member's
# pool are also mutable # pool are also mutable
pool = self._get_db_pool(session, self.pool_id) pool = self._get_db_pool(session, self.pool_id)
# Check the parent is not locked for some reason (ERROR, etc.)
if pool.provisioning_status not in constants.MUTABLE_STATUSES:
raise exceptions.ImmutableObject(resource='Pool', id=self.pool_id)
load_balancer_id = pool.load_balancer_id load_balancer_id = pool.load_balancer_id
if not self.repositories.test_and_set_lb_and_listeners_prov_status( if not self.repositories.test_and_set_lb_and_listeners_prov_status(
session, load_balancer_id, session, load_balancer_id,
@ -129,6 +134,10 @@ class MemberController(base.BaseController):
# do not give any information as to what constraint failed # do not give any information as to what constraint failed
raise exceptions.InvalidOption(value='', option='') raise exceptions.InvalidOption(value='', option='')
def _validate_pool_id(self, member_id, db_member_pool_id):
if db_member_pool_id != self.pool_id:
raise exceptions.NotFound(resource='Member', id=member_id)
@wsme_pecan.wsexpose(member_types.MemberRootResponse, @wsme_pecan.wsexpose(member_types.MemberRootResponse,
body=member_types.MemberRootPOST, status_code=201) body=member_types.MemberRootPOST, status_code=201)
def post(self, member_): def post(self, member_):
@ -214,6 +223,8 @@ class MemberController(base.BaseController):
self._auth_validate_action(context, project_id, constants.RBAC_PUT) self._auth_validate_action(context, project_id, constants.RBAC_PUT)
self._validate_pool_id(id, db_member.pool_id)
# Load the driver early as it also provides validation # Load the driver early as it also provides validation
driver = driver_factory.get_driver(provider) driver = driver_factory.get_driver(provider)
@ -266,6 +277,8 @@ class MemberController(base.BaseController):
self._auth_validate_action(context, project_id, constants.RBAC_DELETE) self._auth_validate_action(context, project_id, constants.RBAC_DELETE)
self._validate_pool_id(id, db_member.pool_id)
# Load the driver early as it also provides validation # Load the driver early as it also provides validation
driver = driver_factory.get_driver(provider) driver = driver_factory.get_driver(provider)

View File

@ -87,6 +87,22 @@ class TaskUtils(object):
"provisioning status to ERROR due to: " "provisioning status to ERROR due to: "
"%(except)s", {'health': health_mon_id, 'except': e}) "%(except)s", {'health': health_mon_id, 'except': e})
def mark_l7policy_prov_status_active(self, l7policy_id):
"""Sets a L7 policy provisioning status to ACTIVE.
NOTE: This should only be called from revert methods.
:param l7policy_id: L7 Policy ID to set provisioning status to ACTIVE
"""
try:
self.l7policy_repo.update(db_apis.get_session(),
id=l7policy_id,
provisioning_status=constants.ACTIVE)
except Exception as e:
LOG.error("Failed to update l7policy %(l7p)s "
"provisioning status to ACTIVE due to: "
"%(except)s", {'l7p': l7policy_id, 'except': e})
def mark_l7policy_prov_status_error(self, l7policy_id): def mark_l7policy_prov_status_error(self, l7policy_id):
"""Sets a L7 policy provisioning status to ERROR. """Sets a L7 policy provisioning status to ERROR.

View File

@ -53,6 +53,7 @@ class HealthMonitorToErrorOnRevertTask(BaseLifecycleTask):
def revert(self, health_mon, listeners, loadbalancer, *args, **kwargs): def revert(self, health_mon, listeners, loadbalancer, *args, **kwargs):
self.task_utils.mark_health_mon_prov_status_error(health_mon.pool_id) self.task_utils.mark_health_mon_prov_status_error(health_mon.pool_id)
self.task_utils.mark_pool_prov_status_active(health_mon.pool_id)
self.task_utils.mark_loadbalancer_prov_status_active(loadbalancer.id) self.task_utils.mark_loadbalancer_prov_status_active(loadbalancer.id)
for listener in listeners: for listener in listeners:
self.task_utils.mark_listener_prov_status_active(listener.id) self.task_utils.mark_listener_prov_status_active(listener.id)
@ -79,6 +80,7 @@ class L7RuleToErrorOnRevertTask(BaseLifecycleTask):
def revert(self, l7rule, listeners, loadbalancer, *args, **kwargs): def revert(self, l7rule, listeners, loadbalancer, *args, **kwargs):
self.task_utils.mark_l7rule_prov_status_error(l7rule.id) self.task_utils.mark_l7rule_prov_status_error(l7rule.id)
self.task_utils.mark_l7policy_prov_status_active(l7rule.l7policy_id)
self.task_utils.mark_loadbalancer_prov_status_active(loadbalancer.id) self.task_utils.mark_loadbalancer_prov_status_active(loadbalancer.id)
for listener in listeners: for listener in listeners:
self.task_utils.mark_listener_prov_status_active(listener.id) self.task_utils.mark_listener_prov_status_active(listener.id)

View File

@ -22,6 +22,7 @@ from octavia.common import constants
import octavia.common.context import octavia.common.context
from octavia.common import data_models from octavia.common import data_models
from octavia.common import exceptions from octavia.common import exceptions
from octavia.db import repositories
from octavia.tests.functional.api.v2 import base from octavia.tests.functional.api.v2 import base
@ -53,6 +54,7 @@ class TestHealthMonitor(base.BaseAPITest):
self.pool_with_listener_id = ( self.pool_with_listener_id = (
self.pool_with_listener.get('pool').get('id')) self.pool_with_listener.get('pool').get('id'))
self.set_lb_status(self.lb_id) self.set_lb_status(self.lb_id)
self.pool_repo = repositories.PoolRepository()
self._setup_udp_lb_resources() self._setup_udp_lb_resources()
def _setup_udp_lb_resources(self): def _setup_udp_lb_resources(self):
@ -999,6 +1001,24 @@ class TestHealthMonitor(base.BaseAPITest):
self.conf.config(group='api_settings', auth_strategy=auth_strategy) self.conf.config(group='api_settings', auth_strategy=auth_strategy)
self.assertEqual(self.NOT_AUTHORIZED_BODY, api_hm) self.assertEqual(self.NOT_AUTHORIZED_BODY, api_hm)
def test_create_pool_in_error(self):
project_id = uuidutils.generate_uuid()
lb1 = self.create_load_balancer(uuidutils.generate_uuid(), name='lb1',
project_id=project_id)
lb1_id = lb1.get('loadbalancer').get('id')
self.set_lb_status(lb1_id)
pool1 = self.create_pool(
lb1_id, constants.PROTOCOL_HTTP,
constants.LB_ALGORITHM_ROUND_ROBIN).get('pool')
pool1_id = pool1.get('id')
self.set_lb_status(lb1_id)
self.set_object_status(self.pool_repo, pool1_id,
provisioning_status=constants.ERROR)
api_hm = self.create_health_monitor(
pool1_id, constants.HEALTH_MONITOR_HTTP, 1, 1, 1, 1, status=409)
ref_msg = 'Pool %s is immutable and cannot be updated.' % pool1_id
self.assertEqual(ref_msg, api_hm.get('faultstring'))
def test_create_with_listener(self): def test_create_with_listener(self):
api_hm = self.create_health_monitor( api_hm = self.create_health_monitor(
self.pool_with_listener_id, constants.HEALTH_MONITOR_HTTP, self.pool_with_listener_id, constants.HEALTH_MONITOR_HTTP,

View File

@ -20,6 +20,7 @@ from oslo_utils import uuidutils
from octavia.common import constants from octavia.common import constants
import octavia.common.context import octavia.common.context
from octavia.common import exceptions from octavia.common import exceptions
from octavia.db import repositories
from octavia.tests.functional.api.v2 import base from octavia.tests.functional.api.v2 import base
@ -46,6 +47,7 @@ class TestL7Rule(base.BaseAPITest):
self.l7rules_path = self.L7RULES_PATH.format( self.l7rules_path = self.L7RULES_PATH.format(
l7policy_id=self.l7policy_id) l7policy_id=self.l7policy_id)
self.l7rule_path = self.l7rules_path + '/{l7rule_id}' self.l7rule_path = self.l7rules_path + '/{l7rule_id}'
self.l7policy_repo = repositories.L7PolicyRepository()
def test_get(self): def test_get(self):
l7rule = self.create_l7rule( l7rule = self.create_l7rule(
@ -541,6 +543,21 @@ class TestL7Rule(base.BaseAPITest):
self.conf.config(group='api_settings', auth_strategy=auth_strategy) self.conf.config(group='api_settings', auth_strategy=auth_strategy)
self.assertEqual(self.NOT_AUTHORIZED_BODY, api_l7rule) self.assertEqual(self.NOT_AUTHORIZED_BODY, api_l7rule)
def test_create_l7policy_in_error(self):
l7policy = self.create_l7policy(
self.listener_id, constants.L7POLICY_ACTION_REJECT)
l7policy_id = l7policy.get('l7policy').get('id')
self.set_lb_status(self.lb_id)
self.set_object_status(self.l7policy_repo, l7policy_id,
provisioning_status=constants.ERROR)
api_l7rule = self.create_l7rule(
l7policy_id, constants.L7RULE_TYPE_HOST_NAME,
constants.L7RULE_COMPARE_TYPE_EQUAL_TO,
'www.example.com', status=409)
ref_msg = ('L7Policy %s is immutable and cannot be updated.' %
l7policy_id)
self.assertEqual(ref_msg, api_l7rule.get('faultstring'))
def test_create_path_rule(self): def test_create_path_rule(self):
api_l7rule = self.create_l7rule( api_l7rule = self.create_l7rule(
self.l7policy_id, constants.L7RULE_TYPE_PATH, self.l7policy_id, constants.L7RULE_TYPE_PATH,

View File

@ -23,6 +23,7 @@ from octavia.common import constants
import octavia.common.context import octavia.common.context
from octavia.common import data_models from octavia.common import data_models
from octavia.common import exceptions from octavia.common import exceptions
from octavia.db import repositories
from octavia.network import base as network_base from octavia.network import base as network_base
from octavia.tests.functional.api.v2 import base from octavia.tests.functional.api.v2 import base
@ -61,6 +62,7 @@ class TestMember(base.BaseAPITest):
self.members_path_listener = self.MEMBERS_PATH.format( self.members_path_listener = self.MEMBERS_PATH.format(
pool_id=self.pool_with_listener_id) pool_id=self.pool_with_listener_id)
self.member_path_listener = self.members_path_listener + '/{member_id}' self.member_path_listener = self.members_path_listener + '/{member_id}'
self.pool_repo = repositories.PoolRepository()
def test_get(self): def test_get(self):
api_member = self.create_member( api_member = self.create_member(
@ -527,6 +529,23 @@ class TestMember(base.BaseAPITest):
self.conf.config(group='api_settings', auth_strategy=auth_strategy) self.conf.config(group='api_settings', auth_strategy=auth_strategy)
self.assertEqual(self.NOT_AUTHORIZED_BODY, api_member) self.assertEqual(self.NOT_AUTHORIZED_BODY, api_member)
def test_create_pool_in_error(self):
project_id = uuidutils.generate_uuid()
lb1 = self.create_load_balancer(uuidutils.generate_uuid(), name='lb1',
project_id=project_id)
lb1_id = lb1.get('loadbalancer').get('id')
self.set_lb_status(lb1_id)
pool1 = self.create_pool(
lb1_id, constants.PROTOCOL_HTTP,
constants.LB_ALGORITHM_ROUND_ROBIN).get('pool')
pool1_id = pool1.get('id')
self.set_lb_status(lb1_id)
self.set_object_status(self.pool_repo, pool1_id,
provisioning_status=constants.ERROR)
api_member = self.create_member(pool1_id, '192.0.2.1', 80, status=409)
ref_msg = 'Pool %s is immutable and cannot be updated.' % pool1_id
self.assertEqual(ref_msg, api_member.get('faultstring'))
# TODO(rm_work) Remove after deprecation of project_id in POST (R series) # TODO(rm_work) Remove after deprecation of project_id in POST (R series)
def test_create_with_project_id_is_ignored(self): def test_create_with_project_id_is_ignored(self):
pid = uuidutils.generate_uuid() pid = uuidutils.generate_uuid()
@ -1162,6 +1181,23 @@ class TestMember(base.BaseAPITest):
self.delete(self.member_path.format( self.delete(self.member_path.format(
member_id=uuidutils.generate_uuid()), status=404) member_id=uuidutils.generate_uuid()), status=404)
def test_delete_mismatch_pool(self):
# Create a pool that will not have the member, but is valid.
self.pool = self.create_pool(self.lb_id, constants.PROTOCOL_HTTP,
constants.LB_ALGORITHM_ROUND_ROBIN)
bad_pool_id = self.pool.get('pool').get('id')
self.set_lb_status(self.lb_id)
# Create a member on our reference pool
api_member = self.create_member(
self.pool_with_listener_id, '192.0.2.1', 80).get(self.root_tag)
self.set_lb_status(self.lb_id)
# Attempt to delete the member using the wrong pool in the path
member_path = self.MEMBERS_PATH.format(
pool_id=bad_pool_id) + '/' + api_member['id']
result = self.delete(member_path, status=404).json
ref_msg = 'Member %s not found.' % api_member['id']
self.assertEqual(ref_msg, result.get('faultstring'))
@mock.patch('octavia.api.drivers.utils.call_provider') @mock.patch('octavia.api.drivers.utils.call_provider')
def test_delete_with_bad_provider(self, mock_provider): def test_delete_with_bad_provider(self, mock_provider):
api_member = self.create_member( api_member = self.create_member(