From 0e6d178939647e61fee1485e7b3611f093adb7c6 Mon Sep 17 00:00:00 2001 From: Bharat Kunwar Date: Tue, 16 Feb 2021 10:55:07 +0000 Subject: [PATCH] Only allow zero node count from microversion 1.10 At present, all clients can request resize of cluster nodes to zero. This PS ensures that only requests with 1.10 microversion header or more are fulfilled. Story: 2007851 Task: 41841 Additionally, unit tests are also included to ensure that the microversions are respected for create, update and resizing clusters with zero node count. Additionally, unit tests for the following APIs are tested explictly: - resize API with microversion 1.7 - upgrade API with microversion 1.8 - nodegroup API with microversion 1.9 Story: 2005054 Task: 41840 Change-Id: Iba9d619d2e92abcbaa3eca5da68f5e0f203dea8d --- magnum/api/controllers/v1/cluster.py | 39 +++++++++++- magnum/api/controllers/v1/cluster_actions.py | 24 +++++++- magnum/common/exception.py | 10 +++ .../unit/api/controllers/v1/test_cluster.py | 41 ++++++++++++- .../controllers/v1/test_cluster_actions.py | 61 ++++++++++++++----- 5 files changed, 155 insertions(+), 20 deletions(-) diff --git a/magnum/api/controllers/v1/cluster.py b/magnum/api/controllers/v1/cluster.py index a2256f5c72..6b3edf42e9 100755 --- a/magnum/api/controllers/v1/cluster.py +++ b/magnum/api/controllers/v1/cluster.py @@ -490,10 +490,23 @@ class ClustersController(base.Controller): "one.") % cluster_limit raise exception.ResourceLimitExceeded(msg=msg) + @base.Controller.api_version("1.1", "1.9") @expose.expose(ClusterID, body=Cluster, status_code=202) @validation.enforce_cluster_type_supported() @validation.enforce_cluster_volume_storage_size() def post(self, cluster): + if cluster.node_count == 0: + raise exception.ZeroNodeCountNotSupported() + return self._post(cluster) + + @base.Controller.api_version("1.10") # noqa + @expose.expose(ClusterID, body=Cluster, status_code=202) + @validation.enforce_cluster_type_supported() + @validation.enforce_cluster_volume_storage_size() + def post(self, cluster): # noqa + return self._post(cluster) + + def _post(self, cluster): """Create a new cluster. :param cluster: a cluster within the request body. @@ -585,12 +598,36 @@ class ClustersController(base.Controller): (cluster, node_count, health_status, health_status_reason) = self._patch(cluster_ident, patch) + if node_count == 0: + raise exception.ZeroNodeCountNotSupported() pecan.request.rpcapi.cluster_update_async(cluster, node_count, health_status, health_status_reason) return ClusterID(cluster.uuid) - @base.Controller.api_version("1.3") # noqa + @base.Controller.api_version("1.3", "1.9") # noqa + @wsme.validate(types.uuid, bool, [ClusterPatchType]) + @expose.expose(ClusterID, types.uuid_or_name, types.boolean, + body=[ClusterPatchType], status_code=202) + def patch(self, cluster_ident, rollback=False, patch=None): # noqa + """Update an existing Cluster. + + :param cluster_ident: UUID or logical name of a cluster. + :param rollback: whether to rollback cluster on update failure. + :param patch: a json PATCH document to apply to this cluster. + """ + (cluster, node_count, + health_status, + health_status_reason) = self._patch(cluster_ident, patch) + if node_count == 0: + raise exception.ZeroNodeCountNotSupported() + pecan.request.rpcapi.cluster_update_async(cluster, node_count, + health_status, + health_status_reason, + rollback) + return ClusterID(cluster.uuid) + + @base.Controller.api_version("1.10") # noqa @wsme.validate(types.uuid, bool, [ClusterPatchType]) @expose.expose(ClusterID, types.uuid_or_name, types.boolean, body=[ClusterPatchType], status_code=202) diff --git a/magnum/api/controllers/v1/cluster_actions.py b/magnum/api/controllers/v1/cluster_actions.py index 0594b314f2..7684d2baae 100644 --- a/magnum/api/controllers/v1/cluster_actions.py +++ b/magnum/api/controllers/v1/cluster_actions.py @@ -81,10 +81,21 @@ class ActionsController(base.Controller): 'upgrade': ['POST'] } - @base.Controller.api_version("1.7") + @base.Controller.api_version("1.7", "1.9") @expose.expose(ClusterID, types.uuid_or_name, body=ClusterResizeRequest, status_code=202) def resize(self, cluster_ident, cluster_resize_req): + if cluster_resize_req.node_count == 0: + raise exception.ZeroNodeCountNotSupported() + return self._resize(cluster_ident, cluster_resize_req) + + @base.Controller.api_version("1.10") # noqa + @expose.expose(ClusterID, types.uuid_or_name, + body=ClusterResizeRequest, status_code=202) + def resize(self, cluster_ident, cluster_resize_req): # noqa + return self._resize(cluster_ident, cluster_resize_req) + + def _resize(self, cluster_ident, cluster_resize_req): """Resize a cluster. :param cluster_ident: UUID of a cluster or logical name of the cluster. @@ -126,10 +137,19 @@ class ActionsController(base.Controller): nodegroup) return ClusterID(cluster.uuid) - @base.Controller.api_version("1.8") + @base.Controller.api_version("1.7", "1.7") @expose.expose(ClusterID, types.uuid_or_name, body=ClusterUpgradeRequest, status_code=202) def upgrade(self, cluster_ident, cluster_upgrade_req): + raise exception.ClusterUpgradeNotSupported() + + @base.Controller.api_version("1.8") # noqa + @expose.expose(ClusterID, types.uuid_or_name, + body=ClusterUpgradeRequest, status_code=202) + def upgrade(self, cluster_ident, cluster_upgrade_req): # noqa + return self._upgrade(cluster_ident, cluster_upgrade_req) + + def _upgrade(self, cluster_ident, cluster_upgrade_req): """Upgrade a cluster. :param cluster_ident: UUID of a cluster or logical name of the cluster. diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 309a070c52..9a7e6f3b44 100755 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -441,6 +441,16 @@ class MasterNGResizeNotSupported(NotSupported): message = _("Resizing a master nodegroup is not supported.") +class ZeroNodeCountNotSupported(NotSupported): + message = _("Resizing a nodegroup to zero is not supported in the " + "provided microversion.") + + +class ClusterUpgradeNotSupported(NotSupported): + message = _("Cluster upgrade is not supported in the " + "provided microversion.") + + class NGResizeOutBounds(Invalid): message = _("Resizing %(nodegroup)s outside the allowed range: " "min_node_count = %(min_nc)s, " diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster.py b/magnum/tests/unit/api/controllers/v1/test_cluster.py index 15d2999c6c..ea0c6e1e5f 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster.py @@ -511,6 +511,30 @@ class TestPatch(api_base.FunctionalTest): self.cluster_obj.health_status_reason, False) self.assertEqual(202, response.status_code) + def test_update_cluster_with_zero_node_count_fail(self): + node_count = 0 + response = self.patch_json( + '/clusters/%s' % self.cluster_obj.uuid, + [{'path': '/node_count', 'value': node_count, + 'op': 'replace'}], + headers={'OpenStack-API-Version': 'container-infra 1.9'}, + expect_errors=True) + + self.assertEqual(400, response.status_code) + + def test_update_cluster_with_zero_node_count(self): + node_count = 0 + response = self.patch_json( + '/clusters/%s' % self.cluster_obj.uuid, + [{'path': '/node_count', 'value': node_count, + 'op': 'replace'}], + headers={'OpenStack-API-Version': 'container-infra 1.10'}) + + self.mock_cluster_update.assert_called_once_with( + mock.ANY, node_count, self.cluster_obj.health_status, + self.cluster_obj.health_status_reason, False) + self.assertEqual(202, response.status_code) + def test_remove_ok(self): response = self.get_json('/clusters/%s' % self.cluster_obj.uuid) self.assertIsNotNone(response['name']) @@ -678,10 +702,21 @@ class TestPost(api_base.FunctionalTest): self.assertEqual('application/json', response.content_type) self.assertEqual(202, response.status_int) - def test_create_cluster_with_node_count_zero(self): + def test_create_cluster_with_zero_node_count_fail(self): bdict = apiutils.cluster_post_data() bdict['node_count'] = 0 - response = self.post_json('/clusters', bdict, expect_errors=True) + response = self.post_json('/clusters', bdict, expect_errors=True, + headers={"Openstack-Api-Version": + "container-infra 1.9"}) + self.assertEqual('application/json', response.content_type) + self.assertEqual(400, response.status_int) + + def test_create_cluster_with_zero_node_count(self): + bdict = apiutils.cluster_post_data() + bdict['node_count'] = 0 + response = self.post_json('/clusters', bdict, + headers={"Openstack-Api-Version": + "container-infra 1.10"}) self.assertEqual('application/json', response.content_type) self.assertEqual(202, response.status_int) @@ -696,7 +731,7 @@ class TestPost(api_base.FunctionalTest): def test_create_cluster_with_no_node_count(self): bdict = apiutils.cluster_post_data() del bdict['node_count'] - response = self.post_json('/clusters', bdict, expect_errors=True) + response = self.post_json('/clusters', bdict) self.assertEqual('application/json', response.content_type) self.assertEqual(202, response.status_int) diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster_actions.py b/magnum/tests/unit/api/controllers/v1/test_cluster_actions.py index 8a512ca0e0..ba9304fe1b 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster_actions.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster_actions.py @@ -46,7 +46,7 @@ class TestClusterResize(api_base.FunctionalTest): self.cluster_obj.uuid, {"node_count": new_node_count}, headers={"Openstack-Api-Version": - "container-infra latest"}) + "container-infra 1.7"}) self.assertEqual(202, response.status_code) response = self.get_json('/clusters/%s' % self.cluster_obj.uuid) @@ -69,7 +69,7 @@ class TestClusterResize(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_resize_req, headers={"Openstack-Api-Version": - "container-infra latest"}) + "container-infra 1.9"}) self.assertEqual(202, response.status_code) response = self.get_json('/clusters/%s' % self.cluster_obj.uuid) @@ -89,7 +89,7 @@ class TestClusterResize(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_resize_req, headers={"Openstack-Api-Version": - "container-infra latest"}, + "container-infra 1.9"}, expect_errors=True) self.assertEqual(400, response.status_code) @@ -106,7 +106,7 @@ class TestClusterResize(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_resize_req, headers={"Openstack-Api-Version": - "container-infra latest"}, + "container-infra 1.9"}, expect_errors=True) self.assertEqual(400, response.status_code) @@ -123,10 +123,43 @@ class TestClusterResize(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_resize_req, headers={"Openstack-Api-Version": - "container-infra latest"}, + "container-infra 1.9"}, expect_errors=True) self.assertEqual(400, response.status_code) + def test_resize_with_zero_node_count_fail(self): + new_node_count = 0 + nodegroup = self.cluster_obj.default_ng_worker + nodegroup.min_node_count = 0 + nodegroup.save() + cluster_resize_req = { + "node_count": new_node_count, + "nodegroup": nodegroup.uuid + } + response = self.post_json('/clusters/%s/actions/resize' % + self.cluster_obj.uuid, + cluster_resize_req, + headers={"Openstack-Api-Version": + "container-infra 1.9"}, + expect_errors=True) + self.assertEqual(400, response.status_code) + + def test_resize_with_zero_node_count(self): + new_node_count = 0 + nodegroup = self.cluster_obj.default_ng_worker + nodegroup.min_node_count = 0 + nodegroup.save() + cluster_resize_req = { + "node_count": new_node_count, + "nodegroup": nodegroup.uuid + } + response = self.post_json('/clusters/%s/actions/resize' % + self.cluster_obj.uuid, + cluster_resize_req, + headers={"Openstack-Api-Version": + "container-infra 1.10"}) + self.assertEqual(202, response.status_code) + class TestClusterUpgrade(api_base.FunctionalTest): def setUp(self): @@ -162,7 +195,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}) + "container-infra 1.8"}) self.assertEqual(202, response.status_code) def test_upgrade_cluster_as_admin(self): @@ -193,7 +226,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): '/clusters/%s/actions/upgrade' % cluster_uuid, cluster_upgrade_req, - headers={"Openstack-Api-Version": "container-infra latest"}) + headers={"Openstack-Api-Version": "container-infra 1.8"}) self.assertEqual(202, response.status_int) @@ -206,7 +239,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}) + "container-infra 1.9"}) self.assertEqual(202, response.status_code) def test_upgrade_default_master(self): @@ -218,7 +251,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}) + "container-infra 1.9"}) self.assertEqual(202, response.status_code) def test_upgrade_non_default_ng(self): @@ -230,7 +263,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}) + "container-infra 1.9"}) self.assertEqual(202, response.status_code) def test_upgrade_cluster_not_found(self): @@ -240,7 +273,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): response = self.post_json('/clusters/not_there/actions/upgrade', cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}, + "container-infra 1.8"}, expect_errors=True) self.assertEqual(404, response.status_code) @@ -252,7 +285,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}, + "container-infra 1.8"}, expect_errors=True) self.assertEqual(404, response.status_code) @@ -265,7 +298,7 @@ class TestClusterUpgrade(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}, + "container-infra 1.9"}, expect_errors=True) self.assertEqual(404, response.status_code) @@ -278,6 +311,6 @@ class TestClusterUpgrade(api_base.FunctionalTest): self.cluster_obj.uuid, cluster_upgrade_req, headers={"Openstack-Api-Version": - "container-infra latest"}, + "container-infra 1.9"}, expect_errors=True) self.assertEqual(409, response.status_code)