Skip to content

Commit eef3e49

Browse files
shwstpprshwstppr
committed
secondary-storage: delete temp directory while deleting entity download
url When expired entity URLs are cleaned up, orphan directories are left behind. This change tries to delete the base temporary directory while cleanup. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent 6bed3d4 commit eef3e49

File tree

3 files changed

+131
-6
lines changed

3 files changed

+131
-6
lines changed

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

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
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;
22+
import java.io.IOException;
2023
import java.net.URI;
2124
import java.net.URISyntaxException;
2225
import java.nio.file.Path;
@@ -30,10 +33,10 @@
3033

3134
import javax.naming.ConfigurationException;
3235

33-
import com.cloud.agent.api.Answer;
34-
3536
import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
37+
import org.apache.commons.lang3.StringUtils;
3638

39+
import com.cloud.agent.api.Answer;
3740
import com.cloud.agent.api.storage.CreateEntityDownloadURLAnswer;
3841
import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand;
3942
import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
@@ -48,15 +51,17 @@
4851
import com.cloud.storage.template.TemplateUploader;
4952
import com.cloud.storage.template.TemplateUploader.Status;
5053
import com.cloud.storage.template.TemplateUploader.UploadCompleteCallback;
54+
import com.cloud.utils.FileUtil;
5155
import com.cloud.utils.NumbersUtil;
56+
import com.cloud.utils.UuidUtils;
5257
import com.cloud.utils.component.ManagerBase;
5358
import com.cloud.utils.exception.CloudRuntimeException;
5459
import com.cloud.utils.script.Script;
5560

56-
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
57-
5861
public class UploadManagerImpl extends ManagerBase implements UploadManager {
5962

63+
protected static final String BASE_EXTRACT_DIR = "/var/www/html/userdata/";
64+
6065
public class Completion implements UploadCompleteCallback {
6166
private final String jobId;
6267

@@ -266,7 +271,7 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
266271
return new CreateEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
267272
}
268273
// Create the directory structure so that its visible under apache server root
269-
String extractDir = "/var/www/html/userdata/";
274+
String extractDir = BASE_EXTRACT_DIR;
270275
extractDir = extractDir + cmd.getFilepathInExtractURL() + File.separator;
271276
Script command = new Script("/bin/su", logger);
272277
command.add("-s");
@@ -330,12 +335,15 @@ public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
330335
String extractUrl = cmd.getExtractUrl();
331336
String result;
332337
if (extractUrl != null) {
333-
command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
338+
String linkPath = extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1);
339+
command.add("unlink " + BASE_EXTRACT_DIR + linkPath);
334340
result = command.execute();
335341
if (result != null) {
336342
// FIXME - Ideally should bail out if you can't delete symlink. Not doing it right now.
337343
// This is because the ssvm might already be destroyed and the symlinks do not exist.
338344
logger.warn("Error in deleting symlink :" + result);
345+
} else {
346+
deleteEntitySymlinkRootPathIfNeeded(cmd, linkPath);
339347
}
340348
}
341349

@@ -356,6 +364,30 @@ public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
356364
return new Answer(cmd, true, "");
357365
}
358366

