Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

server: fix nfs version option during mounts #9559

Open
wants to merge 1 commit into
base: 4.19
Choose a base branch
from
+42 −11
Open

server: fix nfs version option during mounts #9559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.config.dao.ConfigurationDaoImpl;
import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl;
import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
Expand Down Expand Up @@ -83,7 +86,6 @@

public class SystemVmTemplateRegistration {
private static final Logger LOGGER = Logger.getLogger(SystemVmTemplateRegistration.class);
private static final String MOUNT_COMMAND = "sudo mount -t nfs %s %s";
private static final String UMOUNT_COMMAND = "sudo umount %s";
private static final String RELATIVE_TEMPLATE_PATH = "./engine/schema/dist/systemvm-templates/";
private static final String ABSOLUTE_TEMPLATE_PATH = "/usr/share/cloudstack-management/templates/systemvm/";
Expand Down Expand Up @@ -116,6 +118,8 @@
@Inject
ImageStoreDao imageStoreDao;
@Inject
ImageStoreDetailsDao imageStoreDetailsDao;
@Inject
ClusterDao clusterDao;
@Inject
ConfigurationDao configurationDao;
Expand All @@ -129,6 +133,7 @@
templateDataStoreDao = new BasicTemplateDataStoreDaoImpl();
vmInstanceDao = new VMInstanceDaoImpl();
imageStoreDao = new ImageStoreDaoImpl();
imageStoreDetailsDao = new ImageStoreDetailsDaoImpl();

Check warning on line 136 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L136 

Added line #L136 was not covered by tests
clusterDao = new ClusterDaoImpl();
configurationDao = new ConfigurationDaoImpl();
}
Expand All @@ -141,6 +146,14 @@
this.systemVmTemplateVersion = systemVmTemplateVersion;
}

public static String getMountCommand(String nfsVersion, String device, String dir) {
String cmd = "sudo mount -t nfs";

Check warning on line 150 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L149-L150 

Added lines #L149 - L150 were not covered by tests
if (StringUtils.isNotBlank(nfsVersion)) {
cmd = String.format("%s -o vers=%s", cmd, nfsVersion);

Check warning on line 152 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L152 

Added line #L152 was not covered by tests
}
return String.format("%s %s %s", cmd, device, dir);
}

Check warning on line 155 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L154-L155 

Added lines #L154 - L155 were not covered by tests

Comment on lines +149 to +156
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like a good generic method for creating the mount command, but it is in SystemVmTemplateRegistration. Does it make sense to move it to a utility and search for other places to use this, @shwstppr ? (some deduplication may be resulting)

public String getSystemVmTemplateVersion() {
if (StringUtils.isEmpty(systemVmTemplateVersion)) {
return String.format("%s.%s", CS_MAJOR_VERSION, CS_TINY_VERSION);
Expand Down Expand Up @@ -319,14 +332,14 @@
}
};

