Merge deployment containers as well (#1114)

Author: d0x2f

docs/function-controller-configuration.md

@@ -23,7 +23,14 @@ data:
         "template": {
           "annotations": {
             "annotations-to-pod": "value"
-          }
+          },
+          "containers": [{
+            "resources": {
+              "requests": {
+                "cpu": "100m"
+              }
+            }
+          }]
         }
       }
     }

pkg/utils/k8sutil.go

@@ -374,12 +374,29 @@ func MergeDeployments(destinationDeployment *appsv1.Deployment, sourceDeployment
 	initializeEmptyMapsInDeployment(destinationDeployment)
 	initializeEmptyMapsInDeployment(sourceDeployment)
 	err := mergo.Merge(destinationDeployment, sourceDeployment)
-	if err == nil && len(sourceDeployment.Spec.Template.Spec.Containers) > 0 && len(sourceDeployment.Spec.Template.Spec.Containers[0].VolumeMounts) > 0 {
-		// cannot use sourceDeployment.Spec.Template.Spec.Containers[0].VolumeMounts (type []"k8s.io/api/core/v1".VolumeMount) as type "k8s.io/api/core/v1".VolumeMount in append
-		// so iterate over and append
-		for _, volumeMount := range sourceDeployment.Spec.Template.Spec.Containers[0].VolumeMounts {
-			destinationDeployment.Spec.Template.Spec.Containers[0].VolumeMounts = append(destinationDeployment.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMount)
+
+	// Merge containers
+	if err == nil && len(sourceDeployment.Spec.Template.Spec.Containers) > 0 {
+		srcContainers := sourceDeployment.Spec.Template.Spec.Containers
+		dstContainers := destinationDeployment.Spec.Template.Spec.Containers
+
+		// Merge each container individually
+		for i, srcContainer := range srcContainers {
+			if i >= len(dstContainers) {
+				destinationDeployment.Spec.Template.Spec.Containers[i] = srcContainer
+				continue
+			}
+
+			dstContainer := dstContainers[i]
+
+			// Use mergo.WithAppendSlice to append extra volumeMount/env/port definitions
+			err = mergo.Merge(&dstContainer, srcContainer, mergo.WithAppendSlice)
+			if err != nil {
+				break
+			}
+			destinationDeployment.Spec.Template.Spec.Containers[i] = dstContainer
 		}
+
 	}
 	return err
 }

pkg/utils/k8sutil_test.go

@@ -12,6 +12,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	extensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	fakeextensionsapi "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
+	resource "k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/fake"
@@ -125,65 +126,150 @@ func TestInitializeEmptyMapsInDeployment(t *testing.T) {
 }
 
 func TestMergeDeployments(t *testing.T) {
-	var replicas int32
-	replicas = 10
-	volumeMount := corev1.VolumeMount{
-		Name:      "foo",
-		MountPath: "/bar",
-	}
-	container := corev1.Container{
-		VolumeMounts: []corev1.VolumeMount{
-			volumeMount,
+	var dstReplicas int32
+	dstReplicas = 10
+	destinationDeployment := appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Annotations: map[string]string{
+				"foo1-deploy": "bar",
+			},
 		},
-	}
-	podSpec := corev1.PodSpec{
-		Containers: []corev1.Container{
-			container,
+		Spec: appsv1.DeploymentSpec{
+			Replicas: &dstReplicas,
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							VolumeMounts: []corev1.VolumeMount{
+								{
+									Name:      "foo",
+									MountPath: "/bar",
+								},
+							},
+							Resources: corev1.ResourceRequirements{},
+						},
+					},
+				},
+			},
 		},
 	}
-	template := corev1.PodTemplateSpec{
-		Spec: podSpec,
-	}
 
-	destinationDeployment := appsv1.Deployment{
+	var srcReplicas int32
+	srcReplicas = 8
+	sourceDeployment := appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
-				"foo1-deploy": "bar",
+				"foo2-deploy": "bar",
+			},
+		},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: &srcReplicas,
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							VolumeMounts: []corev1.VolumeMount{
+								{
+									Name:      "baz",
+									MountPath: "/qux",
+								},
+							},
+							Resources: corev1.ResourceRequirements{
+								Requests: corev1.ResourceList{
+									corev1.ResourceName(corev1.ResourceCPU):    resource.MustParse("100m"),
+									corev1.ResourceName(corev1.ResourceMemory): resource.MustParse("100Mi"),
+								},
+							},
+						},
+					},
+				},
 			},
 		},
 	}
 
