diff --git a/workflow/controller/container_set_template.go b/workflow/controller/container_set_template.go index 6905c82452f0..e5226c193f44 100644 --- a/workflow/controller/container_set_template.go +++ b/workflow/controller/container_set_template.go @@ -7,7 +7,7 @@ import ( wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" ) -func (woc *wfOperationCtx) executeContainerSet(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) { +func (woc *wfOperationCtx) executeContainerSet(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts, localParams map[string]string) (*wfv1.NodeStatus, error) { node, err := woc.wf.GetNodeByName(nodeName) if err != nil { node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypePod, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodePending, opts.nodeFlag) @@ -21,7 +21,7 @@ func (woc *wfOperationCtx) executeContainerSet(ctx context.Context, nodeName str includeScriptOutput: includeScriptOutput, onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline, - }) + }, localParams) if err != nil { return woc.requeueIfTransientErr(err, node.Name) } diff --git a/workflow/controller/controller.go b/workflow/controller/controller.go index a943471bb3ac..d5491cb78fee 100644 --- a/workflow/controller/controller.go +++ b/workflow/controller/controller.go @@ -632,13 +632,23 @@ func (wfc *WorkflowController) deleteOffloadedNodesForWorkflow(uid string, versi if !ok { return fmt.Errorf("object %+v is not an unstructured", workflows[0]) } + key := un.GetNamespace() + "/" + un.GetName() + wfc.workflowKeyLock.Lock(key) + defer wfc.workflowKeyLock.Unlock(key) + + obj, ok := wfc.getWorkflowByKey(key) + if !ok { + return fmt.Errorf("failed to get workflow by key after locking") + } + un, ok = obj.(*unstructured.Unstructured) + if !ok { + return fmt.Errorf("object %+v is not an unstructured", obj) + } wf, err = util.FromUnstructured(un) if err != nil { return err } - key := wf.ObjectMeta.Namespace + "/" + wf.ObjectMeta.Name - wfc.workflowKeyLock.Lock(key) - defer wfc.workflowKeyLock.Unlock(key) + // workflow might still be hydrated if wfc.hydrator.IsHydrated(wf) { log.WithField("uid", wf.UID).Info("Hydrated workflow encountered") @@ -712,20 +722,14 @@ func (wfc *WorkflowController) processNextItem(ctx context.Context) bool { } defer wfc.wfQueue.Done(key) - obj, exists, err := wfc.wfInformer.GetIndexer().GetByKey(key.(string)) - if err != nil { - log.WithFields(log.Fields{"key": key, "error": err}).Error("Failed to get workflow from informer") - return true - } - if !exists { - // This happens after a workflow was labeled with completed=true - // or was deleted, but the work queue still had an entry for it. - return true - } - wfc.workflowKeyLock.Lock(key.(string)) defer wfc.workflowKeyLock.Unlock(key.(string)) + obj, ok := wfc.getWorkflowByKey(key.(string)) + if !ok { + return true + } + // The workflow informer receives unstructured objects to deal with the possibility of invalid // workflow manifests that are unable to unmarshal to workflow objects un, ok := obj.(*unstructured.Unstructured) @@ -794,6 +798,20 @@ func (wfc *WorkflowController) processNextItem(ctx context.Context) bool { return true } +func (wfc *WorkflowController) getWorkflowByKey(key string) (interface{}, bool) { + obj, exists, err := wfc.wfInformer.GetIndexer().GetByKey(key) + if err != nil { + log.WithFields(log.Fields{"key": key, "error": err}).Error("Failed to get workflow from informer") + return nil, false + } + if !exists { + // This happens after a workflow was labeled with completed=true + // or was deleted, but the work queue still had an entry for it. + return nil, false + } + return obj, true +} + func reconciliationNeeded(wf metav1.Object) bool { return wf.GetLabels()[common.LabelKeyCompleted] != "true" || slices.Contains(wf.GetFinalizers(), common.FinalizerArtifactGC) } @@ -929,6 +947,11 @@ func (wfc *WorkflowController) archiveWorkflow(ctx context.Context, obj interfac } wfc.workflowKeyLock.Lock(key) defer wfc.workflowKeyLock.Unlock(key) + key, err = cache.MetaNamespaceKeyFunc(obj) + if err != nil { + log.Error("failed to get key for object after locking") + return + } err = wfc.archiveWorkflowAux(ctx, obj) if err != nil { log.WithField("key", key).WithError(err).Error("failed to archive workflow") diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 6d2316dac5be..bc8a7d8929aa 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -1057,9 +1057,9 @@ func (woc *wfOperationCtx) processNodeRetries(node *wfv1.NodeStatus, retryStrate retryOnFailed = false retryOnError = true case wfv1.RetryPolicyOnTransientError: + retryOnError = true if (lastChildNode.Phase == wfv1.NodeFailed || lastChildNode.Phase == wfv1.NodeError) && errorsutil.IsTransientErr(errors.InternalError(lastChildNode.Message)) { retryOnFailed = true - retryOnError = true } case wfv1.RetryPolicyOnFailure: retryOnFailed = true @@ -1432,8 +1432,8 @@ func (woc *wfOperationCtx) assessNodeStatus(pod *apiv1.Pod, old *wfv1.NodeStatus } } - // if we are transitioning from Pending to a different state, clear out unchanged message - if old.Phase == wfv1.NodePending && new.Phase != wfv1.NodePending && old.Message == new.Message { + // if we are transitioning from Pending to a different state (except Fail), clear out unchanged message + if old.Phase == wfv1.NodePending && new.Phase != wfv1.NodePending && new.Phase != wfv1.NodeFailed && old.Message == new.Message { new.Message = "" } @@ -1777,6 +1777,10 @@ func getRetryNodeChildrenIds(node *wfv1.NodeStatus, nodes wfv1.Nodes) []string { func buildRetryStrategyLocalScope(node *wfv1.NodeStatus, nodes wfv1.Nodes) map[string]interface{} { localScope := make(map[string]interface{}) + localScope[common.LocalVarRetriesLastExitCode] = "0" + localScope[common.LocalVarRetriesLastStatus] = "" + localScope[common.LocalVarRetriesLastDuration] = "0" + localScope[common.LocalVarRetriesLastMessage] = "" // `retries` variable childNodeIds, lastChildNode := getChildNodeIdsAndLastRetriedNode(node, nodes) @@ -2071,24 +2075,28 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string, localScope, realTimeScope := woc.prepareMetricScope(lastChildNode) woc.computeMetrics(processedTmpl.Metrics.Prometheus, localScope, realTimeScope, false) } + localScope := buildRetryStrategyLocalScope(retryParentNode, woc.wf.Status.Nodes) + for key, value := range localScope { + strKey := fmt.Sprintf("%v", key) + strValue := fmt.Sprintf("%v", value) + localParams[strKey] = strValue + } + retryNum := len(childNodeIDs) + localParams[common.LocalVarRetries] = strconv.Itoa(retryNum) if lastChildNode != nil && !lastChildNode.Fulfilled() { // Last child node is still running. nodeName = lastChildNode.Name node = lastChildNode } else { - retryNum := len(childNodeIDs) // Create a new child node and append it to the retry node. nodeName = fmt.Sprintf("%s(%d)", retryNodeName, retryNum) woc.addChildNode(retryNodeName, nodeName) node = nil - localParams := make(map[string]string) // Change the `pod.name` variable to the new retry node name if processedTmpl.IsPodType() { localParams[common.LocalVarPodName] = woc.getPodName(nodeName, processedTmpl.Name) } - // Inject the retryAttempt number - localParams[common.LocalVarRetries] = strconv.Itoa(retryNum) processedTmpl, err = common.SubstituteParams(processedTmpl, map[string]string{}, localParams) if errorsutil.IsTransientErr(err) { @@ -2102,21 +2110,21 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string, switch processedTmpl.GetType() { case wfv1.TemplateTypeContainer: - node, err = woc.executeContainer(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts) + node, err = woc.executeContainer(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts, localParams) case wfv1.TemplateTypeContainerSet: - node, err = woc.executeContainerSet(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts) + node, err = woc.executeContainerSet(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts, localParams) case wfv1.TemplateTypeSteps: node, err = woc.executeSteps(ctx, nodeName, newTmplCtx, templateScope, processedTmpl, orgTmpl, opts) case wfv1.TemplateTypeScript: - node, err = woc.executeScript(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts) + node, err = woc.executeScript(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts, localParams) case wfv1.TemplateTypeResource: - node, err = woc.executeResource(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts) + node, err = woc.executeResource(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts, localParams) case wfv1.TemplateTypeDAG: node, err = woc.executeDAG(ctx, nodeName, newTmplCtx, templateScope, processedTmpl, orgTmpl, opts) case wfv1.TemplateTypeSuspend: node, err = woc.executeSuspend(nodeName, templateScope, processedTmpl, orgTmpl, opts) case wfv1.TemplateTypeData: - node, err = woc.executeData(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts) + node, err = woc.executeData(ctx, nodeName, templateScope, processedTmpl, orgTmpl, opts, localParams) case wfv1.TemplateTypeHTTP: node = woc.executeHTTPTemplate(nodeName, templateScope, processedTmpl, orgTmpl, opts) case wfv1.TemplateTypePlugin: @@ -2722,7 +2730,7 @@ func (woc *wfOperationCtx) checkParallelism(tmpl *wfv1.Template, node *wfv1.Node return nil } -func (woc *wfOperationCtx) executeContainer(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) { +func (woc *wfOperationCtx) executeContainer(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts, localParams map[string]string) (*wfv1.NodeStatus, error) { node, err := woc.wf.GetNodeByName(nodeName) if err != nil { node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypePod, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodePending, opts.nodeFlag) @@ -2740,7 +2748,7 @@ func (woc *wfOperationCtx) executeContainer(ctx context.Context, nodeName string includeScriptOutput: includeScriptOutput, onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline, - }) + }, localParams) if err != nil { return woc.requeueIfTransientErr(err, node.Name) @@ -2926,7 +2934,7 @@ loop: return nodeName } -func (woc *wfOperationCtx) executeScript(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) { +func (woc *wfOperationCtx) executeScript(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts, localParams map[string]string) (*wfv1.NodeStatus, error) { node, err := woc.wf.GetNodeByName(nodeName) if err != nil { node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypePod, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodePending, opts.nodeFlag) @@ -2951,7 +2959,7 @@ func (woc *wfOperationCtx) executeScript(ctx context.Context, nodeName string, t includeScriptOutput: includeScriptOutput, onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline, - }) + }, localParams) if err != nil { return woc.requeueIfTransientErr(err, node.Name) } @@ -3197,7 +3205,7 @@ func (woc *wfOperationCtx) addChildNode(parent string, child string) { } // executeResource is runs a kubectl command against a manifest -func (woc *wfOperationCtx) executeResource(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) { +func (woc *wfOperationCtx) executeResource(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts, localParams map[string]string) (*wfv1.NodeStatus, error) { node, err := woc.wf.GetNodeByName(nodeName) if err != nil { @@ -3226,7 +3234,7 @@ func (woc *wfOperationCtx) executeResource(ctx context.Context, nodeName string, mainCtr := woc.newExecContainer(common.MainContainerName, tmpl) mainCtr.Command = []string{"argoexec", "resource", tmpl.Resource.Action} - _, err = woc.createWorkflowPod(ctx, nodeName, []apiv1.Container{*mainCtr}, tmpl, &createWorkflowPodOpts{onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline}) + _, err = woc.createWorkflowPod(ctx, nodeName, []apiv1.Container{*mainCtr}, tmpl, &createWorkflowPodOpts{onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline}, localParams) if err != nil { return woc.requeueIfTransientErr(err, node.Name) } @@ -3234,7 +3242,7 @@ func (woc *wfOperationCtx) executeResource(ctx context.Context, nodeName string, return node, err } -func (woc *wfOperationCtx) executeData(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) { +func (woc *wfOperationCtx) executeData(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts, localParams map[string]string) (*wfv1.NodeStatus, error) { node, err := woc.wf.GetNodeByName(nodeName) if err != nil { node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypePod, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodePending, opts.nodeFlag) @@ -3249,7 +3257,7 @@ func (woc *wfOperationCtx) executeData(ctx context.Context, nodeName string, tem mainCtr := woc.newExecContainer(common.MainContainerName, tmpl) mainCtr.Command = []string{"argoexec", "data", string(dataTemplate)} - _, err = woc.createWorkflowPod(ctx, nodeName, []apiv1.Container{*mainCtr}, tmpl, &createWorkflowPodOpts{onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline, includeScriptOutput: true}) + _, err = woc.createWorkflowPod(ctx, nodeName, []apiv1.Container{*mainCtr}, tmpl, &createWorkflowPodOpts{onExitPod: opts.onExitTemplate, executionDeadline: opts.executionDeadline, includeScriptOutput: true}, localParams) if err != nil { return woc.requeueIfTransientErr(err, node.Name) } diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go index 510d112c29e9..890f10ae6d93 100644 --- a/workflow/controller/workflowpod.go +++ b/workflow/controller/workflowpod.go @@ -45,10 +45,6 @@ var ( } ) -func (woc *wfOperationCtx) hasPodSpecPatch(tmpl *wfv1.Template) bool { - return woc.execWf.Spec.HasPodSpecPatch() || tmpl.HasPodSpecPatch() -} - // scheduleOnDifferentHost adds affinity to prevent retry on the same host when // retryStrategy.affinity.nodeAntiAffinity{} is specified func (woc *wfOperationCtx) scheduleOnDifferentHost(node *wfv1.NodeStatus, pod *apiv1.Pod) error { @@ -77,7 +73,7 @@ type createWorkflowPodOpts struct { executionDeadline time.Time } -func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName string, mainCtrs []apiv1.Container, tmpl *wfv1.Template, opts *createWorkflowPodOpts) (*apiv1.Pod, error) { +func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName string, mainCtrs []apiv1.Container, tmpl *wfv1.Template, opts *createWorkflowPodOpts, localParams map[string]string) (*apiv1.Pod, error) { nodeID := woc.wf.NodeID(nodeName) // we must check to see if the pod exists rather than just optimistically creating the pod and see if we get @@ -347,24 +343,23 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin } } - // Apply the patch string from template - if woc.hasPodSpecPatch(tmpl) { - tmpl.PodSpecPatch, err = util.PodSpecPatchMerge(woc.wf, tmpl) - if err != nil { - return nil, errors.Wrap(err, "", "Failed to merge the workflow PodSpecPatch with the template PodSpecPatch due to invalid format") - } - + // Apply the patch string from workflow and template + var podSpecPatchs []string + if woc.execWf.Spec.HasPodSpecPatch() { // Final substitution for workflow level PodSpecPatch - localParams := make(map[string]string) - if tmpl.IsPodType() { - localParams[common.LocalVarPodName] = pod.Name - } - tmpl, err := common.ProcessArgs(tmpl, &wfv1.Arguments{}, woc.globalParams, localParams, false, woc.wf.Namespace, woc.controller.configMapInformer.GetIndexer()) + newTmpl := tmpl.DeepCopy() + newTmpl.PodSpecPatch = woc.execWf.Spec.PodSpecPatch + processedTmpl, err := common.ProcessArgs(newTmpl, &wfv1.Arguments{}, woc.globalParams, localParams, false, woc.wf.Namespace, woc.controller.configMapInformer.GetIndexer()) if err != nil { return nil, errors.Wrap(err, "", "Failed to substitute the PodSpecPatch variables") } - - patchedPodSpec, err := util.ApplyPodSpecPatch(pod.Spec, tmpl.PodSpecPatch) + podSpecPatchs = append(podSpecPatchs, processedTmpl.PodSpecPatch) + } + if tmpl.HasPodSpecPatch() { + podSpecPatchs = append(podSpecPatchs, tmpl.PodSpecPatch) + } + if len(podSpecPatchs) > 0 { + patchedPodSpec, err := util.ApplyPodSpecPatch(pod.Spec, podSpecPatchs...) if err != nil { return nil, errors.Wrap(err, "", "Error applying PodSpecPatch") } diff --git a/workflow/controller/workflowpod_test.go b/workflow/controller/workflowpod_test.go index d2ae3c044199..ac1d6ca753e2 100644 --- a/workflow/controller/workflowpod_test.go +++ b/workflow/controller/workflowpod_test.go @@ -19,6 +19,7 @@ import ( "k8s.io/utils/pointer" "github.com/argoproj/argo-workflows/v3/config" + "github.com/argoproj/argo-workflows/v3/errors" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" "github.com/argoproj/argo-workflows/v3/test/util" armocks "github.com/argoproj/argo-workflows/v3/workflow/artifactrepositories/mocks" @@ -87,7 +88,8 @@ func TestScriptTemplateWithVolume(t *testing.T) { ctx := context.Background() tmpl := unmarshalTemplate(scriptTemplateWithInputArtifact) woc := newWoc() - _, err := woc.executeScript(ctx, tmpl.Name, "", tmpl, &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err := woc.executeScript(ctx, tmpl.Name, "", tmpl, &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) } @@ -161,7 +163,8 @@ func TestScriptTemplateWithoutVolumeOptionalArtifact(t *testing.T) { mainCtr := tmpl.Script.Container mainCtr.Args = append(mainCtr.Args, common.ExecutorScriptSourcePath) ctx := context.Background() - pod, err := woc.createWorkflowPod(ctx, tmpl.Name, []apiv1.Container{mainCtr}, tmpl, &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(ctx, tmpl.Name, []apiv1.Container{mainCtr}, tmpl, &createWorkflowPodOpts{}, lp) assert.NoError(t, err) // Note: pod.Spec.Containers[0] is wait assert.Contains(t, pod.Spec.Containers[1].VolumeMounts, volumeMount) @@ -176,7 +179,7 @@ func TestScriptTemplateWithoutVolumeOptionalArtifact(t *testing.T) { woc = newWoc(*wf) mainCtr = tmpl.Script.Container mainCtr.Args = append(mainCtr.Args, common.ExecutorScriptSourcePath) - pod, err = woc.createWorkflowPod(ctx, tmpl.Name, []apiv1.Container{mainCtr}, tmpl, &createWorkflowPodOpts{includeScriptOutput: true}) + pod, err = woc.createWorkflowPod(ctx, tmpl.Name, []apiv1.Container{mainCtr}, tmpl, &createWorkflowPodOpts{includeScriptOutput: true}, lp) assert.NoError(t, err) assert.NotContains(t, pod.Spec.Containers[1].VolumeMounts, volumeMount) assert.Contains(t, pod.Spec.Containers[1].VolumeMounts, customVolumeMount) @@ -192,7 +195,8 @@ func TestWFLevelServiceAccount(t *testing.T) { assert.NoError(t, err) ctx := context.Background() - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -211,7 +215,8 @@ func TestTmplServiceAccount(t *testing.T) { assert.NoError(t, err) ctx := context.Background() - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) @@ -233,8 +238,8 @@ func TestWFLevelAutomountServiceAccountToken(t *testing.T) { woc.execWf.Spec.Executor = &wfv1.ExecutorConfig{ServiceAccountName: "foo"} tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -257,8 +262,8 @@ func TestTmplLevelAutomountServiceAccountToken(t *testing.T) { woc.execWf.Spec.Templates[0].AutomountServiceAccountToken = &falseValue tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -287,8 +292,8 @@ func TestWFLevelExecutorServiceAccountName(t *testing.T) { woc.execWf.Spec.Executor = &wfv1.ExecutorConfig{ServiceAccountName: "foo"} tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -313,8 +318,8 @@ func TestTmplLevelExecutorServiceAccountName(t *testing.T) { woc.execWf.Spec.Templates[0].Executor = &wfv1.ExecutorConfig{ServiceAccountName: "tmpl"} tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(ctx, metav1.ListOptions{}) assert.NoError(t, err) @@ -340,7 +345,8 @@ func TestTmplLevelExecutorSecurityContext(t *testing.T) { woc.execWf.Spec.Templates[0].Executor = &wfv1.ExecutorConfig{ServiceAccountName: "tmpl"} tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(ctx, metav1.ListOptions{}) assert.NoError(t, err) @@ -367,9 +373,9 @@ func TestImagePullSecrets(t *testing.T) { } tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - + lp := make(map[string]string) ctx := context.Background() - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := woc.controller.kubeclientset.CoreV1().Pods("").List(ctx, metav1.ListOptions{}) assert.NoError(t, err) @@ -403,9 +409,9 @@ func TestAffinity(t *testing.T) { } tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - + lp := make(map[string]string) ctx := context.Background() - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -424,9 +430,9 @@ func TestTolerations(t *testing.T) { }} tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - + lp := make(map[string]string) ctx := context.Background() - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -441,9 +447,9 @@ func TestMetadata(t *testing.T) { woc := newWoc() tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - + lp := make(map[string]string) ctx := context.Background() - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -644,7 +650,8 @@ func Test_createWorkflowPod_rateLimited(t *testing.T) { func Test_createWorkflowPod_containerName(t *testing.T) { woc := newWoc() - pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Name: "invalid", Command: []string{""}}}, &wfv1.Template{}, &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Name: "invalid", Command: []string{""}}}, &wfv1.Template{}, &createWorkflowPodOpts{}, lp) assert.NoError(t, err) assert.Equal(t, common.MainContainerName, pod.Spec.Containers[1].Name) } @@ -652,12 +659,14 @@ func Test_createWorkflowPod_containerName(t *testing.T) { func Test_createWorkflowPod_emissary(t *testing.T) { t.Run("NoCommand", func(t *testing.T) { woc := newWoc() - _, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "docker/whalesay:nope"}}, &wfv1.Template{Name: "my-tmpl"}, &createWorkflowPodOpts{}) + lp := make(map[string]string) + _, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "docker/whalesay:nope"}}, &wfv1.Template{Name: "my-tmpl"}, &createWorkflowPodOpts{}, lp) assert.EqualError(t, err, "failed to look-up entrypoint/cmd for image \"docker/whalesay:nope\", you must either explicitly specify the command, or list the image's command in the index: https://argoproj.github.io/argo-workflows/workflow-executors/#emissary-emissary: GET https://index.docker.io/v2/docker/whalesay/manifests/nope: MANIFEST_UNKNOWN: manifest unknown; unknown tag=nope") }) t.Run("CommandNoArgs", func(t *testing.T) { woc := newWoc() - pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Command: []string{"foo"}}}, &wfv1.Template{}, &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Command: []string{"foo"}}}, &wfv1.Template{}, &createWorkflowPodOpts{}, lp) assert.NoError(t, err) assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, @@ -665,7 +674,8 @@ func Test_createWorkflowPod_emissary(t *testing.T) { }) t.Run("NoCommandWithImageIndex", func(t *testing.T) { woc := newWoc() - pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "my-image"}}, &wfv1.Template{}, &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "my-image"}}, &wfv1.Template{}, &createWorkflowPodOpts{}, lp) if assert.NoError(t, err) { assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, @@ -675,7 +685,8 @@ func Test_createWorkflowPod_emissary(t *testing.T) { }) t.Run("NoCommandWithArgsWithImageIndex", func(t *testing.T) { woc := newWoc() - pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "my-image", Args: []string{"foo"}}}, &wfv1.Template{}, &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Image: "my-image", Args: []string{"foo"}}}, &wfv1.Template{}, &createWorkflowPodOpts{}, lp) if assert.NoError(t, err) { assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, @@ -692,7 +703,8 @@ func Test_createWorkflowPod_emissary(t *testing.T) { }} podSpecPatch, err := json.Marshal(podSpec) assert.NoError(t, err) - pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Command: []string{"foo"}}}, &wfv1.Template{PodSpecPatch: string(podSpecPatch)}, &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(context.Background(), "", []apiv1.Container{{Command: []string{"foo"}}}, &wfv1.Template{PodSpecPatch: string(podSpecPatch)}, &createWorkflowPodOpts{}, lp) assert.NoError(t, err) assert.Equal(t, []string{"/var/run/argo/argoexec", "emissary", "--loglevel", getExecutorLogLevel(), "--log-format", woc.controller.cliExecutorLogFormat, @@ -726,7 +738,8 @@ func TestVolumeAndVolumeMounts(t *testing.T) { tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -795,7 +808,8 @@ func TestVolumesPodSubstitution(t *testing.T) { tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -833,7 +847,8 @@ func TestOutOfCluster(t *testing.T) { tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -859,7 +874,8 @@ func TestOutOfCluster(t *testing.T) { tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -883,7 +899,8 @@ func TestPriority(t *testing.T) { woc.execWf.Spec.Templates[0].Priority = &priority tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -900,7 +917,8 @@ func TestSchedulerName(t *testing.T) { woc.execWf.Spec.Templates[0].SchedulerName = "foo" tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -955,7 +973,8 @@ func TestInitContainers(t *testing.T) { tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -1020,7 +1039,8 @@ func TestSidecars(t *testing.T) { tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -1074,7 +1094,8 @@ func TestTemplateLocalVolumes(t *testing.T) { tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -1098,7 +1119,8 @@ func TestWFLevelHostAliases(t *testing.T) { } tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -1117,7 +1139,8 @@ func TestTmplLevelHostAliases(t *testing.T) { } tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -1136,7 +1159,8 @@ func TestWFLevelSecurityContext(t *testing.T) { } tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -1156,7 +1180,8 @@ func TestTmplLevelSecurityContext(t *testing.T) { } tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "") assert.NoError(t, err) - _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}) + lp := make(map[string]string) + _, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{}, lp) assert.NoError(t, err) pods, err := listPods(woc) assert.NoError(t, err) @@ -1247,7 +1272,8 @@ func Test_createSecretVolumesFromArtifactLocations_SSECUsed(t *testing.T) { mainCtr := woc.execWf.Spec.Templates[0].Container for i := 1; i < 5; i++ { - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) if pod != nil { assert.Contains(t, pod.Spec.Volumes, wantVolume) assert.Len(t, pod.Spec.InitContainers, 1) @@ -1258,6 +1284,30 @@ func Test_createSecretVolumesFromArtifactLocations_SSECUsed(t *testing.T) { } +var helloWorldWfWithTmplAndWFPatch = ` +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + name: hello-world +spec: + entrypoint: whalesay + podSpecPatch: | + containers: + - name: main + securityContext: + runAsNonRoot: true + capabilities: + drop: + - ALL + templates: + - name: whalesay + podSpecPatch: '{"containers":[{"name":"main", "securityContext":{"capabilities":{"add":["ALL"],"drop":null}}}]}' + container: + image: docker/whalesay:latest + command: [cowsay] + args: ["hello world"] +` + var helloWorldWfWithPatch = ` apiVersion: argoproj.io/v1alpha1 kind: Workflow @@ -1337,28 +1387,38 @@ func TestPodSpecPatch(t *testing.T) { ctx := context.Background() woc := newWoc(*wf) mainCtr := woc.execWf.Spec.Templates[0].Container - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "0.800", pod.Spec.Containers[1].Resources.Limits.Cpu().AsDec().String()) wf = wfv1.MustUnmarshalWorkflow(helloWorldWfWithWFPatch) woc = newWoc(*wf) mainCtr = woc.execWf.Spec.Templates[0].Container - pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "0.800", pod.Spec.Containers[1].Resources.Limits.Cpu().AsDec().String()) wf = wfv1.MustUnmarshalWorkflow(helloWorldWfWithWFYAMLPatch) woc = newWoc(*wf) mainCtr = woc.execWf.Spec.Templates[0].Container - pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "0.800", pod.Spec.Containers[1].Resources.Limits.Cpu().AsDec().String()) assert.Equal(t, "104857600", pod.Spec.Containers[1].Resources.Limits.Memory().AsDec().String()) + wf = wfv1.MustUnmarshalWorkflow(helloWorldWfWithTmplAndWFPatch) + woc = newWoc(*wf) + mainCtr = woc.execWf.Spec.Templates[0].Container + pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) + assert.Equal(t, pointer.Bool(true), pod.Spec.Containers[1].SecurityContext.RunAsNonRoot) + assert.Equal(t, apiv1.Capability("ALL"), pod.Spec.Containers[1].SecurityContext.Capabilities.Add[0]) + assert.Equal(t, []apiv1.Capability(nil), pod.Spec.Containers[1].SecurityContext.Capabilities.Drop) + wf = wfv1.MustUnmarshalWorkflow(helloWorldWfWithInvalidPatchFormat) woc = newWoc(*wf) mainCtr = woc.execWf.Spec.Templates[0].Container - _, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) - assert.EqualError(t, err, "Failed to merge the workflow PodSpecPatch with the template PodSpecPatch due to invalid format") + _, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) + assert.EqualError(t, err, "Error applying PodSpecPatch") + assert.EqualError(t, errors.Cause(err), "invalid character '}' after object key") } var helloWorldStepWfWithPatch = ` @@ -1433,7 +1493,8 @@ func TestMainContainerCustomization(t *testing.T) { woc := newWoc(*wf) woc.controller.Config.MainContainer = mainCtrSpec mainCtr := woc.execWf.Spec.Templates[0].Container - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "0.800", pod.Spec.Containers[1].Resources.Limits.Cpu().AsDec().String()) }) // The main container's resources should be changed since the existing @@ -1444,7 +1505,8 @@ func TestMainContainerCustomization(t *testing.T) { woc.controller.Config.MainContainer = mainCtrSpec mainCtr := woc.execWf.Spec.Templates[0].Container mainCtr.Resources = apiv1.ResourceRequirements{Limits: apiv1.ResourceList{}} - pod, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) if assert.NoError(t, err) { ctr := pod.Spec.Containers[1] assert.NotNil(t, ctr.SecurityContext) @@ -1468,7 +1530,8 @@ func TestMainContainerCustomization(t *testing.T) { apiv1.ResourceMemory: resource.MustParse("512Mi"), }, } - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "0.900", pod.Spec.Containers[1].Resources.Limits.Cpu().AsDec().String()) }) @@ -1484,7 +1547,8 @@ func TestMainContainerCustomization(t *testing.T) { apiv1.ResourceMemory: resource.MustParse("123Mi"), }, } - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "1", pod.Spec.Containers[1].Resources.Limits.Cpu().AsDec().String()) assert.Equal(t, "128974848", pod.Spec.Containers[1].Resources.Limits.Memory().AsDec().String()) }) @@ -1537,7 +1601,8 @@ func TestWindowsUNCPathsAreRemoved(t *testing.T) { ctx := context.Background() mainCtr := woc.execWf.Spec.Templates[0].Container - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) waitCtrIdx, err := wfutil.FindWaitCtrIndex(pod) if err != nil { @@ -1576,7 +1641,8 @@ func TestPropagateMaxDuration(t *testing.T) { woc := newWoc() deadline := time.Time{}.Add(time.Second) ctx := context.Background() - pod, err := woc.createWorkflowPod(ctx, tmpl.Name, []apiv1.Container{*tmpl.Container}, tmpl, &createWorkflowPodOpts{executionDeadline: deadline}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(ctx, tmpl.Name, []apiv1.Container{*tmpl.Container}, tmpl, &createWorkflowPodOpts{executionDeadline: deadline}, lp) assert.NoError(t, err) v, err := getPodDeadline(pod) assert.NoError(t, err) @@ -1635,14 +1701,15 @@ func TestPodMetadata(t *testing.T) { ctx := context.Background() woc := newWoc(*wf) mainCtr := woc.execWf.Spec.Templates[0].Container - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "foo", pod.ObjectMeta.Annotations["workflow-level-pod-annotation"]) assert.Equal(t, "bar", pod.ObjectMeta.Labels["workflow-level-pod-label"]) wf = wfv1.MustUnmarshalWorkflow(wfWithPodMetadataAndTemplateMetadata) woc = newWoc(*wf) mainCtr = woc.execWf.Spec.Templates[0].Container - pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "fizz", pod.ObjectMeta.Annotations["workflow-level-pod-annotation"]) assert.Equal(t, "buzz", pod.ObjectMeta.Labels["workflow-level-pod-label"]) assert.Equal(t, "hello", pod.ObjectMeta.Annotations["template-level-pod-annotation"]) @@ -1677,13 +1744,14 @@ func TestPodDefaultContainer(t *testing.T) { wf.Spec.Templates[0].ContainerSet.Containers[0].Name = common.MainContainerName woc := newWoc(*wf) template := woc.execWf.Spec.Templates[0] - pod, _ := woc.createWorkflowPod(ctx, wf.Name, template.ContainerSet.GetContainers(), &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, template.ContainerSet.GetContainers(), &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, common.MainContainerName, pod.ObjectMeta.Annotations[common.AnnotationKeyDefaultContainer]) wf = wfv1.MustUnmarshalWorkflow(wfWithContainerSet) woc = newWoc(*wf) template = woc.execWf.Spec.Templates[0] - pod, _ = woc.createWorkflowPod(ctx, wf.Name, template.ContainerSet.GetContainers(), &template, &createWorkflowPodOpts{}) + pod, _ = woc.createWorkflowPod(ctx, wf.Name, template.ContainerSet.GetContainers(), &template, &createWorkflowPodOpts{}, lp) assert.Equal(t, "b", pod.ObjectMeta.Annotations[common.AnnotationKeyDefaultContainer]) } @@ -1692,7 +1760,8 @@ func TestGetDeadline(t *testing.T) { ctx := context.Background() woc := newWoc(*wf) mainCtr := woc.execWf.Spec.Templates[0].Container - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) deadline, _ := getPodDeadline(pod) assert.Equal(t, time.Time{}, deadline) @@ -1701,7 +1770,7 @@ func TestGetDeadline(t *testing.T) { ctx = context.Background() woc = newWoc(*wf) mainCtr = woc.execWf.Spec.Templates[0].Container - pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{executionDeadline: executionDeadline}) + pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{executionDeadline: executionDeadline}, lp) deadline, _ = getPodDeadline(pod) assert.Equal(t, executionDeadline.Format(time.RFC3339), deadline.Format(time.RFC3339)) } @@ -1731,7 +1800,8 @@ func TestPodMetadataWithWorkflowDefaults(t *testing.T) { err := woc.setExecWorkflow(ctx) assert.NoError(t, err) mainCtr := woc.execWf.Spec.Templates[0].Container - pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, _ := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "annotation-value", pod.ObjectMeta.Annotations["controller-level-pod-annotation"]) assert.Equal(t, "set-by-controller", pod.ObjectMeta.Annotations["workflow-level-pod-annotation"]) assert.Equal(t, "label-value", pod.ObjectMeta.Labels["controller-level-pod-label"]) @@ -1754,7 +1824,7 @@ func TestPodMetadataWithWorkflowDefaults(t *testing.T) { err = woc.setExecWorkflow(ctx) assert.NoError(t, err) mainCtr = woc.execWf.Spec.Templates[0].Container - pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + pod, _ = woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.Equal(t, "foo", pod.ObjectMeta.Annotations["workflow-level-pod-annotation"]) assert.Equal(t, "bar", pod.ObjectMeta.Labels["workflow-level-pod-label"]) assert.Equal(t, "annotation-value", pod.ObjectMeta.Annotations["controller-level-pod-annotation"]) @@ -1772,7 +1842,8 @@ func TestPodExists(t *testing.T) { err := woc.setExecWorkflow(ctx) assert.NoError(t, err) mainCtr := woc.execWf.Spec.Templates[0].Container - pod, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) assert.NoError(t, err) assert.NotNil(t, pod) @@ -1799,7 +1870,8 @@ func TestProgressEnvVars(t *testing.T) { err := woc.setExecWorkflow(ctx) require.NoError(t, err) mainCtr := woc.execWf.Spec.Templates[0].Container - pod, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}) + lp := make(map[string]string) + pod, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{}, lp) require.NoError(t, err) assert.NotNil(t, pod) return cancel, pod diff --git a/workflow/util/util.go b/workflow/util/util.go index bb539f8ee8c8..7f1125c03f8e 100644 --- a/workflow/util/util.go +++ b/workflow/util/util.go @@ -1237,44 +1237,32 @@ func ConvertYAMLToJSON(str string) (string, error) { return str, nil } -// PodSpecPatchMerge will do strategic merge the workflow level PodSpecPatch and template level PodSpecPatch -func PodSpecPatchMerge(wf *wfv1.Workflow, tmpl *wfv1.Template) (string, error) { - wfPatch, err := ConvertYAMLToJSON(wf.Spec.PodSpecPatch) - if err != nil { - return "", err - } - tmplPatch, err := ConvertYAMLToJSON(tmpl.PodSpecPatch) - if err != nil { - return "", err - } - data, err := strategicpatch.StrategicMergePatch([]byte(wfPatch), []byte(tmplPatch), apiv1.PodSpec{}) - return string(data), err -} - -func ApplyPodSpecPatch(podSpec apiv1.PodSpec, podSpecPatchYaml string) (*apiv1.PodSpec, error) { +func ApplyPodSpecPatch(podSpec apiv1.PodSpec, podSpecPatchYamls ...string) (*apiv1.PodSpec, error) { podSpecJson, err := json.Marshal(podSpec) if err != nil { return nil, errors.Wrap(err, "", "Failed to marshal the Pod spec") } - // must convert to json because PodSpec has only json tags - podSpecPatchJson, err := ConvertYAMLToJSON(podSpecPatchYaml) - if err != nil { - return nil, errors.Wrap(err, "", "Failed to convert the PodSpecPatch yaml to json") - } + for _, podSpecPatchYaml := range podSpecPatchYamls { + // must convert to json because PodSpec has only json tags + podSpecPatchJson, err := ConvertYAMLToJSON(podSpecPatchYaml) + if err != nil { + return nil, errors.Wrap(err, "", "Failed to convert the PodSpecPatch yaml to json") + } - // validate the patch to be a PodSpec - if err := json.Unmarshal([]byte(podSpecPatchJson), &apiv1.PodSpec{}); err != nil { - return nil, fmt.Errorf("invalid podSpecPatch %q: %w", podSpecPatchYaml, err) - } + // validate the patch to be a PodSpec + if err := json.Unmarshal([]byte(podSpecPatchJson), &apiv1.PodSpec{}); err != nil { + return nil, fmt.Errorf("invalid podSpecPatch %q: %w", podSpecPatchYaml, err) + } - modJson, err := strategicpatch.StrategicMergePatch(podSpecJson, []byte(podSpecPatchJson), apiv1.PodSpec{}) - if err != nil { - return nil, errors.Wrap(err, "", "Error occurred during strategic merge patch") + podSpecJson, err = strategicpatch.StrategicMergePatch(podSpecJson, []byte(podSpecPatchJson), apiv1.PodSpec{}) + if err != nil { + return nil, errors.Wrap(err, "", "Error occurred during strategic merge patch") + } } var newPodSpec apiv1.PodSpec - err = json.Unmarshal(modJson, &newPodSpec) + err = json.Unmarshal(podSpecJson, &newPodSpec) if err != nil { return nil, errors.Wrap(err, "", "Error in Unmarshalling after merge the patch") } diff --git a/workflow/util/util_test.go b/workflow/util/util_test.go index d392b84eef20..59aa7c49c1c9 100644 --- a/workflow/util/util_test.go +++ b/workflow/util/util_test.go @@ -167,32 +167,6 @@ func TestReadFromSingleorMultiplePathErrorHandling(t *testing.T) { } } -var yamlStr = ` -containers: - - name: main - resources: - limits: - cpu: 1000m -` - -func TestPodSpecPatchMerge(t *testing.T) { - tmpl := wfv1.Template{PodSpecPatch: "{\"containers\":[{\"name\":\"main\", \"resources\":{\"limits\":{\"cpu\": \"1000m\"}}}]}"} - wf := wfv1.Workflow{Spec: wfv1.WorkflowSpec{PodSpecPatch: "{\"containers\":[{\"name\":\"main\", \"resources\":{\"limits\":{\"memory\": \"100Mi\"}}}]}"}} - merged, err := PodSpecPatchMerge(&wf, &tmpl) - assert.NoError(t, err) - var spec v1.PodSpec - wfv1.MustUnmarshal([]byte(merged), &spec) - assert.Equal(t, "1.000", spec.Containers[0].Resources.Limits.Cpu().AsDec().String()) - assert.Equal(t, "104857600", spec.Containers[0].Resources.Limits.Memory().AsDec().String()) - - tmpl = wfv1.Template{PodSpecPatch: yamlStr} - wf = wfv1.Workflow{Spec: wfv1.WorkflowSpec{PodSpecPatch: "{\"containers\":[{\"name\":\"main\", \"resources\":{\"limits\":{\"memory\": \"100Mi\"}}}]}"}} - merged, err = PodSpecPatchMerge(&wf, &tmpl) - assert.NoError(t, err) - wfv1.MustUnmarshal([]byte(merged), &spec) - assert.Equal(t, "1.000", spec.Containers[0].Resources.Limits.Cpu().AsDec().String()) - assert.Equal(t, "104857600", spec.Containers[0].Resources.Limits.Memory().AsDec().String()) -} var suspendedWf = ` apiVersion: argoproj.io/v1alpha1