Skip to content

Commit 7d7e5df

Browse files
Daan HooglandDaanHoogland
Daan Hoogland
authored and
DaanHoogland
committed
Deal with Storage Manager tech debt
1 parent 5bf869c commit 7d7e5df

File tree

15 files changed

+375
-644
lines changed

15 files changed

+375
-644
lines changed

‎api/src/main/java/com/cloud/storage/StorageService.java‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
package com.cloud.storage;
1919

20-
import java.net.UnknownHostException;
2120
import java.util.Map;
2221

2322
import org.apache.cloudstack.api.command.admin.storage.CancelPrimaryStorageMaintenanceCmd;
@@ -35,10 +34,8 @@
3534
import org.apache.cloudstack.api.command.admin.storage.UpdateStoragePoolCmd;
3635

3736
import com.cloud.exception.DiscoveryException;
38-
import com.cloud.exception.InsufficientCapacityException;
3937
import com.cloud.exception.InvalidParameterValueException;
4038
import com.cloud.exception.PermissionDeniedException;
41-
import com.cloud.exception.ResourceInUseException;
4239
import com.cloud.exception.ResourceUnavailableException;
4340
import org.apache.cloudstack.api.command.admin.storage.heuristics.CreateSecondaryStorageSelectorCmd;
4441
import org.apache.cloudstack.api.command.admin.storage.heuristics.RemoveSecondaryStorageSelectorCmd;
@@ -55,12 +52,9 @@ public interface StorageService {
5552
* storage pool.
5653
* @return
5754
* The StoragePool created.
58-
* @throws ResourceInUseException
5955
* @throws IllegalArgumentException
60-
* @throws UnknownHostException
61-
* @throws ResourceUnavailableException
6256
*/
63-
StoragePool createPool(CreateStoragePoolCmd cmd) throws ResourceInUseException, IllegalArgumentException, UnknownHostException, ResourceUnavailableException;
57+
StoragePool createPool(CreateStoragePoolCmd cmd) throws IllegalArgumentException;
6458

6559
ImageStore createSecondaryStagingStore(CreateSecondaryStagingStoreCmd cmd);
6660

@@ -79,10 +73,8 @@ public interface StorageService {
7973
* @param primaryStorageId
8074
* - the primaryStorageId
8175
* @return the primary storage pool
82-
* @throws ResourceUnavailableException
83-
* @throws InsufficientCapacityException
8476
*/
85-
StoragePool preparePrimaryStorageForMaintenance(Long primaryStorageId) throws ResourceUnavailableException, InsufficientCapacityException;
77+
StoragePool preparePrimaryStorageForMaintenance(Long primaryStorageId);
8678

8779
/**
8880
* Complete maintenance for primary storage
@@ -108,7 +100,7 @@ public interface StorageService {
108100

109101
boolean deleteSecondaryStagingStore(DeleteSecondaryStagingStoreCmd cmd);
110102

111-
ImageStore discoverImageStore(String name, String url, String providerName, Long zoneId, Map details) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException;
103+
ImageStore discoverImageStore(String name, String url, String providerName, Long zoneId, Map<String, String> details) throws IllegalArgumentException, InvalidParameterValueException;
112104

113105
/**
114106
* Migrate existing NFS to use object store.
@@ -134,7 +126,7 @@ public interface StorageService {
134126

135127
void removeSecondaryStorageHeuristic(RemoveSecondaryStorageSelectorCmd cmd);
136128

137-
ObjectStore discoverObjectStore(String name, String url, Long size, String providerName, Map details) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException;
129+
ObjectStore discoverObjectStore(String name, String url, Long size, String providerName, Map<String, String> details) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException;
138130

139131
boolean deleteObjectStore(DeleteObjectStoragePoolCmd cmd);
140132

‎api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.apache.cloudstack.api.response.ImageStoreResponse;
2727
import org.apache.cloudstack.api.response.ZoneResponse;
2828

29-
import com.cloud.exception.DiscoveryException;
3029
import com.cloud.storage.ImageStore;
3130
import com.cloud.user.Account;
3231

@@ -67,20 +66,15 @@ public long getEntityOwnerId() {
6766

6867
@Override
6968
public void execute(){
70-
try{
71-
ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), null);
72-
ImageStoreResponse storeResponse = null;
73-
if (result != null ) {
74-
storeResponse = _responseGenerator.createImageStoreResponse(result);
75-
storeResponse.setResponseName(getCommandName());
76-
storeResponse.setObjectName("secondarystorage");
77-
setResponseObject(storeResponse);
78-
} else {
79-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage");
80-
}
81-
} catch (DiscoveryException ex) {
82-
logger.warn("Exception: ", ex);
83-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage());
69+
ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), null);
70+
ImageStoreResponse storeResponse = null;
71+
if (result != null ) {
72+
storeResponse = _responseGenerator.createImageStoreResponse(result);
73+
storeResponse.setResponseName(getCommandName());
74+
storeResponse.setObjectName("secondarystorage");
75+
setResponseObject(storeResponse);
76+
} else {
77+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage");
8478
}
8579
}
8680
}

‎api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreCmd.java‎

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import java.util.Collection;
2020
import java.util.HashMap;
21-
import java.util.Iterator;
2221
import java.util.Map;
2322

2423

@@ -31,7 +30,6 @@
3130
import org.apache.cloudstack.api.response.ImageStoreResponse;
3231
import org.apache.cloudstack.api.response.ZoneResponse;
3332

34-
import com.cloud.exception.DiscoveryException;
3533
import com.cloud.storage.ImageStore;
3634
import com.cloud.user.Account;
3735

@@ -79,11 +77,10 @@ public Long getZoneId() {
7977
public Map<String, String> getDetails() {
8078
Map<String, String> detailsMap = null;
8179
if (details != null && !details.isEmpty()) {
82-
detailsMap = new HashMap<String, String>();
80+
detailsMap = new HashMap<>();
8381
Collection<?> props = details.values();
84-
Iterator<?> iter = props.iterator();
85-
while (iter.hasNext()) {
86-
HashMap<String, String> detail = (HashMap<String, String>)iter.next();
82+
for (Object prop : props) {
83+
HashMap<String, String> detail = (HashMap<String, String>) prop;
8784
String key = detail.get("key");
8885
String value = detail.get("value");
8986
detailsMap.put(key, value);
@@ -123,20 +120,15 @@ public long getEntityOwnerId() {
123120

124121
@Override
125122
public void execute(){
126-
try{
127-
ImageStore result = _storageService.discoverImageStore(getName(), getUrl(), getProviderName(), getZoneId(), getDetails());
128-
ImageStoreResponse storeResponse = null;
129-
if (result != null) {
130-
storeResponse = _responseGenerator.createImageStoreResponse(result);
131-
storeResponse.setResponseName(getCommandName());
132-
storeResponse.setObjectName("imagestore");
133-
setResponseObject(storeResponse);
134-
} else {
135-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage");
136-
}
137-
} catch (DiscoveryException ex) {
138-
logger.warn("Exception: ", ex);
139-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage());
123+
ImageStore result = _storageService.discoverImageStore(getName(), getUrl(), getProviderName(), getZoneId(), getDetails());
124+
ImageStoreResponse storeResponse;
125+
if (result != null) {
126+
storeResponse = _responseGenerator.createImageStoreResponse(result);
127+
storeResponse.setResponseName(getCommandName());
128+
storeResponse.setObjectName("imagestore");
129+
setResponseObject(storeResponse);
130+
} else {
131+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage");
140132
}
141133
}
142134
}

‎api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreS3CMD.java‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,14 @@
4848
import org.apache.cloudstack.api.response.ImageStoreResponse;
4949

5050
import com.cloud.exception.ConcurrentOperationException;
51-
import com.cloud.exception.DiscoveryException;
5251
import com.cloud.exception.InsufficientCapacityException;
5352
import com.cloud.exception.NetworkRuleConflictException;
5453
import com.cloud.exception.ResourceAllocationException;
5554
import com.cloud.exception.ResourceUnavailableException;
5655
import com.cloud.storage.ImageStore;
5756

58-
@APICommand(name = "addImageStoreS3", description = "Adds S3 Image Store", responseObject = ImageStoreResponse.class, since = "4.7.0",
59-
requestHasSensitiveInfo = true, responseHasSensitiveInfo = false)
57+
@APICommand(name = "addImageStoreS3", description = "Adds S3 Image Store", responseObject = ImageStoreResponse.class,
58+
since = "4.7.0", responseHasSensitiveInfo = false)
6059
public final class AddImageStoreS3CMD extends BaseCmd implements ClientOptions {
6160

6261
private static final String s_name = "addImageStoreS3Response";
@@ -73,32 +72,32 @@ public final class AddImageStoreS3CMD extends BaseCmd implements ClientOptions {
7372
@Parameter(name = S3_BUCKET_NAME, type = STRING, required = true, description = "Name of the storage bucket")
7473
private String bucketName;
7574

76-
@Parameter(name = S3_SIGNER, type = STRING, required = false, description = "Signer Algorithm to use, either S3SignerType or AWSS3V4SignerType")
75+
@Parameter(name = S3_SIGNER, type = STRING, description = "Signer Algorithm to use, either S3SignerType or AWSS3V4SignerType")
7776
private String signer;
7877

79-
@Parameter(name = S3_HTTPS_FLAG, type = BOOLEAN, required = false, description = "Use HTTPS instead of HTTP")
78+
@Parameter(name = S3_HTTPS_FLAG, type = BOOLEAN, description = "Use HTTPS instead of HTTP")
8079
private Boolean httpsFlag;
8180

82-
@Parameter(name = S3_CONNECTION_TIMEOUT, type = INTEGER, required = false, description = "Connection timeout (milliseconds)")
81+
@Parameter(name = S3_CONNECTION_TIMEOUT, type = INTEGER, description = "Connection timeout (milliseconds)")
8382
private Integer connectionTimeout;
8483

85-
@Parameter(name = S3_MAX_ERROR_RETRY, type = INTEGER, required = false, description = "Maximum number of times to retry on error")
84+
@Parameter(name = S3_MAX_ERROR_RETRY, type = INTEGER, description = "Maximum number of times to retry on error")
8685
private Integer maxErrorRetry;
8786

88-
@Parameter(name = S3_SOCKET_TIMEOUT, type = INTEGER, required = false, description = "Socket timeout (milliseconds)")
87+
@Parameter(name = S3_SOCKET_TIMEOUT, type = INTEGER, description = "Socket timeout (milliseconds)")
8988
private Integer socketTimeout;
9089

91-
@Parameter(name = S3_CONNECTION_TTL, type = INTEGER, required = false, description = "Connection TTL (milliseconds)")
90+
@Parameter(name = S3_CONNECTION_TTL, type = INTEGER, description = "Connection TTL (milliseconds)")
9291
private Integer connectionTtl;
9392

94-
@Parameter(name = S3_USE_TCP_KEEPALIVE, type = BOOLEAN, required = false, description = "Whether TCP keep-alive is used")
93+
@Parameter(name = S3_USE_TCP_KEEPALIVE, type = BOOLEAN, description = "Whether TCP keep-alive is used")
9594
private Boolean useTCPKeepAlive;
9695

9796
@Override
9897
public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
9998
ResourceAllocationException, NetworkRuleConflictException {
10099

101-
Map<String, String> dm = new HashMap();
100+
Map<String, String> dm = new HashMap<>();
102101

103102
dm.put(ApiConstants.S3_ACCESS_KEY, getAccessKey());
104103
dm.put(ApiConstants.S3_SECRET_KEY, getSecretKey());
@@ -127,20 +126,15 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE
127126
dm.put(ApiConstants.S3_USE_TCP_KEEPALIVE, getUseTCPKeepAlive().toString());
128127
}
129128

130-
try{
131-
ImageStore result = _storageService.discoverImageStore(null, null, "S3", null, dm);
132-
ImageStoreResponse storeResponse;
133-
if (result != null) {
134-
storeResponse = _responseGenerator.createImageStoreResponse(result);
135-
storeResponse.setResponseName(getCommandName());
136-
storeResponse.setObjectName("imagestore");
137-
setResponseObject(storeResponse);
138-
} else {
139-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add S3 Image Store.");
140-
}
141-
} catch (DiscoveryException ex) {
142-
logger.warn("Exception: ", ex);
143-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage());
129+
ImageStore result = _storageService.discoverImageStore(null, null, "S3", null, dm);
130+
ImageStoreResponse storeResponse;
131+
if (result != null) {
132+
storeResponse = _responseGenerator.createImageStoreResponse(result);
133+
storeResponse.setResponseName(getCommandName());
134+
storeResponse.setObjectName("imagestore");
135+
setResponseObject(storeResponse);
136+
} else {
137+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add S3 Image Store.");
144138
}
145139
}
146140

‎api/src/main/java/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java‎

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
// under the License.
1717
package org.apache.cloudstack.api.command.admin.storage;
1818

19-
import java.net.UnknownHostException;
2019
import java.util.Map;
2120

2221

@@ -31,8 +30,6 @@
3130
import org.apache.cloudstack.api.response.StoragePoolResponse;
3231
import org.apache.cloudstack.api.response.ZoneResponse;
3332

34-
import com.cloud.exception.ResourceInUseException;
35-
import com.cloud.exception.ResourceUnavailableException;
3633
import com.cloud.storage.StoragePool;
3734
import com.cloud.user.Account;
3835

@@ -46,53 +43,85 @@ public class CreateStoragePoolCmd extends BaseCmd {
4643
//////////////// API parameters /////////////////////
4744
/////////////////////////////////////////////////////
4845

49-
@Parameter(name = ApiConstants.CLUSTER_ID, type = CommandType.UUID, entityType = ClusterResponse.class, description = "The cluster ID for the storage pool")
46+
@Parameter(name = ApiConstants.CLUSTER_ID,
47+
type = CommandType.UUID,
48+
entityType = ClusterResponse.class,
49+
description = "The cluster ID for the storage pool")
5050
private Long clusterId;
5151

52-
@Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "The details for the storage pool")
52+
@Parameter(name = ApiConstants.DETAILS,
53+
type = CommandType.MAP,
54+
description = "The details for the storage pool")
5355
private Map details;
5456

55-
@Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "The name for the storage pool")
57+
@Parameter(name = ApiConstants.NAME,
58+
type = CommandType.STRING,
59+
required = true,
60+
description = "The name for the storage pool")
5661
private String storagePoolName;
5762

58-
@Parameter(name = ApiConstants.POD_ID, type = CommandType.UUID, entityType = PodResponse.class, description = "The Pod ID for the storage pool")
63+
@Parameter(name = ApiConstants.POD_ID,
64+
type = CommandType.UUID,
65+
entityType = PodResponse.class,
66+
description = "The Pod ID for the storage pool")
5967
private Long podId;
6068

61-
@Parameter(name = ApiConstants.TAGS, type = CommandType.STRING, description = "The tags for the storage pool")
69+
@Parameter(name = ApiConstants.TAGS,
70+
type = CommandType.STRING,
71+
description = "The tags for the storage pool")
6272
private String tags;
6373

64-
@Parameter(name = ApiConstants.STORAGE_ACCESS_GROUPS, type = CommandType.STRING,
74+
@Parameter(name = ApiConstants.STORAGE_ACCESS_GROUPS,
75+
type = CommandType.STRING,
6576
description = "comma separated list of storage access groups for connecting to hosts having those specific groups", since = "4.21.0")
6677
private String storageAccessGroups;
6778

68-
@Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = true, description = "The URL of the storage pool")
79+
@Parameter(name = ApiConstants.URL,
80+
type = CommandType.STRING,
81+
required = true,
82+
description = "The URL of the storage pool")
6983
private String url;
7084

71-
@Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, required = true, description = "The Zone ID for the storage pool")
85+
@Parameter(name = ApiConstants.ZONE_ID,
86+
type = CommandType.UUID,
87+
entityType = ZoneResponse.class,
88+
required = true,
89+
description = "The Zone ID for the storage pool")
7290
private Long zoneId;
7391

74-
@Parameter(name = ApiConstants.PROVIDER, type = CommandType.STRING, required = false, description = "The storage provider name")
92+
@Parameter(name = ApiConstants.PROVIDER,
93+
type = CommandType.STRING,
94+
description = "The storage provider name")
7595
private String storageProviderName;
7696

77-
@Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, required = false, description = "The scope of the storage: cluster or zone")
97+
@Parameter(name = ApiConstants.SCOPE,
98+
type = CommandType.STRING,
99+
description = "The scope of the storage: cluster or zone")
78100
private String scope;
79101

80-
@Parameter(name = ApiConstants.MANAGED, type = CommandType.BOOLEAN, required = false, description = "Whether the storage should be managed by CloudStack")
102+
@Parameter(name = ApiConstants.MANAGED,
103+
type = CommandType.BOOLEAN,
104+
description = "Whether the storage should be managed by CloudStack")
81105
private Boolean managed;
82106

83-
@Parameter(name = ApiConstants.CAPACITY_IOPS, type = CommandType.LONG, required = false, description = "IOPS CloudStack can provision from this storage pool")
107+
@Parameter(name = ApiConstants.CAPACITY_IOPS,
108+
type = CommandType.LONG,
109+
description = "IOPS CloudStack can provision from this storage pool")
84110
private Long capacityIops;
85111

86-
@Parameter(name = ApiConstants.CAPACITY_BYTES, type = CommandType.LONG, required = false, description = "Bytes CloudStack can provision from this storage pool")
112+
@Parameter(name = ApiConstants.CAPACITY_BYTES,
113+
type = CommandType.LONG,
114+
description = "Bytes CloudStack can provision from this storage pool")
87115
private Long capacityBytes;
88116

89117
@Parameter(name = ApiConstants.HYPERVISOR,
90118
type = CommandType.STRING,
91-
required = false,
92119
description = "Hypervisor type of the hosts in zone that will be attached to this storage pool. KVM, VMware supported as of now.")
93120
private String hypervisor;
94121

95-
@Parameter(name = ApiConstants.IS_TAG_A_RULE, type = CommandType.BOOLEAN, description = ApiConstants.PARAMETER_DESCRIPTION_IS_TAG_A_RULE)
122+
@Parameter(name = ApiConstants.IS_TAG_A_RULE,
123+
type = CommandType.BOOLEAN,
124+
description = ApiConstants.PARAMETER_DESCRIPTION_IS_TAG_A_RULE)
96125
private Boolean isTagARule;
97126

98127
/////////////////////////////////////////////////////
@@ -175,15 +204,6 @@ public void execute() {
175204
} else {
176205
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add storage pool");
177206
}
178-
} catch (ResourceUnavailableException ex1) {
179-
logger.warn("Exception: ", ex1);
180-
throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, ex1.getMessage());
181-
} catch (ResourceInUseException ex2) {
182-
logger.warn("Exception: ", ex2);
183-
throw new ServerApiException(ApiErrorCode.RESOURCE_IN_USE_ERROR, ex2.getMessage());
184-
} catch (UnknownHostException ex3) {
185-
logger.warn("Exception: ", ex3);
186-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex3.getMessage());
187207
} catch (Exception ex4) {
188208
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex4.getMessage());
189209
}

‎api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public ApiCommandResourceType getApiResourceType() {
131131
return ApiCommandResourceType.StoragePool;
132132
}
133133

134-
public Map<String,String> getDetails() {
134+
public Map<String,Object> getDetails() {
135135
return details;
136136
}
137137

0 commit comments

Comments
 (0)