fix: Use v1 pod name if no template name or ref. Fixes argoproj#7595 …

…and argoproj#7749 (argoproj#7605) Signed-off-by: J.P. Zivalich <[email protected]>
CashWilliams · Feb 4, 2022 · 9324665 · 9324665
1 parent e9b873a
commit 9324665
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 3 deletions.
diff --git a/ui/src/app/shared/pod-name.test.ts b/ui/src/app/shared/pod-name.test.ts
@@ -1,4 +1,5 @@
-import {createFNVHash, ensurePodNamePrefixLength, getPodName, k8sNamingHashLength, maxK8sResourceNameLength, POD_NAME_V1, POD_NAME_V2} from './pod-name';
+import {Inputs, MemoizationStatus, NodePhase, NodeStatus, NodeType, Outputs, RetryStrategy} from '../../models';
+import {createFNVHash, ensurePodNamePrefixLength, getPodName, getTemplateNameFromNode, k8sNamingHashLength, maxK8sResourceNameLength, POD_NAME_V1, POD_NAME_V2} from './pod-name';
 
 describe('pod names', () => {
  test('createFNVHash', () => {
@@ -34,4 +35,57 @@ describe('pod names', () => {
  const name = getPodName(longWfName, nodeName, longTemplateName, nodeID, POD_NAME_V2);
  expect(name.length).toEqual(maxK8sResourceNameLength);
  });
+
+ test('getTemplateNameFromNode', () => {
+ // case: no template ref or template name
+ // expect fallback to empty string
+ const nodeType: NodeType = 'Pod';
+ const nodePhase: NodePhase = 'Succeeded';
+ const retryStrategy: RetryStrategy = {};
+ const outputs: Outputs = {};
+ const inputs: Inputs = {};
+ const memoizationStatus: MemoizationStatus = {
+ hit: false,
+ key: 'key',
+ cacheName: 'cache'
+ };
+
+ const node: NodeStatus = {
+ id: 'patch-processing-pipeline-ksp78-1623891970',
+ name: 'patch-processing-pipeline-ksp78.retriable-map-authoring-initializer',
+ displayName: 'retriable-map-authoring-initializer',
+ type: nodeType,
+ templateScope: 'local/',
+ phase: nodePhase,
+ boundaryID: '',
+ message: '',
+ startedAt: '',
+ finishedAt: '',
+ podIP: '',
+ daemoned: false,
+ retryStrategy,
+ outputs,
+ children: [],
+ outboundNodes: [],
+ templateName: '',
+ inputs,
+ hostNodeName: '',
+ memoizationStatus
+ };
+
+ expect(getTemplateNameFromNode(node)).toEqual('');
+
+ // case: template ref defined but no template name defined
+ // expect to return templateRef.template
+ node.templateRef = {
+ name: 'test-template-name',
+ template: 'test-template-template'
+ };
+ expect(getTemplateNameFromNode(node)).toEqual(node.templateRef.template);
+
+ // case: template name defined
+ // expect to return templateName
+ node.templateName = 'test-template';
+ expect(getTemplateNameFromNode(node)).toEqual(node.templateName);
+ });
 });
diff --git a/ui/src/app/shared/pod-name.ts b/ui/src/app/shared/pod-name.ts
@@ -6,9 +6,12 @@ export const POD_NAME_V2 = 'v2';
 export const maxK8sResourceNameLength = 253;
 export const k8sNamingHashLength = 10;
 
-// getPodName returns a deterministic pod name
+// getPodName returns a deterministic pod name. It returns a combination of the
+// workflow name, template name, and a hash if the POD_NAME_V2 annotation is
+// set. If the templateName or templateRef is not defined on a given node, it
+// falls back to POD_NAME_V1
 export const getPodName = (workflowName: string, nodeName: string, templateName: string, nodeID: string, version: string): string => {
- if (version === POD_NAME_V2) {
+ if (version === POD_NAME_V2 && templateName !== '') {
  if (workflowName === nodeName) {
  return workflowName;
  }
@@ -51,5 +54,10 @@ export const getTemplateNameFromNode = (node: NodeStatus): string => {
  return node.templateName;
  }
 
+ // fall back to v1 pod names if no templateName or templateRef defined
+ if (!node.templateRef) {
+ return '';
+ }
+
  return node.templateRef.template;
 };