Skip to content

Commit 865c6fd

Browse files
shwstpprshwstppr
committed
allow cleanup of archives on sec store
Signed-off-by: Abhishek Kumar <[email protected]>
1 parent 5525242 commit 865c6fd

File tree

5 files changed

+119
-25
lines changed

5 files changed

+119
-25
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public static enum Status {
4040
}
4141

4242
public static enum Type {
43-
VOLUME, SNAPSHOT, TEMPLATE, ISO
43+
VOLUME, SNAPSHOT, TEMPLATE, ISO, ARCHIVE
4444
}
4545

4646
public static enum Mode {

‎framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsShareManagerImpl.java‎

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import javax.inject.Inject;
4141
import javax.naming.ConfigurationException;
4242

43+
import org.apache.cloudstack.api.ApiConstants;
4344
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
4445
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
4546
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
@@ -55,11 +56,15 @@
5556
import org.apache.cloudstack.framework.config.Configurable;
5657
import org.apache.cloudstack.framework.extensions.ExtensionArchiveDataObject;
5758
import org.apache.cloudstack.framework.extensions.command.DownloadAndSyncExtensionFilesCommand;
59+
import org.apache.cloudstack.framework.extensions.dao.ExtensionDao;
60+
import org.apache.cloudstack.framework.extensions.dao.ExtensionDetailsDao;
61+
import org.apache.cloudstack.framework.extensions.vo.ExtensionVO;
5862
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
5963
import org.apache.cloudstack.management.ManagementServerHost;
6064
import org.apache.cloudstack.storage.command.CommandResult;
6165
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
6266
import org.apache.cloudstack.utils.filesystem.ArchiveUtil;
67+
import org.apache.cloudstack.utils.identity.ManagementServerNode;
6368
import org.apache.cloudstack.utils.security.DigestHelper;
6469
import org.apache.cloudstack.utils.security.HMACSignUtil;
6570
import org.apache.cloudstack.utils.server.ServerPropertiesUtil;
@@ -70,17 +75,24 @@
7075
import com.cloud.agent.api.Answer;
7176
import com.cloud.agent.api.Command;
7277
import com.cloud.cluster.ClusterManager;
78+
import com.cloud.cluster.ManagementServerHostVO;
79+
import com.cloud.cluster.dao.ManagementServerHostDao;
7380
import com.cloud.dc.dao.DataCenterDao;
7481
import com.cloud.serializer.GsonHelper;
7582
import com.cloud.server.ManagementService;
83+
import com.cloud.storage.DataStoreRole;
7684
import com.cloud.storage.Storage;
85+
import com.cloud.storage.Upload;
7786
import com.cloud.utils.FileUtil;
7887
import com.cloud.utils.HttpUtils;
7988
import com.cloud.utils.Pair;
8089
import com.cloud.utils.StringUtils;
8190
import com.cloud.utils.component.ManagerBase;
8291
import com.cloud.utils.concurrency.NamedThreadFactory;
8392
import com.cloud.utils.db.GlobalLock;
93+
import com.cloud.utils.db.Transaction;
94+
import com.cloud.utils.db.TransactionCallbackNoReturn;
95+
import com.cloud.utils.db.TransactionStatus;
8496
import com.cloud.utils.exception.CloudRuntimeException;
8597
import com.cloud.vm.SecondaryStorageVmVO;
8698
import com.cloud.vm.VirtualMachine;
@@ -118,12 +130,21 @@ public class ExtensionsShareManagerImpl extends ManagerBase implements Extension
118130
@Inject
119131
DataStoreManager dataStoreManager;
120132

133+
@Inject
134+
ManagementServerHostDao managementServerHostDao;
135+
121136
@Inject
122137
ManagementService managementService;
123138

124139
@Inject
125140
EndPointSelector endPointSelector;
126141

142+
@Inject
143+
ExtensionDao extensionDao;
144+
145+
@Inject
146+
ExtensionDetailsDao extensionDetailsDao;
147+
127148
private ScheduledExecutorService extensionShareCleanupExecutor;
128149
private int shareLinkValidityInterval;
129150
private boolean serverShareEnabled = true;
@@ -446,7 +467,7 @@ protected void applyExtensionSync(Extension extension, DownloadAndSyncExtensionF
446467
FileUtil.deleteRecursively(applyRoot);
447468
}
448469

449-
protected void cleanupExtensionsShareFiles(long cutoff) throws IOException {
470+
protected void cleanupExtensionsShareFilesOnMS(long cutoff) throws IOException {
450471
Path sharePath = getExtensionsSharePath();
451472
if (!Files.exists(sharePath) || !Files.isDirectory(sharePath)) {
452473
return;
@@ -475,6 +496,18 @@ protected void cleanupExtensionsShareFiles(long cutoff) throws IOException {
475496
}
476497
}
477498

499+
protected void cleanupExtensionsShareFilesOnSecondaryStorage(long cutoff) {
500+
ManagementServerHostVO msHost = managementServerHostDao.findOneByLongestRuntime();
501+
if (msHost == null || (msHost.getMsid() != ManagementServerNode.getManagementServerId())) {
502+
logger.debug("Skipping the secondary storage extensions download files cleanup task on this management server");
503+
return;
504+
}
505+
List<ExtensionVO> extensions = extensionDao.listAll();
506+
for (ExtensionVO extension : extensions) {
507+
cleanupExistingExtensionDownloadArchiveAndDetails(extension, cutoff);
508+
}
509+
}
510+
478511
static protected class DownloadExtensionArchiveOnSecondaryStorageContext<T> extends AsyncRpcContext<T> {
479512
final Extension extension;
480513
final ArchiveInfo archiveInfo;
@@ -532,6 +565,45 @@ protected AsyncCallFuture<DataObject> downloadExtensionArchiveOnSecondaryStorage
532565
return future;
533566
}
534567

568+
protected void cleanupExistingExtensionDownloadArchiveAndDetails(Extension extension, Long cutoff) {
569+
Map<String, String> details = extensionDetailsDao.listDetailsKeyPairs(extension.getId(), false);
570+
if (!details.containsKey("imagestoredownloadurl")) {
571+
return;
572+
}
573+
final String url = details.get("imagestoredownloadurl");
574+
final String storeIdStr = details.get(ApiConstants.IMAGE_STORE_ID);
575+
final String timestampStr = details.get("imagestoredownloadurltimestamp");
576+
final long timestamp = StringUtils.isNotBlank(timestampStr) ? Long.parseLong(timestampStr) : -1L;
577+
if (cutoff != null && timestamp != -1L && timestamp >= cutoff) {
578+
return;
579+
}
580+
final String installPath = details.get("imagestorepath");
581+
final long storeId = StringUtils.isNotBlank(storeIdStr) ? Long.parseLong(storeIdStr) : -1L;
582+
if (StringUtils.isNotBlank(url) && storeId != -1L && timestamp != -1L && StringUtils.isNotBlank(installPath)) {
583+
try {
584+
DataStore store = dataStoreManager.getDataStore(storeId, DataStoreRole.Image);
585+
if (store != null) {
586+
((ImageStoreEntity) store).deleteExtractUrl(installPath, url, Upload.Type.ARCHIVE);
587+
}
588+
} catch (CloudRuntimeException e) {
589+
logger.warn("Failed to cleanup existing extension download archive for {}: {}", extension,
590+
e.getMessage());
591+
if (cutoff == null) {
592+
return;
593+
}
594+
}
595+
}
596+
Transaction.execute(new TransactionCallbackNoReturn() {
597+
@Override
598+
public void doInTransactionWithoutResult(TransactionStatus status) {
599+
extensionDetailsDao.removeDetail(extension.getId(), "imagestoredownloadurl");
600+
extensionDetailsDao.removeDetail(extension.getId(), "imagestoredownloadurltimestamp");
601+
extensionDetailsDao.removeDetail(extension.getId(), ApiConstants.IMAGE_STORE_ID);
602+
extensionDetailsDao.removeDetail(extension.getId(), "imagestorepath");
603+
}
604+
});
605+
}
606+
535607
protected Pair<Boolean, String> downloadExtensionViaSecondaryStorage(Extension extension, ArchiveInfo archiveInfo,
536608
String downloadUrl) {
537609
// Find an active zone, if not available return error
@@ -576,7 +648,19 @@ protected Pair<Boolean, String> downloadExtensionViaSecondaryStorage(Extension e
576648
return new Pair<>(false, msg);
577649
}
578650
ImageStoreEntity imageStoreEntity = (ImageStoreEntity)imageStore;
579-
String url = imageStoreEntity.createEntityExtractUrl(dataObject.getTO().getPath(), Storage.ImageFormat.ZIP, dataObject);
651+
String installPath = dataObject.getTO().getPath();
652+
String url = imageStoreEntity.createEntityExtractUrl(installPath, Storage.ImageFormat.ZIP, dataObject);
653+
cleanupExistingExtensionDownloadArchiveAndDetails(extension, null);
654+
final long imageStoreId = imageStore.getId();
655+
Transaction.execute(new TransactionCallbackNoReturn() {
656+
@Override
657+
public void doInTransactionWithoutResult(TransactionStatus status) {
658+
extensionDetailsDao.addDetail(extension.getId(), "imagestoredownloadurl", url, false);
659+
extensionDetailsDao.addDetail(extension.getId(), "imagestoredownloadurltimestamp", Long.toString(System.currentTimeMillis()), false);
660+
extensionDetailsDao.addDetail(extension.getId(), ApiConstants.IMAGE_STORE_ID, Long.toString(imageStoreId), false);
661+
extensionDetailsDao.addDetail(extension.getId(), "imagestorepath", installPath, false);
662+
}
663+
});
580664
return new Pair<>(true, url);
581665
}
582666

@@ -751,7 +835,8 @@ protected void reallyRun() {
751835
try {
752836
long expiryMillis = shareLinkValidityInterval * 1100L;
753837
long cutoff = System.currentTimeMillis() - expiryMillis;
754-
cleanupExtensionsShareFiles(cutoff);
838+
cleanupExtensionsShareFilesOnMS(cutoff);
839+
cleanupExtensionsShareFilesOnSecondaryStorage(cutoff);
755840
} catch (Exception e) {
756841
logger.warn("Extensions share cleanup failed", e);
757842
}

‎framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsShareManagerImplTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ public void cleanupExtensionsShareFilesDeletesExpiredArchives() throws IOExcepti
784784
Files.setLastModifiedTime(validFile, FileTime.fromMillis(System.currentTimeMillis() + (cutoffDiff / 2)));
785785
try {
786786
doReturn(sharePath).when(extensionsShareManager).getExtensionsSharePath();
787-
extensionsShareManager.cleanupExtensionsShareFiles(System.currentTimeMillis() - cutoffDiff);
787+
extensionsShareManager.cleanupExtensionsShareFilesOnMS(System.currentTimeMillis() - cutoffDiff);
788788
assertFalse(Files.exists(expiredFile));
789789
assertTrue(Files.exists(validFile));
790790
} finally {
@@ -798,7 +798,7 @@ public void cleanupExtensionsShareFilesHandlesNonTgzFilesGracefully() throws IOE
798798
Path nonTgzFile = Files.createFile(sharePath.resolve("file.txt"));
799799
try {
800800
doReturn(sharePath).when(extensionsShareManager).getExtensionsSharePath();
801-
extensionsShareManager.cleanupExtensionsShareFiles(System.currentTimeMillis());
801+
extensionsShareManager.cleanupExtensionsShareFilesOnMS(System.currentTimeMillis());
802802
assertTrue(Files.exists(nonTgzFile));
803803
} finally {
804804
FileUtil.deleteRecursively(sharePath);
@@ -809,7 +809,7 @@ public void cleanupExtensionsShareFilesHandlesNonTgzFilesGracefully() throws IOE
809809
public void cleanupExtensionsShareFilesSkipsNonExistentSharePath() throws IOException {
810810
Path nonExistentPath = Path.of("/nonexistent/sharePath");
811811
doReturn(nonExistentPath).when(extensionsShareManager).getExtensionsSharePath();
812-
extensionsShareManager.cleanupExtensionsShareFiles(System.currentTimeMillis());
812+
extensionsShareManager.cleanupExtensionsShareFilesOnMS(System.currentTimeMillis());
813813
// No exception should be thrown
814814
}
815815

‎services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ private String createTempDirAndPropertiesFile(ResourceType resourceType, String
724724
logger.warn("Unable to create " + tmpDir);
725725
return "Unable to create " + tmpDir;
726726
}
727-
if (resourceType.doesRequirePostDownloadProcessing()) {
727+
if (!resourceType.doesRequirePostDownloadProcessing()) {
728728
return null;
729729
}
730730
// TO DO - define constant for volume properties.

‎services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
// under the License.
1717
package org.apache.cloudstack.storage.template;
1818

19+
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
20+
1921
import java.io.File;
2022
import java.io.IOException;
2123
import java.net.URI;
@@ -32,11 +34,10 @@
3234

3335
import javax.naming.ConfigurationException;
3436

35-
import com.cloud.agent.api.Answer;
36-
37-
import com.cloud.agent.api.ConvertSnapshotCommand;
3837
import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
3938

39+
import com.cloud.agent.api.Answer;
40+
import com.cloud.agent.api.ConvertSnapshotCommand;
4041
import com.cloud.agent.api.storage.CreateEntityDownloadURLAnswer;
4142
import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand;
4243
import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
@@ -56,10 +57,11 @@
5657
import com.cloud.utils.exception.CloudRuntimeException;
5758
import com.cloud.utils.script.Script;
5859

59-
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
60-
6160
public class UploadManagerImpl extends ManagerBase implements UploadManager {
6261

62+
protected static final String BASE_EXTRACT_DIR = "/var/www/html/userdata/";
63+
protected static final String BASE_MOUNT_DIR = "/mnt/SecStorage/";
64+
6365
public class Completion implements UploadCompleteCallback {
6466
private final String jobId;
6567

@@ -101,7 +103,7 @@ public void cleanup() {
101103
private ExecutorService threadPool;
102104
private final Map<String, UploadJob> jobs = new ConcurrentHashMap<String, UploadJob>();
103105
private String parentDir;
104-
private final String extractMountPoint = "/mnt/SecStorage/extractmnt";
106+
private final String extractMountPoint = BASE_MOUNT_DIR + "extractmnt";
105107
private StorageLayer _storage;
106108
private boolean hvm;
107109

@@ -269,8 +271,7 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
269271
return new CreateEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
270272
}
271273
// Create the directory structure so that its visible under apache server root
272-
String extractDir = "/var/www/html/userdata/";
273-
extractDir = extractDir + cmd.getFilepathInExtractURL() + File.separator;
274+
String extractDir = BASE_EXTRACT_DIR + cmd.getFilepathInExtractURL() + File.separator;
274275
Script command = new Script("/bin/su", logger);
275276
command.add("-s");
276277
command.add("/bin/bash");
@@ -284,7 +285,7 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
284285
return new CreateEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
285286
}
286287

287-
File file = new File("/mnt/SecStorage/" + cmd.getParent() + File.separator + cmd.getInstallPath());
288+
File file = new File(BASE_MOUNT_DIR + cmd.getParent() + File.separator + cmd.getInstallPath());
288289
// Return error if the file does not exist or is a directory
289290
if (!file.exists() || file.isDirectory()) {
290291
String errorString = "Error in finding the file " + file.getAbsolutePath();
@@ -297,7 +298,7 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
297298
// Create a symbolic link from the actual directory to the template location. The entity would be directly visible under /var/www/html/userdata/cmd.getInstallPath();
298299
command = new Script("/bin/bash", logger);
299300
command.add("-c");
300-
command.add("ln -sf /mnt/SecStorage/" + cmd.getParent() + File.separator + cmd.getInstallPath() + " " + extractDir + filename);
301+
command.add("ln -sf " + BASE_MOUNT_DIR + cmd.getParent() + File.separator + cmd.getInstallPath() + " " + extractDir + filename);
301302
result = command.execute();
302303
if (result != null) {
303304
String errorString = "Error in linking err=" + result;
@@ -333,7 +334,13 @@ public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
333334
String extractUrl = cmd.getExtractUrl();
334335
String result;
335336
if (extractUrl != null) {
336-
command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
337+
URI uri = URI.create(extractUrl);
338+
String uriPath = uri.getPath();
339+
String marker = "/userdata/";
340+
String linkPath = uriPath.startsWith(marker)
341+
? uriPath.substring(marker.length())
342+
: uriPath.substring(uriPath.indexOf(marker) + marker.length());
343+
command.add("unlink " + BASE_EXTRACT_DIR + linkPath);
337344
result = command.execute();
338345
if (result != null) {
339346
// FIXME - Ideally should bail out if you can't delete symlink. Not doing it right now.
@@ -342,15 +349,17 @@ public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
342349
}
343350
}
344351

345-
// If its a volume also delete the Hard link since it was created only for the purpose of download.
346-
if (cmd.getType() == Upload.Type.VOLUME) {
352+
// If its a volume or archive also delete the Hard link since it was created only for the purpose of download.
353+
if (cmd.getType() == Upload.Type.VOLUME || cmd.getType() == Upload.Type.ARCHIVE) {
354+
String targetPath = BASE_MOUNT_DIR + cmd.getParentPath() + File.separator + path;
347355
command = new Script("/bin/bash", logger);
348356
command.add("-c");
349-
command.add("rm -rf /mnt/SecStorage/" + cmd.getParentPath() + File.separator + path);
350-
logger.warn(" " + parentDir + File.separator + path);
357+
command.add("rm -rf " + targetPath);
358+
logger.warn("Deleted {} {}", cmd.getType().name().toLowerCase(), targetPath);
351359
result = command.execute();
352360
if (result != null) {
353-
String errorString = "Error in deleting volume " + path + " : " + result;
361+
String errorString = String.format("Error in deleting %s %s : %s", cmd.getType().name().toLowerCase(),
362+
path, result);
354363
logger.warn(errorString);
355364
return new Answer(cmd, false, errorString);
356365
}
@@ -359,7 +368,7 @@ public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
359368
if (Upload.Type.SNAPSHOT.equals(cmd.getType())) {
360369
try {
361370
path = path.replace(ConvertSnapshotCommand.TEMP_SNAPSHOT_NAME, "");
362-
String fullPath = String.format("/mnt/SecStorage/%s%s%s%s", cmd.getParentPath(), File.separator, path, ConvertSnapshotCommand.TEMP_SNAPSHOT_NAME);
371+
String fullPath = String.format("%s%s%s%s%s", BASE_EXTRACT_DIR, cmd.getParentPath(), File.separator, path, ConvertSnapshotCommand.TEMP_SNAPSHOT_NAME);
363372
Files.deleteIfExists(Path.of(fullPath));
364373
} catch (IOException e) {
365374
String errorString = String.format("Error deleting temporary snapshot %s%s.", path, ConvertSnapshotCommand.TEMP_SNAPSHOT_NAME);

0 commit comments

Comments
 (0)