From 5b181bec372f7ff44a59f7b4c6e53e514e9734b6 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 24 May 2018 00:17:08 -0300 Subject: [PATCH] Sanitize and refactor --- .../apache/cloudstack/api/ApiConstants.java | 1 - .../cloudstack/api/BaseBackupListCmd.java | 9 +- .../cloudstack/api/ResponseGenerator.java | 4 + .../admin/backup/DeleteBackupPolicyCmd.java | 2 +- .../admin/backup/ImportBackupPolicyCmd.java | 15 +- .../user/backup/ListBackupPoliciesCmd.java | 2 +- .../api/response/BackupPolicyResponse.java | 8 + .../cloudstack/backup/BackupPolicy.java | 1 + .../cloudstack/backup/BackupPolicyTO.java | 13 +- .../cloudstack/backup/BackupPolicyVO.java | 40 ++--- .../backup/dao/BackupPolicyDao.java | 6 + .../backup/dao/BackupPolicyDaoImpl.java | 36 +++++ .../META-INF/db/schema-41110to41200.sql | 2 +- .../main/java/com/cloud/api/ApiDBUtils.java | 11 ++ .../java/com/cloud/api/ApiResponseHelper.java | 7 + .../cloudstack/backup/BackupManagerImpl.java | 33 ++-- .../integration/smoke/test_backup_recovery.py | 143 +++++------------- tools/marvin/marvin/lib/base.py | 2 +- 18 files changed, 162 insertions(+), 173 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 34be8fe3d05..bd845acfd5f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -33,7 +33,6 @@ public class ApiConstants { public static final String LIST_LB_VMIPS = "lbvmips"; public static final String AVAILABLE = "available"; public static final String BACKUP_ID = "backupid"; - public static final String BACKUP_EXTERNAL_POLICY_ID = "externalid"; public static final String BACKUP_POLICY_ID = "backuppolicyid"; public static final String BITS = "bits"; public static final String BOOTABLE = "bootable"; 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 e5d214194c4..b4b5704056d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/BaseBackupListCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/BaseBackupListCmd.java @@ -36,14 +36,7 @@ public abstract class BaseBackupListCmd extends BaseListCmd { if (policy == null) { continue; } - final BackupPolicyResponse backupPolicyResponse = new BackupPolicyResponse(); - if (policy.isImported()) { - backupPolicyResponse.setId(policy.getUuid()); - } - backupPolicyResponse.setName(policy.getName()); - backupPolicyResponse.setDescription(policy.getDescription()); - backupPolicyResponse.setExternalId(policy.getExternalId()); - backupPolicyResponse.setObjectName("policy"); + BackupPolicyResponse backupPolicyResponse = _responseGenerator.createBackupPolicyResponse(policy); responses.add(backupPolicyResponse); } response.setResponses(responses); diff --git a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java index 4c1016ad8f5..5640da3d527 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java +++ b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java @@ -34,6 +34,7 @@ import org.apache.cloudstack.api.response.AsyncJobResponse; import org.apache.cloudstack.api.response.AutoScalePolicyResponse; import org.apache.cloudstack.api.response.AutoScaleVmGroupResponse; import org.apache.cloudstack.api.response.AutoScaleVmProfileResponse; +import org.apache.cloudstack.api.response.BackupPolicyResponse; import org.apache.cloudstack.api.response.BackupResponse; import org.apache.cloudstack.api.response.CapacityResponse; import org.apache.cloudstack.api.response.ClusterResponse; @@ -117,6 +118,7 @@ import org.apache.cloudstack.api.response.VpcOfferingResponse; import org.apache.cloudstack.api.response.VpcResponse; import org.apache.cloudstack.api.response.VpnUsersResponse; import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.cloudstack.backup.BackupPolicy; import org.apache.cloudstack.config.Configuration; import org.apache.cloudstack.backup.Backup; import org.apache.cloudstack.network.lb.ApplicationLoadBalancerRule; @@ -466,4 +468,6 @@ public interface ResponseGenerator { SSHKeyPairResponse createSSHKeyPairResponse(SSHKeyPair sshkeyPair, boolean privatekey); BackupResponse createBackupResponse(Backup backup); + + BackupPolicyResponse createBackupPolicyResponse(BackupPolicy policy); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/DeleteBackupPolicyCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/DeleteBackupPolicyCmd.java index ff662646498..bdbb583a64c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/DeleteBackupPolicyCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/DeleteBackupPolicyCmd.java @@ -51,7 +51,7 @@ public class DeleteBackupPolicyCmd extends BaseCmd { //////////////// API parameters ///////////////////// //////////////////////////////////////////////////// - @Parameter(name = ApiConstants.BACKUP_POLICY_ID, + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = BackupPolicyResponse.class, required = true, 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 0759b0c43c3..b61255af605 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 @@ -105,23 +105,14 @@ public class ImportBackupPolicyCmd extends BaseCmd { /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// - private void setupResponse(BackupPolicy policy) { - BackupPolicyResponse response = new BackupPolicyResponse(); - response.setId(policy.getUuid()); - response.setExternalId(policy.getExternalId()); - response.setName(policy.getName()); - response.setDescription(policy.getDescription()); - response.setObjectName("policy"); - response.setResponseName(getCommandName()); - setResponseObject(response); - } - @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { try { BackupPolicy policy = backupManager.addBackupPolicy(zoneId, policyExternalId, policyName, description); if (policy != null) { - setupResponse(policy); + BackupPolicyResponse response = _responseGenerator.createBackupPolicyResponse(policy); + response.setResponseName(getCommandName()); + setResponseObject(response); } else { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add a Backup policy"); } 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 a00d6875ea0..35c012cfaf0 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 @@ -55,7 +55,7 @@ public class ListBackupPoliciesCmd extends BaseBackupListCmd { ///////////////////////////////////////////////////// @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; @Parameter(name = ApiConstants.EXTERNAL, type = CommandType.BOOLEAN, 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 58454198cb9..9cabcd49449 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 @@ -43,6 +43,10 @@ public class BackupPolicyResponse extends BaseResponse { @Param(description = "policy id on the provider side") private String externalId; + @SerializedName(ApiConstants.ZONE_ID) + @Param(description = "zone id") + private String zoneId; + public void setId(String id) { this.id = id; } @@ -58,4 +62,8 @@ public class BackupPolicyResponse extends BaseResponse { public void setDescription(String description) { this.description = description; } + + public void setZoneId(String zoneId) { + this.zoneId = zoneId; + } } 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 d81cd3f9c2e..d447c4e9c67 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupPolicy.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupPolicy.java @@ -25,5 +25,6 @@ public interface BackupPolicy extends InternalIdentity, Identity { String getName(); String getDescription(); boolean isImported(); + long getZoneId(); } 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 67bee8b1d4f..6c71655f792 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 @@ -19,8 +19,6 @@ package org.apache.cloudstack.backup; -import java.util.UUID; - public class BackupPolicyTO implements BackupPolicy { private String uuid; @@ -28,14 +26,12 @@ public class BackupPolicyTO implements BackupPolicy { private String name; private String description; private String externalId; - private boolean imported = false; + private long zoneId; public BackupPolicyTO() { - this.uuid = UUID.randomUUID().toString(); } public BackupPolicyTO(final String externalId, final String name, final String description) { - this(); this.name = name; this.description = description; this.externalId = externalId; @@ -67,6 +63,11 @@ public class BackupPolicyTO implements BackupPolicy { @Override public boolean isImported() { - return imported; + return false; + } + + @Override + public long getZoneId() { + return zoneId; } } 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 db85231dad4..e143a1fe4f4 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,11 +34,12 @@ public class BackupPolicyVO implements BackupPolicy { this.uuid = UUID.randomUUID().toString(); } - public BackupPolicyVO(final String policyUuid, final String name, final String description) { + public BackupPolicyVO(final long zoneId, final String externalId, final String name, final String description) { this(); + this.zoneId = zoneId; this.name = name; this.description = description; - this.policyUuid = policyUuid; + this.externalId = externalId; } @Id @@ -56,10 +57,10 @@ public class BackupPolicyVO implements BackupPolicy { private String description; @Column(name = "external_id") - private String policyUuid; + private String externalId; - @Column(name = "imported") - private boolean imported = true; + @Column(name = "zone_id") + private long zoneId; public String getUuid() { return uuid; @@ -69,40 +70,25 @@ public class BackupPolicyVO implements BackupPolicy { return id; } - public void setId(long id) { - this.id = id; - } - - public void setUuid(String uuid) { - this.uuid = uuid; - } - public String getName() { return name; } - public void setName(String name) { - this.name = name; - } - public String getExternalId() { - return policyUuid; - } - - public void setPolicyUuid(String policyUuid) { - this.policyUuid = policyUuid; + return externalId; } @Override public boolean isImported() { - return imported; + return true; + } + + @Override + public long getZoneId() { + return zoneId; } public String getDescription() { return description; } - - public void setDescription(String description) { - this.description = description; - } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDao.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDao.java index f7981e6c7b2..42100d28fb8 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDao.java @@ -17,10 +17,16 @@ package org.apache.cloudstack.backup.dao; +import org.apache.cloudstack.api.response.BackupPolicyResponse; +import org.apache.cloudstack.backup.BackupPolicy; import org.apache.cloudstack.backup.BackupPolicyVO; import com.cloud.utils.db.GenericDao; +import java.util.List; + public interface BackupPolicyDao extends GenericDao { + BackupPolicyResponse newBackupPolicyResponse(BackupPolicy policy); + List listByZone(Long zoneId); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDaoImpl.java index dea8b00ee3c..03eb8c5c004 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupPolicyDaoImpl.java @@ -18,16 +18,28 @@ package org.apache.cloudstack.backup.dao; import javax.annotation.PostConstruct; +import javax.inject.Inject; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.utils.db.SearchCriteria; +import org.apache.cloudstack.api.response.BackupPolicyResponse; +import org.apache.cloudstack.backup.BackupPolicy; import org.apache.cloudstack.backup.BackupPolicyVO; import org.springframework.stereotype.Component; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; +import java.util.ArrayList; +import java.util.List; + @Component public class BackupPolicyDaoImpl extends GenericDaoBase implements BackupPolicyDao { + @Inject + DataCenterDao dataCenterDao; + private SearchBuilder backupPoliciesSearch; public BackupPolicyDaoImpl() { @@ -36,6 +48,30 @@ public class BackupPolicyDaoImpl extends GenericDaoBase im @PostConstruct protected void init() { backupPoliciesSearch = createSearchBuilder(); + backupPoliciesSearch.and("zone_id", backupPoliciesSearch.entity().getZoneId(), SearchCriteria.Op.EQ); backupPoliciesSearch.done(); } + + @Override + public BackupPolicyResponse newBackupPolicyResponse(BackupPolicy policy) { + DataCenterVO zone = dataCenterDao.findById(policy.getZoneId()); + + BackupPolicyResponse response = new BackupPolicyResponse(); + if (policy.isImported()) { + response.setId(policy.getUuid()); + response.setZoneId(zone.getUuid()); + } + response.setName(policy.getName()); + response.setDescription(policy.getDescription()); + response.setExternalId(policy.getExternalId()); + response.setObjectName("backuppolicy"); + return response; + } + + @Override + public List listByZone(Long zoneId) { + SearchCriteria sc = backupPoliciesSearch.create(); + sc.setParameters("zone_id", zoneId); + return new ArrayList<>(listBy(sc)); + } } 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 654a8078a84..01d392ee5b1 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 @@ -42,7 +42,7 @@ CREATE TABLE IF NOT EXISTS `cloud`.`backup_policy` ( `name` varchar(255) NOT NULL COMMENT 'backup policy name', `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', + `zone_id` bigint(20) unsigned NOT NULL COMMENT 'zone id', PRIMARY KEY (`id`), UNIQUE KEY `uuid` (`uuid`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/server/src/main/java/com/cloud/api/ApiDBUtils.java b/server/src/main/java/com/cloud/api/ApiDBUtils.java index 58933c484b4..4011fd5d701 100644 --- a/server/src/main/java/com/cloud/api/ApiDBUtils.java +++ b/server/src/main/java/com/cloud/api/ApiDBUtils.java @@ -39,6 +39,7 @@ import org.apache.cloudstack.api.ApiConstants.VMDetails; import org.apache.cloudstack.api.ResponseObject.ResponseView; import org.apache.cloudstack.api.response.AccountResponse; import org.apache.cloudstack.api.response.AsyncJobResponse; +import org.apache.cloudstack.api.response.BackupPolicyResponse; import org.apache.cloudstack.api.response.BackupResponse; import org.apache.cloudstack.api.response.DiskOfferingResponse; import org.apache.cloudstack.api.response.DomainResponse; @@ -62,7 +63,9 @@ import org.apache.cloudstack.api.response.UserResponse; import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.cloudstack.api.response.VolumeResponse; import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.cloudstack.backup.BackupPolicy; import org.apache.cloudstack.backup.dao.BackupDao; +import org.apache.cloudstack.backup.dao.BackupPolicyDao; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; @@ -438,6 +441,7 @@ public class ApiDBUtils { static HostGpuGroupsDao s_hostGpuGroupsDao; static VGPUTypesDao s_vgpuTypesDao; static BackupDao s_backupDao; + static BackupPolicyDao s_backupPolicyDao; @Inject private ManagementServer ms; @@ -672,6 +676,8 @@ public class ApiDBUtils { private VGPUTypesDao vgpuTypesDao; @Inject private BackupDao backupDao; + @Inject + private BackupPolicyDao backupPolicyDao; @PostConstruct void init() { @@ -792,6 +798,7 @@ public class ApiDBUtils { s_hostGpuGroupsDao = hostGpuGroupsDao; s_vgpuTypesDao = vgpuTypesDao; s_backupDao = backupDao; + s_backupPolicyDao = backupPolicyDao; } // /////////////////////////////////////////////////////////// @@ -2011,4 +2018,8 @@ public class ApiDBUtils { public static BackupResponse newBackupResponse(Backup backup) { return s_backupDao.newBackupResponse(backup); } + + public static BackupPolicyResponse newBackupPolicyResponse(BackupPolicy policy) { + return s_backupPolicyDao.newBackupPolicyResponse(policy); + } } diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 53259beffcc..16380745d3d 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -199,6 +199,7 @@ import org.apache.cloudstack.api.response.AsyncJobResponse; import org.apache.cloudstack.api.response.AutoScalePolicyResponse; import org.apache.cloudstack.api.response.AutoScaleVmGroupResponse; import org.apache.cloudstack.api.response.AutoScaleVmProfileResponse; +import org.apache.cloudstack.api.response.BackupPolicyResponse; import org.apache.cloudstack.api.response.BackupResponse; import org.apache.cloudstack.api.response.CapabilityResponse; import org.apache.cloudstack.api.response.CapacityResponse; @@ -290,6 +291,7 @@ import org.apache.cloudstack.api.response.VpcOfferingResponse; import org.apache.cloudstack.api.response.VpcResponse; import org.apache.cloudstack.api.response.VpnUsersResponse; import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.cloudstack.backup.BackupPolicy; import org.apache.cloudstack.config.Configuration; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -3971,4 +3973,9 @@ public class ApiResponseHelper implements ResponseGenerator { public BackupResponse createBackupResponse(Backup backup) { return ApiDBUtils.newBackupResponse(backup); } + + @Override + public BackupPolicyResponse createBackupPolicyResponse(BackupPolicy policy) { + return ApiDBUtils.newBackupPolicyResponse(policy); + } } 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 0d5da6954ca..fbf8817162e 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -78,7 +78,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { throw new CloudRuntimeException("Policy " + policyExternalId + " does not exist on provider " + provider.getName()); } - BackupPolicyVO policy = new BackupPolicyVO(policyExternalId, policyName, policyDescription); + BackupPolicyVO policy = new BackupPolicyVO(zoneId, policyExternalId, policyName, policyDescription); BackupPolicyVO vo = backupPolicyDao.persist(policy); if (vo == null) { throw new CloudRuntimeException("Unable to create backup policy: " + policyExternalId + ", name: " + policyName); @@ -119,17 +119,30 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { return backupDao.listByVmId(vmId); } + /** + * List external backup policies for the Backup and Recovery provider registered in the zone zoneId + */ + private List listExternalPolicies(Long zoneId) { + Account account = CallContext.current().getCallingAccount(); + if (!accountService.isRootAdmin(account.getId())) { + throw new PermissionDeniedException("Parameter external can only be specified by a Root Admin, permission denied"); + } + BackupProvider backupProvider = getBackupProvider(zoneId); + LOG.debug("Listing external backup policies for the backup provider registered in zone " + zoneId); + return backupProvider.listBackupPolicies(); + } + + /** + * List imported backup policies in the zone zoneId + */ + private List listInternalPolicies(Long zoneId) { + LOG.debug("Listing imported backup policies on zone " + zoneId); + return backupPolicyDao.listByZone(zoneId); + } + @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(); - } - return new ArrayList<>(backupPolicyDao.listAll()); + return BooleanUtils.isTrue(external) ? listExternalPolicies(zoneId) : listInternalPolicies(zoneId); } @Override diff --git a/test/integration/smoke/test_backup_recovery.py b/test/integration/smoke/test_backup_recovery.py index f999686448b..18191190d50 100644 --- a/test/integration/smoke/test_backup_recovery.py +++ b/test/integration/smoke/test_backup_recovery.py @@ -17,114 +17,62 @@ # under the License. from marvin.cloudstackTestCase import cloudstackTestCase -from marvin.lib.utils import (random_gen, - cleanup_resources, - validateList) -from marvin.cloudstackAPI import * -from marvin.lib.base import (Domain, - Account, - ServiceOffering, - VirtualMachine, - Network, - User, - NATRule, - Template, - PublicIPAddress, - BackupPolicy, - Configurations) -from marvin.lib.common import (get_domain, - get_zone, - get_template, - list_accounts, - list_virtual_machines, - list_service_offering, - list_templates, - list_users, - get_builtin_template_info, - wait_for_cleanup) +from marvin.lib.utils import (cleanup_resources) +from marvin.lib.base import (Account, ServiceOffering, VirtualMachine, BackupPolicy, Configurations) +from marvin.lib.common import (get_domain, get_zone, get_template) from nose.plugins.attrib import attr -import logging -from marvin.codes import PASS, FAILED +from marvin.codes import FAILED -class TestBackupAndRecovery(cloudstackTestCase): +class TestDummyBackupAndRecovery(cloudstackTestCase): @classmethod def setUpClass(cls): - cls.testClient = super(TestBackupAndRecovery, cls).getClsTestClient() + cls.testClient = super(TestDummyBackupAndRecovery, cls).getClsTestClient() cls.api_client = cls.testClient.getApiClient() - - cls.logger = logging.getLogger('TestBackupAndRecovery') - cls.stream_handler = logging.StreamHandler() - cls.logger.setLevel(logging.DEBUG) - cls.logger.addHandler(cls.stream_handler) - cls.services = cls.testClient.getParsedTestDataConfig() cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) cls.services["mode"] = cls.zone.networktype cls.hypervisor = cls.testClient.getHypervisorInfo() cls.domain = get_domain(cls.api_client) - - cls.template = get_template( - cls.api_client, - cls.zone.id, - cls.services["ostype"] - ) + cls.template = get_template(cls.api_client, cls.zone.id, cls.services["ostype"]) if cls.template == FAILED: assert False, "get_template() failed to return template with description %s" % cls.services["ostype"] - cls.services["small"]["zoneid"] = cls.zone.id cls.services["small"]["template"] = cls.template.id + cls.account = Account.create(cls.api_client, cls.services["account"], domainid=cls.domain.id) + cls.offering = ServiceOffering.create(cls.api_client,cls.services["service_offerings"]["small"]) + cls.vm = VirtualMachine.create(cls.api_client, cls.services["small"], accountid=cls.account.name, + domainid=cls.account.domainid, serviceofferingid=cls.offering.id, + mode=cls.services["mode"]) + cls._cleanup = [cls.offering, cls.account] - cls.account = Account.create( - cls.api_client, - cls.services["account"], - domainid=cls.domain.id - ) - - cls.offering = ServiceOffering.create( - cls.api_client, - cls.services["service_offerings"]["small"] - ) - - cls.vm = VirtualMachine.create( - cls.api_client, - cls.services["small"], - accountid=cls.account.name, - domainid=cls.account.domainid, - serviceofferingid=cls.offering.id, - mode=cls.services["mode"] - ) - - cls._cleanup = [ - cls.offering, - cls.account - ] - - backup_enabled_cfg = Configurations.list( - cls.api_client, - name='backup.framework.enabled' - ) - backup_provider_cfg = Configurations.list( - cls.api_client, - name='backup.framework.provider.plugin' - ) + backup_enabled_cfg = Configurations.list(cls.api_client, name='backup.framework.enabled') + backup_provider_cfg = Configurations.list(cls.api_client, name='backup.framework.provider.plugin') cls.backup_enabled = backup_enabled_cfg[0].value cls.backup_provider = backup_provider_cfg[0].value - cls.backup_available = cls.backup_enabled and cls.backup_provider == "dummy" + if not cls.backup_enabled: + Configurations.update(cls.api_client, 'backup.framework.enabled', 'true', zoneid=cls.zone.id) + if not cls.backup_provider == "dummy": + Configurations.update(cls.api_client, 'backup.framework.provider.plugin', 'dummy', zoneid=cls.zone.id) - 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].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) + cls.external_policies = BackupPolicy.listExternal(cls.api_client, cls.zone.id) + cls.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 @classmethod def tearDownClass(cls): try: + # Restore original backup framework values values + if not cls.backup_enabled: + Configurations.update(cls.api_client, 'backup.framework.enabled', cls.backup_enabled, zoneid=cls.zone.id) + if not cls.backup_provider == "dummy": + Configurations.update(cls.api_client, 'backup.framework.provider.plugin', cls.backup_provider, zoneid=cls.zone.id) + # Cleanup resources used cleanup_resources(cls.api_client, cls._cleanup) except Exception as e: @@ -139,7 +87,6 @@ class TestBackupAndRecovery(cloudstackTestCase): def tearDown(self): try: - # Clean up, terminate the created accounts, domains etc cleanup_resources(self.apiclient, self.cleanup) except Exception as e: raise Exception("Warning: Exception during cleanup : %s" % e) @@ -148,31 +95,19 @@ class TestBackupAndRecovery(cloudstackTestCase): @attr(tags=["advanced", "backup"], required_hardware="false") def test_ImportBackupPolicies(self): """ - Import existing backup policies + Import existing backup policies from Dummy Backup and Recovery Provider """ - if not self.backup_available: - 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.externalid, ext_policy.name)) - policy = BackupPolicy.importExisting(self.apiclient, self.zone.id, ext_policy.externalid, ext_policy.name, ext_policy.description) + self.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( - imported_policies, - list, - "List Backup Policies should return a valid response" - ) - self.assertNotEqual( - len(imported_policies), - 0, - "Check if the list API returns a non-empty response" - ) + self.assertIsInstance(imported_policies, list, "List Backup Policies should return a valid response") + self.assertNotEqual(len(imported_policies), 0, "Check if the list API returns a non-empty response") - self.logger.debug("Listing internal backup policies") - self.logger.debug(imported_policies) - - self.logger.debug("Deleting backup policy %s" % policy.id) + self.debug("Deleting backup policy %s" % policy.id) policy.delete(self.apiclient) @attr(tags=["advanced", "backup"], required_hardware="false") @@ -180,10 +115,8 @@ class TestBackupAndRecovery(cloudstackTestCase): """ Assign a VM to a backup policy """ - if not self.backup_available: - self.skipTest("This test is only available when backup is enabled and dummy provider selected") - self.logger.debug("Assigning VM %s to backup policy %s" % (self.vm.id, self.policy.id)) + self.debug("Assigning VM %s to backup policy %s" % (self.vm.id, self.policy.id)) self.policy.assignVM( self.apiclient, diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index de097f80015..c98435ed826 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -5415,7 +5415,7 @@ class BackupPolicy: """Delete an imported backup policy""" cmd = deleteBackupPolicy.deleteBackupPolicyCmd() - cmd.backuppolicyid = self.id + cmd.id = self.id return (apiclient.deleteBackupPolicy(cmd)) def assignVM(self, apiclient, vmid, zoneid):