From a47df4be662668b63af92ee779e1d36d9bb60bd6 Mon Sep 17 00:00:00 2001 From: okozachenko1203 Date: Thu, 9 Jan 2025 16:52:32 +1100 Subject: [PATCH 1/2] add validation for boot volume size if both boot_volume_size label and flavor's root_gb are zero, cluster node creation fails. This should be filtered at the creation/update operation. Change-Id: Ib73f66a8c7360820e13a7ab24bb4f656584124e5 (cherry picked from commit d8d2016ede09a92ee2a789e2984267bd4c21895f) Signed-off-by: Michal Nasiadka --- magnum/api/attr_validator.py | 20 ++++++++ magnum/api/controllers/v1/cluster.py | 19 ++++++++ magnum/api/controllers/v1/cluster_template.py | 21 +++++++++ magnum/api/controllers/v1/nodegroup.py | 21 +++++++++ magnum/common/exception.py | 5 ++ .../unit/api/controllers/v1/test_cluster.py | 17 +++++++ .../controllers/v1/test_cluster_template.py | 36 ++++++++++++++ .../unit/api/controllers/v1/test_nodegroup.py | 9 ++++ magnum/tests/unit/api/test_attr_validator.py | 47 +++++++++++++++++++ ...ot-volume-size-check-0262c2b61abc7ccf.yaml | 5 ++ 10 files changed, 200 insertions(+) create mode 100644 releasenotes/notes/add-boot-volume-size-check-0262c2b61abc7ccf.yaml diff --git a/magnum/api/attr_validator.py b/magnum/api/attr_validator.py index 22a5b42112..de9639c868 100644 --- a/magnum/api/attr_validator.py +++ b/magnum/api/attr_validator.py @@ -177,6 +177,26 @@ def validate_master_count(context, cluster): "master_count must be 1 when master_lb_enabled is False")) +def validate_flavor_root_volume_size(cli, flavor, boot_volume_size): + """Validate flavor root volume size.""" + + if boot_volume_size > 0: + return + + flavor_obj = None + flavor_list = cli.nova().flavors.list() + for f in flavor_list: + if f.name == flavor or f.id == flavor: + flavor_obj = f + break + + if flavor_obj is None: + raise exception.FlavorNotFound(flavor=flavor) + + if flavor_obj.disk == 0 and boot_volume_size == 0: + raise exception.FlavorZeroRootVolumeNotSupported() + + def validate_federation_hostcluster(cluster_uuid): """Validate Federation `hostcluster_id` parameter. diff --git a/magnum/api/controllers/v1/cluster.py b/magnum/api/controllers/v1/cluster.py index 8ad65d5a36..68e02c5bbf 100644 --- a/magnum/api/controllers/v1/cluster.py +++ b/magnum/api/controllers/v1/cluster.py @@ -33,6 +33,7 @@ from magnum.api import expose from magnum.api import utils as api_utils from magnum.api import validation +from magnum.common import clients from magnum.common import exception from magnum.common import name_generator from magnum.common import policy @@ -542,6 +543,15 @@ def _post(self, cluster): not getattr(cluster, attr)): setattr(cluster, attr, getattr(cluster_template, attr)) + # check root volume size + boot_volume_size = cluster.labels.get( + 'boot_volume_size', CONF.cinder.default_boot_volume_size) + cli = clients.OpenStackClients(context) + attr_validator.validate_flavor_root_volume_size( + cli, cluster.flavor_id, boot_volume_size) + attr_validator.validate_flavor_root_volume_size( + cli, cluster.master_flavor_id, boot_volume_size) + cluster_dict = cluster.as_dict() attr_validator.validate_os_resources(context, @@ -660,6 +670,15 @@ def _patch(self, cluster_ident, patch): if getattr(cluster, field) != getattr(new_cluster, field): delta.add(field) + # check root volume size + boot_volume_size = new_cluster.labels.get( + 'boot_volume_size', CONF.cinder.default_boot_volume_size) + cli = clients.OpenStackClients(context) + attr_validator.validate_flavor_root_volume_size( + cli, new_cluster.flavor_id, boot_volume_size) + attr_validator.validate_flavor_root_volume_size( + cli, new_cluster.master_flavor_id, boot_volume_size) + validation.validate_cluster_properties(delta) # NOTE(brtknr): cluster.node_count is the size of the whole cluster diff --git a/magnum/api/controllers/v1/cluster_template.py b/magnum/api/controllers/v1/cluster_template.py index a082d5c33a..3388c5cfa1 100644 --- a/magnum/api/controllers/v1/cluster_template.py +++ b/magnum/api/controllers/v1/cluster_template.py @@ -34,6 +34,10 @@ from magnum import objects from magnum.objects import fields +from oslo_config import cfg + +CONF = cfg.CONF + LOG = logging.getLogger(__name__) @@ -425,6 +429,14 @@ def post(self, cluster_template): do_raise=False): raise exception.ClusterTemplatePublishDenied() + # check root volume size + boot_volume_size = cluster_template.labels.get( + 'boot_volume_size', CONF.cinder.default_boot_volume_size) + attr_validator.validate_flavor_root_volume_size( + cli, cluster_template.flavor_id, boot_volume_size) + attr_validator.validate_flavor_root_volume_size( + cli, cluster_template.master_flavor_id, boot_volume_size) + if (cluster_template.docker_storage_driver in ('devicemapper', 'overlay')): warnings.warn(self._devicemapper_overlay_deprecation_note, @@ -498,6 +510,15 @@ def patch(self, cluster_template_ident, patch): # noqa do_raise=False): raise exception.ClusterTemplatePublishDenied() + # check root volume size + boot_volume_size = cluster_template.labels.get( + 'boot_volume_size', CONF.cinder.default_boot_volume_size) + cli = clients.OpenStackClients(context) + attr_validator.validate_flavor_root_volume_size( + cli, new_cluster_template.flavor_id, boot_volume_size) + attr_validator.validate_flavor_root_volume_size( + cli, new_cluster_template.master_flavor_id, boot_volume_size) + # Update only the fields that have changed for field in objects.ClusterTemplate.fields: try: diff --git a/magnum/api/controllers/v1/nodegroup.py b/magnum/api/controllers/v1/nodegroup.py index dd7a1b9f87..ef8ddd6b87 100644 --- a/magnum/api/controllers/v1/nodegroup.py +++ b/magnum/api/controllers/v1/nodegroup.py @@ -18,17 +18,22 @@ import wsme from wsme import types as wtypes +from magnum.api import attr_validator from magnum.api.controllers import base from magnum.api.controllers import link from magnum.api.controllers.v1 import collection from magnum.api.controllers.v1 import types from magnum.api import expose from magnum.api import utils as api_utils +from magnum.common import clients from magnum.common import exception from magnum.common import policy +import magnum.conf from magnum import objects from magnum.objects import fields +CONF = magnum.conf.CONF + def _validate_node_count(ng): if ng.max_node_count: @@ -350,6 +355,13 @@ def post(self, cluster_id, nodegroup): labels.update(nodegroup.labels) nodegroup.labels = labels + # check root volume size + boot_volume_size = cluster.labels.get( + 'boot_volume_size', CONF.cinder.default_boot_volume_size) + cli = clients.OpenStackClients(context) + attr_validator.validate_flavor_root_volume_size( + cli, nodegroup.flavor_id, boot_volume_size) + nodegroup_dict = nodegroup.as_dict() nodegroup_dict['cluster_id'] = cluster.uuid nodegroup_dict['project_id'] = context.project_id @@ -414,6 +426,15 @@ def _patch(self, cluster_uuid, nodegroup_id, patch): patch_val = None if nodegroup[field] != patch_val: nodegroup[field] = patch_val + + # check root volume size + cluster = api_utils.get_resource('Cluster', cluster_uuid) + boot_volume_size = cluster.labels.get( + 'boot_volume_size', CONF.cinder.default_boot_volume_size) + cli = clients.OpenStackClients(context) + attr_validator.validate_flavor_root_volume_size( + cli, nodegroup.flavor_id, boot_volume_size) + _validate_node_count(nodegroup) return nodegroup diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 0d2b30e11d..5e6192bd05 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -449,6 +449,11 @@ class ZeroNodeCountNotSupported(NotSupported): "provided microversion.") +class FlavorZeroRootVolumeNotSupported(NotSupported): + message = _("Flavor with zero root volume size is not supported " + "when boot_volume_size is zero.") + + class ClusterUpgradeNotSupported(NotSupported): message = _("Cluster upgrade is not supported in the " "provided microversion.") diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster.py b/magnum/tests/unit/api/controllers/v1/test_cluster.py index 5e70ea9a12..9c37ff40e3 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster.py @@ -288,6 +288,10 @@ def setUp(self): self.mock_cluster_update = p.start() self.mock_cluster_update.side_effect = self._sim_rpc_cluster_update self.addCleanup(p.stop) + p = mock.patch.object( + attr_validator, 'validate_flavor_root_volume_size') + self.mock_valid_flavor_disk = p.start() + self.addCleanup(p.stop) def _sim_rpc_cluster_update(self, cluster, node_count, health_status, health_status_reason, rollback=False): @@ -612,6 +616,10 @@ def setUp(self): p = mock.patch.object(attr_validator, 'validate_os_resources') self.mock_valid_os_res = p.start() self.addCleanup(p.stop) + p = mock.patch.object( + attr_validator, 'validate_flavor_root_volume_size') + self.mock_valid_flavor_disk = p.start() + self.addCleanup(p.stop) def _simulate_cluster_create(self, cluster, master_count, node_count, create_timeout): @@ -873,6 +881,15 @@ def test_create_cluster_with_invalid_flavor(self): self.assertTrue(self.mock_valid_os_res.called) self.assertEqual(400, response.status_int) + def test_create_cluster_with_invalid_flavor_disk_size(self): + bdict = apiutils.cluster_post_data() + self.mock_valid_flavor_disk.side_effect = \ + exception.FlavorZeroRootVolumeNotSupported() + response = self.post_json('/clusters', bdict, expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertTrue(self.mock_valid_flavor_disk.called) + self.assertEqual(400, response.status_int) + def test_create_cluster_with_invalid_ext_network(self): bdict = apiutils.cluster_post_data() self.mock_valid_os_res.side_effect = \ diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py b/magnum/tests/unit/api/controllers/v1/test_cluster_template.py index 50afae0e96..3c835d6a08 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster_template.py @@ -269,6 +269,10 @@ def setUp(self): labels={'key1': 'val1', 'key2': 'val2'}, hidden=False ) + p = mock.patch.object( + attr_validator, 'validate_flavor_root_volume_size') + self.mock_valid_flavor_disk = p.start() + self.addCleanup(p.stop) def test_update_not_found(self): uuid = uuidutils.generate_uuid() @@ -482,6 +486,19 @@ def test_replace_cluster_template_with_no_exist_flavor_id(self): self.assertEqual(400, response.status_code) self.assertTrue(response.json['errors']) + def test_replace_cluster_template_with_invalid_flavor(self): + self.mock_valid_flavor_disk.side_effect = \ + exception.FlavorZeroRootVolumeNotSupported() + response = self.patch_json('/clustertemplates/%s' % + self.cluster_template.uuid, + [{'path': '/flavor_id', 'value': 'aaa', + 'op': 'replace'}], + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(400, response.status_code) + self.assertTrue(self.mock_valid_flavor_disk.called) + self.assertTrue(response.json['errors']) + def test_replace_cluster_template_with_no_exist_keypair_id(self): self.mock_valid_os_res.side_effect = exception.KeyPairNotFound("aaa") response = self.patch_json('/clustertemplates/%s' % @@ -632,6 +649,10 @@ def setUp(self): p = mock.patch.object(attr_validator, 'validate_os_resources') self.mock_valid_os_res = p.start() self.addCleanup(p.stop) + p = mock.patch.object( + attr_validator, 'validate_flavor_root_volume_size') + self.mock_valid_flavor_disk = p.start() + self.addCleanup(p.stop) @mock.patch('magnum.api.attr_validator.validate_image') @mock.patch('oslo_utils.timeutils.utcnow') @@ -1110,6 +1131,21 @@ def test_create_cluster_template_with_no_exist_flavor(self, expect_errors=True) self.assertEqual(400, response.status_int) + @mock.patch('magnum.api.attr_validator.validate_image') + def test_create_cluster_template_with_invalid_flavor( + self, + mock_image_data + ): + self.mock_valid_flavor_disk.side_effect = \ + exception.FlavorZeroRootVolumeNotSupported() + mock_image_data.return_value = {'name': 'mock_name', + 'os_distro': 'fedora-coreos'} + bdict = apiutils.cluster_template_post_data() + response = self.post_json('/clustertemplates', bdict, + expect_errors=True) + self.assertTrue(self.mock_valid_flavor_disk.called) + self.assertEqual(400, response.status_int) + @mock.patch('magnum.api.attr_validator.validate_image') def test_create_cluster_template_with_external_network(self, mock_image_data): diff --git a/magnum/tests/unit/api/controllers/v1/test_nodegroup.py b/magnum/tests/unit/api/controllers/v1/test_nodegroup.py index 68304a10f6..5e61bc16a6 100644 --- a/magnum/tests/unit/api/controllers/v1/test_nodegroup.py +++ b/magnum/tests/unit/api/controllers/v1/test_nodegroup.py @@ -19,6 +19,7 @@ from oslo_utils import timeutils from oslo_utils import uuidutils +from magnum.api import attr_validator from magnum.api.controllers.v1 import nodegroup as api_nodegroup from magnum.conductor import api as rpcapi import magnum.conf @@ -265,6 +266,10 @@ def setUp(self): self.mock_ng_create.side_effect = self._simulate_nodegroup_create self.addCleanup(p.stop) self.url = "/clusters/%s/nodegroups" % self.cluster.uuid + p = mock.patch.object( + attr_validator, 'validate_flavor_root_volume_size') + self.mock_valid_flavor_disk = p.start() + self.addCleanup(p.stop) def _simulate_nodegroup_create(self, cluster, nodegroup): nodegroup.create() @@ -571,6 +576,10 @@ def setUp(self): self.mock_ng_update.side_effect = self._simulate_nodegroup_update self.addCleanup(p.stop) self.url = "/clusters/%s/nodegroups/" % self.cluster.uuid + p = mock.patch.object( + attr_validator, 'validate_flavor_root_volume_size') + self.mock_valid_flavor_disk = p.start() + self.addCleanup(p.stop) def _simulate_nodegroup_update(self, cluster, nodegroup): nodegroup.save() diff --git a/magnum/tests/unit/api/test_attr_validator.py b/magnum/tests/unit/api/test_attr_validator.py index 5a30197550..7c8afc7822 100644 --- a/magnum/tests/unit/api/test_attr_validator.py +++ b/magnum/tests/unit/api/test_attr_validator.py @@ -367,3 +367,50 @@ def test_validate_os_resources_with_cluster(self, mock_os_cli): attr_validator.validate_os_resources(mock_context, mock_cluster_template, mock_cluster) + + def test_validate_flavor_root_volume_size_with_valid_boot_volume_size( + self + ): + boot_volume_size = 100 + mock_flavor = mock.MagicMock() + mock_flavor.name = 'test_flavor' + mock_flavor.id = 'test_flavor_id' + mock_flavor.disk = 0 + mock_flavors = [mock_flavor] + mock_nova = mock.MagicMock() + mock_nova.flavors.list.return_value = mock_flavors + mock_os_cli = mock.MagicMock() + mock_os_cli.nova.return_value = mock_nova + attr_validator.validate_flavor_root_volume_size( + mock_os_cli, 'test_flavor', boot_volume_size) + self.assertFalse(mock_nova.flavors.list.called) + + def test_validate_flavor_root_volume_size_with_valid_flavor(self): + boot_volume_size = 0 + mock_flavor = mock.MagicMock() + mock_flavor.name = 'test_flavor' + mock_flavor.id = 'test_flavor_id' + mock_flavor.disk = 100 + mock_flavors = [mock_flavor] + mock_nova = mock.MagicMock() + mock_nova.flavors.list.return_value = mock_flavors + mock_os_cli = mock.MagicMock() + mock_os_cli.nova.return_value = mock_nova + attr_validator.validate_flavor_root_volume_size( + mock_os_cli, 'test_flavor', boot_volume_size) + self.assertTrue(mock_nova.flavors.list.called) + + def test_validate_flavor_root_volume_size_with_invalid_resources(self): + boot_volume_size = 0 + mock_flavor = mock.MagicMock() + mock_flavor.name = 'test_flavor' + mock_flavor.id = 'test_flavor_id' + mock_flavor.disk = 0 + mock_flavors = [mock_flavor] + mock_nova = mock.MagicMock() + mock_nova.flavors.list.return_value = mock_flavors + mock_os_cli = mock.MagicMock() + mock_os_cli.nova.return_value = mock_nova + self.assertRaises(exception.FlavorZeroRootVolumeNotSupported, + attr_validator.validate_flavor_root_volume_size, + mock_os_cli, 'test_flavor', boot_volume_size) diff --git a/releasenotes/notes/add-boot-volume-size-check-0262c2b61abc7ccf.yaml b/releasenotes/notes/add-boot-volume-size-check-0262c2b61abc7ccf.yaml new file mode 100644 index 0000000000..d8e3ba8612 --- /dev/null +++ b/releasenotes/notes/add-boot-volume-size-check-0262c2b61abc7ccf.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Add a validation for the case when boot_volume_size + label and flavor's disk are both zero. From 075f62e8172b366b70854bc2dc1908ab5f60632a Mon Sep 17 00:00:00 2001 From: okozachenko Date: Tue, 3 Mar 2026 14:51:48 +0100 Subject: [PATCH 2/2] fix TypeError when boot_volume_size label is passed as string Change-Id: I246494a4e32018465c7092d22ffa5f09bbaf179a Signed-off-by: okozachenko Closes-bug: #2144473 (cherry picked from commit c73f9e152598a99cff5e7d64de466f41c3c7810e) --- magnum/api/attr_validator.py | 4 ++++ .../fix-boot-volume-size-label-type-a5625def4c6db32b.yaml | 5 +++++ 2 files changed, 9 insertions(+) create mode 100644 releasenotes/notes/fix-boot-volume-size-label-type-a5625def4c6db32b.yaml diff --git a/magnum/api/attr_validator.py b/magnum/api/attr_validator.py index de9639c868..962e907c16 100644 --- a/magnum/api/attr_validator.py +++ b/magnum/api/attr_validator.py @@ -14,6 +14,7 @@ from glanceclient import exc as glance_exception from novaclient import exceptions as nova_exception +from oslo_utils import strutils from magnum.api import utils as api_utils from magnum.common import clients @@ -180,6 +181,9 @@ def validate_master_count(context, cluster): def validate_flavor_root_volume_size(cli, flavor, boot_volume_size): """Validate flavor root volume size.""" + boot_volume_size = strutils.validate_integer(boot_volume_size, + "boot_volume_size") + if boot_volume_size > 0: return diff --git a/releasenotes/notes/fix-boot-volume-size-label-type-a5625def4c6db32b.yaml b/releasenotes/notes/fix-boot-volume-size-label-type-a5625def4c6db32b.yaml new file mode 100644 index 0000000000..c3db6e8c44 --- /dev/null +++ b/releasenotes/notes/fix-boot-volume-size-label-type-a5625def4c6db32b.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + The boot_volume_size label value is passed as a string from the API. + The validator now converts it to an integer before comparison.