From 932466540a109550b98714f41a5c6e1d3fc13158 Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Fri, 4 Feb 2022 12:06:40 -0500 Subject: [PATCH] fix: Use v1 pod name if no template name or ref. Fixes #7595 and #7749 (#7605) Signed-off-by: J.P. Zivalich --- ui/src/app/shared/pod-name.test.ts | 56 +++++++++++++++++++++++++++++- ui/src/app/shared/pod-name.ts | 12 +++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/ui/src/app/shared/pod-name.test.ts b/ui/src/app/shared/pod-name.test.ts index 6aed683c5f44..01ed2d5070c5 100644 --- 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 index 09dd2f94fda9..39eb900c1a8d 100644 --- 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; };