Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.99.x-prod][SRVLOGIC-221] Pod instances keep spawning and terminating when deploying the workflow #16

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controllers/profiles/dev/object_creators_dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func mountDevConfigMapsMutateVisitor(workflow *operatorapi.SonataFlow, flowDefCM
}

if len(deployment.Spec.Template.Spec.Volumes) == 0 {
deployment.Spec.Template.Spec.Volumes = make([]corev1.Volume, 0, len(resourceVolumes)+2)
deployment.Spec.Template.Spec.Volumes = make([]corev1.Volume, 0, len(resourceVolumes)+1)
}
kubeutil.AddOrReplaceVolume(&deployment.Spec.Template.Spec, defaultResourcesVolume)
kubeutil.AddOrReplaceVolume(&deployment.Spec.Template.Spec, resourceVolumes...)
Expand Down
53 changes: 34 additions & 19 deletions utils/kubernetes/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,38 +101,53 @@ func VolumeMountAdd(volumeMount []corev1.VolumeMount, name, mountPath string) []
// AddOrReplaceVolume adds or removes the given volumes to the PodSpec.
// If there's already a volume with the same name, it's replaced.
func AddOrReplaceVolume(podSpec *corev1.PodSpec, volumes ...corev1.Volume) {
// indexing the volumes to add to the podSpec we avoid O(n) operation when searching for a volume to replace
volumesToAdd := make(map[string]corev1.Volume, len(volumes))
// volumes iterated here are read/constructed by the caller following the order defined in the original CRD, and that
// order must be preserved. If not preserved, in the reconciliation cycles an order change in the volumes might be
// interpreted as configuration change in the original resource, causing undesired side effects like creating
// a new ReplicaSet for a deployment with the subsequent pods spawning reported here.
volumesToAdd := make([]corev1.Volume, 0)
wasAdded := false
for _, volume := range volumes {
volumesToAdd[volume.Name] = volume
}
// replace
for i := range podSpec.Volumes {
if vol, ok := volumesToAdd[podSpec.Volumes[i].Name]; ok {
podSpec.Volumes[i] = vol
delete(volumesToAdd, vol.Name)
wasAdded = false
for i := 0; !wasAdded && i < len(podSpec.Volumes); i++ {
if volume.Name == podSpec.Volumes[i].Name {
// replace existing
podSpec.Volumes[i] = volume
wasAdded = true
}
}
if !wasAdded {
// remember to add it later in order
volumesToAdd = append(volumesToAdd, volume)
}
}
// append what's left
for _, volume := range volumesToAdd {
podSpec.Volumes = append(podSpec.Volumes, volume)
}
}

// AddOrReplaceVolumeMount same as AddOrReplaceVolume, but with VolumeMounts in a specific container
func AddOrReplaceVolumeMount(containerIndex int, podSpec *corev1.PodSpec, mounts ...corev1.VolumeMount) {
mountsToAdd := make(map[string]corev1.VolumeMount, len(mounts))
// analogous to AddOrReplaceVolume function, the processing must be realized en order.
// see: AddOrReplaceVolume
mountsToAdd := make([]corev1.VolumeMount, 0)
wasAdded := false
container := &podSpec.Containers[containerIndex]
for _, mount := range mounts {
mountsToAdd[mount.Name] = mount
}
for i := range podSpec.Containers[containerIndex].VolumeMounts {
if mount, ok := mountsToAdd[podSpec.Containers[containerIndex].VolumeMounts[i].Name]; ok {
podSpec.Containers[containerIndex].VolumeMounts[i] = mount
delete(mountsToAdd, mount.Name)
wasAdded = false
for i := 0; !wasAdded && i < len(container.VolumeMounts); i++ {
if mount.Name == container.VolumeMounts[i].Name {
// replace existing
container.VolumeMounts[i] = mount
wasAdded = true
}
}
if !wasAdded {
// remember to add it later in order
mountsToAdd = append(mountsToAdd, mount)
}
}
// append what's left
for _, mount := range mountsToAdd {
podSpec.Containers[containerIndex].VolumeMounts = append(podSpec.Containers[containerIndex].VolumeMounts, mount)
container.VolumeMounts = append(container.VolumeMounts, mount)
}
}
16 changes: 0 additions & 16 deletions utils/kubernetes/volumes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package kubernetes

import (
"sort"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -46,9 +45,6 @@ func TestReplaceOrAddVolume(t *testing.T) {
AddOrReplaceVolume(&podSpec, volumes...)

assert.Len(t, podSpec.Volumes, 3)
sort.Slice(podSpec.Volumes, func(i, j int) bool {
return podSpec.Volumes[i].Name < podSpec.Volumes[j].Name
})
assert.Equal(t, "cmA", podSpec.Volumes[0].ConfigMap.Name)
assert.Equal(t, "cmB", podSpec.Volumes[1].ConfigMap.Name)
assert.Equal(t, "cmC", podSpec.Volumes[2].ConfigMap.Name)
Expand All @@ -69,9 +65,6 @@ func TestReplaceOrAddVolume_Append(t *testing.T) {
AddOrReplaceVolume(&podSpec, volumes...)

assert.Len(t, podSpec.Volumes, 2)
sort.Slice(podSpec.Volumes, func(i, j int) bool {
return podSpec.Volumes[i].Name < podSpec.Volumes[j].Name
})
assert.Equal(t, "cm1", podSpec.Volumes[0].ConfigMap.Name)
assert.Equal(t, "cmB", podSpec.Volumes[1].ConfigMap.Name)
}
Expand All @@ -90,9 +83,6 @@ func TestReplaceOrAddVolume_EmptyVolumes(t *testing.T) {
AddOrReplaceVolume(&podSpec, volumes...)

assert.Len(t, podSpec.Volumes, 2)
sort.Slice(podSpec.Volumes, func(i, j int) bool {
return podSpec.Volumes[i].Name < podSpec.Volumes[j].Name
})
assert.Equal(t, "cm1", podSpec.Volumes[0].ConfigMap.Name)
assert.Equal(t, "cm2", podSpec.Volumes[1].ConfigMap.Name)
}
Expand All @@ -114,9 +104,6 @@ func TestReplaceOrAddVolume_EmptyPodVolumes(t *testing.T) {
AddOrReplaceVolume(&podSpec, volumes...)

assert.Len(t, podSpec.Volumes, 3)
sort.Slice(podSpec.Volumes, func(i, j int) bool {
return podSpec.Volumes[i].Name < podSpec.Volumes[j].Name
})
assert.Equal(t, "cmA", podSpec.Volumes[0].ConfigMap.Name)
assert.Equal(t, "cmB", podSpec.Volumes[1].ConfigMap.Name)
assert.Equal(t, "cmC", podSpec.Volumes[2].ConfigMap.Name)
Expand All @@ -137,9 +124,6 @@ func TestAddOrReplaceVolumeMount(t *testing.T) {

AddOrReplaceVolumeMount(0, &podSpec, mounts...)
assert.Len(t, podSpec.Containers[0].VolumeMounts, 2)
sort.Slice(podSpec.Containers[0].VolumeMounts, func(i, j int) bool {
return podSpec.Containers[0].VolumeMounts[i].Name < podSpec.Containers[0].VolumeMounts[j].Name
})
assert.Equal(t, "/dev", podSpec.Containers[0].VolumeMounts[0].MountPath)
assert.Equal(t, "/tmp/any/path", podSpec.Containers[0].VolumeMounts[1].MountPath)
}
Loading