From 2d4821c8c0851a971419da8e2919671043554da6 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Wed, 10 Jul 2013 18:08:35 +0400 Subject: [PATCH] LBaaS: add status field to PoolMonitorAssociation table also fixes branch in migrations Fixes bug 1166395 Change-Id: I451603cf152d84c750edb6fb1da59b4c7563cbce --- neutron/db/loadbalancer/loadbalancer_db.py | 35 +++++++---- .../11c6e18605c8_pool_monitor_status_.py | 63 +++++++++++++++++++ neutron/extensions/loadbalancer.py | 5 ++ .../loadbalancer/drivers/abstract_driver.py | 6 ++ .../drivers/haproxy/plugin_driver.py | 6 +- neutron/services/loadbalancer/plugin.py | 5 +- .../db/loadbalancer/test_db_loadbalancer.py | 33 +++++++++- 7 files changed, 132 insertions(+), 21 deletions(-) create mode 100644 neutron/db/migration/alembic_migrations/versions/11c6e18605c8_pool_monitor_status_.py diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py index ab25428f5..3c9511310 100644 --- a/neutron/db/loadbalancer/loadbalancer_db.py +++ b/neutron/db/loadbalancer/loadbalancer_db.py @@ -152,7 +152,8 @@ class HealthMonitor(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant, ) -class PoolMonitorAssociation(model_base.BASEV2): +class PoolMonitorAssociation(model_base.BASEV2, + models_v2.HasStatusDescription): """Many-to-many association between pool and healthMonitor classes.""" pool_id = sa.Column(sa.String(36), @@ -568,7 +569,8 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, pool = self._get_resource(context, Pool, pool_id) assoc = PoolMonitorAssociation(pool_id=pool_id, - monitor_id=monitor_id) + monitor_id=monitor_id, + status=constants.PENDING_CREATE) pool.monitors.append(assoc) monitors = [monitor['monitor_id'] for monitor in pool['monitors']] @@ -577,19 +579,25 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, def delete_pool_health_monitor(self, context, id, pool_id): with context.session.begin(subtransactions=True): + assoc = self.get_pool_health_monitor(context, id, pool_id) pool = self._get_resource(context, Pool, pool_id) - try: - monitor_qry = context.session.query(PoolMonitorAssociation) - monitor = monitor_qry.filter_by(monitor_id=id, - pool_id=pool_id).one() - pool.monitors.remove(monitor) - except exc.NoResultFound: - raise loadbalancer.HealthMonitorNotFound(monitor_id=id) + pool.monitors.remove(assoc) def get_pool_health_monitor(self, context, id, pool_id, fields=None): - # TODO(markmcclain) look into why pool_id is ignored - healthmonitor = self._get_resource(context, HealthMonitor, id) - return self._make_health_monitor_dict(healthmonitor, fields) + try: + assoc_qry = context.session.query(PoolMonitorAssociation) + return assoc_qry.filter_by(monitor_id=id, pool_id=pool_id).one() + except exc.NoResultFound: + raise loadbalancer.PoolMonitorAssociationNotFound( + monitor_id=id, pool_id=pool_id) + + def update_pool_health_monitor(self, context, id, pool_id, + status, status_description=None): + with context.session.begin(subtransactions=True): + assoc = self.get_pool_health_monitor(context, id, pool_id) + self.assert_modification_allowed(assoc) + assoc.status = status + assoc.status_description = status_description ######################################################## # Member DB access @@ -674,6 +682,7 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, v = health_monitor['health_monitor'] tenant_id = self._get_tenant_id_for_create(context, v) with context.session.begin(subtransactions=True): + # setting ACTIVE status sinse healthmon is shared DB object monitor_db = HealthMonitor(id=uuidutils.generate_uuid(), tenant_id=tenant_id, type=v['type'], @@ -684,7 +693,7 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, url_path=v['url_path'], expected_codes=v['expected_codes'], admin_state_up=v['admin_state_up'], - status=constants.PENDING_CREATE) + status=constants.ACTIVE) context.session.add(monitor_db) return self._make_health_monitor_dict(monitor_db) diff --git a/neutron/db/migration/alembic_migrations/versions/11c6e18605c8_pool_monitor_status_.py b/neutron/db/migration/alembic_migrations/versions/11c6e18605c8_pool_monitor_status_.py new file mode 100644 index 000000000..4a9e511e6 --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/11c6e18605c8_pool_monitor_status_.py @@ -0,0 +1,63 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 +# +# Copyright 2013 OpenStack Foundation +# +# 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. +# + +"""Pool Monitor status field + +Revision ID: 11c6e18605c8 +Revises: 39cf3f799352 +Create Date: 2013-07-10 06:07:20.878520 + +""" + +# revision identifiers, used by Alembic. +revision = '11c6e18605c8' +down_revision = '39cf3f799352' + +# Change to ['*'] if this migration applies to all plugins + +migration_for_plugins = ['*'] + +from alembic import op +import sqlalchemy as sa + +from neutron.db import migration + + +# Change to ['*'] if this migration applies to all plugins +migration_for_plugins = ['*'] + + +def upgrade(active_plugin=None, options=None): + if not migration.should_run(active_plugin, migration_for_plugins): + return + + op.add_column('poolmonitorassociations', sa.Column('status', + sa.String(16), + nullable=False)) + op.add_column('poolmonitorassociations', sa.Column('status_description', + sa.String(255))) + + # Set status to ACTIVE for existing associations + op.execute("UPDATE poolmonitorassociations SET status='ACTIVE'") + + +def downgrade(active_plugin=None, options=None): + if not migration.should_run(active_plugin, migration_for_plugins): + return + + op.drop_column('poolmonitorassociations', 'status') + op.drop_column('poolmonitorassociations', 'status_description') diff --git a/neutron/extensions/loadbalancer.py b/neutron/extensions/loadbalancer.py index d93360f03..82b4a8e66 100644 --- a/neutron/extensions/loadbalancer.py +++ b/neutron/extensions/loadbalancer.py @@ -49,6 +49,11 @@ class HealthMonitorNotFound(qexception.NotFound): message = _("Health_monitor %(monitor_id)s could not be found") +class PoolMonitorAssociationNotFound(qexception.NotFound): + message = _("Monitor %(monitor_id)s is not associated " + "with Pool %(pool_id)s") + + class StateInvalid(qexception.NeutronException): message = _("Invalid state %(state)s of Loadbalancer resource %(id)s") diff --git a/neutron/services/loadbalancer/drivers/abstract_driver.py b/neutron/services/loadbalancer/drivers/abstract_driver.py index 1be9f934f..c20886e91 100644 --- a/neutron/services/loadbalancer/drivers/abstract_driver.py +++ b/neutron/services/loadbalancer/drivers/abstract_driver.py @@ -131,6 +131,12 @@ class LoadBalancerAbstractDriver(object): def create_pool_health_monitor(self, context, health_monitor, pool_id): + """Driver may call the code below in order to update the status. + self.plugin.update_pool_health_monitor(context, + health_monitor["id"], + pool_id, + constants.ACTIVE) + """ pass @abc.abstractmethod diff --git a/neutron/services/loadbalancer/drivers/haproxy/plugin_driver.py b/neutron/services/loadbalancer/drivers/haproxy/plugin_driver.py index d50c6e28b..e9f2707cb 100644 --- a/neutron/services/loadbalancer/drivers/haproxy/plugin_driver.py +++ b/neutron/services/loadbalancer/drivers/haproxy/plugin_driver.py @@ -110,8 +110,8 @@ class LoadBalancerCallbacks(object): m.status = constants.ACTIVE for hm in pool.monitors: - if hm.healthmonitor.status in ACTIVE_PENDING: - hm.healthmonitor.status = constants.ACTIVE + if hm.status in ACTIVE_PENDING: + hm.status = constants.ACTIVE if (pool.status != constants.ACTIVE or pool.vip.status != constants.ACTIVE): @@ -137,7 +137,7 @@ class LoadBalancerCallbacks(object): retval['healthmonitors'] = [ self.plugin._make_health_monitor_dict(hm.healthmonitor) for hm in pool.monitors - if hm.healthmonitor.status == constants.ACTIVE + if hm.status == constants.ACTIVE ] return retval diff --git a/neutron/services/loadbalancer/plugin.py b/neutron/services/loadbalancer/plugin.py index 11b9c4df3..2f6e2de84 100644 --- a/neutron/services/loadbalancer/plugin.py +++ b/neutron/services/loadbalancer/plugin.py @@ -206,14 +206,13 @@ class LoadBalancerPlugin(loadbalancer_db.LoadBalancerPluginDb, health_monitor, pool_id ) - # open issue: PoolMonitorAssociation has no status field - # so we cant set the status to pending and let the driver - # set the real status of the association self.driver.create_pool_health_monitor( context, health_monitor, pool_id) return retval def delete_pool_health_monitor(self, context, id, pool_id): + self.update_pool_health_monitor(context, id, pool_id, + constants.PENDING_DELETE) hm = self.get_health_monitor(context, id) self.driver.delete_pool_health_monitor( context, hm, pool_id) diff --git a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py index 61a03c133..6b94ded66 100644 --- a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py +++ b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py @@ -832,7 +832,7 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): ('timeout', 10), ('max_retries', 3), ('admin_state_up', True), - ('status', 'PENDING_CREATE')] + ('status', 'ACTIVE')] with self.health_monitor() as monitor: for k, v in keys: self.assertEqual(monitor['health_monitor'][k], v) @@ -916,7 +916,7 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): ('timeout', 10), ('max_retries', 3), ('admin_state_up', True), - ('status', 'PENDING_CREATE')] + ('status', 'ACTIVE')] req = self.new_show_request('health_monitors', monitor['health_monitor']['id'], fmt=self.fmt) @@ -1225,6 +1225,35 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): self.assertEqual(updated_pool['status'], 'ACTIVE') self.assertFalse(pool['pool']['status_description']) + def test_update_pool_health_monitor(self): + with self.pool() as pool: + with self.health_monitor() as hm: + res = self.plugin.create_pool_health_monitor( + context.get_admin_context(), + hm, pool['pool']['id']) + self.assertEqual({'health_monitor': + [hm['health_monitor']['id']]}, + res) + + assoc = self.plugin.get_pool_health_monitor( + context.get_admin_context(), + hm['health_monitor']['id'], + pool['pool']['id']) + self.assertEqual(assoc['status'], 'PENDING_CREATE') + self.assertIsNone(assoc['status_description']) + + self.plugin.update_pool_health_monitor( + context.get_admin_context(), + hm['health_monitor']['id'], + pool['pool']['id'], + 'ACTIVE', 'ok') + assoc = self.plugin.get_pool_health_monitor( + context.get_admin_context(), + hm['health_monitor']['id'], + pool['pool']['id']) + self.assertEqual(assoc['status'], 'ACTIVE') + self.assertEqual(assoc['status_description'], 'ok') + class TestLoadBalancerXML(TestLoadBalancer): fmt = 'xml'