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]

Reply via email to