diff --git a/octavia/api/drivers/driver_lib.py b/octavia/api/drivers/driver_lib.py index 242b995a68..2f605cfa3b 100644 --- a/octavia/api/drivers/driver_lib.py +++ b/octavia/api/drivers/driver_lib.py @@ -14,6 +14,7 @@ from octavia.api.drivers import exceptions as driver_exceptions from octavia.common import constants as consts +from octavia.common import utils from octavia.db import api as db_apis from octavia.db import repositories as repo @@ -33,6 +34,16 @@ class DriverLibrary(object): self.db_session = db_apis.get_session() super(DriverLibrary, self).__init__(**kwargs) + def _check_for_lb_vip_deallocate(self, repo, lb_id): + lb = repo.get(self.db_session, id=lb_id) + if lb.vip.octavia_owned: + vip = lb.vip + # We need a backreference + vip.load_balancer = lb + # Only lookup the network driver if we have a VIP to deallocate + network_driver = utils.get_network_driver() + network_driver.deallocate_vip(vip) + def _process_status_update(self, repo, object_name, record, delete_record=False): # Zero it out so that if the ID is missing from a record we do not @@ -43,11 +54,13 @@ class DriverLibrary(object): record_kwargs = {} prov_status = record.get(consts.PROVISIONING_STATUS, None) if prov_status: - if prov_status == consts.DELETED and delete_record: + if (prov_status == consts.DELETED and + object_name == consts.LOADBALANCERS): + self._check_for_lb_vip_deallocate(repo, record_id) + elif prov_status == consts.DELETED and delete_record: repo.delete(self.db_session, id=record_id) return - else: - record_kwargs[consts.PROVISIONING_STATUS] = prov_status + record_kwargs[consts.PROVISIONING_STATUS] = prov_status op_status = record.get(consts.OPERATING_STATUS, None) if op_status: record_kwargs[consts.OPERATING_STATUS] = op_status diff --git a/octavia/api/v2/controllers/load_balancer.py b/octavia/api/v2/controllers/load_balancer.py index 9dec7f74e6..5adef77403 100644 --- a/octavia/api/v2/controllers/load_balancer.py +++ b/octavia/api/v2/controllers/load_balancer.py @@ -290,6 +290,7 @@ class LoadBalancersController(base.BaseController): lock_session, lb_dict, vip_dict) # See if the provider driver wants to create the VIP port + octavia_owned = False try: provider_vip_dict = driver_utils.vip_dict_to_provider_dict( vip_dict) @@ -303,13 +304,15 @@ class LoadBalancersController(base.BaseController): vip = self._create_vip_port_if_not_exist(db_lb) LOG.info('Created VIP port %s for provider %s.', vip.port_id, driver.name) + # If a port_id wasn't passed in and we made it this far + # we created the VIP + if 'port_id' not in vip_dict or not vip_dict['port_id']: + octavia_owned = True self.repositories.vip.update( - lock_session, db_lb.id, - ip_address=vip.ip_address, - port_id=vip.port_id, - network_id=vip.network_id, - subnet_id=vip.subnet_id) + lock_session, db_lb.id, ip_address=vip.ip_address, + port_id=vip.port_id, network_id=vip.network_id, + subnet_id=vip.subnet_id, octavia_owned=octavia_owned) if listeners or pools: db_pools, db_lists = self._graph_create( diff --git a/octavia/common/data_models.py b/octavia/common/data_models.py index f0af12c8ff..38d6067303 100644 --- a/octavia/common/data_models.py +++ b/octavia/common/data_models.py @@ -471,7 +471,7 @@ class Vip(BaseDataModel): def __init__(self, load_balancer_id=None, ip_address=None, subnet_id=None, network_id=None, port_id=None, - load_balancer=None, qos_policy_id=None): + load_balancer=None, qos_policy_id=None, octavia_owned=None): self.load_balancer_id = load_balancer_id self.ip_address = ip_address self.subnet_id = subnet_id @@ -479,6 +479,7 @@ class Vip(BaseDataModel): self.port_id = port_id self.load_balancer = load_balancer self.qos_policy_id = qos_policy_id + self.octavia_owned = octavia_owned class SNI(BaseDataModel): diff --git a/octavia/db/migration/alembic_migrations/versions/ebbcc72b4e5e_add_octavia_owned_vip_column_to_vip_.py b/octavia/db/migration/alembic_migrations/versions/ebbcc72b4e5e_add_octavia_owned_vip_column_to_vip_.py new file mode 100644 index 0000000000..b595bdb0df --- /dev/null +++ b/octavia/db/migration/alembic_migrations/versions/ebbcc72b4e5e_add_octavia_owned_vip_column_to_vip_.py @@ -0,0 +1,34 @@ +# Copyright 2018 Rackspace, US Inc. +# +# 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 Octavia owned VIP column to VIP table + +Revision ID: ebbcc72b4e5e +Revises: 0f242cf02c74 +Create Date: 2018-07-09 17:25:30.137527 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'ebbcc72b4e5e' +down_revision = '0f242cf02c74' + + +def upgrade(): + op.add_column( + u'vip', + sa.Column(u'octavia_owned', sa.Boolean(), nullable=True) + ) diff --git a/octavia/db/models.py b/octavia/db/models.py index d0703afbd2..d7adff989b 100644 --- a/octavia/db/models.py +++ b/octavia/db/models.py @@ -389,6 +389,7 @@ class Vip(base_models.BASE): backref=orm.backref("vip", uselist=False, cascade="delete")) qos_policy_id = sa.Column(sa.String(36), nullable=True) + octavia_owned = sa.Column(sa.Boolean(), nullable=True) class Listener(base_models.BASE, base_models.IdMixin, diff --git a/octavia/tests/functional/db/test_repositories.py b/octavia/tests/functional/db/test_repositories.py index 38ea8fbf1c..df88e3a94f 100644 --- a/octavia/tests/functional/db/test_repositories.py +++ b/octavia/tests/functional/db/test_repositories.py @@ -135,7 +135,7 @@ class AllRepositoriesTest(base.OctaviaDBTestBase): 'port_id': uuidutils.generate_uuid(), 'subnet_id': uuidutils.generate_uuid(), 'network_id': uuidutils.generate_uuid(), - 'qos_policy_id': None} + 'qos_policy_id': None, 'octavia_owned': True} lb_dm = self.repos.create_load_balancer_and_vip(self.session, lb, vip) lb_dm_dict = lb_dm.to_dict() del lb_dm_dict['vip'] diff --git a/octavia/tests/unit/api/drivers/test_driver_lib.py b/octavia/tests/unit/api/drivers/test_driver_lib.py index ee3797864b..8b53ec0aa9 100644 --- a/octavia/tests/unit/api/drivers/test_driver_lib.py +++ b/octavia/tests/unit/api/drivers/test_driver_lib.py @@ -70,7 +70,28 @@ class TestDriverLib(base.TestCase): "total_connections": 100}] self.listener_stats_dict = {"listeners": listener_stats_list} - def test_process_status_update(self): + @mock.patch('octavia.common.utils.get_network_driver') + def test_check_for_lb_vip_deallocate(self, mock_get_driver): + mock_repo = mock.MagicMock() + mock_lb = mock.MagicMock() + + # Test VIP not owned by Octavia + mock_lb.vip.octavia_owned = False + mock_repo.get.return_value = mock_lb + self.driver_lib._check_for_lb_vip_deallocate(mock_repo, 4) + mock_get_driver.assert_not_called() + + # Test VIP is owned by Octavia + mock_lb.vip.octavia_owned = True + mock_repo.get.return_value = mock_lb + mock_net_driver = mock.MagicMock() + mock_get_driver.return_value = mock_net_driver + self.driver_lib._check_for_lb_vip_deallocate(mock_repo, 4) + mock_net_driver.deallocate_vip.assert_called_once_with(mock_lb.vip) + + @mock.patch('octavia.api.drivers.driver_lib.DriverLibrary.' + '_check_for_lb_vip_deallocate') + def test_process_status_update(self, mock_deallocate): mock_repo = mock.MagicMock() list_dict = {"id": 2, constants.PROVISIONING_STATUS: constants.ACTIVE, constants.OPERATING_STATUS: constants.ONLINE} @@ -129,6 +150,12 @@ class TestDriverLib(base.TestCase): mock_repo.delete.assert_called_once_with(self.mock_session, id=2) mock_repo.update.assert_not_called() + # Test with LB Delete + mock_repo.reset_mock() + self.driver_lib._process_status_update( + mock_repo, constants.LOADBALANCERS, list_deleted_dict) + mock_deallocate.assert_called_once_with(mock_repo, 2) + # Test with an exception mock_repo.reset_mock() mock_repo.update.side_effect = Exception('boom')