From ecc6b64f4aa1b212b9b6f819fea2bfdf6d5b49fd Mon Sep 17 00:00:00 2001 From: jiaopengju Date: Sun, 2 Dec 2018 20:59:00 +0800 Subject: [PATCH] Add support to reset checkpoint state Now when doing checkpoint copy failed, checkpoint will be wait_copying status forever, and so we can not do the restore anymore. So we should add an API to support checkpoint status reset if we deeply knows that the checkpoint is ok. This patch added API support for doing checkpoint state reset. Implements: bp checkpoint-status-reset Change-Id: Iabaa98c9900fba554be2ad0833d438901e01147a --- karbor/api/schemas/checkpoints.py | 19 ++++ karbor/api/v1/providers.py | 39 ++++++++ karbor/api/v1/router.py | 6 ++ karbor/exception.py | 4 + karbor/policies/providers.py | 12 +++ karbor/services/protection/api.py | 8 ++ karbor/services/protection/manager.py | 24 +++++ karbor/services/protection/rpcapi.py | 9 ++ karbor/tests/unit/api/v1/test_providers.py | 96 +++++++++++++++++++ karbor/tests/unit/protection/test_manager.py | 48 ++++++++++ ...ckpoint-status-reset-d714b4a04da2f44d.yaml | 4 + 11 files changed, 269 insertions(+) create mode 100644 releasenotes/notes/checkpoint-status-reset-d714b4a04da2f44d.yaml diff --git a/karbor/api/schemas/checkpoints.py b/karbor/api/schemas/checkpoints.py index 9e1fb8c2..d6da9673 100644 --- a/karbor/api/schemas/checkpoints.py +++ b/karbor/api/schemas/checkpoints.py @@ -35,3 +35,22 @@ create = { 'required': ['checkpoint'], 'additionalProperties': False, } + +update = { + 'type': 'object', + 'properties': { + 'os-resetState': { + 'type': 'object', + 'properties': { + 'state': { + 'type': 'string', + 'enum': ['available', 'error'], + }, + }, + 'required': ['state'], + 'additionalProperties': False, + }, + }, + 'required': [], + 'additionalProperties': False, +} diff --git a/karbor/api/v1/providers.py b/karbor/api/v1/providers.py index 96854d18..3549b031 100644 --- a/karbor/api/v1/providers.py +++ b/karbor/api/v1/providers.py @@ -476,6 +476,45 @@ class ProvidersController(wsgi.Controller): LOG.info("Delete checkpoint request issued successfully.") return {} + def _checkpoint_reset_state(self, context, provider_id, + checkpoint_id, state): + try: + self.protection_api.reset_state(context, provider_id, + checkpoint_id, state) + except exception.AccessCheckpointNotAllowed as error: + raise exc.HTTPForbidden(explanation=error.msg) + except exception.CheckpointNotFound as error: + raise exc.HTTPNotFound(explanation=error.msg) + except exception.CheckpointNotBeReset as error: + raise exc.HTTPBadRequest(explanation=error.msg) + LOG.info("Reset checkpoint state request issued successfully.") + return {} + + @validation.schema(checkpoint_schema.update) + def checkpoints_update(self, req, provider_id, checkpoint_id, body): + """Reset a checkpoint's state""" + context = req.environ['karbor.context'] + + LOG.info("Reset checkpoint state with id: %s", checkpoint_id) + LOG.info("provider_id: %s.", provider_id) + + if not uuidutils.is_uuid_like(provider_id): + msg = _("Invalid provider id provided.") + raise exc.HTTPBadRequest(explanation=msg) + + if not uuidutils.is_uuid_like(checkpoint_id): + msg = _("Invalid checkpoint id provided.") + raise exc.HTTPBadRequest(explanation=msg) + + context.can(provider_policy.CHECKPOINT_UPDATE_POLICY) + if body.get("os-resetState"): + state = body["os-resetState"]["state"] + return self._checkpoint_reset_state( + context, provider_id, checkpoint_id, state) + else: + msg = _("Invalid input.") + raise exc.HTTPBadRequest(explanation=msg) + def create_resource(): return wsgi.Resource(ProvidersController()) diff --git a/karbor/api/v1/router.py b/karbor/api/v1/router.py index a39159d6..4838289c 100644 --- a/karbor/api/v1/router.py +++ b/karbor/api/v1/router.py @@ -96,6 +96,12 @@ class APIRouter(base_wsgi.Router): controller=providers_resources, action='checkpoints_delete', conditions={"method": ['DELETE']}) + mapper.connect("provider", + "/{project_id}/providers/{provider_id}/checkpoints/" + "{checkpoint_id}", + controller=providers_resources, + action='checkpoints_update', + conditions={'method': ['PUT']}) mapper.resource("trigger", "triggers", controller=trigger_resources, collection={}, diff --git a/karbor/exception.py b/karbor/exception.py index 18cff957..f5fc1274 100644 --- a/karbor/exception.py +++ b/karbor/exception.py @@ -390,6 +390,10 @@ class CheckpointNotBeDeleted(KarborException): message = _("The checkpoint %(checkpoint_id)s can not be deleted.") +class CheckpointNotBeReset(KarborException): + message = _("The checkpoint %(checkpoint_id)s can not be reset.") + + class GetProtectionNetworkSubResourceFailed(KarborException): message = _("Get protection network sub-resources of type %(type)s failed:" " %(reason)s") diff --git a/karbor/policies/providers.py b/karbor/policies/providers.py index a3f4865c..eb6f547d 100644 --- a/karbor/policies/providers.py +++ b/karbor/policies/providers.py @@ -24,6 +24,7 @@ CHECKPOINT_GET_POLICY = 'provider:checkpoint_get' CHECKPOINT_GET_ALL_POLICY = 'provider:checkpoint_get_all' CHECKPOINT_CREATE_POLICY = 'provider:checkpoint_create' CHECKPOINT_DELETE_POLICY = 'provider:checkpoint_delete' +CHECKPOINT_UPDATE_POLICY = 'provider:checkpoint_update' providers_policies = [ @@ -87,6 +88,17 @@ providers_policies = [ 'path': '/providers/{provider_id}/checkpoints/{checkpoint_id}' } ]), + policy.DocumentedRuleDefault( + name=CHECKPOINT_UPDATE_POLICY, + check_str=base.RULE_ADMIN_OR_OWNER, + description='Reset checkpoint state.', + operations=[ + { + 'method': 'PUT', + 'path': '/providers/{provider_id}/checkpoints/{checkpoint_id}' + } + ] + ) ] diff --git a/karbor/services/protection/api.py b/karbor/services/protection/api.py index d8c01efe..89b67826 100644 --- a/karbor/services/protection/api.py +++ b/karbor/services/protection/api.py @@ -44,6 +44,14 @@ class API(base.Base): checkpoint_id ) + def reset_state(self, context, provider_id, checkpoint_id, state): + return self.protection_rpcapi.reset_state( + context, + provider_id, + checkpoint_id, + state + ) + def show_checkpoint(self, context, provider_id, checkpoint_id): return self.protection_rpcapi.show_checkpoint( context, diff --git a/karbor/services/protection/manager.py b/karbor/services/protection/manager.py index a2de22ee..0bb74298 100644 --- a/karbor/services/protection/manager.py +++ b/karbor/services/protection/manager.py @@ -363,6 +363,30 @@ class ProtectionManager(manager.Manager): )) self._spawn(self.worker.run_flow, flow) + @messaging.expected_exceptions(exception.AccessCheckpointNotAllowed, + exception.CheckpointNotBeReset) + def reset_state(self, context, provider_id, checkpoint_id, state): + provider = self.provider_registry.show_provider(provider_id) + + checkpoint = provider.get_checkpoint(checkpoint_id, context=context) + checkpoint_dict = checkpoint.to_dict() + if not context.is_admin and ( + context.project_id != checkpoint_dict['project_id']): + raise exception.AccessCheckpointNotAllowed( + checkpoint_id=checkpoint_id) + + if checkpoint.status not in [ + constants.CHECKPOINT_STATUS_AVAILABLE, + constants.CHECKPOINT_STATUS_ERROR, + constants.CHECKPOINT_STATUS_COPYING, + constants.CHECKPOINT_STATUS_WAIT_COPYING, + constants.CHECKPOINT_STATUS_COPY_FINISHED + ]: + raise exception.CheckpointNotBeReset( + checkpoint_id=checkpoint_id) + checkpoint.status = state + checkpoint.commit() + def start(self, plan): # TODO(wangliuan) pass diff --git a/karbor/services/protection/rpcapi.py b/karbor/services/protection/rpcapi.py index 7fd2f81d..ce734c91 100644 --- a/karbor/services/protection/rpcapi.py +++ b/karbor/services/protection/rpcapi.py @@ -82,6 +82,15 @@ class ProtectionAPI(object): provider_id=provider_id, checkpoint_id=checkpoint_id) + def reset_state(self, ctxt, provider_id, checkpoint_id, state): + cctxt = self.client.prepare(version='1.0') + return cctxt.call( + ctxt, + 'reset_state', + provider_id=provider_id, + checkpoint_id=checkpoint_id, + state=state) + def show_checkpoint(self, ctxt, provider_id, checkpoint_id): cctxt = self.client.prepare(version='1.0') return cctxt.call( diff --git a/karbor/tests/unit/api/v1/test_providers.py b/karbor/tests/unit/api/v1/test_providers.py index 65cffd3c..56a2826c 100644 --- a/karbor/tests/unit/api/v1/test_providers.py +++ b/karbor/tests/unit/api/v1/test_providers.py @@ -18,6 +18,7 @@ from webob import exc from karbor.api.v1 import providers from karbor import context +from karbor import exception from karbor.tests import base from karbor.tests.unit.api import fakes @@ -138,3 +139,98 @@ class ProvidersApiTest(base.TestCase): body=body) self.assertTrue(mock_plan_create.called) self.assertTrue(mock_protect.called) + + @mock.patch('karbor.services.protection.api.API.reset_state') + def test_checkpoints_update_reset_state(self, mock_reset_state): + req = fakes.HTTPRequest.blank('/v1/providers/{provider_id}/' + 'checkpoints/{checkpoint_id}') + body = { + 'os-resetState': {'state': 'error'} + } + self.controller.checkpoints_update( + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body=body) + self.assertTrue(mock_reset_state.called) + + def test_checkpoints_update_reset_state_with_invalid_provider_id(self): + req = fakes.HTTPRequest.blank('/v1/providers/{provider_id}/' + 'checkpoints/{checkpoint_id}') + body = { + 'os-resetState': {'state': 'error'} + } + self.assertRaises( + exc.HTTPBadRequest, + self.controller.checkpoints_update, + req, + 'invalid_provider_id', + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body=body) + + def test_checkpoints_update_reset_state_with_invalid_checkpoint_id(self): + req = fakes.HTTPRequest.blank('/v1/providers/{provider_id}/' + 'checkpoints/{checkpoint_id}') + body = { + 'os-resetState': {'state': 'error'} + } + self.assertRaises( + exc.HTTPBadRequest, + self.controller.checkpoints_update, + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + 'invalid_checkpoint_id', + body=body) + + def test_checkpoints_update_reset_state_with_invalid_body(self): + req = fakes.HTTPRequest.blank('/v1/providers/{provider_id}/' + 'checkpoints/{checkpoint_id}') + self.assertRaises( + exc.HTTPBadRequest, + self.controller.checkpoints_update, + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body={}) + self.assertRaises( + exception.ValidationError, + self.controller.checkpoints_update, + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body={'os-resetState': {'state': 'invalid_state'}}) + + @mock.patch('karbor.services.protection.api.API.reset_state') + def test_checkpoints_update_reset_state_with_protection_api_exceptions( + self, mock_reset_state): + req = fakes.HTTPRequest.blank('/v1/providers/{provider_id}/' + 'checkpoints/{checkpoint_id}') + body = { + 'os-resetState': {'state': 'error'} + } + mock_reset_state.side_effect = exception.AccessCheckpointNotAllowed( + checkpoint_id='2220f8b1-975d-4621-a872-fa9afb43cb6c') + self.assertRaises(exc.HTTPForbidden, + self.controller.checkpoints_update, + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body=body) + + mock_reset_state.side_effect = exception.CheckpointNotFound( + checkpoint_id='2220f8b1-975d-4621-a872-fa9afb43cb6c') + self.assertRaises(exc.HTTPNotFound, + self.controller.checkpoints_update, + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body=body) + + mock_reset_state.side_effect = exception.CheckpointNotBeReset( + checkpoint_id='2220f8b1-975d-4621-a872-fa9afb43cb6c') + self.assertRaises(exc.HTTPBadRequest, + self.controller.checkpoints_update, + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body=body) diff --git a/karbor/tests/unit/protection/test_manager.py b/karbor/tests/unit/protection/test_manager.py index 6ecf2636..132304bc 100644 --- a/karbor/tests/unit/protection/test_manager.py +++ b/karbor/tests/unit/protection/test_manager.py @@ -233,6 +233,54 @@ class ProtectionServiceTest(base.TestCase): 'provider1', 'non_existent_checkpoint') + @mock.patch.object(provider.ProviderRegistry, 'show_provider') + def test_checkpoint_state_reset(self, mock_provider): + fake_provider = fakes.FakeProvider() + fake_checkpoint = fakes.FakeCheckpoint() + fake_checkpoint.commit = mock.MagicMock() + fake_provider.get_checkpoint = mock.MagicMock( + return_value=fake_checkpoint) + mock_provider.return_value = fake_provider + context = mock.MagicMock(project_id='fake_project_id', is_admin=True) + self.pro_manager.reset_state(context, 'provider1', 'fake_checkpoint', + 'error') + self.assertEqual(fake_checkpoint.status, 'error') + self.assertEqual(True, fake_checkpoint.commit.called) + + @mock.patch.object(provider.ProviderRegistry, 'show_provider') + def test_checkpoint_state_reset_with_access_not_allowed( + self, mock_provider): + fake_provider = fakes.FakeProvider() + fake_checkpoint = fakes.FakeCheckpoint() + fake_provider.get_checkpoint = mock.MagicMock( + return_value=fake_checkpoint) + mock_provider.return_value = fake_provider + context = mock.MagicMock(project_id='fake_project_id_01', + is_admin=False) + self.assertRaises(oslo_messaging.ExpectedException, + self.pro_manager.reset_state, + context, + 'fake_project_id', + 'fake_checkpoint_id', + 'error') + + @mock.patch.object(provider.ProviderRegistry, 'show_provider') + def test_checkpoint_state_reset_with_wrong_checkpoint_state( + self, mock_provider): + fake_provider = fakes.FakeProvider() + fake_checkpoint = fakes.FakeCheckpoint() + fake_checkpoint.status = 'deleting' + fake_provider.get_checkpoint = mock.MagicMock( + return_value=fake_checkpoint) + mock_provider.return_value = fake_provider + context = mock.MagicMock(project_id='fake_project_id', is_admin=True) + self.assertRaises(oslo_messaging.ExpectedException, + self.pro_manager.reset_state, + context, + 'fake_project_id', + 'fake_checkpoint_id', + 'error') + def tearDown(self): flow_manager.Worker._load_engine = self.load_engine super(ProtectionServiceTest, self).tearDown() diff --git a/releasenotes/notes/checkpoint-status-reset-d714b4a04da2f44d.yaml b/releasenotes/notes/checkpoint-status-reset-d714b4a04da2f44d.yaml new file mode 100644 index 00000000..aee4633f --- /dev/null +++ b/releasenotes/notes/checkpoint-status-reset-d714b4a04da2f44d.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Added support for checkpoint state reset by admin and owner.