From 66efc6c8d93cc6fc9f533ec442f238dfa5d0b132 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Tue, 21 Mar 2017 16:08:09 +0100 Subject: [PATCH] Fix host check in is_backend_frozen When we fixed the freeze mechanism we introduced a DB function called ``is_backend_frozen`` that is used to check in the DB whether a resource (volume, group, etc.) is frozen or not. The issue is that this function is not taking into consideration that in some cases host/cluster_name may be comming with the pool information, so the check will fail. This patch fixes this by removing the pool information from host and cluster_name parameters before doing the check in the DB. TrivialFix Change-Id: Ie776adf9e746cf4cb7a2856d64d0b94423149b8d Closes-Bug: #1616974 --- cinder/db/sqlalchemy/api.py | 5 +++-- cinder/tests/unit/test_db_api.py | 24 +++++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 4f91f5612be..2da07c61304 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -60,6 +60,7 @@ from cinder import exception from cinder.i18n import _ from cinder.objects import fields from cinder import utils +from cinder.volume import utils as vol_utils CONF = cfg.CONF @@ -575,10 +576,10 @@ def is_backend_frozen(context, host, cluster_name): """Check if a storage backend is frozen based on host and cluster_name.""" if cluster_name: model = models.Cluster - conditions = [model.name == cluster_name] + conditions = [model.name == vol_utils.extract_host(cluster_name)] else: model = models.Service - conditions = [model.host == host] + conditions = [model.host == vol_utils.extract_host(host)] conditions.extend((~model.deleted, model.frozen)) query = get_session().query(sql.exists().where(and_(*conditions))) frozen = query.scalar() diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 3b39f289f19..ac12549df19 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -2947,16 +2947,21 @@ class DBAPIGenericTestCase(BaseTest): @ddt.ddt class DBAPIBackendTestCase(BaseTest): - @ddt.data(True, False) - def test_is_backend_frozen_service(self, frozen): + @ddt.data((True, True), (True, False), (False, True), (False, False)) + @ddt.unpack + def test_is_backend_frozen_service(self, frozen, pool): service = utils.create_service(self.ctxt, {'frozen': frozen}) utils.create_service(self.ctxt, {'host': service.host + '2', 'frozen': not frozen}) - self.assertEqual(frozen, db.is_backend_frozen(self.ctxt, service.host, + host = service.host + if pool: + host += '#poolname' + self.assertEqual(frozen, db.is_backend_frozen(self.ctxt, host, service.cluster_name)) - @ddt.data(True, False) - def test_is_backend_frozen_cluster(self, frozen): + @ddt.data((True, True), (True, False), (False, True), (False, False)) + @ddt.unpack + def test_is_backend_frozen_cluster(self, frozen, pool): cluster = utils.create_cluster(self.ctxt, frozen=frozen) utils.create_service(self.ctxt, {'frozen': frozen, 'host': 'hostA', 'cluster_name': cluster.name}) @@ -2966,5 +2971,10 @@ class DBAPIBackendTestCase(BaseTest): 'cluster_name': cluster.name}) utils.create_populated_cluster(self.ctxt, 3, 0, frozen=not frozen, name=cluster.name + '2') - self.assertEqual(frozen, db.is_backend_frozen(self.ctxt, service.host, - service.cluster_name)) + host = service.host + cluster = service.cluster_name + if pool: + host += '#poolname' + cluster += '#poolname' + self.assertEqual(frozen, + db.is_backend_frozen(self.ctxt, host, cluster))