From c5e8f63452ac61431b1722400ce5c19289cbe187 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 20 Jan 2025 17:50:17 +0530 Subject: [PATCH 1/5] Fix NPE issues during host rolling maintenance, due to host tags and custom constrained/unconstrained service offering (#9844) --- .../RollingMaintenanceManagerImpl.java | 59 ++++++++++++++++-- .../RollingMaintenanceManagerImplTest.java | 60 +++++++++++++++++++ 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java b/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java index 25b2ad53bf2..4ada43308ee 100644 --- a/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java @@ -37,6 +37,7 @@ import org.apache.cloudstack.api.command.admin.resource.StartRollingMaintenanceC import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.log4j.Logger; import com.cloud.agent.AgentManager; @@ -65,12 +66,16 @@ import com.cloud.org.Grouping; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.utils.Pair; +import com.cloud.utils.StringUtils; import com.cloud.utils.Ternary; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.UserVmDetailVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VirtualMachineProfileImpl; +import com.cloud.vm.VmDetailConstants; +import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; public class RollingMaintenanceManagerImpl extends ManagerBase implements RollingMaintenanceManager { @@ -86,6 +91,8 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin @Inject private VMInstanceDao vmInstanceDao; @Inject + protected UserVmDetailsDao userVmDetailsDao; + @Inject private ServiceOfferingDao serviceOfferingDao; @Inject private ClusterDetailsDao clusterDetailsDao; @@ -621,10 +628,19 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin int successfullyCheckedVmMigrations = 0; for (VMInstanceVO runningVM : vmsRunning) { boolean canMigrateVm = false; + Ternary cpuSpeedAndRamSize = getComputeResourcesCpuSpeedAndRamSize(runningVM); + Integer cpu = cpuSpeedAndRamSize.first(); + Integer speed = cpuSpeedAndRamSize.second(); + Integer ramSize = cpuSpeedAndRamSize.third(); + if (ObjectUtils.anyNull(cpu, speed, ramSize)) { + s_logger.warn(String.format("Cannot fetch compute resources for the VM %s, skipping it from the capacity check", runningVM)); + continue; + } + ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId()); for (Host hostInCluster : hostsInCluster) { if (!checkHostTags(hostTags, hostTagsDao.getHostTags(hostInCluster.getId()), serviceOffering.getHostTag())) { - s_logger.debug(String.format("Host tags mismatch between %s and %s Skipping it from the capacity check", host, hostInCluster)); + s_logger.warn(String.format("Host tags mismatch between %s and %s, skipping it from the capacity check", host, hostInCluster)); continue; } DeployDestination deployDestination = new DeployDestination(null, null, null, host); @@ -634,13 +650,13 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin affinityChecks = affinityChecks && affinityProcessor.check(vmProfile, deployDestination); } if (!affinityChecks) { - s_logger.debug(String.format("Affinity check failed between %s and %s Skipping it from the capacity check", host, hostInCluster)); + s_logger.warn(String.format("Affinity check failed between %s and %s, skipping it from the capacity check", host, hostInCluster)); continue; } boolean maxGuestLimit = capacityManager.checkIfHostReachMaxGuestLimit(host); - boolean hostHasCPUCapacity = capacityManager.checkIfHostHasCpuCapability(hostInCluster.getId(), serviceOffering.getCpu(), serviceOffering.getSpeed()); - int cpuRequested = serviceOffering.getCpu() * serviceOffering.getSpeed(); - long ramRequested = serviceOffering.getRamSize() * 1024L * 1024L; + boolean hostHasCPUCapacity = capacityManager.checkIfHostHasCpuCapability(hostInCluster.getId(), cpu, speed); + int cpuRequested = cpu * speed; + long ramRequested = ramSize * 1024L * 1024L; ClusterDetailsVO clusterDetailsCpuOvercommit = clusterDetailsDao.findDetail(cluster.getId(), "cpuOvercommitRatio"); ClusterDetailsVO clusterDetailsRamOvercommmt = clusterDetailsDao.findDetail(cluster.getId(), "memoryOvercommitRatio"); Float cpuOvercommitRatio = Float.parseFloat(clusterDetailsCpuOvercommit.getValue()); @@ -666,11 +682,42 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin return new Pair<>(true, "OK"); } + protected Ternary getComputeResourcesCpuSpeedAndRamSize(VMInstanceVO runningVM) { + ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId()); + Integer cpu = serviceOffering.getCpu(); + Integer speed = serviceOffering.getSpeed(); + Integer ramSize = serviceOffering.getRamSize(); + if (!serviceOffering.isDynamic()) { + return new Ternary<>(cpu, speed, ramSize); + } + + List vmDetails = userVmDetailsDao.listDetails(runningVM.getId()); + if (CollectionUtils.isEmpty(vmDetails)) { + return new Ternary<>(cpu, speed, ramSize); + } + + for (UserVmDetailVO vmDetail : vmDetails) { + if (StringUtils.isBlank(vmDetail.getName()) || StringUtils.isBlank(vmDetail.getValue())) { + continue; + } + + if (cpu == null && VmDetailConstants.CPU_NUMBER.equals(vmDetail.getName())) { + cpu = Integer.valueOf(vmDetail.getValue()); + } else if (speed == null && VmDetailConstants.CPU_SPEED.equals(vmDetail.getName())) { + speed = Integer.valueOf(vmDetail.getValue()); + } else if (ramSize == null && VmDetailConstants.MEMORY.equals(vmDetail.getName())) { + ramSize = Integer.valueOf(vmDetail.getValue()); + } + } + + return new Ternary<>(cpu, speed, ramSize); + } + /** * Check hosts tags */ private boolean checkHostTags(List hostTags, List hostInClusterTags, String offeringTag) { - if (CollectionUtils.isEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) { + if ((CollectionUtils.isEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) || StringUtils.isBlank(offeringTag)) { return true; } else if ((CollectionUtils.isNotEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) || (CollectionUtils.isEmpty(hostTags) && CollectionUtils.isNotEmpty(hostInClusterTags))) { diff --git a/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java b/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java index ef0277fd372..d8363964f05 100644 --- a/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java @@ -23,7 +23,15 @@ import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.org.Cluster; +import com.cloud.service.ServiceOfferingVO; +import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.utils.Ternary; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.UserVmDetailVO; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VmDetailConstants; +import com.cloud.vm.dao.UserVmDetailsDao; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -53,6 +61,12 @@ public class RollingMaintenanceManagerImplTest { HostVO host4; @Mock Cluster cluster; + @Mock + VMInstanceVO vm; + @Mock + ServiceOfferingDao serviceOfferingDao; + @Mock + UserVmDetailsDao userVmDetailsDao; @Spy @InjectMocks @@ -164,4 +178,50 @@ public class RollingMaintenanceManagerImplTest { Assert.assertEquals(1, hosts.size()); } + + @Test + public void testGetComputeResourcesCpuSpeedAndRamSize_ForNormalOffering() { + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(serviceOffering.isDynamic()).thenReturn(false); + Mockito.when(serviceOffering.getCpu()).thenReturn(1); + Mockito.when(serviceOffering.getSpeed()).thenReturn(500); + Mockito.when(serviceOffering.getRamSize()).thenReturn(512); + + Mockito.when(vm.getServiceOfferingId()).thenReturn(1L); + Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(serviceOffering); + + Ternary cpuSpeedAndRamSize = manager.getComputeResourcesCpuSpeedAndRamSize(vm); + + Assert.assertEquals(1, cpuSpeedAndRamSize.first().intValue()); + Assert.assertEquals(500, cpuSpeedAndRamSize.second().intValue()); + Assert.assertEquals(512, cpuSpeedAndRamSize.third().intValue()); + } + + @Test + public void testGetComputeResourcesCpuSpeedAndRamSize_ForCustomOffering() { + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(serviceOffering.isDynamic()).thenReturn(true); + Mockito.when(serviceOffering.getCpu()).thenReturn(null); + Mockito.when(serviceOffering.getSpeed()).thenReturn(null); + Mockito.when(serviceOffering.getRamSize()).thenReturn(null); + + List vmDetails = new ArrayList<>(); + UserVmDetailVO cpuDetail = new UserVmDetailVO(1L, VmDetailConstants.CPU_NUMBER, "2", false); + vmDetails.add(cpuDetail); + UserVmDetailVO speedDetail = new UserVmDetailVO(1L, VmDetailConstants.CPU_SPEED, "1000", false); + vmDetails.add(speedDetail); + UserVmDetailVO ramSizeDetail = new UserVmDetailVO(1L, VmDetailConstants.MEMORY, "1024", false); + vmDetails.add(ramSizeDetail); + + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(vm.getServiceOfferingId()).thenReturn(1L); + Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(serviceOffering); + Mockito.when(userVmDetailsDao.listDetails(1L)).thenReturn(vmDetails); + + Ternary cpuSpeedAndRamSize = manager.getComputeResourcesCpuSpeedAndRamSize(vm); + + Assert.assertEquals(2, cpuSpeedAndRamSize.first().intValue()); + Assert.assertEquals(1000, cpuSpeedAndRamSize.second().intValue()); + Assert.assertEquals(1024, cpuSpeedAndRamSize.third().intValue()); + } } From 00c659b7a76d0f93d7467a70b5673e1ec31dc844 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 20 Jan 2025 18:59:27 +0530 Subject: [PATCH 2/5] api: fix access for listSystemVmUsageHistory (#10032) Signed-off-by: Abhishek Kumar --- .../org/apache/cloudstack/api/ListSystemVMsUsageHistoryCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListSystemVMsUsageHistoryCmd.java b/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListSystemVMsUsageHistoryCmd.java index e2d3af24aef..5b279eb8bcb 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListSystemVMsUsageHistoryCmd.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListSystemVMsUsageHistoryCmd.java @@ -26,7 +26,7 @@ import org.apache.cloudstack.response.VmMetricsStatsResponse; @APICommand(name = "listSystemVmsUsageHistory", description = "Lists System VM stats", responseObject = VmMetricsStatsResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.18.0", - authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin}) + authorized = {RoleType.Admin}) public class ListSystemVMsUsageHistoryCmd extends BaseResourceUsageHistoryCmd { ///////////////////////////////////////////////////// From 0b8076c38cf7e5f4236d3035f0294dedd4eb1921 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Tue, 21 Jan 2025 13:58:51 +0530 Subject: [PATCH 3/5] Configure org.eclipse.jetty.server.Request.maxFormKeys from server.properties and increase the default value (#10214) --- client/conf/server.properties.in | 3 +++ .../main/java/org/apache/cloudstack/ServerDaemon.java | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/client/conf/server.properties.in b/client/conf/server.properties.in index 57d81c81217..0a6078048d3 100644 --- a/client/conf/server.properties.in +++ b/client/conf/server.properties.in @@ -32,6 +32,9 @@ session.timeout=30 # Max allowed API request payload/content size in bytes request.content.size=1048576 +# Max allowed API request form keys +request.max.form.keys=5000 + # Options to configure and enable HTTPS on the management server # # For the management server to pick up these configuration settings, the configured diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java index fb84e1297e6..e33a4084e4e 100644 --- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java +++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java @@ -81,6 +81,8 @@ public class ServerDaemon implements Daemon { private static final String ACCESS_LOG = "access.log"; private static final String REQUEST_CONTENT_SIZE_KEY = "request.content.size"; private static final int DEFAULT_REQUEST_CONTENT_SIZE = 1048576; + private static final String REQUEST_MAX_FORM_KEYS_KEY = "request.max.form.keys"; + private static final int DEFAULT_REQUEST_MAX_FORM_KEYS = 5000; //////////////////////////////////////////////////////// /////////////// Server Configuration /////////////////// @@ -93,6 +95,7 @@ public class ServerDaemon implements Daemon { private int httpsPort = 8443; private int sessionTimeout = 30; private int maxFormContentSize = DEFAULT_REQUEST_CONTENT_SIZE; + private int maxFormKeys = DEFAULT_REQUEST_MAX_FORM_KEYS; private boolean httpsEnable = false; private String accessLogFile = "access.log"; private String bindInterface = null; @@ -140,6 +143,7 @@ public class ServerDaemon implements Daemon { setAccessLogFile(properties.getProperty(ACCESS_LOG, "access.log")); setSessionTimeout(Integer.valueOf(properties.getProperty(SESSION_TIMEOUT, "30"))); setMaxFormContentSize(Integer.valueOf(properties.getProperty(REQUEST_CONTENT_SIZE_KEY, String.valueOf(DEFAULT_REQUEST_CONTENT_SIZE)))); + setMaxFormKeys(Integer.valueOf(properties.getProperty(REQUEST_MAX_FORM_KEYS_KEY, String.valueOf(DEFAULT_REQUEST_MAX_FORM_KEYS)))); } catch (final IOException e) { LOG.warn("Failed to read configuration from server.properties file", e); } finally { @@ -191,6 +195,7 @@ public class ServerDaemon implements Daemon { // Extra config options server.setStopAtShutdown(true); server.setAttribute(ContextHandler.MAX_FORM_CONTENT_SIZE_KEY, maxFormContentSize); + server.setAttribute(ContextHandler.MAX_FORM_KEYS_KEY, maxFormKeys); // HTTPS Connector createHttpsConnector(httpConfig); @@ -263,6 +268,7 @@ public class ServerDaemon implements Daemon { webApp.setContextPath(contextPath); webApp.setInitParameter("org.eclipse.jetty.servlet.Default.dirAllowed", "false"); webApp.setMaxFormContentSize(maxFormContentSize); + webApp.setMaxFormKeys(maxFormKeys); // GZIP handler final GzipHandler gzipHandler = new GzipHandler(); @@ -365,4 +371,8 @@ public class ServerDaemon implements Daemon { public void setMaxFormContentSize(int maxFormContentSize) { this.maxFormContentSize = maxFormContentSize; } + + public void setMaxFormKeys(int maxFormKeys) { + this.maxFormKeys = maxFormKeys; + } } From 70776b067a4420e60ded8e0f825fc1b6c88721cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Tue, 21 Jan 2025 06:33:55 -0300 Subject: [PATCH 4/5] fix listing of VMs by network (#10204) --- server/src/main/java/com/cloud/api/query/QueryManagerImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 389a16a5542..42128525782 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -1377,6 +1377,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q if (networkId != null || vpcId != null) { SearchBuilder nicSearch = nicDao.createSearchBuilder(); nicSearch.and("networkId", nicSearch.entity().getNetworkId(), Op.EQ); + nicSearch.and("removed", nicSearch.entity().getRemoved(), Op.NULL); if (vpcId != null) { SearchBuilder networkSearch = networkDao.createSearchBuilder(); networkSearch.and("vpcId", networkSearch.entity().getVpcId(), Op.EQ); From 1ff68cf9b10d9064dbdb10b840000ad161f734bd Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Tue, 21 Jan 2025 11:10:17 +0100 Subject: [PATCH 5/5] linstor: Fix ZFS snapshot backup (#10219) Linstor plugin used the wrong zfs dataset path to hide/unhide the snapshot device. Also don't use the full path to the zfs binary. --- plugins/storage/volume/linstor/CHANGELOG.md | 6 ++++++ .../LinstorBackupSnapshotCommandWrapper.java | 14 ++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/linstor/CHANGELOG.md b/plugins/storage/volume/linstor/CHANGELOG.md index 957377e2978..419a7f983ee 100644 --- a/plugins/storage/volume/linstor/CHANGELOG.md +++ b/plugins/storage/volume/linstor/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to Linstor CloudStack plugin will be documented in this file The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2025-01-20] + +### Fixed + +- Volume snapshots on zfs used the wrong dataset path to hide/unhide snapdev + ## [2024-12-13] ### Fixed diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java index a210d53d7e7..a572759c35a 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java @@ -45,11 +45,17 @@ public final class LinstorBackupSnapshotCommandWrapper { private static final Logger s_logger = Logger.getLogger(LinstorBackupSnapshotCommandWrapper.class); + private static String zfsDatasetName(String zfsFullSnapshotUrl) { + String zfsFullPath = zfsFullSnapshotUrl.substring(6); + int atPos = zfsFullPath.indexOf('@'); + return atPos >= 0 ? zfsFullPath.substring(0, atPos) : zfsFullPath; + } + private String zfsSnapdev(boolean hide, String zfsUrl) { - Script script = new Script("/usr/bin/zfs", Duration.millis(5000)); + Script script = new Script("zfs", Duration.millis(5000)); script.add("set"); script.add("snapdev=" + (hide ? "hidden" : "visible")); - script.add(zfsUrl.substring(6)); // cutting zfs:// + script.add(zfsDatasetName(zfsUrl)); // cutting zfs:// and @snapshotname return script.execute(); } @@ -133,10 +139,10 @@ public final class LinstorBackupSnapshotCommandWrapper s_logger.info("Src: " + srcPath + " | " + src.getName()); if (srcPath.startsWith("zfs://")) { zfsHidden = true; - if (zfsSnapdev(false, srcPath) != null) { + if (zfsSnapdev(false, src.getPath()) != null) { return new CopyCmdAnswer("Unable to unhide zfs snapshot device."); } - srcPath = "/dev/" + srcPath.substring(6); + srcPath = "/dev/zvol/" + srcPath.substring(6); } secondaryPool = storagePoolMgr.getStoragePoolByURI(dstDataStore.getUrl());