Merge pull request #16 from wmedvede/9.99.x-prod-kie-kogito-serverles…

…s-operator-391

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

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...)

utils/kubernetes/volumes.go

@@ -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)
 	}
 }

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)
 }