Skip to content

Commit cb576f1

Browse files
author
Dingane Hlaluku
committed
Fix diagnostics dir permissions in secondary store to allow for GC
1 parent f76117c commit cb576f1

File tree

4 files changed

+99
-46
lines changed

4 files changed

+99
-46
lines changed

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyToSecondaryStorageWrapper.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
package com.cloud.hypervisor.kvm.resource.wrapper;
1818

1919
import java.io.File;
20+
import java.nio.file.Files;
21+
import java.nio.file.Path;
22+
import java.nio.file.Paths;
23+
import java.nio.file.attribute.PosixFileAttributes;
24+
import java.nio.file.attribute.PosixFilePermission;
25+
import java.util.Set;
2026

2127
import com.cloud.agent.api.Answer;
2228
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
@@ -25,11 +31,13 @@
2531
import com.cloud.resource.CommandWrapper;
2632
import com.cloud.resource.ResourceWrapper;
2733
import com.cloud.utils.ssh.SshHelper;
28-
2934
import org.apache.cloudstack.diagnostics.CopyToSecondaryStorageAnswer;
3035
import org.apache.cloudstack.diagnostics.CopyToSecondaryStorageCommand;
36+
import org.apache.cloudstack.diagnostics.DiagnosticsHelper;
3137
import org.apache.log4j.Logger;
3238

