Merge branch '4.22' into add-performance-indexes

Author: DaanHoogland

api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java

@@ -115,7 +115,7 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd {
     @Parameter(name = ApiConstants.TEMPLATE_ID,
             type = CommandType.UUID,
             entityType = TemplateResponse.class,
-            description = "The ID of the Template for the Instance")
+            description = "The ID of the Template for the Instance. Required when the instance to import is an unmanaged KVM Instance.")
     private Long templateId;
 
     @Parameter(name = ApiConstants.SERVICE_OFFERING_ID,

engine/schema/src/main/resources/META-INF/db/schema-42210to42220-cleanup.sql

@@ -18,4 +18,3 @@
 --;
 -- Schema upgrade cleanup from 4.22.1.0 to 4.22.2.0
 --;
-

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

@@ -19,6 +19,7 @@
 
 package com.cloud.hypervisor.kvm.resource.wrapper;
 
+import org.apache.commons.lang.StringEscapeUtils;
 import org.libvirt.Connect;
 import org.libvirt.Domain;
 import org.libvirt.DomainInfo.DomainState;
@@ -57,7 +58,7 @@ public Answer execute(final CreateVMSnapshotCommand cmd, final LibvirtComputingR
                         "Create Instance Snapshot Failed due to Instance is not running: " + vmName + " with domainState = " + domainState);
             }
 
-            String vmSnapshotXML = "<domainsnapshot>" + "  <name>" + vmSnapshotName + "</name>"
+            String vmSnapshotXML = "<domainsnapshot>" + "  <name>" + StringEscapeUtils.escapeXml(vmSnapshotName) + "</name>"
                     + "  <memory snapshot='internal' />" + "</domainsnapshot>";
 
             dm.snapshotCreateXML(vmSnapshotXML);

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

@@ -22,6 +22,7 @@
 import java.io.File;
 import java.text.MessageFormat;
 
+import org.apache.commons.lang.StringEscapeUtils;
 import org.libvirt.Connect;
 import org.libvirt.Domain;
 import org.libvirt.DomainInfo.DomainState;
@@ -79,7 +80,7 @@ public Answer execute(final ManageSnapshotCommand command, final LibvirtComputin
                         + "            <uuid>{1}</uuid>" + "        </domain>" + "    </domainsnapshot>");
 
                 final String vmUuid = vm.getUUIDString();
-                final Object[] args = new Object[] {snapshotName, vmUuid};
+                final Object[] args = new Object[] {StringEscapeUtils.escapeXml(snapshotName), vmUuid};
                 final String snapshot = snapshotXML.format(args);
                 logger.debug(snapshot);
                 if (command.getCommandSwitch().equalsIgnoreCase(ManageSnapshotCommand.CREATE_SNAPSHOT)) {

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

@@ -26,6 +26,7 @@
 import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
 import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
 import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
 import org.libvirt.Connect;
@@ -112,9 +113,9 @@ public Script buildScript(final String scriptPath) {
     }
 
     public String generateVMSnapshotXML(VMSnapshotTO snapshot, VMSnapshotTO parent, String domainXmlDesc) {
-        String parentName = (parent == null)? "": ("  <parent><name>" + parent.getSnapshotName() + "</name></parent>\n");
+        String parentName = (parent == null)? "": ("  <parent><name>" + StringEscapeUtils.escapeXml(parent.getSnapshotName()) + "</name></parent>\n");
         String vmSnapshotXML = "<domainsnapshot>\n"
-                + "  <name>" + snapshot.getSnapshotName() + "</name>\n"
+                + "  <name>" + StringEscapeUtils.escapeXml(snapshot.getSnapshotName()) + "</name>\n"
                 + "  <state>running</state>\n"
                 + parentName
                 + "  <creationTime>" + (int) Math.rint(snapshot.getCreateTime()/1000) + "</creationTime>\n"

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUtilitiesHelperTest.java

@@ -25,6 +25,7 @@
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
+import com.cloud.agent.api.VMSnapshotTO;
 import com.cloud.utils.Pair;
 
 import junit.framework.TestCase;
@@ -75,4 +76,30 @@ public void validateIsLibvirtVersionEqualOrHigherThanVersionInParameterLibvirtVe
         Assert.assertEquals(String.valueOf(libvirtVersion), result.first());
         Assert.assertTrue(result.second());
     }
+
+    @Test
+    public void generateVMSnapshotXMLEscapesSnapshotName() {
+        VMSnapshotTO snapshot = Mockito.mock(VMSnapshotTO.class);
+        Mockito.doReturn("i-2-3-VM_VS_</name><disks><disk name='vda'/></disks><name>x").when(snapshot).getSnapshotName();
+        Mockito.doReturn(1000L).when(snapshot).getCreateTime();
+
+        String xml = libvirtUtilitiesHelperSpy.generateVMSnapshotXML(snapshot, null, "<domain/>");
+
+        Assert.assertTrue(xml.contains("<name>i-2-3-VM_VS_&lt;/name&gt;&lt;disks&gt;&lt;disk name=&apos;vda&apos;/&gt;&lt;/disks&gt;&lt;name&gt;x</name>"));
+        Assert.assertFalse(xml.contains("<disks>"));
+    }
+
+    @Test
+    public void generateVMSnapshotXMLEscapesParentSnapshotName() {
+        VMSnapshotTO snapshot = Mockito.mock(VMSnapshotTO.class);
+        Mockito.doReturn("child").when(snapshot).getSnapshotName();
+        Mockito.doReturn(1000L).when(snapshot).getCreateTime();
+        VMSnapshotTO parent = Mockito.mock(VMSnapshotTO.class);
+        Mockito.doReturn("parent</name><disks/>").when(parent).getSnapshotName();
+
+        String xml = libvirtUtilitiesHelperSpy.generateVMSnapshotXML(snapshot, parent, "<domain/>");
+
+        Assert.assertTrue(xml.contains("<parent><name>parent&lt;/name&gt;&lt;disks/&gt;</name></parent>"));
+        Assert.assertFalse(xml.contains("<disks/>"));
+    }
 }

