argoproj · terrytangyuan · Nov 10, 2022 · Nov 9, 2022
diff --git a/cmd/argo/commands/common/get.go b/cmd/argo/commands/common/get.go
@@ -479,7 +479,9 @@ func printNode(w *tabwriter.Writer, node wfv1.NodeStatus, wfName, nodePrefix str
 	var args []interface{}
 	duration := humanize.RelativeDurationShort(node.StartedAt.Time, node.FinishedAt.Time)
 	if node.Type == wfv1.NodeTypePod {
-		podName := util.PodName(wfName, nodeName, templateName, node.ID, podNameVersion)
+		// node.Name is used here because nodeName may contain additionally formatting.
+		// We want to use the original naming to ensure the correct hash is dervied
+		podName := util.GeneratePodName(wfName, node.Name, templateName, node.ID, podNameVersion)
 		args = []interface{}{nodePrefix, nodeName, templateName, podName, duration, node.Message, ""}
 	} else {
 		args = []interface{}{nodePrefix, nodeName, templateName, "", "", node.Message, ""}

diff --git a/cmd/argo/commands/common/get_test.go b/cmd/argo/commands/common/get_test.go
@@ -3,7 +3,6 @@ package common
 import (
 	"bytes"
 	"fmt"
-	"hash/fnv"
 	"testing"
 	"text/tabwriter"
 	"time"
@@ -16,9 +15,7 @@ import (
 	"github.com/argoproj/argo-workflows/v3/workflow/util"
 )
 
-var (
-	workflowName string = "testWF"
-)
+var workflowName string = "testWF"
 
 func init() {
 	// these values get used as part of determining node name and would normally be set as part of
@@ -63,33 +60,32 @@ func TestPrintNode(t *testing.T) {
 		Time: time.Now(),
 	}
 	node := wfv1.NodeStatus{
-		Name:        nodeName,
-		Phase:       wfv1.NodeRunning,
-		DisplayName: nodeName,
-		Type:        wfv1.NodeTypePod,
-		ID:          nodeID,
-		StartedAt:   timestamp,
-		FinishedAt:  timestamp,
-		Message:     nodeMessage,
+		Name:         nodeName,
+		Phase:        wfv1.NodeRunning,
+		DisplayName:  nodeName,
+		Type:         wfv1.NodeTypePod,
+		ID:           nodeID,
+		StartedAt:    timestamp,
+		FinishedAt:   timestamp,
+		Message:      nodeMessage,
+		TemplateName: nodeTemplateName,
 	}
-
 	node.HostNodeName = kubernetesNodeName
 	// derive expected pod name:
-	h := fnv.New32a()
-	_, _ = h.Write([]byte(fmt.Sprintf("%s %s", JobStatusIconMap[wfv1.NodeRunning], nodeName)))
-	expectedPodName := fmt.Sprintf("%s-%s-%v", workflowName, node.TemplateName, h.Sum32())
-	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t%s\t%s\t%s\t%s\t%s\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, "", expectedPodName, "0s", nodeMessage, ""), node, getArgs)
+	expectedPodName := util.GeneratePodName(workflowName, nodeName, nodeTemplateName, nodeID, util.GetPodNameVersion())
+	t.Log(expectedPodName)
+	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t%s\t%s\t%s\t%s\t%s\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, nodeTemplateName, expectedPodName, "0s", nodeMessage, ""), node, getArgs)
 
 	// Compatibility test
 	getArgs.Status = "Running"
-	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t\t%s\t%s\t%s\t\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, expectedPodName, "0s", nodeMessage), node, getArgs)
+	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t%s\t%s\t%s\t%s\t%s\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, nodeTemplateName, expectedPodName, "0s", nodeMessage, ""), node, getArgs)
 
 	getArgs.Status = ""
 	getArgs.NodeFieldSelectorString = "phase=Running"
-	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t\t%s\t%s\t%s\t\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, expectedPodName, "0s", nodeMessage), node, getArgs)
+	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t%s\t%s\t%s\t%s\t%s\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, nodeTemplateName, expectedPodName, "0s", nodeMessage, ""), node, getArgs)
 
 	getArgs.NodeFieldSelectorString = "phase!=foobar"
-	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t\t%s\t%s\t%s\t\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, expectedPodName, "0s", nodeMessage), node, getArgs)
+	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t%s\t%s\t%s\t%s\t%s\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, nodeTemplateName, expectedPodName, "0s", nodeMessage, ""), node, getArgs)
 
 	getArgs.NodeFieldSelectorString = "phase!=Running"
 	testPrintNodeImpl(t, "", node, getArgs)
@@ -108,7 +104,7 @@ func TestPrintNode(t *testing.T) {
 	}
 
 	node.TemplateName = nodeTemplateName
-	expectedPodName = fmt.Sprintf("%s-%s-%v", workflowName, node.TemplateName, h.Sum32())
+	expectedPodName = util.GeneratePodName(workflowName, nodeName, nodeTemplateName, nodeID, util.GetPodNameVersion())
 	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t%s\t%s\t%s\t%s\t%s\n", JobStatusIconMap[wfv1.NodeRunning], nodeName, nodeTemplateName, expectedPodName, "0s", nodeMessage, ""), node, getArgs)
 
 	node.Type = wfv1.NodeTypeSuspend
