fix(controller): use correct pod.name in retry/podspecpatch scenario. F…

…ixes #7007 (#7008)

Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>

workflow/controller/operator.go

@@ -1621,7 +1621,7 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string,
 	// Inject the pod name. If the pod has a retry strategy, the pod name will be changed and will be injected when it
 	// is determined
 	if resolvedTmpl.IsPodType() && woc.retryStrategy(resolvedTmpl) == nil {
-		localParams[common.LocalVarPodName] = woc.wf.NodeID(nodeName)
+		localParams[common.LocalVarPodName] = wfutil.PodName(woc.wf.Name, nodeName, resolvedTmpl.Name, woc.wf.NodeID(nodeName))
 	}
 
 	// Merge Template defaults to template
@@ -1810,7 +1810,7 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string,
 			localParams := make(map[string]string)
 			// Change the `pod.name` variable to the new retry node name
 			if processedTmpl.IsPodType() {
-				localParams[common.LocalVarPodName] = woc.wf.NodeID(nodeName)
+				localParams[common.LocalVarPodName] = wfutil.PodName(woc.wf.Name, nodeName, processedTmpl.Name, woc.wf.NodeID(nodeName))
 			}
 			// Inject the retryAttempt number
 			localParams[common.LocalVarRetries] = strconv.Itoa(len(retryParentNode.Children))

workflow/controller/operator_test.go

@@ -2788,20 +2788,30 @@ spec:
 `
 
 func TestResolvePodNameInRetries(t *testing.T) {
-	ctx := context.Background()
-	wf := wfv1.MustUnmarshalWorkflow(podNameInRetries)
-	woc := newWoc(*wf)
-	woc.operate(ctx)
-	assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase)
-	pods, err := woc.controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(ctx, metav1.ListOptions{})
-	assert.NoError(t, err)
-	assert.True(t, len(pods.Items) > 0, "pod was not created successfully")
+	tests := []struct {
+		podNameVersion string
+		wantPodName    string
+	}{
+		{"v1", "output-value-placeholders-wf-3033990984"},
+		{"v2", "output-value-placeholders-wf-tell-pod-name-3033990984"},
+	}
+	for _, tt := range tests {
+		os.Setenv("POD_NAMES", tt.podNameVersion)
+		ctx := context.Background()
+		wf := wfv1.MustUnmarshalWorkflow(podNameInRetries)
+		woc := newWoc(*wf)
+		woc.operate(ctx)
+		assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase)
+		pods, err := woc.controller.kubeclientset.CoreV1().Pods(wf.ObjectMeta.Namespace).List(ctx, metav1.ListOptions{})
+		assert.NoError(t, err)
+		assert.True(t, len(pods.Items) > 0, "pod was not created successfully")
 
-	template, err := getPodTemplate(&pods.Items[0])
-	assert.NoError(t, err)
-	parameterValue := template.Outputs.Parameters[0].Value
-	assert.NotNil(t, parameterValue)
-	assert.Equal(t, "output-value-placeholders-wf-3033990984", parameterValue.String())
+		template, err := getPodTemplate(&pods.Items[0])
+		assert.NoError(t, err)
+		parameterValue := template.Outputs.Parameters[0].Value
+		assert.NotNil(t, parameterValue)
+		assert.Equal(t, tt.wantPodName, parameterValue.String())
+	}
 }
 
 var outputStatuses = `

workflow/controller/workflowpod.go

@@ -377,7 +377,7 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
 		// Final substitution for workflow level PodSpecPatch
 		localParams := make(map[string]string)
 		if tmpl.IsPodType() {
-			localParams[common.LocalVarPodName] = woc.wf.NodeID(nodeName)
+			localParams[common.LocalVarPodName] = pod.Name
 		}
 		tmpl, err := common.ProcessArgs(tmpl, &wfv1.Arguments{}, woc.globalParams, localParams, false, woc.wf.Namespace, woc.controller.configMapInformer)
 		if err != nil {

workflow/controller/workflowpod_test.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 	"strconv"
 	"testing"
@@ -1223,6 +1224,10 @@ spec:
       image: docker/whalesay:latest
       command: [cowsay]
       args: ["hello world"]
+    outputs:
+      parameters:
+      - name: pod-name
+        value: "{{pod.name}}"
 `
 
 var helloWorldWfWithWFPatch = `
@@ -1308,6 +1313,59 @@ func TestPodSpecPatch(t *testing.T) {
 	assert.EqualError(t, err, "Failed to merge the workflow PodSpecPatch with the template PodSpecPatch due to invalid format")
 }
 
+var helloWorldStepWfWithPatch = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-world
+spec:
+  entrypoint: hello
+  templates:
+  - name: hello
+    steps:
+    - - name: hello
+        template: whalesay
+  - name: whalesay
+    podSpecPatch: '{"containers":[{"name":"main", "resources":{"limits":{"cpu": "800m"}}}]}'
+    container:
+      image: docker/whalesay:latest
+      command: [cowsay]
+      args: ["hello world"]
+    outputs:
+      parameters:
+      - name: pod-name
+        value: "{{pod.name}}"
+`
+
+func TestPodSpecPatchPodName(t *testing.T) {
+	tests := []struct {
+		podNameVersion string
+		wantPodName    string
+		workflowYaml   string
+	}{
+		{"v1", "hello-world", helloWorldWfWithPatch},
+		{"v2", "hello-world", helloWorldWfWithPatch},
+		{"v1", "hello-world-3731220306", helloWorldStepWfWithPatch},
+		{"v2", "hello-world-whalesay-3731220306", helloWorldStepWfWithPatch},
+	}
+	for _, tt := range tests {
+		os.Setenv("POD_NAMES", tt.podNameVersion)
+		ctx := context.Background()
+		wf := wfv1.MustUnmarshalWorkflow(tt.workflowYaml)
+		woc := newWoc(*wf)
+		woc.operate(ctx)
+		assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase)
+		pods, err := listPods(woc)
+		assert.NoError(t, err)
+		assert.True(t, len(pods.Items) > 0, "pod was not created successfully")
+		template, err := getPodTemplate(&pods.Items[0])
+		assert.NoError(t, err)
+		parameterValue := template.Outputs.Parameters[0].Value
+		assert.NotNil(t, parameterValue)
+		assert.Equal(t, tt.wantPodName, parameterValue.String())
+	}
+}
+
 func TestMainContainerCustomization(t *testing.T) {
 	mainCtrSpec := &apiv1.Container{
 		Name: common.MainContainerName,