From 7cb5d2d47ce96858859a774c4789d67e3e1894b0 Mon Sep 17 00:00:00 2001
From: Vicente Cheng <vicente.cheng@suse.com>
Date: Fri, 17 Feb 2023 17:23:06 +0800
Subject: [PATCH 1/2] virt-controller: render hp-volume- pod should respect
 blockdevices

    We found that the VolumeDevices does not generate as the first
    time we render on hp-volume- pod. That would cause the global path
    not to be generated by kubelet again when the hp-volume- is
    dead, then render again.

    We should respect the VolumeDevics on the Spec. That would make
    kubelet behavior is the same as the first time when the hp-volume-
    pod is killed.

Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
---
 pkg/virt-controller/services/template.go | 35 ++++++++++++------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/pkg/virt-controller/services/template.go b/pkg/virt-controller/services/template.go
index 8c2ca44c1..e5ecfc8a1 100644
--- a/pkg/virt-controller/services/template.go
+++ b/pkg/virt-controller/services/template.go
@@ -1615,23 +1615,24 @@ func (t *templateService) RenderHotplugAttachmentPodTemplate(volumes []*v1.Volum
 				},
 			},
 		})
-		if !skipMount {
-			pvc := claimMap[volume.Name]
-			if pvc != nil {
-				if pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == k8sv1.PersistentVolumeBlock {
-					pod.Spec.Containers[0].VolumeDevices = append(pod.Spec.Containers[0].VolumeDevices, k8sv1.VolumeDevice{
-						Name:       volume.Name,
-						DevicePath: fmt.Sprintf("/path/%s/%s", volume.Name, pvc.GetUID()),
-					})
-					pod.Spec.SecurityContext = &k8sv1.PodSecurityContext{
-						RunAsUser: &[]int64{0}[0],
-					}
-				} else {
-					pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, k8sv1.VolumeMount{
-						Name:      volume.Name,
-						MountPath: fmt.Sprintf("/%s", volume.Name),
-					})
-				}
+		pvc := claimMap[volume.Name]
+		if pvc == nil {
+			continue
+		}
+		if types.IsPVCBlock(pvc.Spec.VolumeMode) {
+			pod.Spec.Containers[0].VolumeDevices = append(pod.Spec.Containers[0].VolumeDevices, k8sv1.VolumeDevice{
+				Name:       volume.Name,
+				DevicePath: fmt.Sprintf("/path/%s/%s", volume.Name, pvc.GetUID()),
+			})
+			pod.Spec.SecurityContext = &k8sv1.PodSecurityContext{
+				RunAsUser: &[]int64{0}[0],
+			}
+		} else {
+			if !skipMount {
+				pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, k8sv1.VolumeMount{
+					Name:      volume.Name,
+					MountPath: fmt.Sprintf("/%s", volume.Name),
+				})
 			}
 		}
 	}
-- 
2.40.1


From ca7f25dc72f8921f0ce1aff1fe1afeba954c0f7c Mon Sep 17 00:00:00 2001
From: Vicente Cheng <vicente.cheng@suse.com>
Date: Wed, 10 May 2023 14:35:56 +0800
Subject: [PATCH 2/2] test: add unittest to check the template of hotplug
 volume

    - add unittest for both FS and Block mode hotplug volume

Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
---
 pkg/virt-controller/services/template_test.go | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/pkg/virt-controller/services/template_test.go b/pkg/virt-controller/services/template_test.go
index 4da8f6109..673c7cdb4 100644
--- a/pkg/virt-controller/services/template_test.go
+++ b/pkg/virt-controller/services/template_test.go
@@ -3192,6 +3192,92 @@ var _ = Describe("Template", func() {
 			Expect(false).To(BeTrue())
 		})
 
+		It("should compute the correct volumeDevice context when rendering hotplug attachment pods with the FS PersistentVolumeClaim", func() {
+			vmi := api.NewMinimalVMI("fake-vmi")
+			ownerPod, err := svc.RenderLaunchManifest(vmi)
+			Expect(err).ToNot(HaveOccurred())
+
+			volumeName := "testVolume"
+			pvcName := "pvcDevice"
+			namespace := "testns"
+			mode := kubev1.PersistentVolumeFilesystem
+			pvc := kubev1.PersistentVolumeClaim{
+				TypeMeta:   metav1.TypeMeta{Kind: "PersistentVolumeClaim", APIVersion: "v1"},
+				ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: pvcName},
+				Spec: kubev1.PersistentVolumeClaimSpec{
+					VolumeMode: &mode,
+				},
+			}
+			claimMap := map[string]*kubev1.PersistentVolumeClaim{volumeName: &pvc}
+
+			volumes := []*v1.Volume{}
+			volumes = append(volumes, &v1.Volume{
+				Name: volumeName,
+				VolumeSource: v1.VolumeSource{
+					PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
+						PersistentVolumeClaimVolumeSource: kubev1.PersistentVolumeClaimVolumeSource{
+							ClaimName: pvcName,
+						},
+					},
+				},
+			})
+			pod, err := svc.RenderHotplugAttachmentPodTemplate(volumes, ownerPod, vmi, claimMap, false)
+			prop := kubev1.MountPropagationHostToContainer
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pod.Spec.Containers[0].VolumeMounts).To(HaveLen(2))
+			Expect(pod.Spec.Containers[0].VolumeMounts).To(Equal([]kubev1.VolumeMount{
+				{
+					Name:             "hotplug-disks",
+					MountPath:        "/path",
+					MountPropagation: &prop,
+				},
+				{
+					Name:      volumeName,
+					MountPath: "/" + volumeName,
+				},
+			}))
+		})
+
+		It("should compute the correct volumeDevice context when rendering hotplug attachment pods with the Block PersistentVolumeClaim", func() {
+			vmi := api.NewMinimalVMI("fake-vmi")
+			ownerPod, err := svc.RenderLaunchManifest(vmi)
+			Expect(err).ToNot(HaveOccurred())
+
+			volumeName := "testVolume"
+			pvcName := "pvcDevice"
+			namespace := "testns"
+			mode := kubev1.PersistentVolumeBlock
+			pvc := kubev1.PersistentVolumeClaim{
+				TypeMeta:   metav1.TypeMeta{Kind: "PersistentVolumeClaim", APIVersion: "v1"},
+				ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: pvcName},
+				Spec: kubev1.PersistentVolumeClaimSpec{
+					VolumeMode: &mode,
+				},
+			}
+			claimMap := map[string]*kubev1.PersistentVolumeClaim{volumeName: &pvc}
+
+			volumes := []*v1.Volume{}
+			volumes = append(volumes, &v1.Volume{
+				Name: volumeName,
+				VolumeSource: v1.VolumeSource{
+					PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
+						PersistentVolumeClaimVolumeSource: kubev1.PersistentVolumeClaimVolumeSource{
+							ClaimName: pvcName,
+						},
+					},
+				},
+			})
+			pod, err := svc.RenderHotplugAttachmentPodTemplate(volumes, ownerPod, vmi, claimMap, false)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pod.Spec.Containers[0].VolumeDevices).ToNot(BeNil())
+			Expect(pod.Spec.Containers[0].VolumeDevices).To(Equal([]kubev1.VolumeDevice{
+				{
+					Name:       volumeName,
+					DevicePath: "/path/" + volumeName + "/",
+				},
+			}))
+		})
+
 		It("Should run as non-root except compute", func() {
 			vmi := newMinimalWithContainerDisk("ranom")
 
-- 
2.40.1