39+
import static org.apache.cloudstack.diagnostics.DiagnosticsHelper.setDirFilePermissions;
40+
3341
@ResourceWrapper(handles = CopyToSecondaryStorageCommand.class)
3442
public class LibvirtCopyToSecondaryStorageWrapper extends CommandWrapper<CopyToSecondaryStorageCommand, Answer, LibvirtComputingResource> {
3543
public static final Logger LOGGER = Logger.getLogger(LibvirtCopyToSecondaryStorageWrapper.class);
@@ -50,11 +58,14 @@ public Answer execute(CopyToSecondaryStorageCommand command, LibvirtComputingRes
5058
String mountPoint = secondaryPool.getLocalPath();
5159

5260
// /mnt/SecStorage/uuid/diagnostics_data
53-
String dataDirectoryInSecondaryStore = String.format("%s/%s", mountPoint, "diagnostics_data");
61+
String dataDirectoryInSecondaryStore = String.format("%s/%s", mountPoint, DiagnosticsHelper.DIAGNOSTICS_DATA_DIR);
5462
try {
5563
File dataDirectory = new File(dataDirectoryInSecondaryStore);
5664
boolean existsInSecondaryStore = dataDirectory.exists() || dataDirectory.mkdir();
5765

66+
// Modify directory file permissions
67+
Path path = Paths.get(dataDirectory.getAbsolutePath());
68+
setDirFilePermissions(path);
5869
if (existsInSecondaryStore) {
5970
LOGGER.info(String.format("Copying %s from %s to secondary store %s", diagnosticsZipFile, vmSshIp, secondaryStorageUrl));
6071
int port = Integer.valueOf(LibvirtComputingResource.DEFAULTDOMRSSHPORT);
@@ -76,4 +87,5 @@ public Answer execute(CopyToSecondaryStorageCommand command, LibvirtComputingRes
7687
secondaryPool.delete();
7788
}
7889
}
90+
7991
}

‎plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.net.URL;
3030
import java.net.URLConnection;
3131
import java.nio.charset.Charset;
32+
import java.nio.file.Path;
33+
import java.nio.file.Paths;
3234
import java.util.ArrayList;
3335
import java.util.Date;
3436
import java.util.HashMap;
@@ -51,6 +53,7 @@
5153

5254
import org.apache.cloudstack.diagnostics.CopyToSecondaryStorageAnswer;
5355
import org.apache.cloudstack.diagnostics.CopyToSecondaryStorageCommand;
56+
import org.apache.cloudstack.diagnostics.DiagnosticsHelper;
5457
import org.apache.cloudstack.storage.to.TemplateObjectTO;
5558
import org.apache.cloudstack.storage.to.VolumeObjectTO;
5659
import org.apache.commons.collections.CollectionUtils;
@@ -161,6 +164,8 @@
161164
import com.xensource.xenapi.VM;
162165
import com.xensource.xenapi.XenAPIObject;
163166

167+
import static org.apache.cloudstack.diagnostics.DiagnosticsHelper.setDirFilePermissions;
168+
164169
/**
165170
* CitrixResourceBase encapsulates the calls to the XenServer Xapi process to
166171
* perform the required functionalities for CloudStack.
@@ -5636,11 +5641,14 @@ public Answer copyDiagnosticsFileToSecondaryStorage(Connection conn, CopyToSecon
56365641
return new CopyToSecondaryStorageAnswer(cmd, false, "Could not mount secondary storage " + secondaryStorageMountPath + " on host " + localDir);
56375642
}
56385643

5639-
String diagnosticsFileDir = "diagnostics_data";
5640-
5641-
String dataDirectoryInSecondaryStore = localDir + "/" + diagnosticsFileDir;
5644+
String dataDirectoryInSecondaryStore = localDir + "/" + DiagnosticsHelper.DIAGNOSTICS_DATA_DIR;
56425645
File dataDirectory = new File(dataDirectoryInSecondaryStore);
56435646
boolean existsInSecondaryStore = dataDirectory.exists() || dataDirectory.mkdir();
5647+
5648+
// Modify directory file permissions
5649+
Path path = Paths.get(dataDirectory.getAbsolutePath());
5650+
setDirFilePermissions(path);
5651+
56445652
if (existsInSecondaryStore) {
56455653
int port = 3922;
56465654
File permKey = new File("/root/.ssh/id_rsa.cloud");

‎server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsHelper.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,33 @@
2525
import java.nio.file.attribute.BasicFileAttributeView;
2626
import java.nio.file.attribute.BasicFileAttributes;
2727
import java.nio.file.attribute.FileTime;
28+
import java.nio.file.attribute.PosixFileAttributes;
29+
import java.nio.file.attribute.PosixFilePermission;
30+
import java.util.Set;
2831

32+
import com.cloud.utils.script.Script2;
2933
import org.apache.commons.lang3.StringUtils;
3034
import org.apache.log4j.Logger;
3135

32-
import com.cloud.utils.script.Script2;
33-
3436
public class DiagnosticsHelper {
3537
private static final Logger LOGGER = Logger.getLogger(DiagnosticsHelper.class);
3638

39+
public static final String DIAGNOSTICS_DATA_DIR = "diagnostics_data";
40+
41+
public static void setDirFilePermissions(Path path) throws java.io.IOException {
42+
Set<PosixFilePermission> perms = Files.readAttributes(path, PosixFileAttributes.class).permissions();
43+
perms.add(PosixFilePermission.OWNER_WRITE);
44+
perms.add(PosixFilePermission.OWNER_READ);
45+
perms.add(PosixFilePermission.OWNER_EXECUTE);
46+
perms.add(PosixFilePermission.GROUP_WRITE);
47+
perms.add(PosixFilePermission.GROUP_READ);
48+
perms.add(PosixFilePermission.GROUP_EXECUTE);
49+
perms.add(PosixFilePermission.OTHERS_WRITE);
50+
perms.add(PosixFilePermission.OTHERS_READ);
51+
perms.add(PosixFilePermission.OTHERS_EXECUTE);
52+
Files.setPosixFilePermissions(path, perms);
53+
}
54+
3755
public static void umountSecondaryStorage(String mountPoint) {
3856
if (StringUtils.isNotBlank(mountPoint)) {
3957
Script2 umountCmd = new Script2("/bin/bash", LOGGER);

‎server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java‎

Lines changed: 54 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
package org.apache.cloudstack.diagnostics;
1919

2020
import java.io.File;
21+
import java.nio.file.Path;
22+
import java.nio.file.Paths;
2123
import java.util.ArrayList;
2224
import java.util.List;
2325
import java.util.Map;
@@ -26,32 +28,12 @@
2628
import javax.inject.Inject;
2729
import javax.naming.ConfigurationException;
2830

29-
import org.apache.cloudstack.api.command.admin.diagnostics.GetDiagnosticsDataCmd;
30-
import org.apache.cloudstack.api.command.admin.diagnostics.RunDiagnosticsCmd;
31-
import org.apache.cloudstack.diagnostics.fileprocessor.DiagnosticsFilesList;
32-
import org.apache.cloudstack.diagnostics.fileprocessor.DiagnosticsFilesListFactory;
33-
import org.apache.cloudstack.diagnostics.to.DiagnosticsDataObject;
34-
import org.apache.cloudstack.diagnostics.to.DiagnosticsDataTO;
35-
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
36-
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
37-
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
38-
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
39-
import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
40-
import org.apache.cloudstack.framework.config.ConfigKey;
41-
import org.apache.cloudstack.framework.config.Configurable;
42-
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
43-
import org.apache.cloudstack.poll.BackgroundPollManager;
44-
import org.apache.cloudstack.poll.BackgroundPollTask;
45-
import org.apache.cloudstack.storage.NfsMountManager;
46-
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
47-
import org.apache.commons.collections.CollectionUtils;
48-
import org.apache.commons.lang3.StringUtils;
49-
import org.apache.log4j.Logger;
50-
5131
import com.cloud.agent.AgentManager;
5232
import com.cloud.agent.api.Answer;
5333
import com.cloud.agent.api.routing.NetworkElementCommand;
5434
import com.cloud.agent.api.to.DataTO;
35+
import com.cloud.dc.DataCenterVO;
36+
import com.cloud.dc.dao.DataCenterDao;
5537
import com.cloud.event.ActionEvent;
5638
import com.cloud.event.EventTypes;
5739
import com.cloud.exception.InvalidParameterValueException;
@@ -69,8 +51,30 @@
6951
import com.cloud.vm.VirtualMachineManager;
7052
import com.cloud.vm.dao.VMInstanceDao;
7153
import com.google.common.base.Strings;
54+
import org.apache.cloudstack.api.command.admin.diagnostics.GetDiagnosticsDataCmd;
55+
import org.apache.cloudstack.api.command.admin.diagnostics.RunDiagnosticsCmd;
56+
import org.apache.cloudstack.diagnostics.fileprocessor.DiagnosticsFilesList;
57+
import org.apache.cloudstack.diagnostics.fileprocessor.DiagnosticsFilesListFactory;
58+
import org.apache.cloudstack.diagnostics.to.DiagnosticsDataObject;
59+
import org.apache.cloudstack.diagnostics.to.DiagnosticsDataTO;
60+
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
61+
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
62+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
63+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
64+
import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
65+
import org.apache.cloudstack.framework.config.ConfigKey;
66+
import org.apache.cloudstack.framework.config.Configurable;
67+
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
68+
import org.apache.cloudstack.poll.BackgroundPollManager;
69+
import org.apache.cloudstack.poll.BackgroundPollTask;
70+
import org.apache.cloudstack.storage.NfsMountManager;
71+
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
72+
import org.apache.commons.collections.CollectionUtils;
73+
import org.apache.commons.lang3.StringUtils;
74+
import org.apache.log4j.Logger;
7275

7376
import static org.apache.cloudstack.diagnostics.DiagnosticsHelper.getTimeDifference;
77+
import static org.apache.cloudstack.diagnostics.DiagnosticsHelper.setDirFilePermissions;
7478
import static org.apache.cloudstack.diagnostics.DiagnosticsHelper.umountSecondaryStorage;
7579
import static org.apache.cloudstack.diagnostics.fileprocessor.DiagnosticsFilesList.CpvmDefaultSupportedFiles;
7680
import static org.apache.cloudstack.diagnostics.fileprocessor.DiagnosticsFilesList.SsvmDefaultSupportedFiles;
@@ -97,12 +101,14 @@ public class DiagnosticsServiceImpl extends ManagerBase implements PluggableServ
97101
private ImageStoreDetailsUtil imageStoreDetailsUtil;
98102
@Inject
99103
private NfsMountManager mountManager;
104+
@Inject
105+
private DataCenterDao dataCenterDao;
100106

101107
// This 2 settings should require a restart of the management server?
102108
private static final ConfigKey<Boolean> EnableGarbageCollector = new ConfigKey<>("Advanced", Boolean.class,
103-
"diagnostics.data.gc.enable", "true", "enable the diagnostics data files garbage collector", false);
109+
"diagnostics.data.gc.enable", "true", "enable the diagnostics data files garbage collector", true);
104110
private static final ConfigKey<Integer> GarbageCollectionInterval = new ConfigKey<>("Advanced", Integer.class,
105-
"diagnostics.data.gc.interval", "86400", "garbage collection interval in seconds", false);
111+
"diagnostics.data.gc.interval", "86400", "garbage collection interval in seconds", true);
106112

107113
// These are easily computed properties and need not need a restart of the management server
108114
private static final ConfigKey<Long> DataRetrievalTimeout = new ConfigKey<>("Advanced", Long.class,
@@ -329,6 +335,11 @@ private Pair<Boolean, String> orchestrateCopyToSecondaryStorageVMware(final Data
329335
try {
330336
File dataDirectory = new File(dataDirectoryInSecondaryStore);
331337
boolean existsInSecondaryStore = dataDirectory.exists() || dataDirectory.mkdir();
338+
339+
// Modify directory file permissions
340+
Path path = Paths.get(dataDirectory.getAbsolutePath());
341+
setDirFilePermissions(path);
342+
332343
if (existsInSecondaryStore) {
333344
// scp from system VM to mounted sec storage directory
334345
int port = 3922;
@@ -449,7 +460,8 @@ private static void deleteOldDiagnosticsFiles(File directory, String storeName)
449460
for (File file : fileList) {
450461
if (file.isFile()) {
451462
if (MaximumFileAgeforGarbageCollection.value() <= getTimeDifference(file)) {
452-
file.delete();
463+
boolean success = file.delete();
464+
LOGGER.info(file.getName() + " delete status: " + success);
453465
}
454466
}
455467
}
@@ -458,23 +470,26 @@ private static void deleteOldDiagnosticsFiles(File directory, String storeName)
458470

459471
@Override
460472
protected void runInContext() {
461-
// Get All Image Stores in current running Zone
462-
List<DataStore> storeList = serviceImpl.storeMgr.listImageStores();
463-
464-
for (DataStore store : storeList) {
465-
String mountPoint = null;
466-
try {
467-
mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
468-
if (StringUtils.isNotBlank(mountPoint)) {
469-
File directory = new File(mountPoint + "/" + DIAGNOSTICS_DATA_DIRECTORY);
470-
if (directory.isDirectory()) {
471-
deleteOldDiagnosticsFiles(directory, store.getName());
473+
List<DataCenterVO> dcList = serviceImpl.dataCenterDao.listEnabledZones();
474+
for (DataCenterVO vo: dcList) {
475+
// Get All Image Stores in current running Zone
476+
List<DataStore> storeList = serviceImpl.storeMgr.getImageStoresByScope(new ZoneScope(vo.getId()));
477+
for (DataStore store : storeList) {
478+
String mountPoint = null;
479+
try {
480+
mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
481+
if (StringUtils.isNotBlank(mountPoint)) {
482+
File directory = new File(mountPoint + "/" + DIAGNOSTICS_DATA_DIRECTORY);
483+
if (directory.isDirectory()) {
484+
deleteOldDiagnosticsFiles(directory, store.getName());
485+
}
472486
}
487+
} finally {
488+
// umount secondary storage
489+
umountSecondaryStorage(mountPoint);
473490
}
474-
} finally {
475-
// umount secondary storage
476-
umountSecondaryStorage(mountPoint);
477491
}
492+
478493
}
479494
}
480495

0 commit comments

Comments
 (0)