diff --git a/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py b/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py index 188058306..d74889991 100644 --- a/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py +++ b/distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py @@ -25,7 +25,6 @@ from dcmanager.common import exceptions from dcmanager.common.i18n import _ from dcmanager.common import utils from dcmanager.db import api as db_api -from dcmanager.db.sqlalchemy.models import Subcloud from dcmanager.rpc import client as rpc_client CONF = cfg.CONF @@ -146,12 +145,24 @@ class SubcloudBackupController(object): operation (string): Subcloud backup operation """ subclouds = request_entity.subclouds - error_msg = '' + error_msg = _('Subcloud(s) must be in a valid state for backup %s.' % operation) has_valid_subclouds = False for subcloud in subclouds: try: - if utils.is_valid_for_backup_operation(operation, subcloud): - has_valid_subclouds = True + is_valid = utils.is_valid_for_backup_operation(operation, subcloud) + + if operation == 'create': + backup_in_progress = subcloud.backup_status in \ + consts.STATES_FOR_ONGOING_BACKUP + if is_valid and not backup_in_progress: + has_valid_subclouds = True + else: + error_msg = _('Subcloud(s) already have a backup ' + 'operation in progress.') + else: + if is_valid: + has_valid_subclouds = True + except exceptions.ValidateFail as e: error_msg = e.message @@ -202,19 +213,6 @@ class SubcloudBackupController(object): else: pecan.abort(400, _("'subcloud' or 'group' parameter is required")) - @staticmethod - def _reset_backup_status(context, subclouds): - subcloud_ids = [] - for subcloud in subclouds: - subcloud.backup_status = consts.BACKUP_STATE_INITIAL - subcloud_ids.append(subcloud.id) - - update_form = { - Subcloud.backup_status.name: consts.BACKUP_STATE_INITIAL - } - - db_api.subcloud_bulk_update_by_ids(context, subcloud_ids, update_form) - @utils.synchronized(LOCK_NAME) @index.when(method='POST', template='json') def post(self): @@ -237,12 +235,9 @@ class SubcloudBackupController(object): # Set subcloud/group ID as reference instead of name to ease processing payload[request_entity.type] = request_entity.id - subclouds = request_entity.subclouds - self._convert_param_to_bool(payload, ['local_only', 'registry_images']) try: - self._reset_backup_status(context, subclouds) self.dcmanager_rpc_client.backup_subclouds(context, payload) return utils.subcloud_db_list_to_dict(request_entity.subclouds) except RemoteError as e: diff --git a/distributedcloud/dcmanager/common/consts.py b/distributedcloud/dcmanager/common/consts.py index f5a683e4b..fa2d958b2 100644 --- a/distributedcloud/dcmanager/common/consts.py +++ b/distributedcloud/dcmanager/common/consts.py @@ -298,7 +298,15 @@ VALID_DEPLOY_STATES_FOR_BACKUP = [DEPLOY_STATE_DONE, INVALID_DEPLOY_STATES_FOR_RESTORE = [DEPLOY_STATE_INSTALLING, DEPLOY_STATE_BOOTSTRAPPING, DEPLOY_STATE_DEPLOYING, - DEPLOY_STATE_REHOMING] + DEPLOY_STATE_REHOMING, + DEPLOY_STATE_PRE_RESTORE, + DEPLOY_STATE_RESTORING] + +# States to indicate if a backup operation is currently in progress +STATES_FOR_ONGOING_BACKUP = [BACKUP_STATE_INITIAL, + BACKUP_STATE_VALIDATING, + BACKUP_STATE_PRE_BACKUP, + BACKUP_STATE_IN_PROGRESS] # The k8s secret that holds openldap CA certificate OPENLDAP_CA_CERT_SECRET_NAME = "system-local-ca" diff --git a/distributedcloud/dcmanager/manager/subcloud_manager.py b/distributedcloud/dcmanager/manager/subcloud_manager.py index ab4e83c7f..66f603bad 100644 --- a/distributedcloud/dcmanager/manager/subcloud_manager.py +++ b/distributedcloud/dcmanager/manager/subcloud_manager.py @@ -608,6 +608,10 @@ class SubcloudManager(manager.Manager): subclouds = [db_api.subcloud_get(context, subcloud_id)] if subcloud_id\ else db_api.subcloud_get_for_group(context, group_id) + self._filter_subclouds_with_ongoing_backup(subclouds) + self._update_backup_status(context, subclouds, + consts.BACKUP_STATE_INITIAL) + # Validate the subclouds and filter the ones applicable for backup self._update_backup_status(context, subclouds, consts.BACKUP_STATE_VALIDATING) @@ -728,6 +732,17 @@ class SubcloudManager(manager.Manager): invalid_subclouds) return + def _filter_subclouds_with_ongoing_backup(self, subclouds): + i = 0 + while i < len(subclouds): + subcloud = subclouds[i] + if subcloud.backup_status in consts.STATES_FOR_ONGOING_BACKUP: + LOG.info(_('Subcloud %s already has a backup operation in ' + 'progress' % subcloud.name)) + subclouds.pop(i) + else: + i += 1 + def _validate_subclouds_for_backup(self, subclouds, operation): valid_subclouds = [] invalid_subclouds = [] diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py index b597d0aed..a1fe9b01a 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py @@ -596,6 +596,52 @@ class TestSubcloudCreate(testroot.DCManagerApiTest): self.assertEqual(response.status_int, 200) + @mock.patch('dcmanager.common.utils.OpenStackDriver') + @mock.patch('dcmanager.common.utils.SysinvClient') + @mock.patch.object(rpc_client, 'ManagerClient') + def test_create_concurrent_backup(self, mock_rpc_client, mock_sysinv, + mock_openstack): + + mock_sysinv().get_system_health.return_value = FAKE_GOOD_SYSTEM_HEALTH + mock_rpc_client().backup_subclouds.return_value = True + subcloud = fake_subcloud.create_fake_subcloud(self.ctx) + fake_password = (base64.b64encode('testpass'.encode("utf-8"))).decode('ascii') + data = {'sysadmin_password': fake_password, + 'subcloud': '1'} + db_api.subcloud_update(self.ctx, + subcloud.id, + deploy_status=consts.DEPLOY_STATE_DONE, + availability_status=dccommon_consts.AVAILABILITY_ONLINE, + management_state=dccommon_consts.MANAGEMENT_MANAGED) + + ongoing_backup_states = [consts.BACKUP_STATE_INITIAL, + consts.BACKUP_STATE_VALIDATING, + consts.BACKUP_STATE_PRE_BACKUP, + consts.BACKUP_STATE_IN_PROGRESS] + + final_backup_states = [consts.BACKUP_STATE_VALIDATE_FAILED, + consts.BACKUP_STATE_PREP_FAILED, + consts.BACKUP_STATE_FAILED, + consts.BACKUP_STATE_UNKNOWN, + consts.BACKUP_STATE_COMPLETE_CENTRAL, + consts.BACKUP_STATE_COMPLETE_LOCAL, + consts.BACKUP_STATE_IN_PROGRESS] + + for backup_state in ongoing_backup_states + final_backup_states: + db_api.subcloud_update(self.ctx, subcloud.id, + backup_status=backup_state) + if backup_state in ongoing_backup_states: + # Expect the operation to fail + six.assertRaisesRegex(self, webtest.app.AppError, "400 *", + self.app.post_json, FAKE_URL_CREATE, + headers=FAKE_HEADERS, params=data) + else: + # Expect the operation to succeed + response = self.app.post_json(FAKE_URL_CREATE, + headers=FAKE_HEADERS, + params=data) + self.assertEqual(response.status_int, 200) + class TestSubcloudDelete(testroot.DCManagerApiTest): @@ -749,9 +795,6 @@ class TestSubcloudDelete(testroot.DCManagerApiTest): self.app.patch_json, FAKE_URL_DELETE + release_version, headers=FAKE_HEADERS, params=data) -# from dccommon.drivers.openstack.sdk_platform import OpenStackDriver -# from dccommon.drivers.openstack.sysinv_v1 import SysinvClient - @mock.patch.object(rpc_client, 'ManagerClient') def test_backup_delete_invalid_url(self, mock_rpc_client):