367+
protected void deleteEntitySymlinkRootPathIfNeeded(DeleteEntityDownloadURLCommand cmd, String linkPath) {
368+
if (StringUtils.isEmpty(linkPath)) {
369+
return;
370+
}
371+
String[] parts = linkPath.split("/");
372+
if (parts.length == 0) {
373+
return;
374+
}
375+
String rootDir = parts[0];
376+
if (StringUtils.isEmpty(rootDir) || !UuidUtils.isUuid(rootDir)) {
377+
return;
378+
}
379+
logger.info("Deleting symlink root directory: {} for {}", rootDir, cmd.getExtractUrl());
380+
Path rootDirPath = Path.of(BASE_EXTRACT_DIR + rootDir);
381+
String failMsg = "Failed to delete symlink root directory: {} for {}";
382+
try {
383+
if (!FileUtil.deleteRecursively(rootDirPath)) {
384+
logger.warn(failMsg, rootDir, cmd.getExtractUrl());
385+
}
386+
} catch (IOException e) {
387+
logger.warn(failMsg, rootDir, cmd.getExtractUrl(), e);
388+
}
389+
}
390+
359391
private String getInstallPath(String jobId) {
360392
// TODO Auto-generated method stub
361393
return null;

‎services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.storage.template;
18+
19+
import static org.mockito.ArgumentMatchers.any;
20+
import static org.mockito.Mockito.mock;
21+
import static org.mockito.Mockito.mockStatic;
22+
import static org.mockito.Mockito.never;
23+
import static org.mockito.Mockito.times;
24+
25+
import java.nio.file.Path;
26+
27+
import org.junit.After;
28+
import org.junit.Before;
29+
import org.junit.Test;
30+
import org.junit.runner.RunWith;
31+
import org.mockito.InjectMocks;
32+
import org.mockito.MockedStatic;
33+
import org.mockito.Mockito;
34+
import org.mockito.junit.MockitoJUnitRunner;
35+
36+
import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
37+
import com.cloud.utils.FileUtil;
38+
39+
@RunWith(MockitoJUnitRunner.class)
40+
public class UploadManagerImplTest {
41+
42+
@InjectMocks
43+
UploadManagerImpl uploadManager;
44+
45+
MockedStatic<FileUtil> fileUtilMock;
46+
47+
@Before
48+
public void setup() {
49+
fileUtilMock = mockStatic(FileUtil.class, Mockito.CALLS_REAL_METHODS);
50+
fileUtilMock.when(() -> FileUtil.deleteRecursively(any(Path.class))).thenReturn(true);
51+
}
52+
53+
@After
54+
public void tearDown() {
55+
fileUtilMock.close();
56+
}
57+
58+
@Test
59+
public void doesNotDeleteWhenLinkPathIsEmpty() {
60+
String emptyLinkPath = "";
61+
uploadManager.deleteEntitySymlinkRootPathIfNeeded(mock(DeleteEntityDownloadURLCommand.class), emptyLinkPath);
62+
fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never());
63+
}
64+
65+
@Test
66+
public void doesNotDeleteWhenRootDirIsNotUuid() {
67+
String invalidLinkPath = "invalidRootDir/file";
68+
uploadManager.deleteEntitySymlinkRootPathIfNeeded(mock(DeleteEntityDownloadURLCommand.class), invalidLinkPath);
69+
fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never());
70+
}
71+
72+
@Test
73+
public void deletesSymlinkRootDirectoryWhenValidUuid() {
74+
String validLinkPath = "123e4567-e89b-12d3-a456-426614174000/file";
75+
uploadManager.deleteEntitySymlinkRootPathIfNeeded(mock(DeleteEntityDownloadURLCommand.class), validLinkPath);
76+
fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), times(1));
77+
}
78+
}

‎utils/src/main/java/com/cloud/utils/FileUtil.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,19 @@ public static boolean writeToFile(String fileName, String content) {
160160
public static String readResourceFile(String resource) throws IOException {
161161
return IOUtils.toString(Objects.requireNonNull(Thread.currentThread().getContextClassLoader().getResourceAsStream(resource)), com.cloud.utils.StringUtils.getPreferredCharset());
162162
}
163+
164+
public static boolean deleteRecursively(Path path) throws IOException {
165+
LOGGER.debug("Deleting path: {}", path);
166+
if (Files.isDirectory(path)) {
167+
try (Stream<Path> entries = Files.list(path)) {
168+
List<Path> list = entries.collect(Collectors.toList());
169+
for (Path entry : list) {
170+
if (!deleteRecursively(entry)) {
171+
return false;
172+
}
173+
}
174+
}
175+
}
176+
return Files.deleteIfExists(path);
177+
}
163178
}

0 commit comments

Comments
 (0)