fix: Match cli display pod names with k8s. Fixes #7646 (#7653)

* fix: Match cli display pod names with k8s. Fixes #7646 Signed-off-by: J.P. Zivalich <jp@pipekit.io> * fix: Remove stray console statement Signed-off-by: J.P. Zivalich <jp@pipekit.io> * fix: Factor out pod name version in pod names fn Signed-off-by: J.P. Zivalich <jp@pipekit.io> * refactor: Use GetPodNameVersion in tests Signed-off-by: J.P. Zivalich <jp@pipekit.io> * refactor: Use wf object in test fixture Signed-off-by: J.P. Zivalich <jp@pipekit.io>
argoproj · Jan 27, 2022 · 1159afc · 1159afc
1 parent f2e15ab
commit 1159afc
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 16 deletions.
diff --git a/cmd/argo/commands/get.go b/cmd/argo/commands/get.go
@@ -524,7 +524,7 @@ func renderChild(w *tabwriter.Writer, wf *wfv1.Workflow, nInfo renderNode, depth
 }
 
 // Main method to print information of node in get
-func printNode(w *tabwriter.Writer, node wfv1.NodeStatus, nodePrefix string, getArgs getFlags) {
+func printNode(w *tabwriter.Writer, node wfv1.NodeStatus, wfName, nodePrefix string, getArgs getFlags, podNameVersion util.PodNameVersion) {
 	nodeName := fmt.Sprintf("%s %s", jobStatusIconMap[node.Phase], node.DisplayName)
 	if node.IsActiveSuspendNode() {
 		nodeName = fmt.Sprintf("%s %s", nodeTypeIconMap[node.Type], node.DisplayName)
@@ -538,7 +538,8 @@ func printNode(w *tabwriter.Writer, node wfv1.NodeStatus, nodePrefix string, get
 	var args []interface{}
 	duration := humanize.RelativeDurationShort(node.StartedAt.Time, node.FinishedAt.Time)
 	if node.Type == wfv1.NodeTypePod {
-		args = []interface{}{nodePrefix, nodeName, templateName, node.ID, duration, node.Message, ""}
+		podName := util.PodName(wfName, nodeName, templateName, node.ID, podNameVersion)
+		args = []interface{}{nodePrefix, nodeName, templateName, podName, duration, node.Message, ""}
 	} else {
 		args = []interface{}{nodePrefix, nodeName, templateName, "", "", node.Message, ""}
 	}
@@ -566,7 +567,8 @@ func printNode(w *tabwriter.Writer, node wfv1.NodeStatus, nodePrefix string, get
 func (nodeInfo *boundaryNode) renderNodes(w *tabwriter.Writer, wf *wfv1.Workflow, depth int, nodePrefix string, childPrefix string, getArgs getFlags) {
 	filtered, childIndent := filterNode(nodeInfo.getNodeStatus(wf), getArgs)
 	if !filtered {
-		printNode(w, nodeInfo.getNodeStatus(wf), nodePrefix, getArgs)
+		version := util.GetWorkflowPodNameVersion(wf)
+		printNode(w, nodeInfo.getNodeStatus(wf), wf.ObjectMeta.Name, nodePrefix, getArgs, version)
 	}
 
 	for i, nInfo := range nodeInfo.boundaryContained {
@@ -579,7 +581,8 @@ func (nodeInfo *boundaryNode) renderNodes(w *tabwriter.Writer, wf *wfv1.Workflow
 func (nodeInfo *nonBoundaryParentNode) renderNodes(w *tabwriter.Writer, wf *wfv1.Workflow, depth int, nodePrefix string, childPrefix string, getArgs getFlags) {
 	filtered, childIndent := filterNode(nodeInfo.getNodeStatus(wf), getArgs)
 	if !filtered {
-		printNode(w, nodeInfo.getNodeStatus(wf), nodePrefix, getArgs)
+		version := util.GetWorkflowPodNameVersion(wf)
+		printNode(w, nodeInfo.getNodeStatus(wf), wf.ObjectMeta.Name, nodePrefix, getArgs, version)
 	}
 
 	for i, nInfo := range nodeInfo.children {
@@ -592,7 +595,8 @@ func (nodeInfo *nonBoundaryParentNode) renderNodes(w *tabwriter.Writer, wf *wfv1
 func (nodeInfo *executionNode) renderNodes(w *tabwriter.Writer, wf *wfv1.Workflow, _ int, nodePrefix string, _ string, getArgs getFlags) {
 	filtered, _ := filterNode(nodeInfo.getNodeStatus(wf), getArgs)
 	if !filtered {
-		printNode(w, nodeInfo.getNodeStatus(wf), nodePrefix, getArgs)
+		version := util.GetWorkflowPodNameVersion(wf)
+		printNode(w, nodeInfo.getNodeStatus(wf), wf.ObjectMeta.Name, nodePrefix, getArgs, version)
 	}
 }
 

diff --git a/cmd/argo/commands/get_test.go b/cmd/argo/commands/get_test.go
@@ -12,14 +12,15 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo-workflows/v3/workflow/util"
 )
 
 func testPrintNodeImpl(t *testing.T, expected string, node wfv1.NodeStatus, getArgs getFlags) {
 	var result bytes.Buffer
 	w := tabwriter.NewWriter(&result, 0, 8, 1, '\t', 0)
 	filtered, _ := filterNode(node, getArgs)
 	if !filtered {
-		printNode(w, node, "", getArgs)
+		printNode(w, node, "testWf", "", getArgs, util.GetPodNameVersion())
 	}
 	err := w.Flush()
 	assert.NoError(t, err)

diff --git a/test/e2e/fixtures/then.go b/test/e2e/fixtures/then.go
@@ -86,9 +86,14 @@ func (t *Then) ExpectWorkflowNode(selector func(status wfv1.NodeStatus) bool, f
 		if n != nil {
 			_, _ = fmt.Println("Found node", "id="+n.ID, "type="+n.Type)
 			if n.Type == wfv1.NodeTypePod {
+				wf := &wfv1.Workflow{
+					ObjectMeta: *metadata,
+				}
+				version := util.GetWorkflowPodNameVersion(wf)
+				podName := util.PodName(t.wf.Name, n.Name, n.TemplateName, n.ID, version)
+
 				var err error
 				ctx := context.Background()
-				podName := util.PodName(t.wf.Name, n.Name, n.TemplateName, n.ID)
 				p, err = t.kubeClient.CoreV1().Pods(t.wf.Namespace).Get(ctx, podName, metav1.GetOptions{})
 				if err != nil {
 					if !apierr.IsNotFound(err) {

diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
@@ -1623,7 +1623,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] = wfutil.PodName(woc.wf.Name, nodeName, resolvedTmpl.Name, woc.wf.NodeID(nodeName))
+		localParams[common.LocalVarPodName] = woc.getPodName(nodeName, resolvedTmpl.Name)
 	}
 
 	// Merge Template defaults to template
@@ -1814,7 +1814,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] = wfutil.PodName(woc.wf.Name, nodeName, processedTmpl.Name, woc.wf.NodeID(nodeName))
+				localParams[common.LocalVarPodName] = woc.getPodName(nodeName, processedTmpl.Name)
 			}
 			// Inject the retryAttempt number
 			localParams[common.LocalVarRetries] = strconv.Itoa(len(retryParentNode.Children))
@@ -2218,7 +2218,8 @@ func (woc *wfOperationCtx) getPodByNode(node *wfv1.NodeStatus) (*apiv1.Pod, erro
 	if node.Type != wfv1.NodeTypePod {
 		return nil, fmt.Errorf("Expected node type %s, got %s", wfv1.NodeTypePod, node.Type)
 	}
-	podName := wfutil.PodName(woc.wf.Name, node.Name, node.TemplateName, node.ID)
+
+	podName := woc.getPodName(node.Name, node.TemplateName)
 	return woc.controller.getPod(woc.wf.GetNamespace(), podName)
 }
 
@@ -3552,6 +3553,12 @@ func (woc *wfOperationCtx) substituteGlobalVariables() error {
 	return nil
 }
 
+// getPodName gets the appropriate pod name for a workflow based on the
+// POD_NAMES environment variable
+func (woc *wfOperationCtx) getPodName(nodeName, templateName string) string {
+	return wfutil.PodName(woc.wf.Name, nodeName, templateName, woc.wf.NodeID(nodeName), wfutil.GetPodNameVersion())
+}
+
 // setWfPodNamesAnnotation sets an annotation on a workflow with the pod naming
 // convention version
 func setWfPodNamesAnnotation(wf *wfv1.Workflow) {

diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go
@@ -204,7 +204,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),
+			Name:      util.PodName(woc.wf.Name, nodeName, tmpl.Name, nodeID, util.GetPodNameVersion()),
 			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
@@ -4,6 +4,9 @@ import (
 	"fmt"
 	"hash/fnv"
 	"os"
+
+	"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo-workflows/v3/workflow/common"
 )
 
 const (
@@ -42,8 +45,8 @@ func GetPodNameVersion() PodNameVersion {
 }
 
 // PodName return a deterministic pod name
-func PodName(workflowName, nodeName, templateName, nodeID string) string {
-	if GetPodNameVersion() == PodNameV1 {
+func PodName(workflowName, nodeName, templateName, nodeID string, version PodNameVersion) string {
+	if version == PodNameV1 {
 		return nodeID
 	}
 
@@ -68,3 +71,16 @@ func ensurePodNamePrefixLength(prefix string) string {
 
 	return prefix
 }
+
+// GetWorkflowPodNameVersion gets the pod name version from the annotation of a
+// given workflow
+func GetWorkflowPodNameVersion(wf *v1alpha1.Workflow) PodNameVersion {
+	annotations := wf.GetAnnotations()
+	version := annotations[common.AnnotationKeyPodNameVersion]
+
+	if version == PodNameV2.String() {
+		return PodNameV2
+	}
+
+	return PodNameV1
+}
diff --git a/workflow/util/pod_name_test.go b/workflow/util/pod_name_test.go
@@ -19,7 +19,7 @@ func TestPodName(t *testing.T) {
 	actual := ensurePodNamePrefixLength(expected)
 	assert.Equal(t, expected, actual)
 
-	name := PodName(shortWfName, nodeName, shortTemplateName, nodeID)
+	name := PodName(shortWfName, nodeName, shortTemplateName, nodeID, GetPodNameVersion())
 	assert.Equal(t, nodeID, name)
 
 	// long case
@@ -34,6 +34,6 @@ func TestPodName(t *testing.T) {
 
 	assert.Equal(t, maxK8sResourceNameLength-k8sNamingHashLength-1, len(actual))
 
-	name = PodName(longWfName, nodeName, longTemplateName, nodeID)
+	name = PodName(longWfName, nodeName, longTemplateName, nodeID, GetPodNameVersion())
 	assert.Equal(t, nodeID, name)
 }
diff --git a/workflow/util/util.go b/workflow/util/util.go
@@ -822,7 +822,8 @@ func retryWorkflow(ctx context.Context, kubeClient kubernetes.Interface, hydrato
 		}
 		if node.Type == wfv1.NodeTypePod {
 			templateName := getTemplateFromNode(node)
-			podName := PodName(wf.Name, node.Name, templateName, node.ID)
+			version := GetWorkflowPodNameVersion(wf)
+			podName := PodName(wf.Name, node.Name, templateName, node.ID, version)
 			log.Infof("Deleting pod: %s", podName)
 			err := podIf.Delete(ctx, podName, metav1.DeleteOptions{})
 			if err != nil && !apierr.IsNotFound(err) {