From 9c3db124c5c95fa33ad9a54626120c04047ebe4b Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 23 May 2018 20:40:14 -0300 Subject: [PATCH] Refactor --- .../cloudstack/api/BaseBackupListCmd.java | 3 ++- .../admin/backup/ImportBackupPolicyCmd.java | 17 +++++++++--- .../user/backup/AssignBackupPolicyCmd.java | 6 +++-- .../command/user/backup/CreateBackupCmd.java | 20 -------------- .../command/user/backup/DeleteBackupCmd.java | 20 -------------- .../user/backup/ListBackupPoliciesCmd.java | 11 +++++--- .../command/user/backup/ListBackupsCmd.java | 4 ++- .../command/user/backup/RestoreBackupCmd.java | 6 +++-- .../user/backup/RestoreBackupVolumeCmd.java | 6 +++-- .../api/response/BackupPolicyResponse.java | 16 ++++++++--- .../cloudstack/backup/BackupManager.java | 20 +++++++------- .../cloudstack/backup/BackupPolicy.java | 3 ++- .../cloudstack/backup/BackupPolicyTO.java | 17 ++++++++---- .../cloudstack/backup/BackupPolicyVO.java | 18 ++++++++++--- .../META-INF/db/schema-41110to41200.sql | 3 ++- .../backup/DummyBackupProvider.java | 4 +-- .../cloudstack/backup/BackupManagerImpl.java | 27 ++++++++++--------- .../integration/smoke/test_backup_recovery.py | 9 ++++--- tools/marvin/marvin/lib/base.py | 3 ++- 19 files changed, 112 insertions(+), 101 deletions(-) delete mode 100644 api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupCmd.java delete mode 100644 api/src/main/java/org/apache/cloudstack/api/command/user/backup/DeleteBackupCmd.java diff --git a/api/src/main/java/org/apache/cloudstack/api/BaseBackupListCmd.java b/api/src/main/java/org/apache/cloudstack/api/BaseBackupListCmd.java index 22e630b70fd..e5d214194c4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/BaseBackupListCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/BaseBackupListCmd.java @@ -41,7 +41,8 @@ public abstract class BaseBackupListCmd extends BaseListCmd { backupPolicyResponse.setId(policy.getUuid()); } backupPolicyResponse.setName(policy.getName()); - backupPolicyResponse.setPolicyId(policy.getPolicyUuid()); + backupPolicyResponse.setDescription(policy.getDescription()); + backupPolicyResponse.setExternalId(policy.getExternalId()); backupPolicyResponse.setObjectName("policy"); responses.add(backupPolicyResponse); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupPolicyCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupPolicyCmd.java index 1c07a9bd15b..0759b0c43c3 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupPolicyCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupPolicyCmd.java @@ -57,14 +57,18 @@ public class ImportBackupPolicyCmd extends BaseCmd { description = "the name of the backup policy") private String policyName; - @Parameter(name = ApiConstants.BACKUP_EXTERNAL_POLICY_ID, + @Parameter(name = ApiConstants.DESCRIPTION, type = CommandType.STRING, required = true, + description = "the description of the backup policy") + private String description; + + @Parameter(name = ApiConstants.EXTERNAL_ID, type = CommandType.STRING, required = true, description = "The backup policy ID (on backup provider side)") private String policyExternalId; @Parameter(name = ApiConstants.ZONE_ID, type = BaseCmd.CommandType.UUID, entityType = ZoneResponse.class, - description = "The zone ID") + description = "The zone ID", required = true) private Long zoneId; ///////////////////////////////////////////////////// @@ -83,6 +87,10 @@ public class ImportBackupPolicyCmd extends BaseCmd { return zoneId; } + public String getDescription() { + return description; + } + @Override public String getCommandName() { return APINAME.toLowerCase() + RESPONSE_SUFFIX; @@ -100,8 +108,9 @@ public class ImportBackupPolicyCmd extends BaseCmd { private void setupResponse(BackupPolicy policy) { BackupPolicyResponse response = new BackupPolicyResponse(); response.setId(policy.getUuid()); - response.setPolicyId(policy.getPolicyUuid()); + response.setExternalId(policy.getExternalId()); response.setName(policy.getName()); + response.setDescription(policy.getDescription()); response.setObjectName("policy"); response.setResponseName(getCommandName()); setResponseObject(response); @@ -110,7 +119,7 @@ public class ImportBackupPolicyCmd extends BaseCmd { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { try { - BackupPolicy policy = backupManager.addBackupPolicy(policyExternalId, policyName, zoneId); + BackupPolicy policy = backupManager.addBackupPolicy(zoneId, policyExternalId, policyName, description); if (policy != null) { setupResponse(policy); } else { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/AssignBackupPolicyCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/AssignBackupPolicyCmd.java index 6dd59304be2..aca2457e201 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/AssignBackupPolicyCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/AssignBackupPolicyCmd.java @@ -18,6 +18,7 @@ package org.apache.cloudstack.api.command.user.backup; import javax.inject.Inject; +import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -39,7 +40,8 @@ import com.cloud.exception.ResourceUnavailableException; @APICommand(name = AssignBackupPolicyCmd.APINAME, description = "Assigns a VM to an existing backup policy", - responseObject = SuccessResponse.class, since = "4.12.0") + responseObject = SuccessResponse.class, since = "4.12.0", + authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class AssignBackupPolicyCmd extends BaseCmd { public static final String APINAME = "assignBackupPolicy"; @@ -104,7 +106,7 @@ public class AssignBackupPolicyCmd extends BaseCmd { Long virtualMachineId = getVirtualMachineId(); Long policyId = getPolicyId(); Long zoneId = getZoneId(); - boolean result = backupManager.assignVMToBackupPolicy(policyId, virtualMachineId, zoneId); + boolean result = backupManager.assignVMToBackupPolicy(zoneId, policyId, virtualMachineId); if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); response.setResponseName(getCommandName()); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupCmd.java deleted file mode 100644 index 8f2b651c70c..00000000000 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupCmd.java +++ /dev/null @@ -1,20 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -package org.apache.cloudstack.api.command.user.backup; - -public class CreateBackupCmd { -} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/DeleteBackupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/DeleteBackupCmd.java deleted file mode 100644 index eb8558534e0..00000000000 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/DeleteBackupCmd.java +++ /dev/null @@ -1,20 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -package org.apache.cloudstack.api.command.user.backup; - -public class DeleteBackupCmd { -} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupPoliciesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupPoliciesCmd.java index dc40ce4e173..a00d6875ea0 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupPoliciesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupPoliciesCmd.java @@ -20,6 +20,7 @@ import java.util.List; import javax.inject.Inject; +import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -36,10 +37,12 @@ import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.commons.lang.BooleanUtils; @APICommand(name = ListBackupPoliciesCmd.APINAME, description = "Lists backup policies", - responseObject = BackupPolicyResponse.class, since = "4.12.0") + responseObject = BackupPolicyResponse.class, since = "4.12.0", + authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class ListBackupPoliciesCmd extends BaseBackupListCmd { public static final String APINAME = "listBackupPolicies"; @@ -56,7 +59,7 @@ public class ListBackupPoliciesCmd extends BaseBackupListCmd { private Long zoneId; @Parameter(name = ApiConstants.EXTERNAL, type = CommandType.BOOLEAN, - description = "True if list external backup policies (provider policies)") + description = "True if list external backup policies (provider policies)", authorized = {RoleType.Admin}) private Boolean external; ///////////////////////////////////////////////////// @@ -67,8 +70,8 @@ public class ListBackupPoliciesCmd extends BaseBackupListCmd { return zoneId; } - public Boolean isExternal() { - return external; + public boolean isExternal() { + return BooleanUtils.isTrue(external); } @Override diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupsCmd.java index aa9d9b64cda..c74dccd3036 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupsCmd.java @@ -22,6 +22,7 @@ import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; +import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -40,7 +41,8 @@ import java.util.List; @APICommand(name = ListBackupsCmd.APINAME, description = "Lists backups", - responseObject = BackupResponse.class, since = "4.12.0") + responseObject = BackupResponse.class, since = "4.12.0", + authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class ListBackupsCmd extends BaseBackupListCmd { public static final String APINAME = "listBackups"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupCmd.java index 62285c0e70c..ae4f98f3d68 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupCmd.java @@ -23,6 +23,7 @@ import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -40,7 +41,8 @@ import javax.inject.Inject; @APICommand(name = RestoreBackupCmd.APINAME, description = "Restore backup", - responseObject = SuccessResponse.class, since = "4.12.0") + responseObject = SuccessResponse.class, since = "4.12.0", + authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class RestoreBackupCmd extends BaseCmd { public static final String APINAME = "restoreBackup"; @@ -102,7 +104,7 @@ public class RestoreBackupCmd extends BaseCmd { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { try { - boolean result = backupManager.restoreBackup(virtualMachineId, backupId, zoneId); + boolean result = backupManager.restoreBackup(zoneId, virtualMachineId, backupId); if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); response.setResponseName(getCommandName()); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupVolumeCmd.java index f3d57a9ccee..a2d8358443b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupVolumeCmd.java @@ -23,6 +23,7 @@ import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -41,7 +42,8 @@ import javax.inject.Inject; @APICommand(name = RestoreBackupVolumeCmd.APINAME, description = "Restore and attach a backed up volume to VM", - responseObject = SuccessResponse.class, since = "4.12.0") + responseObject = SuccessResponse.class, since = "4.12.0", + authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class RestoreBackupVolumeCmd extends BaseCmd { public static final String APINAME = "restoreBackupVolume"; @@ -115,7 +117,7 @@ public class RestoreBackupVolumeCmd extends BaseCmd { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { try { - boolean result = backupManager.restoreBackupVolume(volumeId, virtualMachineId, backupId, zoneId); + boolean result = backupManager.restoreBackupVolume(zoneId, volumeId, virtualMachineId, backupId); if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); response.setResponseName(getCommandName()); diff --git a/api/src/main/java/org/apache/cloudstack/api/response/BackupPolicyResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/BackupPolicyResponse.java index d886859496e..58454198cb9 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/BackupPolicyResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/BackupPolicyResponse.java @@ -35,19 +35,27 @@ public class BackupPolicyResponse extends BaseResponse { @Param(description = "internal name for the backup policy") private String name; - @SerializedName(ApiConstants.POLICY_ID) + @SerializedName(ApiConstants.DESCRIPTION) + @Param(description = "internal description for the backup policy") + private String description; + + @SerializedName(ApiConstants.EXTERNAL_ID) @Param(description = "policy id on the provider side") - private String policyId; + private String externalId; public void setId(String id) { this.id = id; } - public void setPolicyId(String policyId) { - this.policyId = policyId; + public void setExternalId(String externalId) { + this.externalId = externalId; } public void setName(String name) { this.name = name; } + + public void setDescription(String description) { + this.description = description; + } } diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java index 10cb935de5c..18e8d032ea3 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -19,7 +19,6 @@ package org.apache.cloudstack.backup; import java.util.List; -import org.apache.cloudstack.api.response.BackupPolicyResponse; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; @@ -42,19 +41,18 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer "The backup and recovery provider plugin.", true, ConfigKey.Scope.Zone); /** - * Generate a response from the Backup Policy VO + * Add a new Backup and Recovery policy to CloudStack by mapping an existing external backup policy to a name and description + * @param zoneId zone id + * @param policyExternalId backup policy external id + * @param policyName internal name for the backup policy + * @param policyDescription internal description for the backup policy */ - BackupPolicyResponse createBackupPolicyResponse(BackupPolicy policyVO); - - /** - * Add a new Backup and Recovery policy - */ - BackupPolicy addBackupPolicy(String policyExternalId, String policyName, Long zoneId); + BackupPolicy addBackupPolicy(Long zoneId, String policyExternalId, String policyName, String policyDescription); /** * Assign VM to existing backup policy */ - boolean assignVMToBackupPolicy(Long policyId, Long virtualMachineId, Long zoneId); + boolean assignVMToBackupPolicy(Long zoneId, Long policyId, Long virtualMachineId); /** * List existing backups for a VM @@ -71,10 +69,10 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer /** * Restore a full backed up VM */ - boolean restoreBackup(Long vmId, Long backupId, Long zoneId); + boolean restoreBackup(Long zoneId, Long vmId, Long backupId); //TODO - boolean restoreBackupVolume(Long volumeId, Long vmId, Long backupId, Long zoneId); + boolean restoreBackupVolume(Long zoneId, Long volumeId, Long vmId, Long backupId); /** * Deletes a backup policy diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupPolicy.java b/api/src/main/java/org/apache/cloudstack/backup/BackupPolicy.java index 2dcc665c095..d81cd3f9c2e 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupPolicy.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupPolicy.java @@ -21,8 +21,9 @@ import org.apache.cloudstack.api.InternalIdentity; public interface BackupPolicy extends InternalIdentity, Identity { - String getPolicyUuid(); + String getExternalId(); String getName(); + String getDescription(); boolean isImported(); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyTO.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyTO.java index 0400c1b0947..67bee8b1d4f 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyTO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyTO.java @@ -26,28 +26,35 @@ public class BackupPolicyTO implements BackupPolicy { private String uuid; private long id; private String name; - private String policyUuid; + private String description; + private String externalId; private boolean imported = false; public BackupPolicyTO() { this.uuid = UUID.randomUUID().toString(); } - public BackupPolicyTO(final String name, final String policyUuid) { + public BackupPolicyTO(final String externalId, final String name, final String description) { this(); this.name = name; - this.policyUuid = policyUuid; + this.description = description; + this.externalId = externalId; } @Override - public String getPolicyUuid() { - return policyUuid; + public String getExternalId() { + return externalId; } public String getName() { return name; } + @Override + public String getDescription() { + return description; + } + @Override public String getUuid() { return uuid; diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyVO.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyVO.java index 52129fb45e6..db85231dad4 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupPolicyVO.java @@ -34,9 +34,10 @@ public class BackupPolicyVO implements BackupPolicy { this.uuid = UUID.randomUUID().toString(); } - public BackupPolicyVO(final String name, final String policyUuid) { + public BackupPolicyVO(final String policyUuid, final String name, final String description) { this(); this.name = name; + this.description = description; this.policyUuid = policyUuid; } @@ -51,7 +52,10 @@ public class BackupPolicyVO implements BackupPolicy { @Column(name = "name") private String name; - @Column(name = "policy_uuid") + @Column(name = "description") + private String description; + + @Column(name = "external_id") private String policyUuid; @Column(name = "imported") @@ -81,7 +85,7 @@ public class BackupPolicyVO implements BackupPolicy { this.name = name; } - public String getPolicyUuid() { + public String getExternalId() { return policyUuid; } @@ -93,4 +97,12 @@ public class BackupPolicyVO implements BackupPolicy { public boolean isImported() { return imported; } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql b/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql index c599caacad5..654a8078a84 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql @@ -40,7 +40,8 @@ CREATE TABLE IF NOT EXISTS `cloud`.`backup_policy` ( `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT, `uuid` varchar(40) NOT NULL, `name` varchar(255) NOT NULL COMMENT 'backup policy name', - `policy_uuid` varchar(40) NOT NULL COMMENT 'backup policy ID on provider side', + `description` varchar(255) NOT NULL COMMENT 'backup policy description', + `external_id` varchar(40) NOT NULL COMMENT 'backup policy ID on provider side', `imported` tinyint(1) unsigned NOT NULL DEFAULT '0' COMMENT 'true if policy has been imported from the backup provider', PRIMARY KEY (`id`), UNIQUE KEY `uuid` (`uuid`) diff --git a/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java b/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java index 431b05ce4e7..b5ea7cdc9b2 100644 --- a/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java +++ b/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java @@ -45,8 +45,8 @@ public class DummyBackupProvider extends AdapterBase implements BackupProvider { @Override public List listBackupPolicies() { s_logger.debug("Listing backup policies on Dummy B&R Plugin"); - BackupPolicy policy1 = new BackupPolicyTO("Golden Policy", "aaaa-aaaa"); - BackupPolicy policy2 = new BackupPolicyTO("Silver Policy", "bbbb-bbbb"); + BackupPolicy policy1 = new BackupPolicyTO("aaaa-aaaa", "Golden Policy", "Gold description"); + BackupPolicy policy2 = new BackupPolicyTO("bbbb-bbbb", "Silver Policy", "Silver description"); return Arrays.asList(policy1, policy2); } diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 921940a24d3..0d5da6954ca 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -24,6 +24,9 @@ import java.util.Map; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.user.Account; +import com.cloud.user.AccountService; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.api.command.admin.backup.DeleteBackupPolicyCmd; @@ -34,16 +37,15 @@ import org.apache.cloudstack.api.command.admin.backup.ListBackupProvidersCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupsCmd; import org.apache.cloudstack.api.command.user.backup.RestoreBackupCmd; import org.apache.cloudstack.api.command.user.backup.RestoreBackupVolumeCmd; -import org.apache.cloudstack.api.response.BackupPolicyResponse; import org.apache.cloudstack.backup.dao.BackupDao; import org.apache.cloudstack.backup.dao.BackupPolicyDao; import org.apache.cloudstack.backup.dao.BackupPolicyVMMapDao; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.commons.lang.BooleanUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; -import com.cloud.dc.dao.DataCenterDao; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.exception.CloudRuntimeException; @@ -58,7 +60,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { VMInstanceDao vmInstanceDao; @Inject - DataCenterDao dataCenterDao; + private AccountService accountService; @Inject BackupPolicyVMMapDao backupPolicyVMMapDao; @@ -70,13 +72,13 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { private List backupProviders; @Override - public BackupPolicy addBackupPolicy(String policyExternalId, String policyName, Long zoneId) { + public BackupPolicy addBackupPolicy(Long zoneId, String policyExternalId, String policyName, String policyDescription) { BackupProvider provider = getBackupProvider(zoneId); if (!provider.isBackupPolicy(policyExternalId)) { throw new CloudRuntimeException("Policy " + policyExternalId + " does not exist on provider " + provider.getName()); } - BackupPolicyVO policy = new BackupPolicyVO(policyName, policyExternalId); + BackupPolicyVO policy = new BackupPolicyVO(policyExternalId, policyName, policyDescription); BackupPolicyVO vo = backupPolicyDao.persist(policy); if (vo == null) { throw new CloudRuntimeException("Unable to create backup policy: " + policyExternalId + ", name: " + policyName); @@ -86,7 +88,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { } @Override - public boolean assignVMToBackupPolicy(Long policyId, Long virtualMachineId, Long zoneId) { + public boolean assignVMToBackupPolicy(Long zoneId, Long policyId, Long virtualMachineId) { VMInstanceVO vmInstanceVO = vmInstanceDao.findById(virtualMachineId); if (vmInstanceVO == null) { throw new CloudRuntimeException("VM " + virtualMachineId + " does not exist"); @@ -119,7 +121,11 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { @Override public List listBackupPolicies(Long zoneId, Boolean external) { + Account account = CallContext.current().getCallingAccount(); if (BooleanUtils.isTrue(external)) { + if (!accountService.isRootAdmin(account.getId())) { + throw new PermissionDeniedException("Parameter external can only be specified by a Root Admin, permission denied"); + } BackupProvider backupProvider = getBackupProvider(zoneId); return backupProvider.listBackupPolicies(); } @@ -127,7 +133,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { } @Override - public boolean restoreBackup(Long vmId, Long backupId, Long zoneId) { + public boolean restoreBackup(Long zoneId, Long vmId, Long backupId) { BackupProvider backupProvider = getBackupProvider(zoneId); VMInstanceVO vm = vmInstanceDao.findById(vmId); if (vm == null) { @@ -141,7 +147,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { } @Override - public boolean restoreBackupVolume(Long volumeId, Long vmId, Long backupId, Long zoneId) { + public boolean restoreBackupVolume(Long zoneId, Long volumeId, Long vmId, Long backupId) { //TODO return false; } @@ -155,11 +161,6 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { return backupPolicyDao.expunge(policy.getId()); } - @Override - public BackupPolicyResponse createBackupPolicyResponse(BackupPolicy policyVO) { - return null; - } - @Override public boolean configure(String name, Map params) throws ConfigurationException { super.configure(name, params); diff --git a/test/integration/smoke/test_backup_recovery.py b/test/integration/smoke/test_backup_recovery.py index f116693acf3..f999686448b 100644 --- a/test/integration/smoke/test_backup_recovery.py +++ b/test/integration/smoke/test_backup_recovery.py @@ -115,8 +115,9 @@ class TestBackupAndRecovery(cloudstackTestCase): if cls.backup_available: cls.external_policies = BackupPolicy.listExternal(cls.api_client, cls.zone.id) - cls.logger.debug("Importing backup policy %s - %s" % (cls.external_policies[0].policyid, cls.external_policies[0].name)) - cls.policy = BackupPolicy.importExisting(cls.api_client, cls.zone.id, cls.external_policies[0].policyid, cls.external_policies[0].name) + cls.logger.debug("Importing backup policy %s - %s" % (cls.external_policies[0].externalid, cls.external_policies[0].name)) + cls.policy = BackupPolicy.importExisting(cls.api_client, cls.zone.id, cls.external_policies[0].externalid, + cls.external_policies[0].name, cls.external_policies[0].description) cls._cleanup.append(cls.policy) return @@ -153,8 +154,8 @@ class TestBackupAndRecovery(cloudstackTestCase): self.skipTest("This test is only available when backup is enabled and dummy provider selected") ext_policy = self.external_policies[1] - self.logger.debug("Importing backup policy %s - %s" % (ext_policy.policyid, ext_policy.name)) - policy = BackupPolicy.importExisting(self.apiclient, self.zone.id, ext_policy.policyid, ext_policy.name) + self.logger.debug("Importing backup policy %s - %s" % (ext_policy.externalid, ext_policy.name)) + policy = BackupPolicy.importExisting(self.apiclient, self.zone.id, ext_policy.externalid, ext_policy.name, ext_policy.description) imported_policies = BackupPolicy.listInternal(self.apiclient, self.zone.id) self.assertIsInstance( diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index a1f5f1b6dd0..de097f80015 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -5384,13 +5384,14 @@ class BackupPolicy: self.__dict__.update(items) @classmethod - def importExisting(self, apiclient, zoneid, externalid, name): + def importExisting(self, apiclient, zoneid, externalid, name, description): """Import existing backup policy from the provider""" cmd = importBackupPolicy.importBackupPolicyCmd() cmd.zoneid = zoneid cmd.externalid = externalid cmd.name = name + cmd.description = description return BackupPolicy(apiclient.importBackupPolicy(cmd).__dict__) @classmethod