diff --git a/magnum/api/controllers/v1/cluster.py b/magnum/api/controllers/v1/cluster.py index dccf10f874..fde06a68e0 100755 --- a/magnum/api/controllers/v1/cluster.py +++ b/magnum/api/controllers/v1/cluster.py @@ -68,25 +68,6 @@ class Cluster(base.APIBase): between the internal object model and the API representation of a Cluster. """ - _cluster_template_id = None - - def _get_cluster_template_id(self): - return self._cluster_template_id - - def _set_cluster_template_id(self, value): - if value and self._cluster_template_id != value: - try: - cluster_template = api_utils.get_resource('ClusterTemplate', - value) - self._cluster_template_id = cluster_template.uuid - except exception.ClusterTemplateNotFound as e: - # Change error code because 404 (NotFound) is inappropriate - # response for a POST request to create a Cluster - e.code = 400 # BadRequest - raise - elif value == wtypes.Unset: - self._cluster_template_id = wtypes.Unset - uuid = types.uuid """Unique UUID for this cluster""" @@ -95,10 +76,7 @@ class Cluster(base.APIBase): """Name of this cluster, max length is limited to 242 because of heat stack requires max length limit to 255, and Magnum amend a uuid length""" - cluster_template_id = wsme.wsproperty(wtypes.text, - _get_cluster_template_id, - _set_cluster_template_id, - mandatory=True) + cluster_template_id = wsme.wsattr(wtypes.text, mandatory=True) """The cluster_template UUID""" keypair = wsme.wsattr(wtypes.StringType(min_length=1, max_length=255), @@ -493,6 +471,7 @@ class ClustersController(base.Controller): @base.Controller.api_version("1.1", "1.9") @expose.expose(ClusterID, body=Cluster, status_code=202) + @validation.ct_not_found_to_bad_request() @validation.enforce_cluster_type_supported() @validation.enforce_cluster_volume_storage_size() def post(self, cluster): @@ -519,8 +498,10 @@ class ClustersController(base.Controller): self._check_cluster_quota_limit(context) temp_id = cluster.cluster_template_id - cluster_template = objects.ClusterTemplate.get_by_uuid(context, - temp_id) + cluster_template = objects.ClusterTemplate.get(context, temp_id) + # We are not sure if we got a uuid or name here. So just set + # explicitly the uuid of the cluster template in the cluster. + cluster.cluster_template_id = cluster_template.uuid # If keypair not present, use cluster_template value if cluster.keypair is None: cluster.keypair = cluster_template.keypair_id diff --git a/magnum/api/validation.py b/magnum/api/validation.py old mode 100644 new mode 100755 index e04a2236d3..de9b096276 --- a/magnum/api/validation.py +++ b/magnum/api/validation.py @@ -34,11 +34,25 @@ cluster_update_allowed_properties = set(['node_count', 'health_status', federation_update_allowed_properties = set(['member_ids', 'properties']) +def ct_not_found_to_bad_request(): + @decorator.decorator + def wrapper(func, *args, **kwargs): + try: + return func(*args, **kwargs) + except exception.ClusterTemplateNotFound as e: + # Change error code because 404 (NotFound) is inappropriate + # response for a POST request to create a Cluster + e.code = 400 # BadRequest + raise + + return wrapper + + def enforce_cluster_type_supported(): @decorator.decorator def wrapper(func, *args, **kwargs): cluster = args[1] - cluster_template = objects.ClusterTemplate.get_by_uuid( + cluster_template = objects.ClusterTemplate.get( pecan.request.context, cluster.cluster_template_id) cluster_type = (cluster_template.server_type, cluster_template.cluster_distro, @@ -77,7 +91,7 @@ def enforce_cluster_volume_storage_size(): @decorator.decorator def wrapper(func, *args, **kwargs): cluster = args[1] - cluster_template = objects.ClusterTemplate.get_by_uuid( + cluster_template = objects.ClusterTemplate.get( pecan.request.context, cluster.cluster_template_id) _enforce_volume_storage_size( cluster_template.as_dict(), cluster.as_dict()) diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 9a7e6f3b44..81781191d8 100755 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -480,3 +480,8 @@ class InvalidClusterTemplateForUpgrade(Conflict): class ClusterAPIAddressUnavailable(Conflict): message = _("Cluster API address is not available yet") + + +class ObjectError(MagnumException): + message = _("Failed to perform action %{action}s on %{obj_name}s with " + "uuid %{obj_id}s: %{reason}s") diff --git a/magnum/objects/cluster.py b/magnum/objects/cluster.py old mode 100644 new mode 100755 index 896006b1d2..a65321b97b --- a/magnum/objects/cluster.py +++ b/magnum/objects/cluster.py @@ -22,6 +22,9 @@ from magnum.objects import fields as m_fields from magnum.objects.nodegroup import NodeGroup +LAZY_LOADED_ATTRS = ['cluster_template'] + + @base.MagnumObjectRegistry.register class Cluster(base.MagnumPersistentObject, base.MagnumObject, base.MagnumObjectDictCompat): @@ -100,16 +103,11 @@ class Cluster(base.MagnumPersistentObject, base.MagnumObject, def _from_db_object(cluster, db_cluster): """Converts a database entity to a formal object.""" for field in cluster.fields: + # cluster_template will be loaded lazily when it is needed + # by obj_load_attr. if field != 'cluster_template': cluster[field] = db_cluster[field] - # Note(eliqiao): The following line needs to be placed outside the - # loop because there is a dependency from cluster_template to - # cluster_template_id. The cluster_template_id must be populated - # first in the loop before it can be used to find the cluster_template. - cluster['cluster_template'] = ClusterTemplate.get_by_uuid( - cluster._context, cluster.cluster_template_id) - cluster.obj_reset_changes() return cluster @@ -331,6 +329,17 @@ class Cluster(base.MagnumPersistentObject, base.MagnumObject, if self.obj_attr_is_set(field) and self[field] != current[field]: self[field] = current[field] + def obj_load_attr(self, attrname): + if attrname not in LAZY_LOADED_ATTRS: + raise exception.ObjectError( + action='obj_load_attr', obj_name=self.name, obj_id=self.uuid, + reason='unable to lazy-load %s' % attrname) + + self['cluster_template'] = ClusterTemplate.get_by_uuid( + self._context, self.cluster_template_id) + + self.obj_reset_changes(['cluster_template']) + def as_dict(self): dict_ = super(Cluster, self).as_dict() # Update the dict with the attributes coming form diff --git a/magnum/objects/cluster_template.py b/magnum/objects/cluster_template.py old mode 100644 new mode 100755 index de25577b68..d352feb6dd --- a/magnum/objects/cluster_template.py +++ b/magnum/objects/cluster_template.py @@ -14,7 +14,6 @@ from oslo_utils import strutils from oslo_utils import uuidutils from oslo_versionedobjects import fields -from magnum.common import exception from magnum.db import api as dbapi from magnum.objects import base from magnum.objects import fields as m_fields @@ -112,7 +111,7 @@ class ClusterTemplate(base.MagnumPersistentObject, base.MagnumObject, elif uuidutils.is_uuid_like(cluster_template_id): return cls.get_by_uuid(context, cluster_template_id) else: - raise exception.InvalidIdentity(identity=cluster_template_id) + return cls.get_by_name(context, cluster_template_id) @base.remotable_classmethod def get_by_id(cls, context, cluster_template_id): diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster.py b/magnum/tests/unit/api/controllers/v1/test_cluster.py old mode 100644 new mode 100755 index ea0c6e1e5f..4e5132d19b --- a/magnum/tests/unit/api/controllers/v1/test_cluster.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster.py @@ -187,7 +187,9 @@ class TestListCluster(api_base.FunctionalTest): @mock.patch("magnum.common.policy.enforce") @mock.patch("magnum.common.context.make_context") - def test_get_all_with_all_projects(self, mock_context, mock_policy): + @mock.patch("magnum.objects.Cluster.obj_load_attr") + @mock.patch("magnum.objects.Cluster.cluster_template") + def test_get_all_with_all_projects(self, mock_context, mock_policy, mock_load, mock_template): for id_ in range(4): temp_uuid = uuidutils.generate_uuid() obj_utils.create_test_cluster(self.context, id=id_, diff --git a/magnum/tests/unit/api/test_validation.py b/magnum/tests/unit/api/test_validation.py old mode 100644 new mode 100755 index e6d89e3129..91d9f8f9b0 --- a/magnum/tests/unit/api/test_validation.py +++ b/magnum/tests/unit/api/test_validation.py @@ -30,7 +30,7 @@ CONF = magnum.conf.CONF class TestValidation(base.BaseTestCase): def _test_enforce_cluster_type_supported( - self, mock_cluster_template_get_by_uuid, mock_cluster_get_by_uuid, + self, mock_cluster_template_get, mock_cluster_get_by_uuid, mock_pecan_request, cluster_type, assert_raised=False): @v.enforce_cluster_type_supported() @@ -41,7 +41,7 @@ class TestValidation(base.BaseTestCase): cluster_template = obj_utils.get_test_cluster_template( mock_pecan_request.context, uuid='cluster_template_id', coe=coe, cluster_distro=cluster_distro, server_type=server_type) - mock_cluster_template_get_by_uuid.return_value = cluster_template + mock_cluster_template_get.return_value = cluster_template cluster = mock.MagicMock() cluster.cluster_template_id = 'cluster_template_id' @@ -56,26 +56,26 @@ class TestValidation(base.BaseTestCase): @mock.patch('pecan.request') @mock.patch('magnum.objects.Cluster.get_by_uuid') - @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid') + @mock.patch('magnum.objects.ClusterTemplate.get') def test_enforce_cluster_type_supported( - self, mock_cluster_template_get_by_uuid, mock_cluster_get_by_uuid, + self, mock_cluster_template_get, mock_cluster_get_by_uuid, mock_pecan_request): cluster_type = ('vm', 'fedora-atomic', 'kubernetes') self._test_enforce_cluster_type_supported( - mock_cluster_template_get_by_uuid, mock_cluster_get_by_uuid, + mock_cluster_template_get, mock_cluster_get_by_uuid, mock_pecan_request, cluster_type) @mock.patch('pecan.request') @mock.patch('magnum.objects.Cluster.get_by_uuid') - @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid') + @mock.patch('magnum.objects.ClusterTemplate.get') def test_enforce_cluster_type_not_supported( - self, mock_cluster_template_get_by_uuid, mock_cluster_get_by_uuid, + self, mock_cluster_template_get, mock_cluster_get_by_uuid, mock_pecan_request): cluster_type = ('vm', 'foo', 'kubernetes') exc = self._test_enforce_cluster_type_supported( - mock_cluster_template_get_by_uuid, mock_cluster_get_by_uuid, + mock_cluster_template_get, mock_cluster_get_by_uuid, mock_pecan_request, cluster_type, assert_raised=True) self.assertEqual('Cluster type (vm, foo, kubernetes) not supported.', exc.message) diff --git a/magnum/tests/unit/objects/test_cluster.py b/magnum/tests/unit/objects/test_cluster.py old mode 100644 new mode 100755 index 10abab59d2..186de31824 --- a/magnum/tests/unit/objects/test_cluster.py +++ b/magnum/tests/unit/objects/test_cluster.py @@ -117,13 +117,12 @@ class TestClusterObject(base.DbTestCase): self.assertThat(clusters, HasLength(1)) self.assertIsInstance(clusters[0], objects.Cluster) self.assertEqual(self.context, clusters[0]._context) + mock_cluster_template_get.assert_not_called() - @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid') - def test_list_with_filters(self, mock_cluster_template_get): + def test_list_with_filters(self): with mock.patch.object(self.dbapi, 'get_cluster_list', autospec=True) as mock_get_list: mock_get_list.return_value = [self.fake_cluster] - mock_cluster_template_get.return_value = self.fake_cluster_template filters = {'name': 'cluster1'} clusters = objects.Cluster.list(self.context, filters=filters) @@ -136,24 +135,20 @@ class TestClusterObject(base.DbTestCase): self.assertIsInstance(clusters[0], objects.Cluster) self.assertEqual(self.context, clusters[0]._context) - @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid') - def test_create(self, mock_cluster_template_get): + def test_create(self): with mock.patch.object(self.dbapi, 'create_cluster', autospec=True) as mock_create_cluster: - mock_cluster_template_get.return_value = self.fake_cluster_template mock_create_cluster.return_value = self.fake_cluster cluster = objects.Cluster(self.context, **self.fake_cluster) cluster.create() mock_create_cluster.assert_called_once_with(self.fake_cluster) self.assertEqual(self.context, cluster._context) - @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid') - def test_destroy(self, mock_cluster_template_get): + def test_destroy(self): uuid = self.fake_cluster['uuid'] with mock.patch.object(self.dbapi, 'get_cluster_by_uuid', autospec=True) as mock_get_cluster: mock_get_cluster.return_value = self.fake_cluster - mock_cluster_template_get.return_value = self.fake_cluster_template with mock.patch.object(self.dbapi, 'destroy_cluster', autospec=True) as mock_destroy_cluster: cluster = objects.Cluster.get_by_uuid(self.context, uuid) @@ -162,12 +157,10 @@ class TestClusterObject(base.DbTestCase): mock_destroy_cluster.assert_called_once_with(uuid) self.assertEqual(self.context, cluster._context) - @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid') - def test_save(self, mock_cluster_template_get): + def test_save(self): uuid = self.fake_cluster['uuid'] with mock.patch.object(self.dbapi, 'get_cluster_by_uuid', autospec=True) as mock_get_cluster: - mock_cluster_template_get.return_value = self.fake_cluster_template mock_get_cluster.return_value = self.fake_cluster with mock.patch.object(self.dbapi, 'update_cluster', autospec=True) as mock_update_cluster: @@ -177,12 +170,10 @@ class TestClusterObject(base.DbTestCase): mock_get_cluster.assert_called_once_with(self.context, uuid) mock_update_cluster.assert_called_once_with( - uuid, {'status': 'DELETE_IN_PROGRESS', - 'cluster_template': self.fake_cluster_template}) + uuid, {'status': 'DELETE_IN_PROGRESS'}) self.assertEqual(self.context, cluster._context) - @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid') - def test_refresh(self, mock_cluster_template_get): + def test_refresh(self): uuid = self.fake_cluster['uuid'] new_uuid = uuidutils.generate_uuid() returns = [dict(self.fake_cluster, uuid=uuid), @@ -192,7 +183,6 @@ class TestClusterObject(base.DbTestCase): with mock.patch.object(self.dbapi, 'get_cluster_by_uuid', side_effect=returns, autospec=True) as mock_get_cluster: - mock_cluster_template_get.return_value = self.fake_cluster_template cluster = objects.Cluster.get_by_uuid(self.context, uuid) self.assertEqual(uuid, cluster.uuid) cluster.refresh() diff --git a/magnum/tests/unit/objects/test_cluster_template.py b/magnum/tests/unit/objects/test_cluster_template.py old mode 100644 new mode 100755 index dc868b5772..4df97b9296 --- a/magnum/tests/unit/objects/test_cluster_template.py +++ b/magnum/tests/unit/objects/test_cluster_template.py @@ -52,7 +52,7 @@ class TestClusterTemplateObject(base.DbTestCase): self.assertEqual(self.context, cluster_template._context) def test_get_bad_id_and_uuid(self): - self.assertRaises(exception.InvalidIdentity, + self.assertRaises(exception.ClusterTemplateNotFound, objects.ClusterTemplate.get, self.context, 'not-a-uuid')