@@ -119,7 +115,7 @@ func TestPrintNode(t *testing.T) {
 		Template: nodeTemplateRefName,
 	}
 	templateName := fmt.Sprintf("%s/%s", node.TemplateRef.Name, node.TemplateRef.Template)
-	expectedPodName = fmt.Sprintf("%s-%s-%v", workflowName, templateName, h.Sum32())
+	expectedPodName = util.GeneratePodName(workflowName, nodeName, templateName, nodeID, util.GetPodNameVersion())
 	testPrintNodeImpl(t, fmt.Sprintf("%s %s\t%s/%s\t%s\t%s\t%s\t%s\n", NodeTypeIconMap[wfv1.NodeTypeSuspend], nodeName, nodeTemplateRefName, nodeTemplateRefName, "", "", nodeMessage, ""), node, getArgs)
 
 	getArgs.Output = "wide"
@@ -397,20 +393,14 @@ status:
 		output := PrintWorkflowHelper(&wf, GetFlags{})
 
 		// derive expected pod name:
-		h := fnv.New32a()
-		_, _ = h.Write([]byte(fmt.Sprintf("%s %s", JobStatusIconMap[wfv1.NodeSucceeded], "sleep(9:nine)")))
-		expectedPodName := fmt.Sprintf("many-items-z26lj-sleep-%v", h.Sum32())
-		assert.Contains(t, output, fmt.Sprintf("sleep(9:nine)     sleep           %s   19s", expectedPodName))
+		expectedPodName := util.GeneratePodName(wf.GetObjectMeta().GetName(), "many-items-z26lj[0].sleep(9:nine)", "sleep", "many-items-z26lj-2619926859", util.GetPodNameVersion())
+		assert.Contains(t, output, fmt.Sprintf("sleep(9:nine)     sleep           %s  19s", expectedPodName))
 
-		h.Reset()
-		_, _ = h.Write([]byte(fmt.Sprintf("%s %s", JobStatusIconMap[wfv1.NodeSucceeded], "sleep(10:ten)")))
-		expectedPodName = fmt.Sprintf("many-items-z26lj-sleep-%v", h.Sum32())
+		expectedPodName = util.GeneratePodName(wf.GetObjectMeta().GetName(), "many-items-z26lj[0].sleep(10:ten)", "sleep", "many-items-z26lj-1052882686", util.GetPodNameVersion())
 		assert.Contains(t, output, fmt.Sprintf("sleep(10:ten)     sleep           %s  23s", expectedPodName))
 
-		h.Reset()
-		_, _ = h.Write([]byte(fmt.Sprintf("%s %s", JobStatusIconMap[wfv1.NodeSucceeded], "sleep(11:eleven)")))
-		expectedPodName = fmt.Sprintf("many-items-z26lj-sleep-%v", h.Sum32())
-		assert.Contains(t, output, fmt.Sprintf("sleep(11:eleven)  sleep           %s   22s", expectedPodName))
+		expectedPodName = util.GeneratePodName(wf.GetObjectMeta().GetName(), "many-items-z26lj[0].sleep(11:eleven)", "sleep", "many-items-z26lj-3011405271", util.GetPodNameVersion())
+		assert.Contains(t, output, fmt.Sprintf("sleep(11:eleven)  sleep           %s  22s", expectedPodName))
 	})
 }
 

diff --git a/test/e2e/fixtures/then.go b/test/e2e/fixtures/then.go
@@ -92,7 +92,7 @@ func (t *Then) ExpectWorkflowNode(selector func(status wfv1.NodeStatus) bool, f
 					ObjectMeta: *metadata,
 				}
 				version := util.GetWorkflowPodNameVersion(wf)
-				podName := util.PodName(t.wf.Name, n.Name, n.TemplateName, n.ID, version)
+				podName := util.GeneratePodName(t.wf.Name, n.Name, n.TemplateName, n.ID, version)
 
 				var err error
 				ctx := context.Background()