-	sourceDeployment := appsv1.Deployment{
+	var expectedReplicas int32
+	expectedReplicas = 10
+	expectedDeployment := appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
+				"foo1-deploy": "bar",
 				"foo2-deploy": "bar",
 			},
 		},
 		Spec: appsv1.DeploymentSpec{
-			Replicas: &replicas,
-			Template: template,
+			Replicas: &expectedReplicas,
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							VolumeMounts: []corev1.VolumeMount{
+								{
+									Name:      "foo",
+									MountPath: "/bar",
+								},
+								{
+									Name:      "baz",
+									MountPath: "/qux",
+								},
+							},
+							Resources: corev1.ResourceRequirements{
+								Requests: corev1.ResourceList{
+									corev1.ResourceName(corev1.ResourceCPU):    resource.MustParse("100m"),
+									corev1.ResourceName(corev1.ResourceMemory): resource.MustParse("100Mi"),
+								},
+							},
+						},
+					},
+				},
+			},
 		},
 	}
 
 	MergeDeployments(&destinationDeployment, &sourceDeployment)
-	expectedAnnotations := map[string]string{
-		"foo1-deploy": "bar",
-		"foo2-deploy": "bar",
+
+	mergedContainerCount := len(destinationDeployment.Spec.Template.Spec.Containers)
+	if mergedContainerCount != 1 {
+		t.Fatalf("Expecting 1 container but received %v", mergedContainerCount)
 	}
-	expectedVolumneMounts := len(destinationDeployment.Spec.Template.Spec.Containers[0].VolumeMounts)
-	mergedVolumneMounts := len(sourceDeployment.Spec.Template.Spec.Containers[0].VolumeMounts)
+
+	expectedAnnotations := expectedDeployment.ObjectMeta.Annotations
+	mergedAnnotations := destinationDeployment.ObjectMeta.Annotations
 	for i := range expectedAnnotations {
-		if destinationDeployment.ObjectMeta.Annotations[i] != expectedAnnotations[i] {
-			t.Fatalf("Expecting annotation %s but received %s", destinationDeployment.ObjectMeta.Annotations[i], expectedAnnotations[i])
+		if mergedAnnotations[i] != expectedAnnotations[i] {
+			t.Fatalf("Expecting annotation %s but received %s", expectedAnnotations[i], mergedAnnotations[i])
 		}
 	}
-	if *destinationDeployment.Spec.Replicas != replicas {
-		t.Fatalf("Expecting replicas as 10 but received %v", *destinationDeployment.Spec.Replicas)
+
+	mergedReplicas := *destinationDeployment.Spec.Replicas
+	if mergedReplicas != expectedReplicas {
+		t.Fatalf("Expecting 8 replicas but received %v", *destinationDeployment.Spec.Replicas)
 	}
-	if mergedVolumneMounts != expectedVolumneMounts {
-		t.Fatalf("Expecting %v volumeMounts but received %v", expectedVolumneMounts, mergedVolumneMounts)
+
+	expectedVolumeMountCount := 2
+	mergedVolumeMountCount := len(destinationDeployment.Spec.Template.Spec.Containers[0].VolumeMounts)
+	if mergedVolumeMountCount != expectedVolumeMountCount {
+		t.Fatalf("Expecting %v volumeMounts but received %v", expectedVolumeMountCount, mergedVolumeMountCount)
 	}
 
+	expectedCPURequest := expectedDeployment.Spec.Template.Spec.Containers[0].Resources.Requests[corev1.ResourceName(corev1.ResourceCPU)]
+	mergedCPURequest := destinationDeployment.Spec.Template.Spec.Containers[0].Resources.Requests[corev1.ResourceName(corev1.ResourceCPU)]
+	if mergedCPURequest != expectedCPURequest {
+		t.Fatalf(
+			"Expecting %s cpu resource request but received %s",
+			expectedCPURequest.String(),
+			mergedCPURequest.String(),
+		)
+	}
+
+	expectedMemoryRequest := expectedDeployment.Spec.Template.Spec.Containers[0].Resources.Requests[corev1.ResourceName(corev1.ResourceMemory)]
+	mergedMemoryRequest := destinationDeployment.Spec.Template.Spec.Containers[0].Resources.Requests[corev1.ResourceName(corev1.ResourceMemory)]
+	if mergedMemoryRequest != expectedMemoryRequest {
+		t.Fatalf(
+			"Expecting %s memory resource request but received %s",
+			expectedMemoryRequest.String(),
+			mergedMemoryRequest.String(),
+		)
+	}
 }
 
 func TestGetAnnotationsFromCRD(t *testing.T) {