PWX-34256_op-23.10.2: node-wiper fix for custom-dir installs (#1388) (#…

…1389) Signed-off-by: Zoran Rajic <zox@portworx.com> Manually fixed Conflicts: drivers/storage/portworx/util/util.go
libopenstorage · Jan 16, 2024 · 27e7cd5 · 27e7cd5
1 parent 98618a4
commit 27e7cd5
Show file tree

Hide file tree

Showing 3 changed files with 155 additions and 0 deletions.
diff --git a/drivers/storage/portworx/uninstall.go b/drivers/storage/portworx/uninstall.go
@@ -407,6 +407,7 @@ func (u *uninstallPortworx) RunNodeWiper(
 	if strings.Contains(wiperImage, "monitor") {
 		logrus.Warnf("Using oci-monitor %s as node-wiper image", wiperImage)
 		ds.Spec.Template.Spec.Containers[0].Command = []string{"/px-node-wiper"}
+		pxutil.AppendUserVolumeMounts(&ds.Spec.Template.Spec, u.cluster.Spec.Volumes)
 	}
 
 	if u.cluster.Spec.ImagePullSecret != nil && *u.cluster.Spec.ImagePullSecret != "" {

diff --git a/drivers/storage/portworx/util/util.go b/drivers/storage/portworx/util/util.go
@@ -1262,3 +1262,45 @@ func GetTLSCipherSuites(cluster *corev1.StorageCluster) (string, error) {
 	}
 	return strings.Join(outList, ","), nil
 }
+
+// AppendUserVolumeMounts appends "user" vol specs to the pod spec
+//   - note, the user volume specs will override container mounts, if the mount
+//     destination directory is the same
+//   - caveat: caller needs to ensure that the volume specs NAMES are unique
+func AppendUserVolumeMounts(
+	podSpec *v1.PodSpec,
+	userVolSpecList []corev1.VolumeSpec,
+) {
+	if podSpec == nil {
+		return
+	} else if len(userVolSpecList) == 0 {
+		return
+	}
+
+	// make map of user-volumes, also append vols to pod spec
+	usrSpecMap := make(map[string]corev1.VolumeSpec)
+	for _, v := range userVolSpecList {
+		usrSpecMap[v.MountPath] = v
+		podSpec.Volumes = append(podSpec.Volumes, v1.Volume{
+			Name:         UserVolumeName(v.Name),
+			VolumeSource: v.VolumeSource,
+		})
+	}
+
+	// update container volumes, when destination-dir matches
+	for idx1, cntr := range podSpec.Containers {
+		for idx2, cv := range cntr.VolumeMounts {
+			if uv, has := usrSpecMap[cv.MountPath]; has {
+				logrus.Debugf("Replacing container %s:%s mount '%s' with user-mount '%s'",
+					cntr.Name, cv.MountPath, cv.Name, uv.Name)
+
+				podSpec.Containers[idx1].VolumeMounts[idx2] = v1.VolumeMount{
+					Name:             UserVolumeName(uv.Name),
+					MountPath:        uv.MountPath,
+					ReadOnly:         uv.ReadOnly,
+					MountPropagation: uv.MountPropagation,
+				}
+			}
+		}
+	}
+}
diff --git a/drivers/storage/portworx/util/util_test.go b/drivers/storage/portworx/util/util_test.go
@@ -332,3 +332,115 @@ func TestGetTLSCipherSuites(t *testing.T) {
 		}
 	}
 }
+
+func TestAppendUserVolumeMounts(t *testing.T) {
+	podSpecReference := &v1.PodSpec{
+		Containers: []v1.Container{
+			{
+				Name: "px",
+				VolumeMounts: []v1.VolumeMount{
+					{
+						Name:      "install-vol",
+						MountPath: "/opt/pwx",
+					},
+					{
+						Name:      "creds-vol",
+						MountPath: "/var/lib/serviceaccount",
+					},
+					{
+						Name:      "osd-vol",
+						MountPath: "/var/lib/osd",
+					},
+				},
+			},
+		},
+		Volumes: []v1.Volume{
+			{
+				Name: "install-vol",
+				VolumeSource: v1.VolumeSource{
+					HostPath: &v1.HostPathVolumeSource{
+						Path: "/opt/pwx",
+					},
+				},
+			},
+			{
+				Name: "creds-vol",
+				VolumeSource: v1.VolumeSource{
+					HostPath: &v1.HostPathVolumeSource{
+						Path: "/var/lib/kubelet/pod123/serviceaccount",
+					},
+				},
+			},
+			{
+				Name: "osd-vol",
+				VolumeSource: v1.VolumeSource{
+					HostPath: &v1.HostPathVolumeSource{
+						Path: "/var/lib/osd",
+					},
+				},
+			},
+		},
+	}
+
+	// no user volume mounts
+	podSpec := podSpecReference.DeepCopy()
+	AppendUserVolumeMounts(podSpec, []corev1.VolumeSpec{})
+	assert.Equal(t, podSpecReference, podSpec)
+
+	// non-overlapping volume mounts
+	userMounts := []corev1.VolumeSpec{
+		{
+			Name:      "my-vol1",
+			MountPath: "/mnt/user-vol1",
+			VolumeSource: v1.VolumeSource{
+				HostPath: &v1.HostPathVolumeSource{
+					Path: "/mnt/user-vol1",
+				},
+			},
+		},
+		{
+			Name:      "my-vol2",
+			MountPath: "/mnt/my-vol2",
+			VolumeSource: v1.VolumeSource{
+				HostPath: &v1.HostPathVolumeSource{
+					Path: "/mnt/my-vol2",
+				},
+			},
+		},
+	}
+	AppendUserVolumeMounts(podSpec, userMounts)
+	assert.Equal(t, podSpecReference.Containers, podSpec.Containers)
+	assert.Equal(t, len(podSpecReference.Volumes)+2, len(podSpec.Volumes))
+
+	// add destination-overlapping volume mounts
+	userMounts = []corev1.VolumeSpec{
+		{
+			Name:      "custom-install",
+			MountPath: "/opt/pwx",
+			VolumeSource: v1.VolumeSource{
+				HostPath: &v1.HostPathVolumeSource{
+					Path: "/usr/local/portworx",
+				},
+			},
+		},
+		{
+			Name:      "custom-osd",
+			MountPath: "/var/lib/osd",
+			VolumeSource: v1.VolumeSource{
+				HostPath: &v1.HostPathVolumeSource{
+					Path: "/var/lib/vcap/store/osd",
+				},
+			},
+		},
+	}
+	podSpec = podSpecReference.DeepCopy()
+	AppendUserVolumeMounts(podSpec, userMounts)
+	assert.NotEqual(t, podSpecReference.Containers, podSpec.Containers)
+	assert.Equal(t, len(podSpecReference.Volumes)+2, len(podSpec.Volumes))
+	assert.Equal(t, "user-custom-install", podSpec.Containers[0].VolumeMounts[0].Name)
+	assert.Equal(t, "creds-vol", podSpec.Containers[0].VolumeMounts[1].Name)
+	assert.Equal(t, "user-custom-osd", podSpec.Containers[0].VolumeMounts[2].Name)
+	assert.Equal(t, podSpecReference.Containers[0].VolumeMounts[0].MountPath, podSpec.Containers[0].VolumeMounts[0].MountPath)
+	assert.Equal(t, podSpecReference.Containers[0].VolumeMounts[1].MountPath, podSpec.Containers[0].VolumeMounts[1].MountPath)
+	assert.Equal(t, podSpecReference.Containers[0].VolumeMounts[2].MountPath, podSpec.Containers[0].VolumeMounts[2].MountPath)
+}