Skip to content

Commit

Permalink
PWX-32428,PWX-27672: shared mounts fix for PKS and privileged:false
Browse files Browse the repository at this point in the history 
* fixes shared mounts for PKS (PWX-32428, regression introduced via PWX-31842)
* fixed shared mounts for privileged:false (PWX-27672)

Signed-off-by: Zoran Rajic <zox@portworx.com>
  • Loading branch information
@zoxpx
zoxpx committed Aug 3, 2023
1 parent 48cef8e commit b11fcc8
Show file tree
Hide file tree
Showing 6 changed files with 223 additions and 32 deletions.
95 changes: 74 additions & 21 deletions drivers/storage/portworx/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ const (
SecretKeyKvdbACLToken = "acl-token"
)

var (
pxVer2_3_2, _ = version.NewVersion("2.3.2")
pxVer2_5_5, _ = version.NewVersion("2.5.5")
pxVer2_6, _ = version.NewVersion("2.6")
pxVer2_8, _ = version.NewVersion("2.8")
pxVer2_9_1, _ = version.NewVersion("2.9.1")
pxVer3_0, _ = version.NewVersion("3.0")
pxVer3_0_1, _ = version.NewVersion("3.0.1")
)

type volumeInfo struct {
name string
hostPath string // The path on the host
Expand All @@ -63,8 +73,9 @@ type volumeInfo struct {
}

type pksVolumeInfo struct {
hostPath string
mountPath string
hostPath string
mountPath string
mountPropagation *v1.MountPropagationMode
}

var (
Expand Down Expand Up @@ -290,6 +301,14 @@ func (p *portworx) GetStoragePodSpec(
t.cloudConfig = cloudConfig
}

if _, has := t.cluster.Annotations[pxutil.AnnotationIsPrivileged]; has {
if pxutil.GetPortworxVersion(cluster).LessThan(pxVer3_0_1) {
err = fmt.Errorf("failed to create pod spec: need portworx %s or higher to use annotation '%s'",
pxVer3_0_1, pxutil.AnnotationIsPrivileged)
return v1.PodSpec{}, err
}
}

containers := t.portworxContainer(cluster)
podSpec := v1.PodSpec{
HostPID: pxutil.IsHostPidEnabled(cluster),
Expand Down Expand Up @@ -942,9 +961,8 @@ func (t *template) getArguments() []string {
}

} else if t.cluster.Spec.CloudStorage != nil {
pxVer, _ := version.NewVersion("2.8")
// Cloud provider parameter was added in newer version.
if t.pxVersion.GreaterThanOrEqual(pxVer) {
if t.pxVersion.GreaterThanOrEqual(pxVer2_8) {
cloudProvider := t.getCloudProvider()
if len(cloudProvider) > 0 {
args = append(args, "-cloud_provider", cloudProvider)
Expand Down Expand Up @@ -1016,6 +1034,14 @@ func (t *template) getArguments() []string {
if t.cluster.Annotations[pxutil.AnnotationLogFile] != "" {
args = append(args, "--log", t.cluster.Annotations[pxutil.AnnotationLogFile])
}
// for non-privileged and PKS, add shared mounts via parameters
if t.pxVersion.GreaterThanOrEqual(pxVer3_0_1) &&
(!pxutil.IsPrivileged(t.cluster) || pxutil.IsPKS(t.cluster)) {
args = append(args,
"-v", "/var/lib/osd/pxns:/var/lib/osd/pxns:shared",
"-v", "/var/lib/osd/mounts:/var/lib/osd/mounts:shared",
)
}

rtOpts := make([]string, 0)
for k, v := range t.cluster.Spec.RuntimeOpts {
Expand Down Expand Up @@ -1068,7 +1094,6 @@ func (t *template) getArguments() []string {

marketplaceName := strings.TrimSpace(os.Getenv(pxutil.EnvKeyMarketplaceName))
if marketplaceName != "" {
pxVer2_5_5, _ := version.NewVersion("2.5.5")
if t.pxVersion.GreaterThanOrEqual(pxVer2_5_5) {
args = append(args, "-marketplace_name", marketplaceName)
}
Expand Down Expand Up @@ -1107,7 +1132,6 @@ func (t *template) getEnvList() []v1.EnvVar {
},
}

pxVer2_6, _ := version.NewVersion("2.6")
if t.pxVersion.LessThan(pxVer2_6) {
envMap["AUTO_NODE_RECOVERY_TIMEOUT_IN_SECS"] = &v1.EnvVar{
Name: "AUTO_NODE_RECOVERY_TIMEOUT_IN_SECS",
Expand All @@ -1128,9 +1152,13 @@ func (t *template) getEnvList() []v1.EnvVar {
}

if t.isPKS {
ev := "if [ ! -x /bin/systemctl ]; then apt-get update; apt-get install -y systemd; fi"
if t.pxVersion.GreaterThanOrEqual(pxVer3_0_1) {
ev = "rm -fr /var/lib/osd/driver"
}
envMap["PRE-EXEC"] = &v1.EnvVar{
Name: "PRE-EXEC",
Value: "rm -fr /var/lib/osd/driver",
Value: ev,
}
}

Expand All @@ -1156,7 +1184,6 @@ func (t *template) getEnvList() []v1.EnvVar {
}

if t.cluster.Spec.ImagePullSecret != nil && *t.cluster.Spec.ImagePullSecret != "" {
pxVer2_3_2, _ := version.NewVersion("2.3.2")
if t.pxVersion.LessThan(pxVer2_3_2) {
envMap["REGISTRY_CONFIG"] = &v1.EnvVar{
Name: "REGISTRY_CONFIG",
Expand Down Expand Up @@ -1288,8 +1315,7 @@ func (t *template) getVolumeMounts() []v1.VolumeMount {
if t.cluster.Annotations != nil {
preFltCheck = strings.TrimSpace(strings.ToLower(t.cluster.Annotations[pxutil.AnnotationPreflightCheck]))
}
pxVer30, _ := version.NewVersion("3.0")
if t.pxVersion.GreaterThanOrEqual(pxVer30) && preFltCheck != "true" {
if t.pxVersion.GreaterThanOrEqual(pxVer3_0) && preFltCheck != "true" {
extensions = append(extensions, t.getTelemetryPhoneHomeVolumeInfoList)
}
for _, fn := range extensions {
Expand All @@ -1316,8 +1342,18 @@ func (t *template) mountsFromVolInfo(vols []volumeInfo) []v1.VolumeMount {
v.hostPath, v.mountPath, pxutil.AnnotationIsPrivileged)
volMount.MountPropagation = nil
}
if t.isPKS && v.pks != nil && v.pks.mountPath != "" {
volMount.MountPath = v.pks.mountPath

if t.isPKS && v.pks != nil {
if v.pks.mountPath != "" {
volMount.MountPath = v.pks.mountPath
}
if v.pks.mountPropagation != nil {
if *(v.pks.mountPropagation) == "-" {
volMount.MountPropagation = nil
} else {
volMount.MountPropagation = v.pks.mountPropagation
}
}
}
if volMount.MountPath != "" {
volumeMounts = append(volumeMounts, volMount)
Expand Down Expand Up @@ -1365,8 +1401,7 @@ func (t *template) getVolumes() []v1.Volume {
if t.cluster.Annotations != nil {
preFltCheck = strings.TrimSpace(strings.ToLower(t.cluster.Annotations[pxutil.AnnotationPreflightCheck]))
}
pxVer30, _ := version.NewVersion("3.0")
if t.pxVersion.GreaterThanOrEqual(pxVer30) && preFltCheck != "true" {
if t.pxVersion.GreaterThanOrEqual(pxVer3_0) && preFltCheck != "true" {
extensions = append(extensions, t.getTelemetryPhoneHomeVolumeInfoList)
}

Expand Down Expand Up @@ -1817,17 +1852,35 @@ func getDefaultVolumeInfoList(pxVersion *version.Version) []volumeInfo {
// getCommonVolumeList returns a common list of volumes across all containers
func getCommonVolumeList(pxVersion *version.Version) []volumeInfo {
list := make([]volumeInfo, 0)
pxVer2_9_1, _ := version.NewVersion("2.9.1")
if pxVersion.GreaterThanOrEqual(pxVer2_9_1) {

if pxVersion.GreaterThanOrEqual(pxVer3_0_1) {
list = append(list, volumeInfo{
name: "varlibosd",
hostPath: "/var/lib/osd",
mountPath: "/var/lib/osd",
name: "varlibosd",
hostPath: "/var/lib/osd",
mountPath: "/var/lib/osd",
mountPropagation: mountPropagationModePtr(v1.MountPropagationBidirectional),
pks: &pksVolumeInfo{
hostPath: "/var/vcap/store/lib/osd",
hostPath: "/var/vcap/store/lib/osd",
mountPropagation: mountPropagationModePtr(v1.MountPropagationMode("-")),
},
mountPropagation: mountPropagationModePtr(v1.MountPropagationBidirectional),
})
} else {
list = append(list, volumeInfo{
name: "pxlogs",
pks: &pksVolumeInfo{
mountPath: "/var/lib/osd/log",
hostPath: "/var/vcap/store/lib/osd/log",
},
})

if pxVersion.GreaterThanOrEqual(pxVer2_9_1) {
list = append(list, volumeInfo{
name: "varlibosd",
hostPath: "/var/lib/osd",
mountPath: "/var/lib/osd",
mountPropagation: mountPropagationModePtr(v1.MountPropagationBidirectional),
})
}
}

list = append(list, []volumeInfo{
Expand Down
136 changes: 129 additions & 7 deletions drivers/storage/portworx/deployment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2539,6 +2539,40 @@ func TestPKSPodSpec(t *testing.T) {
assert.NoError(t, err, "Unexpected error on GetStoragePodSpec")

assertPodSpecEqual(t, expected, &actual)

// check PKS when using px-2.9.1

cluster.Spec.Image = "portworx/oci-monitor:2.9.1"
expected_2_9_1 := expected.DeepCopy()
expected_2_9_1.Containers[0].Image = "docker.io/" + cluster.Spec.Image
podSpecAddMount(expected_2_9_1, "varlibosd", "/var/lib/osd:/var/lib/osd:shared")
podSpecRemoveEnv(expected_2_9_1, "AUTO_NODE_RECOVERY_TIMEOUT_IN_SECS")

actual, err = driver.GetStoragePodSpec(cluster, nodeName)
assert.NoError(t, err, "Unexpected error on GetStoragePodSpec")
assertPodSpecEqual(t, expected_2_9_1, &actual)

// check PKS when using px-3.0.1

cluster.Spec.Image = "portworx/oci-monitor:3.0.1"
expected_3_0_1 := expected_2_9_1.DeepCopy()
expected_3_0_1.Containers[0].Image = "docker.io/" + cluster.Spec.Image
expected_3_0_1.Containers[0].Args = append(expected_3_0_1.Containers[0].Args,
"-v", "/var/lib/osd/pxns:/var/lib/osd/pxns:shared",
"-v", "/var/lib/osd/mounts:/var/lib/osd/mounts:shared",
)
podSpecRemoveMount(expected_3_0_1, "varlibosd")
podSpecRemoveMount(expected_3_0_1, "pxlogs")
podSpecAddMount(expected_3_0_1, "varlibosd", "/var/vcap/store/lib/osd:/var/lib/osd")
podSpecRemoveEnv(expected_3_0_1, "PRE-EXEC")
expected_3_0_1.Containers[0].Env = append(expected_3_0_1.Containers[0].Env, v1.EnvVar{
Name: "PRE-EXEC", Value: "rm -fr /var/lib/osd/driver",
})

actual, err = driver.GetStoragePodSpec(cluster, nodeName)
assert.NoError(t, err, "Unexpected error on GetStoragePodSpec")

assertPodSpecEqual(t, expected_3_0_1, &actual)
}

func TestOpenshiftRuncPodSpec(t *testing.T) {
Expand Down Expand Up @@ -2602,16 +2636,39 @@ func TestOpenshiftRuncPodSpec(t *testing.T) {

// Test securityContxt w/ privileged=false
cluster.ObjectMeta.Annotations[pxutil.AnnotationIsPrivileged] = "false"

actual, err = driver.GetStoragePodSpec(cluster, nodeName)
require.Error(t, err)
assert.Contains(t, err.Error(), "need portworx 3.0.1 or higher to use annotation '"+
pxutil.AnnotationIsPrivileged+"'")

cluster.Spec.Image = "portworx/oci-monitor:3.0.1"
expected_3_0_1 := expected.DeepCopy()
expected_3_0_1.Containers[0].Image = "docker.io/" + cluster.Spec.Image
expected_3_0_1.Containers[0].Args = append(expected_3_0_1.Containers[0].Args,
"-v", "/var/lib/osd/pxns:/var/lib/osd/pxns:shared",
"-v", "/var/lib/osd/mounts:/var/lib/osd/mounts:shared",
)
expected_3_0_1.Containers[0].SecurityContext = &v1.SecurityContext{
Privileged: boolPtr(false),
Capabilities: &v1.Capabilities{
Add: []v1.Capability{
"SYS_ADMIN", "SYS_PTRACE", "SYS_RAWIO", "SYS_MODULE", "LINUX_IMMUTABLE",
},
},
}
podSpecAddMount(expected_3_0_1, "varlibosd", "/var/lib/osd:/var/lib/osd")
podSpecRemoveEnv(expected_3_0_1, "AUTO_NODE_RECOVERY_TIMEOUT_IN_SECS")

actual, err = driver.GetStoragePodSpec(cluster, nodeName)
require.NoError(t, err)

pxc := actual.Containers[0]
require.Equal(t, "portworx", pxc.Name)
require.NotNil(t, pxc.SecurityContext.Privileged)
assert.False(t, *pxc.SecurityContext.Privileged)
require.NotNil(t, pxc.SecurityContext.Capabilities)
assert.Equal(t, 5, len(pxc.SecurityContext.Capabilities.Add))
assert.Empty(t, pxc.SecurityContext.Capabilities.Drop)
assertPodSpecEqual(t, expected_3_0_1, &actual)

require.Equal(t, 1, len(actual.Containers))
for _, v := range actual.Containers[0].VolumeMounts {
assert.Nil(t, v.MountPropagation, "Wrong propagation on %v", v)
}
}

func TestPodSpecForK3s(t *testing.T) {
Expand Down Expand Up @@ -4324,3 +4381,68 @@ func assertContainerEqual(t *testing.T, expected, actual v1.Container) {
assert.ElementsMatch(t, expected.Env, actual.Env)
assert.ElementsMatch(t, expected.VolumeMounts, actual.VolumeMounts)
}

func podSpecAddMount(ps *v1.PodSpec, name, mntSpec string) {
if ps == nil || name == "" || mntSpec == "" {
return
}
parts := strings.Split(mntSpec, ":")
src := v1.Volume{
Name: name,
VolumeSource: v1.VolumeSource{
HostPath: &v1.HostPathVolumeSource{
Path: parts[0],
},
},
}
dst := v1.VolumeMount{
Name: name,
MountPath: parts[1],
}
if len(parts) > 2 {
if parts[2] == "shared" || parts[2] == "rshared" {
dst.MountPropagation = mountPropagationModePtr(v1.MountPropagationBidirectional)
}
}
ps.Volumes = append(ps.Volumes, src)
ps.Containers[0].VolumeMounts = append(ps.Containers[0].VolumeMounts, dst)
}

func podSpecRemoveMount(ps *v1.PodSpec, name string) {
if ps == nil || name == "" {
return
}

newVols := make([]v1.Volume, 0, len(ps.Volumes))
for _, v := range ps.Volumes {
if v.Name == name {
continue
}
newVols = append(newVols, v)
}
ps.Volumes = newVols

newVolMmounts := make([]v1.VolumeMount, 0, len(ps.Containers[0].VolumeMounts))
for _, v := range ps.Containers[0].VolumeMounts {
if v.Name == name {
continue
}
newVolMmounts = append(newVolMmounts, v)
}
ps.Containers[0].VolumeMounts = newVolMmounts
}

func podSpecRemoveEnv(ps *v1.PodSpec, name string) {
if ps == nil || name == "" {
return
}

newEnv := make([]v1.EnvVar, 0, len(ps.Containers[0].Env))
for _, v := range ps.Containers[0].Env {
if v.Name == name {
continue
}
newEnv = append(newEnv, v)
}
ps.Containers[0].Env = newEnv
}
7 changes: 6 additions & 1 deletion drivers/storage/portworx/testspec/pks.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ spec:
- name: "PX_TEMPLATE_VERSION"
value: "v4"
- name: "PRE-EXEC"
value: "rm -fr /var/lib/osd/driver"
value: "if [ ! -x /bin/systemctl ]; then apt-get update; apt-get install -y systemd; fi"
livenessProbe:
periodSeconds: 30
initialDelaySeconds: 840 # allow image pull in slow networks
Expand Down Expand Up @@ -85,6 +85,8 @@ spec:
mountPath: /etc/pwx
- name: optpwx
mountPath: /opt/pwx
- name: pxlogs
mountPath: /var/lib/osd/log
- name: procmount
mountPath: /host_proc
- name: sysdmount
Expand Down Expand Up @@ -127,6 +129,9 @@ spec:
- name: optpwx
hostPath:
path: /var/vcap/store/opt/pwx
- name: pxlogs
hostPath:
path: /var/vcap/store/lib/osd/log
- name: procmount
hostPath:
path: /proc
Expand Down
Loading

0 comments on commit b11fcc8

Please sign in to comment.