You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by ri...@apache.org on 2024/02/14 18:20:08 UTC

(incubator-kie-kogito-serverless-operator) branch main updated: kie-kogito-serverless-operator-391: Pod instances keep spawning and terminating when deploying the workflow (#392)

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 <wm...@gmail.com>
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: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org