diff --git a/workflow/controller/exec_control.go b/workflow/controller/exec_control.go
@@ -100,7 +100,7 @@ func (woc *wfOperationCtx) killDaemonedChildren(nodeID string) {
 		if !childNode.IsDaemoned() {
 			continue
 		}
-		podName := util.PodName(woc.wf.Name, childNode.Name, childNode.TemplateName, childNode.ID, util.GetWorkflowPodNameVersion(woc.wf))
+		podName := util.GeneratePodName(woc.wf.Name, childNode.Name, childNode.TemplateName, childNode.ID, util.GetWorkflowPodNameVersion(woc.wf))
 		woc.controller.queuePodForCleanup(woc.wf.Namespace, podName, terminateContainers)
 		childNode.Phase = wfv1.NodeSucceeded
 		childNode.Daemoned = nil

diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
@@ -3737,7 +3737,7 @@ func (woc *wfOperationCtx) substituteGlobalVariables(params common.Parameters) e
 // POD_NAMES environment variable
 func (woc *wfOperationCtx) getPodName(nodeName, templateName string) string {
 	version := wfutil.GetWorkflowPodNameVersion(woc.wf)
-	return wfutil.PodName(woc.wf.Name, nodeName, templateName, woc.wf.NodeID(nodeName), version)
+	return wfutil.GeneratePodName(woc.wf.Name, nodeName, templateName, woc.wf.NodeID(nodeName), version)
 }
 
 func (woc *wfOperationCtx) getServiceAccountTokenName(ctx context.Context, name string) (string, error) {

diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go
@@ -159,7 +159,7 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
 
 	pod := &apiv1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      util.PodName(woc.wf.Name, nodeName, tmpl.Name, nodeID, util.GetWorkflowPodNameVersion(woc.wf)),
+			Name:      util.GeneratePodName(woc.wf.Name, nodeName, tmpl.Name, nodeID, util.GetWorkflowPodNameVersion(woc.wf)),
 			Namespace: woc.wf.ObjectMeta.Namespace,
 			Labels: map[string]string{
 				common.LabelKeyWorkflow:  woc.wf.ObjectMeta.Name, // Allows filtering by pods related to specific workflow

diff --git a/workflow/util/pod_name.go b/workflow/util/pod_name.go
@@ -45,8 +45,8 @@ func GetPodNameVersion() PodNameVersion {
 	}
 }
 
-// PodName return a deterministic pod name
-func PodName(workflowName, nodeName, templateName, nodeID string, version PodNameVersion) string {
+// GeneratePodName return a deterministic pod name
+func GeneratePodName(workflowName, nodeName, templateName, nodeID string, version PodNameVersion) string {
 	if version == PodNameV1 {
 		return nodeID
 	}

diff --git a/workflow/util/pod_name_v1_test.go b/workflow/util/pod_name_v1_test.go
@@ -19,7 +19,7 @@ func TestPodNameV1(t *testing.T) {
 	actual := ensurePodNamePrefixLength(expected)
 	assert.Equal(t, expected, actual)
 
-	name := PodName(shortWfName, nodeName, shortTemplateName, nodeID, PodNameV1)
+	name := GeneratePodName(shortWfName, nodeName, shortTemplateName, nodeID, PodNameV1)
 	assert.Equal(t, nodeID, name)
 
 	// long case
@@ -34,7 +34,7 @@ func TestPodNameV1(t *testing.T) {
 
 	assert.Equal(t, maxK8sResourceNameLength-k8sNamingHashLength-1, len(actual))
 
-	name = PodName(longWfName, nodeName, longTemplateName, nodeID, PodNameV1)
+	name = GeneratePodName(longWfName, nodeName, longTemplateName, nodeID, PodNameV1)
 	assert.Equal(t, nodeID, name)
 
 }
diff --git a/workflow/util/pod_name_v2_test.go b/workflow/util/pod_name_v2_test.go
@@ -27,7 +27,7 @@ func TestPodNameV2(t *testing.T) {
 	_, _ = h.Write([]byte(nodeName))
 	expectedPodName := fmt.Sprintf("wfname-templatename-%v", h.Sum32())
 
-	name := PodName(shortWfName, nodeName, shortTemplateName, nodeID, PodNameV2)
+	name := GeneratePodName(shortWfName, nodeName, shortTemplateName, nodeID, PodNameV2)
 	assert.Equal(t, expectedPodName, name)
 
 	///////////////////////////////////////////////////////////////////////////////////////////
@@ -47,7 +47,7 @@ func TestPodNameV2(t *testing.T) {
 	longPrefix := fmt.Sprintf("%s-%s", longWfName, longTemplateName)
 	expectedPodName = fmt.Sprintf("%s-%v", longPrefix[0:maxK8sResourceNameLength-k8sNamingHashLength-1], h.Sum32())
 
-	name = PodName(longWfName, nodeName, longTemplateName, nodeID, PodNameV2)
+	name = GeneratePodName(longWfName, nodeName, longTemplateName, nodeID, PodNameV2)
 	assert.Equal(t, expectedPodName, name)
 
 }
diff --git a/workflow/util/util.go b/workflow/util/util.go
@@ -757,7 +757,7 @@ func getDescendantNodeIDs(wf *wfv1.Workflow, node wfv1.NodeStatus) []string {
 func deletePodNodeDuringRetryWorkflow(wf *wfv1.Workflow, node wfv1.NodeStatus, deletedPods map[string]bool, podsToDelete []string) (map[string]bool, []string) {
 	templateName := GetTemplateFromNode(node)
 	version := GetWorkflowPodNameVersion(wf)
-	podName := PodName(wf.Name, node.Name, templateName, node.ID, version)
+	podName := GeneratePodName(wf.Name, node.Name, templateName, node.ID, version)
 	if _, ok := deletedPods[podName]; !ok {
 		deletedPods[podName] = true
 		podsToDelete = append(podsToDelete, podName)