public static boolean validateIfSeeded(String url, String path) {
public static boolean validateIfSeeded(String url, String path, String nfsVersion) {

Check warning on line 335 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L335 

Added line #L335 was not covered by tests
String filePath = null;
try {
filePath = Files.createTempDirectory(TEMPORARY_SECONDARY_STORE).toString();
if (filePath == null) {
throw new CloudRuntimeException("Failed to create temporary directory to mount secondary store");
}
mountStore(url, filePath);
mountStore(url, filePath, nfsVersion);

Check warning on line 342 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L342 

Added line #L342 was not covered by tests
int lastIdx = path.lastIndexOf(File.separator);
String partialDirPath = path.substring(0, lastIdx);
String templatePath = filePath + File.separator + partialDirPath;
Expand Down Expand Up @@ -426,14 +439,13 @@
return new Pair<>(url, storeId);
}

public static void mountStore(String storeUrl, String path) {
public static void mountStore(String storeUrl, String path, String nfsVersion) {

Check warning on line 442 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L442 

Added line #L442 was not covered by tests
try {
if (storeUrl != null) {
URI uri = new URI(UriUtils.encodeURIComponent(storeUrl));
String host = uri.getHost();
String mountPath = uri.getPath();
String mount = String.format(MOUNT_COMMAND, host + ":" + mountPath, path);
Script.runSimpleBashScript(mount);
Script.runSimpleBashScript(getMountCommand(nfsVersion, host + ":" + mountPath, path));

Check warning on line 448 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L448 

Added line #L448 was not covered by tests
}
} catch (Exception e) {
String msg = "NFS Store URL is not in the correct format";
Expand Down Expand Up @@ -772,7 +784,8 @@
throw new CloudRuntimeException("Failed to create temporary file path to mount the store");
}
Pair<String, Long> storeUrlAndId = getNfsStoreInZone(zoneId);
mountStore(storeUrlAndId.first(), filePath);
String nfsVersion = getNfsVersion(storeUrlAndId.second());
mountStore(storeUrlAndId.first(), filePath, nfsVersion);

Check warning on line 788 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L787-L788 

Added lines #L787 - L788 were not covered by tests
List<String> hypervisorList = fetchAllHypervisors(zoneId);
for (String hypervisor : hypervisorList) {
Hypervisor.HypervisorType name = Hypervisor.HypervisorType.getType(hypervisor);
Expand All @@ -783,7 +796,7 @@
VMTemplateVO templateVO = vmTemplateDao.findById(templateId);
TemplateDataStoreVO templateDataStoreVO = templateDataStoreDao.findByTemplate(templateId, DataStoreRole.Image);
String installPath = templateDataStoreVO.getInstallPath();
if (validateIfSeeded(storeUrlAndId.first(), installPath)) {
if (validateIfSeeded(storeUrlAndId.first(), installPath, nfsVersion)) {
continue;
} else if (templateVO != null) {
registerTemplate(hypervisorAndTemplateName, storeUrlAndId, templateVO, templateDataStoreVO, filePath);
Expand Down Expand Up @@ -888,4 +901,17 @@
}
});
}

public String getNfsVersion(long storeId) {
final String configKey = "secstorage.nfs.version";
weizhouapache marked this conversation as resolved.
Show resolved Hide resolved
final Map<String, String> storeDetails = imageStoreDetailsDao.getDetails(storeId);

Check warning on line 907 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L905-L907 

Added lines #L905 - L907 were not covered by tests
if (storeDetails != null && storeDetails.containsKey(configKey)) {
return storeDetails.get(configKey);

Check warning on line 909 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L909 

Added line #L909 was not covered by tests
}
ConfigurationVO globalNfsVersion = configurationDao.findByName(configKey);

Check warning on line 911 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L911 

Added line #L911 was not covered by tests
if (globalNfsVersion != null) {
return globalNfsVersion.getValue();

Check warning on line 913 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L913 

Added line #L913 was not covered by tests
}
return null;
}

Check warning on line 916 in engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java#L915-L916 

Added lines #L915 - L916 were not covered by tests
}
8 changes: 6 additions & 2 deletions server/src/main/java/com/cloud/storage/StorageManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@
ConfigDepot configDepot;
@Inject
ConfigurationDao configurationDao;
@Inject
private ImageStoreDetailsUtil imageStoreDetailsUtil;

protected List<StoragePoolDiscoverer> _discoverers;

Expand Down Expand Up @@ -3425,6 +3427,7 @@
throw new CloudRuntimeException("Failed to create temporary file path to mount the store");
}
Pair<String, Long> storeUrlAndId = new Pair<>(url, store.getId());
String nfsVersion = imageStoreDetailsUtil.getNfsVersion(store.getId());

Check warning on line 3430 in server/src/main/java/com/cloud/storage/StorageManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/storage/StorageManagerImpl.java#L3430 

Added line #L3430 was not covered by tests
for (HypervisorType hypervisorType : hypSet) {
try {
if (HypervisorType.Simulator == hypervisorType) {
Expand All @@ -3441,15 +3444,16 @@
templateVO = _templateStoreDao.findByTemplate(templateId, DataStoreRole.Image);
if (templateVO != null) {
try {
if (SystemVmTemplateRegistration.validateIfSeeded(url, templateVO.getInstallPath())) {
if (SystemVmTemplateRegistration.validateIfSeeded(
url, templateVO.getInstallPath(), nfsVersion)) {
continue;
}
} catch (Exception e) {
s_logger.error("Failed to validated if template is seeded", e);
}
}
}
SystemVmTemplateRegistration.mountStore(storeUrlAndId.first(), filePath);
SystemVmTemplateRegistration.mountStore(storeUrlAndId.first(), filePath, nfsVersion);

Check warning on line 3456 in server/src/main/java/com/cloud/storage/StorageManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/storage/StorageManagerImpl.java#L3456 

Added line #L3456 was not covered by tests
if (templateVO != null && vmTemplateVO != null) {
systemVmTemplateRegistration.registerTemplate(hypervisorAndTemplateName, storeUrlAndId, vmTemplateVO, templateVO, filePath);
} else {
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,8 @@

private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(),
serviceImpl.imageStoreDetailsUtil.getNfsVersion(store.getId()));

Check warning on line 484 in server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java#L483-L484 

Added lines #L483 - L484 were not covered by tests
if (StringUtils.isNotBlank(mountPoint)) {
File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
if (directory.isDirectory()) {
Expand Down
Loading