Skip to content

Commit

Permalink
Merge pull request #657 from openshift-cherrypick-robot/cherry-pick-6…
Browse files Browse the repository at this point in the history 
…54-to-release-4.9

Bug 2004568: lib/resourcemerge/core: Remove unrecognized volumes and mounts
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Sep 16, 2021
2 parents 513a2fc + 16f51af commit 43d63b8
Show file tree
Hide file tree
Showing 2 changed files with 223 additions and 35 deletions.
97 changes: 62 additions & 35 deletions lib/resourcemerge/core.go
View file
Expand Up @@ -36,23 +36,7 @@ func ensurePodTemplateSpec(modified *bool, existing *corev1.PodTemplateSpec, req
func ensurePodSpec(modified *bool, existing *corev1.PodSpec, required corev1.PodSpec) {
ensureContainers(modified, &existing.InitContainers, required.InitContainers, required.HostNetwork)
ensureContainers(modified, &existing.Containers, required.Containers, required.HostNetwork)

// any volume we specify, we require.
for _, required := range required.Volumes {
var existingCurr *corev1.Volume
for j, curr := range existing.Volumes {
if curr.Name == required.Name {
existingCurr = &existing.Volumes[j]
break
}
}
if existingCurr == nil {
*modified = true
existing.Volumes = append(existing.Volumes, corev1.Volume{})
existingCurr = &existing.Volumes[len(existing.Volumes)-1]
}
ensureVolume(modified, existingCurr, required)
}
ensureVolumes(modified, &existing.Volumes, required.Volumes)

if len(required.RestartPolicy) > 0 {
if existing.RestartPolicy != required.RestartPolicy {
Expand Down Expand Up @@ -118,24 +102,7 @@ func ensureContainer(modified *bool, existing *corev1.Container, required corev1
setStringIfSet(modified, &existing.WorkingDir, required.WorkingDir)
ensureResourceRequirements(modified, &existing.Resources, required.Resources)
ensureContainerPorts(modified, &existing.Ports, required.Ports, hostNetwork)

// any volume mount we specify, we require
for _, required := range required.VolumeMounts {
var existingCurr *corev1.VolumeMount
for j, curr := range existing.VolumeMounts {
if curr.Name == required.Name {
existingCurr = &existing.VolumeMounts[j]
break
}
}
if existingCurr == nil {
*modified = true
existing.VolumeMounts = append(existing.VolumeMounts, corev1.VolumeMount{})
existingCurr = &existing.VolumeMounts[len(existing.VolumeMounts)-1]
}
ensureVolumeMount(modified, existingCurr, required)
}

ensureVolumeMounts(modified, &existing.VolumeMounts, required.VolumeMounts)
ensureProbePtr(modified, &existing.LivenessProbe, required.LivenessProbe)
ensureProbePtr(modified, &existing.ReadinessProbe, required.ReadinessProbe)

Expand Down Expand Up @@ -347,13 +314,73 @@ func ensureServicePortDefaults(servicePort *corev1.ServicePort) {
}
}

func ensureVolumeMounts(modified *bool, existing *[]corev1.VolumeMount, required []corev1.VolumeMount) {
// any volume mount we specify, we require
exists := struct{}{}
requiredNames := make(map[string]struct{}, len(required))
for _, requiredVolumeMount := range required {
requiredNames[requiredVolumeMount.Name] = exists
var existingCurr *corev1.VolumeMount
for j, curr := range *existing {
if curr.Name == requiredVolumeMount.Name {
existingCurr = &(*existing)[j]
break
}
}
if existingCurr == nil {
*modified = true
*existing = append(*existing, corev1.VolumeMount{})
existingCurr = &(*existing)[len(*existing)-1]
}
ensureVolumeMount(modified, existingCurr, requiredVolumeMount)
}

// any unrecognized volume mount, we remove
for eidx := len(*existing) - 1; eidx >= 0; eidx-- {
if _, ok := requiredNames[(*existing)[eidx].Name]; !ok {
*modified = true
*existing = append((*existing)[:eidx], (*existing)[eidx+1:]...)
}
}
}

func ensureVolumeMount(modified *bool, existing *corev1.VolumeMount, required corev1.VolumeMount) {
if !equality.Semantic.DeepEqual(required, *existing) {
*modified = true
*existing = required
}
}

func ensureVolumes(modified *bool, existing *[]corev1.Volume, required []corev1.Volume) {
// any volume we specify, we require.
exists := struct{}{}
requiredNames := make(map[string]struct{}, len(required))
for _, requiredVolume := range required {
requiredNames[requiredVolume.Name] = exists
var existingCurr *corev1.Volume
for j, curr := range *existing {
if curr.Name == requiredVolume.Name {
existingCurr = &(*existing)[j]
break
}
}
if existingCurr == nil {
*modified = true
*existing = append(*existing, corev1.Volume{})
existingCurr = &(*existing)[len(*existing)-1]
}
ensureVolume(modified, existingCurr, requiredVolume)
}

// any unrecognized volume mount, we remove
for eidx := len(*existing) - 1; eidx >= 0; eidx-- {
if _, ok := requiredNames[(*existing)[eidx].Name]; !ok {
*modified = true
*existing = append((*existing)[:eidx], (*existing)[eidx+1:]...)
}
}
}

func ensureVolume(modified *bool, existing *corev1.Volume, required corev1.Volume) {
if pointer.AllPtrFieldsNil(&required.VolumeSource) {
required.VolumeSource = corev1.VolumeSource{
Expand Down
161 changes: 161 additions & 0 deletions lib/resourcemerge/core_test.go
View file
Expand Up @@ -451,6 +451,167 @@ func TestEnsurePodSpec(t *testing.T) {
},
},
},
{
name: "add volumes on container",
existing: corev1.PodSpec{
Containers: []corev1.Container{
{Name: "test"},
},
},
input: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "test",
VolumeMounts: []corev1.VolumeMount{
{
Name: "test-volume",
MountPath: "/mnt",
},
},
},
},
Volumes: []corev1.Volume{
{
Name: "test-volume",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},
},
},
expectedModified: true,
expected: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "test",
VolumeMounts: []corev1.VolumeMount{
{
Name: "test-volume",
MountPath: "/mnt",
},
},
},
},
Volumes: []corev1.Volume{
{
Name: "test-volume",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},
},
},
},
{
name: "modify volumes on container",
existing: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "test",
VolumeMounts: []corev1.VolumeMount{
{
Name: "test-volume",
MountPath: "/mnt/a",
},
},
},
},
Volumes: []corev1.Volume{
{
Name: "test-volume",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},
},
},
input: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "test",
VolumeMounts: []corev1.VolumeMount{
{
Name: "test-volume",
MountPath: "/mnt/b",
},
},
},
},
Volumes: []corev1.Volume{
{
Name: "test-volume",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "test-config-map",
},
},
},
},
},
},
expectedModified: true,
expected: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "test",
VolumeMounts: []corev1.VolumeMount{
{
Name: "test-volume",
MountPath: "/mnt/b",
},
},
},
},
Volumes: []corev1.Volume{
{
Name: "test-volume",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "test-config-map",
},
},
},
},
},
},
},
{
name: "remove container volumes",
existing: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "test",
VolumeMounts: []corev1.VolumeMount{
{
Name: "test-volume",
MountPath: "/mnt",
},
},
},
},
Volumes: []corev1.Volume{
{
Name: "test-volume",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},
},
},
input: corev1.PodSpec{
Containers: []corev1.Container{
{Name: "test"},
},
},
expectedModified: true,
expected: corev1.PodSpec{
Containers: []corev1.Container{
{Name: "test"},
},
},
},
}

for _, test := range tests {
Expand Down

0 comments on commit 43d63b8

Please sign in to comment.