plugins/integrations/kubernetes-service/src/main/resources/script/delete-pv-reclaimpolicy-delete

@@ -33,23 +33,33 @@ delete_workloads_using_pvc() {
     local deleted_count=0
 
     # Find & delete any deployment using the PVC
-    /opt/bin/kubectl get deployments -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
-    while IFS= read -r deployment; do
-        if [ -n "$deployment" ]; then
-            deployment_name=$(echo "$deployment" | cut -d'/' -f2)
-            echo "$(timestamp) -   Deleting Deployment: $deployment_name"
-            /opt/bin/kubectl delete deployment "$deployment_name" -n "$namespace" --ignore-not-found=true
+    # Iterate resource names and check the volumes via jsonpath to avoid grep -l on JSON (which prints "(standard input)")
+    /opt/bin/kubectl get deployments -n "$namespace" -o name 2>/dev/null | \
+    while IFS= read -r resource; do
+        if [ -z "$resource" ]; then
+            continue
+        fi
+        name=${resource#*/}
+        dep_volumes=$(/opt/bin/kubectl get deployment "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+        if [ -n "$dep_volumes" ] && echo "$dep_volumes" | grep -F -q "$pvc_name"; then
+            echo "$(timestamp) -   Deleting Deployment: $name"
+            /opt/bin/kubectl delete deployment "$name" -n "$namespace" --ignore-not-found=true
             deleted_count=$((deleted_count + 1))
         fi
     done
 
     # Find and delete any StatefulSet using the PVC
-    /opt/bin/kubectl get statefulsets -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
-    while IFS= read -r sts; do
-        if [ -n "$sts" ]; then
-            sts_name=$(echo "$sts" | cut -d'/' -f2)
-            echo "$(timestamp) -   Deleting StatefulSet: $sts_name"
-            /opt/bin/kubectl delete statefulset "$sts_name" -n "$namespace" --ignore-not-found=true
+    /opt/bin/kubectl get statefulsets -n "$namespace" -o name 2>/dev/null | \
+    while IFS= read -r resource; do
+        if [ -z "$resource" ]; then
+            continue
+        fi
+        name=${resource#*/}
+        # Check both template volumes and volumeClaimTemplates
+        sts_volumes=$(/opt/bin/kubectl get statefulset "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName} {.spec.volumeClaimTemplates[*].metadata.name}' 2>/dev/null || echo "")
+        if [ -n "$sts_volumes" ] && echo "$sts_volumes" | grep -F -q "$pvc_name"; then
+            echo "$(timestamp) -   Deleting StatefulSet: $name"
+            /opt/bin/kubectl delete statefulset "$name" -n "$namespace" --ignore-not-found=true
             deleted_count=$((deleted_count + 1))
         fi
     done
@@ -71,34 +81,46 @@ delete_workloads_using_pvc() {
     done
 
     # Find and delete any DaemonSet using the PVC
-    /opt/bin/kubectl get daemonsets -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
-    while IFS= read -r ds; do
-        if [ -n "$ds" ]; then
-            ds_name=$(echo "$ds" | cut -d'/' -f2)
-            echo "$(timestamp) -   Deleting DaemonSet: $ds_name"
-            /opt/bin/kubectl delete daemonset "$ds_name" -n "$namespace" --ignore-not-found=true
+    /opt/bin/kubectl get daemonsets -n "$namespace" -o name 2>/dev/null | \
+    while IFS= read -r resource; do
+        if [ -z "$resource" ]; then
+            continue
+        fi
+        name=${resource#*/}
+        ds_volumes=$(/opt/bin/kubectl get daemonset "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+        if [ -n "$ds_volumes" ] && echo "$ds_volumes" | grep -F -q "$pvc_name"; then
+            echo "$(timestamp) -   Deleting DaemonSet: $name"
+            /opt/bin/kubectl delete daemonset "$name" -n "$namespace" --ignore-not-found=true
             deleted_count=$((deleted_count + 1))
         fi
     done
 
     # Find and delete any Job using the PVC
-    /opt/bin/kubectl get jobs -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
-    while IFS= read -r job; do
-        if [ -n "$job" ]; then
-            job_name=$(echo "$job" | cut -d'/' -f2)
-            echo "$(timestamp) -   Deleting Job: $job_name"
-            /opt/bin/kubectl delete job "$job_name" -n "$namespace" --ignore-not-found=true
+    /opt/bin/kubectl get jobs -n "$namespace" -o name 2>/dev/null | \
+    while IFS= read -r resource; do
+        if [ -z "$resource" ]; then
+            continue
+        fi
+        name=${resource#*/}
+        job_volumes=$(/opt/bin/kubectl get job "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+        if [ -n "$job_volumes" ] && echo "$job_volumes" | grep -F -q "$pvc_name"; then
+            echo "$(timestamp) -   Deleting Job: $name"
+            /opt/bin/kubectl delete job "$name" -n "$namespace" --ignore-not-found=true
             deleted_count=$((deleted_count + 1))
         fi
     done
 
     # Find and delete any CronJobs using the PVC
-    /opt/bin/kubectl get cronjobs -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
-    while IFS= read -r cronjob; do
-        if [ -n "$cronjob" ]; then
-            cronjob_name=$(echo "$cronjob" | cut -d'/' -f2)
-            echo "$(timestamp) -   Deleting CronJob: $cronjob_name"
-            /opt/bin/kubectl delete cronjob "$cronjob_name" -n "$namespace" --ignore-not-found=true
+    /opt/bin/kubectl get cronjobs -n "$namespace" -o name 2>/dev/null | \
+    while IFS= read -r resource; do
+        if [ -z "$resource" ]; then
+            continue
+        fi
+        name=${resource#*/}
+        cron_volumes=$(/opt/bin/kubectl get cronjob "$name" -n "$namespace" -o jsonpath='{.spec.jobTemplate.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+        if [ -n "$cron_volumes" ] && echo "$cron_volumes" | grep -F -q "$pvc_name"; then
+            echo "$(timestamp) -   Deleting CronJob: $name"
+            /opt/bin/kubectl delete cronjob "$name" -n "$namespace" --ignore-not-found=true
             deleted_count=$((deleted_count + 1))
         fi
     done

plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java

@@ -204,8 +204,10 @@ public static String getSnapshotPath(com.linbit.linstor.api.model.StoragePool sp
         final String path;
         switch (sp.getProviderKind()) {
             case LVM_THIN:
+                // device-mapper doubles every dash in the VG and LV name, so the VG part needs escaping too
+                final String vgName = backingPool.split("/")[0];
                 path = String.format("/dev/mapper/%s-%s_%s_%s",
-                    backingPool.split("/")[0], rscName.replace("-", "--"), suffix, snapshotName.replace("-", "--"));
+                    vgName.replace("-", "--"), rscName.replace("-", "--"), suffix, snapshotName.replace("-", "--"));
                 break;
             case ZFS:
             case ZFS_THIN:

plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java

@@ -103,6 +103,20 @@ public void testGetSnapshotPath() {
             Assert.assertEquals("/dev/mapper/storage-cs--cb32532a--dd8f--47e0--a81c--8a75573d3545_00000_snap3", snapPath);
         }
 
+        {
+            // dashes in the volume group name must be escaped as well (GH issue #14011)
+            StoragePool spLVMThin = new StoragePool();
+            Properties lvmThinProps = new Properties();
+            lvmThinProps.put("StorDriver/StorPoolName", "linstor_pool-lvm-thin/thin");
+            spLVMThin.setProps(lvmThinProps);
+            spLVMThin.setProviderKind(ProviderKind.LVM_THIN);
+            String snapPath = LinstorUtil.getSnapshotPath(spLVMThin,
+                "cs-12fc4055-3985-4025-8eb5-d6fd53effe37", "cs-d7aea646-5f40-46a2-b9dc-77e41ea29336");
+            Assert.assertEquals(
+                "/dev/mapper/linstor_pool--lvm--thin-cs--12fc4055--3985--4025--8eb5--d6fd53effe37_00000_cs--d7aea646--5f40--46a2--b9dc--77e41ea29336",
+                snapPath);
+        }
+
         {
             StoragePool spZFS = new StoragePool();
             Properties zfsProps = new Properties();