diff --git a/nailgun/nailgun/api/v1/validators/release.py b/nailgun/nailgun/api/v1/validators/release.py index 916eba74a6..25af85c283 100644 --- a/nailgun/nailgun/api/v1/validators/release.py +++ b/nailgun/nailgun/api/v1/validators/release.py @@ -94,27 +94,29 @@ class ReleaseValidator(BasicValidator): ) if 'roles_metadata' in d: - new_roles = set(d['roles_metadata']) - clusters = [cluster.id for cluster in instance.clusters] + deleted_roles = (set(instance.roles_metadata) - + set(d['roles_metadata'])) + clusters_ids = (cluster.id for cluster in instance.clusters) - new_roles_array = sa.cast( - psql.array(new_roles), + deleted_roles_array = sa.cast( + psql.array(deleted_roles), psql.ARRAY(sa.String(consts.ROLE_NAME_MAX_SIZE))) node = db().query(models.Node).filter( - models.Node.cluster_id.in_(clusters) - ).filter(sa.not_(sa.and_( - models.Node.roles.contained_by(new_roles_array), - models.Node.pending_roles.contained_by(new_roles_array) - ))).first() + models.Node.cluster_id.in_(clusters_ids) + ).filter(sa.or_( + models.Node.roles.overlap(deleted_roles_array), + models.Node.pending_roles.overlap(deleted_roles_array) + )).first() if node: used_role = set(node.roles + node.pending_roles) - used_role -= new_roles + used_role = used_role.intersection(deleted_roles) raise errors.CannotDelete( - "Cannot delete roles already assigned " - "to nodes: {0}".format(','.join(used_role)) + "The following roles: {0} cannot be deleted " + "since they are already assigned " + "to nodes.".format(','.join(used_role)) ) return d diff --git a/nailgun/nailgun/test/integration/test_roles.py b/nailgun/nailgun/test/integration/test_roles.py index 39930647a9..1338dabde0 100644 --- a/nailgun/nailgun/test/integration/test_roles.py +++ b/nailgun/nailgun/test/integration/test_roles.py @@ -146,5 +146,6 @@ class TestRoles(BaseIntegrationTest): self.assertEqual(resp.status_code, 400) self.assertEqual( resp.json_body["message"], - "Cannot delete roles already assigned to nodes: controller" + "The following roles: controller cannot be deleted " + "since they are already assigned to nodes." ) diff --git a/nailgun/nailgun/test/unit/test_release_validator.py b/nailgun/nailgun/test/unit/test_release_validator.py index 5778dd2088..5c0c93067e 100644 --- a/nailgun/nailgun/test/unit/test_release_validator.py +++ b/nailgun/nailgun/test/unit/test_release_validator.py @@ -16,6 +16,7 @@ from oslo_serialization import jsonutils from nailgun.api.v1.validators.release import ReleaseValidator from nailgun import errors from nailgun.test.base import BaseTestCase +from nailgun.utils import reverse class TestReleaseValidator(BaseTestCase): @@ -62,3 +63,40 @@ class TestReleaseValidator(BaseTestCase): def test_default_are_good(self): self.validator.validate(self.get_release(self.release)) + + def test_release_delete_role(self): + cluster = self.env.create( + nodes_kwargs=[{ + "roles": ['test_plugin_role_1'], + "pending_roles": ['cinder', 'test_plugin_role_2'], + }]) + + resp = self.app.get( + reverse('ReleaseHandler', + kwargs={'obj_id': cluster.release.id}), + headers=self.default_headers + ) + + data = dict(resp.json_body) + resp = self.app.put( + reverse('ReleaseHandler', + kwargs={'obj_id': cluster.release.id}), + params=jsonutils.dumps(data), + headers=self.default_headers, + expect_errors=False + ) + + data['roles_metadata'].pop('cinder') + resp = self.app.put( + reverse('ReleaseHandler', + kwargs={'obj_id': cluster.release.id}), + params=jsonutils.dumps(data), + headers=self.default_headers, + expect_errors=True + ) + + self.assertEqual(resp.status_code, 400) + self.assertIn( + "The following roles: cinder cannot be deleted", + resp.json_body["message"], + )