From f38e77cb181e1d94cfc8456f16600b0860419792 Mon Sep 17 00:00:00 2001 From: John Griffith Date: Sat, 6 Jan 2018 11:56:19 -0700 Subject: [PATCH] Add multiattach policy This patch adds the policy for creating and retyping volumes with volume types that indicate multi attach capabilities. There are two policies being added: 1. MULTIATTACH_POLICY General policy to disallow creating volumes of type multiattach as well as retyping volumes to/from a multiattach type. 2. MULTIATTACH_BOOTABLE_VOLUME_POLICY Specific policy to disallow creating multiple attachments for any volume that is `bootable`. We use policy to control the use of this particular type, and we also limit the ability to retype a volumes multi attach settings to only volumes that are in available status. We need to to do this because multi attach has implications for things on the Compute side (ie libvirt) that would require detach/reattach to keep things synced correctly. Currently there's no back end reporting the `multiattach=True` capability, that will be in the next patch of this series. Change-Id: I3fd8afe9cbae3c733a6530dce7be6fef8d53cfa6 blueprint: multi-attach-v3-attach --- cinder/policies/attachments.py | 11 +++++ cinder/policies/volumes.py | 11 +++++ .../unit/attachments/test_attachments_api.py | 27 +++++++++++++ cinder/tests/unit/volume/test_volume.py | 21 ++++++++++ cinder/volume/api.py | 40 +++++++++++++++++++ ...multiattach_policies-8e0b22505ed6cbd8.yaml | 27 +++++++++++++ 6 files changed, 137 insertions(+) create mode 100644 releasenotes/notes/add_multiattach_policies-8e0b22505ed6cbd8.yaml diff --git a/cinder/policies/attachments.py b/cinder/policies/attachments.py index 3dc13168d72..1378e4477fc 100644 --- a/cinder/policies/attachments.py +++ b/cinder/policies/attachments.py @@ -21,6 +21,7 @@ from cinder.policies import base CREATE_POLICY = 'volume:attachment_create' UPDATE_POLICY = 'volume:attachment_update' DELETE_POLICY = 'volume:attachment_delete' +MULTIATTACH_BOOTABLE_VOLUME_POLICY = 'volume:multiattach_bootable_volume' attachments_policies = [ policy.DocumentedRuleDefault( @@ -53,6 +54,16 @@ attachments_policies = [ 'path': '/attachments/{attachment_id}' } ]), + policy.DocumentedRuleDefault( + name=MULTIATTACH_BOOTABLE_VOLUME_POLICY, + check_str=base.RULE_ADMIN_OR_OWNER, + description="Allow multiattach of bootable volumes.", + operations=[ + { + 'method': 'POST', + 'path': '/attachments' + } + ]), ] diff --git a/cinder/policies/volumes.py b/cinder/policies/volumes.py index 15b9245be11..d9d8d024bd7 100644 --- a/cinder/policies/volumes.py +++ b/cinder/policies/volumes.py @@ -29,6 +29,7 @@ HOST_ATTRIBUTE_POLICY = "volume_extension:volume_host_attribute" TENANT_ATTRIBUTE_POLICY = "volume_extension:volume_tenant_attribute" MIG_ATTRIBUTE_POLICY = "volume_extension:volume_mig_status_attribute" ENCRYPTION_METADATA_POLICY = "volume_extension:volume_encryption_metadata" +MULTIATTACH_POLICY = "volume:multiattach" volumes_policies = [ policy.DocumentedRuleDefault( @@ -161,6 +162,16 @@ volumes_policies = [ 'path': '/volumes/{volume_id}/encryption/{encryption_key}' } ]), + policy.DocumentedRuleDefault( + name=MULTIATTACH_POLICY, + check_str=base.RULE_ADMIN_OR_OWNER, + description="Create multiattach capable volume.", + operations=[ + { + 'method': 'POST', + 'path': '/volumes' + } + ]), ] diff --git a/cinder/tests/unit/attachments/test_attachments_api.py b/cinder/tests/unit/attachments/test_attachments_api.py index 0bcf15b6ded..fbfc6bd9eeb 100644 --- a/cinder/tests/unit/attachments/test_attachments_api.py +++ b/cinder/tests/unit/attachments/test_attachments_api.py @@ -12,11 +12,15 @@ import mock from oslo_config import cfg +from oslo_policy import policy as oslo_policy from cinder import context from cinder import db from cinder import exception from cinder import objects +from cinder.policies import attachments as attachment_policy +from cinder.policies import base as base_policy +from cinder import policy from cinder import test from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import utils as tests_utils @@ -38,6 +42,9 @@ class AttachmentManagerTestCase(test.TestCase): self.project_id = fake.PROJECT3_ID self.context.project_id = self.project_id self.volume_api = volume_api.API() + self.user_context = context.RequestContext( + user_id=fake.USER_ID, + project_id=fake.PROJECT3_ID) def test_attachment_create_no_connector(self): """Test attachment_create no connector.""" @@ -237,3 +244,23 @@ class AttachmentManagerTestCase(test.TestCase): vref.id) self.assertEqual('reserved', vref.status) self.assertEqual(1, len(vref.volume_attachment)) + + def test_attachment_create_bootable_multiattach_policy(self): + """Test attachment_create no connector.""" + volume_params = {'status': 'available'} + + vref = tests_utils.create_volume(self.context, **volume_params) + vref.multiattach = True + vref.bootable = True + vref.status = 'in-use' + + rules = { + attachment_policy.MULTIATTACH_BOOTABLE_VOLUME_POLICY: base_policy.RULE_ADMIN_API # noqa + } + policy.set_rules(oslo_policy.Rules.from_dict(rules)) + self.addCleanup(policy.reset) + self.assertRaises(exception.PolicyNotAuthorized, + self.volume_api.attachment_create, + self.user_context, + vref, + fake.UUID2) diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index d8649ec9ff7..ce4df54988e 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -54,6 +54,7 @@ from cinder.volume import driver from cinder.volume import manager as vol_manager from cinder.volume import rpcapi as volume_rpcapi import cinder.volume.targets.tgt +from cinder.volume import volume_types QUOTAS = quota.QUOTAS @@ -606,6 +607,26 @@ class VolumeTestCase(base.BaseVolumeTestCase): volume_type=db_vol_type) self.assertEqual(db_vol_type.get('id'), volume['volume_type_id']) + def test_create_volume_with_multiattach_volume_type(self): + """Test volume creation with multiattach volume type.""" + elevated = context.get_admin_context() + volume_api = cinder.volume.api.API() + + especs = dict(multiattach=" True") + volume_types.create(elevated, + "multiattach-type", + especs, + description="test-multiattach") + foo = objects.VolumeType.get_by_name_or_id(elevated, + "multiattach-type") + + vol = volume_api.create(self.context, + 1, + 'admin-vol', + 'description', + volume_type=foo) + self.assertEqual(foo['id'], vol['volume_type_id']) + @mock.patch.object(key_manager, 'API', fake_keymgr.fake_api) def test_create_volume_with_encrypted_volume_type_aes(self): ctxt = context.get_admin_context() diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 79bc7b10d83..fdf84705263 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -193,6 +193,10 @@ class API(base.Base): return (volume['migration_status'] not in self.AVAILABLE_MIGRATION_STATUS) + def _is_multiattach(self, volume_type): + specs = getattr(volume_type, 'extra_specs', {}) + return specs.get('multiattach', 'False') == ' True' + def create(self, context, size, name, description, snapshot=None, image_id=None, volume_type=None, metadata=None, availability_zone=None, source_volume=None, @@ -285,6 +289,9 @@ class API(base.Base): utils.check_metadata_properties(metadata) + if (volume_type and self._is_multiattach(volume_type)): + context.authorize(vol_policy.MULTIATTACH_POLICY) + create_what = { 'context': context, 'raw_size': size, @@ -1622,6 +1629,30 @@ class API(base.Base): vol_type_id = vol_type['id'] + # NOTE(jdg): We check here if multiattach is involved in either side + # of the retype, we can't change multiattach on an in-use volume + # because there's things the hypervisor needs when attaching, so + # we just disallow retype of in-use volumes in this case. You still + # have to get through scheduling if all the conditions are met, we + # should consider an up front capabilities check to give fast feedback + # rather than "No hosts found" and error status + + src_is_multiattach = volume.multiattach + tgt_is_multiattach = False + if (vol_type and + self._is_multiattach(vol_type)): + tgt_is_multiattach = True + + if src_is_multiattach != tgt_is_multiattach: + if volume.status != "available": + msg = _('Invalid volume_type passed, retypes affecting ' + 'multiattach are only allowed on available volumes, ' + 'the specified volume however currently has a status ' + 'of: %s.') % volume.status + LOG.info(msg) + raise exception.InvalidInput(reason=msg) + context.authorize(vol_policy.MULTIATTACH_POLICY) + # We're checking here in so that we can report any quota issues as # early as possible, but won't commit until we change the type. We # pass the reservations onward in case we need to roll back. @@ -2021,6 +2052,14 @@ class API(base.Base): # creation of other new reserves/attachments while in this state # so we avoid contention issues with shared connections + # Multiattach of bootable volumes is a special case with it's own + # policy, check that here right off the bat + if (vref.get('multiattach', False) and + vref.status == 'in-use' and + vref.bootable): + context.authorize( + attachment_policy.MULTIATTACH_BOOTABLE_VOLUME_POLICY) + # FIXME(JDG): We want to be able to do things here like reserve a # volume for Nova to do BFV WHILE the volume may be in the process of # downloading image, we add downloading here; that's easy enough but @@ -2032,6 +2071,7 @@ class API(base.Base): 'status': (('available', 'in-use', 'downloading') if vref.multiattach else ('available', 'downloading'))} + result = vref.conditional_update({'status': 'reserved'}, expected) if not result: diff --git a/releasenotes/notes/add_multiattach_policies-8e0b22505ed6cbd8.yaml b/releasenotes/notes/add_multiattach_policies-8e0b22505ed6cbd8.yaml new file mode 100644 index 00000000000..162cc610d2a --- /dev/null +++ b/releasenotes/notes/add_multiattach_policies-8e0b22505ed6cbd8.yaml @@ -0,0 +1,27 @@ +--- +features: + - Added policies to disallow multiattach operations. This includes two + policies, the first being a general policy to allow the creation or + retyping of multiattach volumes is a volume create policy with the name + ``MULTIATTACH_POLICY``. + + The second policy is specifically for disallowing the ability to create + multiple attachments on a volume that is marked as bootable, and is an + attachment policy with the name ``MULTIATTACH_BOOTABLE_VOLUME_POLICY``. + + The default for these new policies is ``ADMIN_OR_OWNER``; be aware that + if you wish to disable either of these policies for your users you will + need to modify the default policy settings. +upgrade: + - Added policies to disallow multiattach operations. This includes two + policies, the first being a general policy to allow the creation or + retyping of multiattach volumes is a volume create policy with the name + ``MULTIATTACH_POLICY``. + + The second policy is specifically for disallowing the ability to create + multiple attachments on a volume that is marked as bootable, and is an + attachment policy with the name ``MULTIATTACH_BOOTABLE_VOLUME_POLICY``. + + The default for these new policies is ``ADMIN_OR_OWNER``; be aware that + if you wish to disable either of these policies for your users you will + need to modify the default policy settings.