This is an automated email from the ASF dual-hosted git repository.
ricardozanini pushed a commit to branch main
in repository
https://gitbox.apache.org/repos/asf/incubator-kie-kogito-serverless-operator.git
The following commit(s) were added to refs/heads/main by this push:
new f6f61388 kie-kogito-serverless-operator-391: Pod instances keep
spawning and terminating when deploying the workflow (#392)
f6f61388 is described below
commit f6f61388d20b443a737c18d01f0111c3973b0a56
Author: Walter Medvedeo <[email protected]>
AuthorDate: Wed Feb 14 19:20:03 2024 +0100
kie-kogito-serverless-operator-391: Pod instances keep spawning and
terminating when deploying the workflow (#392)
---
controllers/profiles/dev/object_creators_dev.go | 2 +-
utils/kubernetes/volumes.go | 53 ++++++++++++++++---------
utils/kubernetes/volumes_test.go | 16 --------
3 files changed, 35 insertions(+), 36 deletions(-)
diff --git a/controllers/profiles/dev/object_creators_dev.go
b/controllers/profiles/dev/object_creators_dev.go
index 03b85750..47638320 100644
--- a/controllers/profiles/dev/object_creators_dev.go
+++ b/controllers/profiles/dev/object_creators_dev.go
@@ -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...)
diff --git a/utils/kubernetes/volumes.go b/utils/kubernetes/volumes.go
index 4c7855f1..71b56d62 100644
--- a/utils/kubernetes/volumes.go
+++ b/utils/kubernetes/volumes.go
@@ -101,19 +101,26 @@ 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)
}
@@ -121,18 +128,26 @@ func AddOrReplaceVolume(podSpec *corev1.PodSpec, volumes
...corev1.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)
}
}
diff --git a/utils/kubernetes/volumes_test.go b/utils/kubernetes/volumes_test.go
index 618e6277..6b296c67 100644
--- a/utils/kubernetes/volumes_test.go
+++ b/utils/kubernetes/volumes_test.go
@@ -15,7 +15,6 @@
package kubernetes
import (
- "sort"
"testing"
"github.com/stretchr/testify/assert"
@@ -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)
@@ -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)
}
@@ -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)
}
@@ -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)
@@ -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)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]