fix: copy "container" override to "background" and "output" overrides for MultiContainerRun

pkg/function/args.go

@@ -55,16 +55,21 @@ func ArgExists(args map[string]interface{}, argName string) bool {
 
 // GetPodSpecOverride merges PodOverride specs passed in args and TemplateParams and returns combined Override specs
 func GetPodSpecOverride(tp param.TemplateParams, args map[string]interface{}, argName string) (crv1alpha1.JSONMap, error) {
+	return GetAndMergePodSpecOverride(tp.PodOverride, args, argName)
+}
+
+// GetAndMergePodSpecOverride merges PodOverride specs passed in args and actionSetOverride and returns combined Override specs
+func GetAndMergePodSpecOverride(actionSetOverride crv1alpha1.JSONMap, args map[string]interface{}, argName string) (crv1alpha1.JSONMap, error) {
 	var podOverride crv1alpha1.JSONMap
 	var err error
-	if err = OptArg(args, KubeTaskPodOverrideArg, &podOverride, tp.PodOverride); err != nil {
+	if err = OptArg(args, argName, &podOverride, actionSetOverride); err != nil {
 		return nil, err
 	}
 
 	// Check if PodOverride specs are passed through actionset
 	// If yes, override podOverride specs
-	if tp.PodOverride != nil {
-		podOverride, err = kube.CreateAndMergeJSONPatch(podOverride, tp.PodOverride)
+	if actionSetOverride != nil {
+		podOverride, err = kube.CreateAndMergeJSONPatch(podOverride, actionSetOverride)
 		if err != nil {
 			return nil, err
 		}

pkg/function/multi_container_run.go

@@ -16,6 +16,7 @@ package function
 
 import (
 	"context"
+	"encoding/json"
 	"sort"
 	"time"
 
@@ -302,7 +303,12 @@ func (ktpf *multiContainerRunFunc) Exec(ctx context.Context, tp param.TemplatePa
 		return nil, err
 	}
 
-	ktpf.podOverride, err = GetPodSpecOverride(tp, args, MultiContainerRunPodOverrideArg)
+	actionSetOverride, err := prepareActionSetPodSpecOverride(tp.PodOverride)
+	if err != nil {
+		return nil, errkit.Wrap(err, "Unable to process podOverride from ActionSet spec")
+	}
+
+	ktpf.podOverride, err = GetAndMergePodSpecOverride(actionSetOverride, args, MultiContainerRunPodOverrideArg)
 	if err != nil {
 		return nil, err
 	}
@@ -312,6 +318,71 @@ func (ktpf *multiContainerRunFunc) Exec(ctx context.Context, tp param.TemplatePa
 	return ktpf.run(ctx)
 }
 
+// Since we use different container names compared to other functions, users might
+// specify override for "container" container meaning to override "worker containers"
+// We take override of the "container" container and copy that to worker containers
+// TODO: This is a temporary solution until phase-specific podOverride argument is
+// implemented for tha actionset
+func prepareActionSetPodSpecOverride(podOverride crv1alpha1.JSONMap) (crv1alpha1.JSONMap, error) {
+	containers, ok := getContainersFromOverride(podOverride)
+	if !ok { // No containers overridden
+		return podOverride, nil
+	}
+	hasContainer := false
+	hasBackgroundOrOutput := false
+
+	var containerOverride corev1.Container
+	resultContainers := []corev1.Container{}
+
+	for _, container := range containers {
+		switch container.Name {
+		case "container":
+			hasContainer = true
+			containerOverride = container
+		case "background":
+			hasBackgroundOrOutput = true
+		case "output":
+			hasBackgroundOrOutput = true
+		default:
+			resultContainers = append(resultContainers, container)
+		}
+	}
+
+	// "container" override is defined, but not specific overrides
+	// Assume the intention was to override "worker containers"
+	if hasContainer && !hasBackgroundOrOutput {
+		backgroundContainer := containerOverride
+		backgroundContainer.Name = "background"
+		outputContainer := containerOverride
+		outputContainer.Name = "output"
+		resultContainers := append(resultContainers, backgroundContainer, outputContainer)
+		podOverride["containers"] = nil
+		return kube.CreateAndMergeJSONPatch(podOverride, crv1alpha1.JSONMap{"containers": resultContainers})
+	}
+
+	// If "container" override is not defined, nothing to do
+
+	// If both "container" and either "background" or "output" overrides are defined
+	// Assume user knows what they're doing and keep override as is
+
+	return podOverride, nil
+}
+
+func getContainersFromOverride(podOverride crv1alpha1.JSONMap) ([]corev1.Container, bool) {
+	containersRaw, ok := podOverride["containers"]
+	if !ok {
+		return nil, false
+	}
+	// we're trying to enforce stricter types here
+	jsonString, err := json.Marshal(containersRaw)
+	if err != nil {
+		return nil, false
+	}
+	var containers []corev1.Container
+	err = json.Unmarshal(jsonString, &containers)
+	return containers, err == nil
+}
+
 func (ktpf *multiContainerRunFunc) setLabelsAndAnnotations(tp param.TemplateParams, labels, annotation map[string]string) {
 	ktpf.labels = labels
 	ktpf.annotations = annotation

pkg/function/multi_container_run_test.go

@@ -268,3 +268,156 @@ func (s *MultiContainerRunSuite) TestMultiContainerRunWithoutNamespace(c *C) {
 		}
 	}
 }
+
+func (s *MultiContainerRunSuite) TestPrepareActionSetPodSpecOverride(c *C) {
+	originalOverride := crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name":            "background",
+				"imagePullPolicy": "Always",
+				"resources":       map[string]interface{}{},
+			},
+			map[string]interface{}{
+				"name":            "output",
+				"imagePullPolicy": "Always",
+				"resources":       map[string]interface{}{},
+			},
+		},
+	}
+	podOverride, err := prepareActionSetPodSpecOverride(originalOverride)
+	c.Assert(err, IsNil)
+	c.Assert(podOverride, DeepEquals, originalOverride)
+
+	originalOverride = crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name":            "other_container",
+				"imagePullPolicy": "Always",
+				"resources":       map[string]interface{}{},
+			},
+		},
+	}
+	podOverride, err = prepareActionSetPodSpecOverride(originalOverride)
+	c.Assert(err, IsNil)
+	c.Assert(podOverride, DeepEquals, originalOverride)
+
+	originalOverride = crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name":            "container",
+				"imagePullPolicy": "Always",
+				"resources":       map[string]interface{}{},
+			},
+			map[string]interface{}{
+				"name":            "other_container",
+				"imagePullPolicy": "Never",
+				"resources":       map[string]interface{}{},
+			},
+		},
+	}
+
+	expectedOverride := crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name":            "other_container",
+				"imagePullPolicy": "Never",
+				"resources":       map[string]interface{}{},
+			},
+			map[string]interface{}{
+				"name":            "background",
+				"imagePullPolicy": "Always",
+				"resources":       map[string]interface{}{},
+			},
+			map[string]interface{}{
+				"name":            "output",
+				"imagePullPolicy": "Always",
+				"resources":       map[string]interface{}{},
+			},
+		},
+	}
+	podOverride, err = prepareActionSetPodSpecOverride(originalOverride)
+	c.Assert(err, IsNil)
+	c.Assert(podOverride, DeepEquals, expectedOverride)
+
+	originalOverride = crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name": "container",
+				"resources": map[string]interface{}{
+					"limits": map[string]interface{}{
+						"memory": "128Mi",
+					},
+				},
+			},
+		},
+	}
+
+	expectedOverride = crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name": "background",
+				"resources": map[string]interface{}{
+					"limits": map[string]interface{}{
+						"memory": "128Mi",
+					},
+				},
+			},
+			map[string]interface{}{
+				"name": "output",
+				"resources": map[string]interface{}{
+					"limits": map[string]interface{}{
+						"memory": "128Mi",
+					},
+				},
+			},
+		},
+	}
+	podOverride, err = prepareActionSetPodSpecOverride(originalOverride)
+	c.Assert(err, IsNil)
+	c.Assert(podOverride, DeepEquals, expectedOverride)
+
+	originalOverride = crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name": "container",
+				"resources": map[string]interface{}{
+					"limits": map[string]interface{}{
+						"memory": "128Mi",
+					},
+				},
+			},
+			map[string]interface{}{
+				"name": "background",
+				"resources": map[string]interface{}{
+					"limits": map[string]interface{}{
+						"memory": "128Mi",
+					},
+				},
+			},
+		},
+	}
+
+	expectedOverride = crv1alpha1.JSONMap{
+		"containers": []interface{}{
+			map[string]interface{}{
+				"name": "container",
+				"resources": map[string]interface{}{
+					"limits": map[string]interface{}{
+						"memory": "128Mi",
+					},
+				},
+			},
+			map[string]interface{}{
+				"name": "background",
+				"resources": map[string]interface{}{
+					"limits": map[string]interface{}{
+						"memory": "128Mi",
+					},
+				},
+			},
+		},
+	}
+	podOverride, err = prepareActionSetPodSpecOverride(originalOverride)
+	c.Assert(err, IsNil)
+	c.Assert(podOverride, DeepEquals, expectedOverride)
+}