Skip to content

Commit b1c51df

Browse files
agilgur5agilgur5
committed
refactor: simplify getPodName and make consistent with back-end (argoproj#12964)
- resolved minor merge conflicts with file relocation, function style, and new lines Signed-off-by: Anton Gilgur <[email protected]> (cherry picked from commit 671320d)
1 parent 5e81697 commit b1c51df

File tree

6 files changed

+64
-104
lines changed

6 files changed

+64
-104
lines changed

‎ui/src/app/shared/pod-name.test.ts‎

Lines changed: 31 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import {Inputs, MemoizationStatus, NodePhase, NodeStatus, NodeType, Outputs, RetryStrategy} from '../../models';
1+
import {NodeStatus, Workflow} from '../../models';
2+
import {ANNOTATION_KEY_POD_NAME_VERSION} from './annotations';
3+
24
import {createFNVHash, ensurePodNamePrefixLength, getPodName, getTemplateNameFromNode, k8sNamingHashLength, maxK8sResourceNameLength, POD_NAME_V1, POD_NAME_V2} from './pod-name';
35

46
describe('pod names', () => {
@@ -9,9 +11,6 @@ describe('pod names', () => {
911
});
1012

1113
// note: the below is intended to be equivalent to the server-side Go code in workflow/util/pod_name_test.go
12-
const nodeName = 'nodename';
13-
const nodeID = '1';
14-
1514
const shortWfName = 'wfname';
1615
const shortTemplateName = 'templatename';
1716

@@ -29,66 +28,51 @@ describe('pod names', () => {
2928
});
3029

3130
test('getPodName', () => {
32-
const v1podName = nodeID;
33-
const v2podName = `${shortWfName}-${shortTemplateName}-${createFNVHash(nodeName)}`;
34-
expect(getPodName(shortWfName, nodeName, shortTemplateName, nodeID, POD_NAME_V2)).toEqual(v2podName);
35-
expect(getPodName(shortWfName, nodeName, shortTemplateName, nodeID, POD_NAME_V1)).toEqual(v1podName);
36-
expect(getPodName(shortWfName, nodeName, shortTemplateName, nodeID, '')).toEqual(v2podName);
37-
expect(getPodName(shortWfName, nodeName, shortTemplateName, nodeID, undefined)).toEqual(v2podName);
31+
const node = {
32+
name: 'nodename',
33+
id: '1',
34+
templateName: shortTemplateName
35+
} as unknown as NodeStatus;
36+
const wf = {
37+
metadata: {
38+
name: shortWfName,
39+
annotations: {
40+
[ANNOTATION_KEY_POD_NAME_VERSION]: POD_NAME_V1
41+
}
42+
}
43+
} as unknown as Workflow;
44+
45+
const v1podName = node.id;
46+
const v2podName = `${shortWfName}-${shortTemplateName}-${createFNVHash(node.name)}`;
47+
48+
expect(getPodName(wf, node)).toEqual(v1podName);
49+
wf.metadata.annotations[ANNOTATION_KEY_POD_NAME_VERSION] = POD_NAME_V2;
50+
expect(getPodName(wf, node)).toEqual(v2podName);
51+
wf.metadata.annotations[ANNOTATION_KEY_POD_NAME_VERSION] = '';
52+
expect(getPodName(wf, node)).toEqual(v2podName);
53+
delete wf.metadata.annotations;
54+
expect(getPodName(wf, node)).toEqual(v2podName);
3855

39-
const name = getPodName(longWfName, nodeName, longTemplateName, nodeID, POD_NAME_V2);
56+
wf.metadata.name = longWfName;
57+
node.templateName = longTemplateName;
58+
const name = getPodName(wf, node);
4059
expect(name.length).toEqual(maxK8sResourceNameLength);
4160
});
4261

4362
test('getTemplateNameFromNode', () => {
4463
// case: no template ref or template name
4564
// expect fallback to empty string
46-
const nodeType: NodeType = 'Pod';
47-
const nodePhase: NodePhase = 'Succeeded';
48-
const retryStrategy: RetryStrategy = {};
49-
const outputs: Outputs = {};
50-
const inputs: Inputs = {};
51-
const memoizationStatus: MemoizationStatus = {
52-
hit: false,
53-
key: 'key',
54-
cacheName: 'cache'
55-
};
56-
57-
const node: NodeStatus = {
58-
id: 'patch-processing-pipeline-ksp78-1623891970',
59-
name: 'patch-processing-pipeline-ksp78.retriable-map-authoring-initializer',
60-
displayName: 'retriable-map-authoring-initializer',
61-
type: nodeType,
62-
templateScope: 'local/',
63-
phase: nodePhase,
64-
boundaryID: '',
65-
message: '',
66-
startedAt: '',
67-
finishedAt: '',
68-
podIP: '',
69-
daemoned: false,
70-
retryStrategy,
71-
outputs,
72-
children: [],
73-
outboundNodes: [],
74-
templateName: '',
75-
inputs,
76-
hostNodeName: '',
77-
memoizationStatus
78-
};
79-
65+
const node = {} as unknown as NodeStatus;
8066
expect(getTemplateNameFromNode(node)).toEqual('');
8167

8268
// case: template ref defined but no template name defined
83-
// expect to return templateRef.template
8469
node.templateRef = {
8570
name: 'test-template-name',
8671
template: 'test-template-template'
8772
};
8873
expect(getTemplateNameFromNode(node)).toEqual(node.templateRef.template);
8974

9075
// case: template name defined
91-
// expect to return templateName
9276
node.templateName = 'test-template';
9377
expect(getTemplateNameFromNode(node)).toEqual(node.templateName);
9478
});

‎ui/src/app/shared/pod-name.ts‎

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,40 @@
1-
import {NodeStatus} from '../../models';
1+
import {NodeStatus, Workflow} from '../../models';
2+
import {ANNOTATION_KEY_POD_NAME_VERSION} from './annotations';
23

34
export const POD_NAME_V1 = 'v1';
45
export const POD_NAME_V2 = 'v2';
56

67
export const maxK8sResourceNameLength = 253;
78
export const k8sNamingHashLength = 10;
9+
const maxPrefixLength = maxK8sResourceNameLength - k8sNamingHashLength;
810

911
// getPodName returns a deterministic pod name
1012
// In case templateName is not defined or that version is explicitly set to POD_NAME_V1, it will return the nodeID (v1)
1113
// In other cases it will return a combination of workflow name, template name, and a hash (v2)
1214
// note: this is intended to be equivalent to the server-side Go code in workflow/util/pod_name.go
13-
export const getPodName = (workflowName: string, nodeName: string, templateName: string, nodeID: string, version: string): string => {
14-
if (version !== POD_NAME_V1 && templateName !== '') {
15-
if (workflowName === nodeName) {
16-
return workflowName;
17-
}
18-
19-
const prefix = ensurePodNamePrefixLength(`${workflowName}-${templateName}`);
15+
export function getPodName(workflow: Workflow, node: NodeStatus): string {
16+
const version = workflow.metadata?.annotations?.[ANNOTATION_KEY_POD_NAME_VERSION];
17+
if (version === POD_NAME_V1) {
18+
return node.id;
19+
}
2020

21-
const hash = createFNVHash(nodeName);
22-
return `${prefix}-${hash}`;
21+
const workflowName = workflow.metadata.name;
22+
if (workflowName === node.name) {
23+
return workflowName;
2324
}
2425

25-
return nodeID;
26-
};
26+
const templateName = getTemplateNameFromNode(node);
27+
let prefix = workflowName;
28+
if (templateName) {
29+
prefix += `-${templateName}`;
30+
}
31+
prefix = ensurePodNamePrefixLength(prefix);
2732

28-
export const ensurePodNamePrefixLength = (prefix: string): string => {
29-
const maxPrefixLength = maxK8sResourceNameLength - k8sNamingHashLength;
33+
const hash = createFNVHash(node.name);
34+
return `${prefix}-${hash}`;
35+
}
3036

37+
export function ensurePodNamePrefixLength(prefix: string): string {
3138
if (prefix.length > maxPrefixLength - 1) {
3239
return prefix.substring(0, maxPrefixLength - 1);
3340
}
@@ -49,15 +56,7 @@ export const createFNVHash = (input: string): number => {
4956
/* tslint:enable:no-bitwise */
5057
};
5158

52-
export const getTemplateNameFromNode = (node: NodeStatus): string => {
53-
if (node.templateName && node.templateName !== '') {
54-
return node.templateName;
55-
}
56-
59+
export function getTemplateNameFromNode(node: NodeStatus): string {
5760
// fall back to v1 pod names if no templateName or templateRef defined
58-
if (node?.templateRef === undefined || node?.templateRef.template === '') {
59-
return '';
60-
}
61-
62-
return node.templateRef.template;
63-
};
61+
return node.templateName || node.templateRef?.template || '';
62+
}

‎ui/src/app/workflows/components/workflow-details/workflow-details.tsx‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import classNames from 'classnames';
33
import * as React from 'react';
44
import {useContext, useEffect, useRef, useState} from 'react';
55
import {RouteComponentProps} from 'react-router';
6-
import {archivalStatus, ArtifactRepository, execSpec, isArchivedWorkflow, isWorkflowInCluster, Link, NodeStatus, Parameter, Workflow} from '../../../../models';
7-
import {ANNOTATION_KEY_POD_NAME_VERSION} from '../../../shared/annotations';
6+
7+
import {archivalStatus, ArtifactRepository, execSpec, isArchivedWorkflow, isWorkflowInCluster, Link, Parameter, Workflow} from '../../../../models';
88
import {artifactRepoHasLocation, findArtifact} from '../../../shared/artifacts';
99
import {uiUrl} from '../../../shared/base';
1010
import {CostOptimisationNudge} from '../../../shared/components/cost-optimisation-nudge';
@@ -16,7 +16,7 @@ import {useCollectEvent} from '../../../shared/components/use-collect-event';
1616
import {hasArtifactGCError, hasWarningConditionBadge} from '../../../shared/conditions-panel';
1717
import {Context} from '../../../shared/context';
1818
import {historyUrl} from '../../../shared/history';
19-
import {getPodName, getTemplateNameFromNode} from '../../../shared/pod-name';
19+
import {getPodName} from '../../../shared/pod-name';
2020
import {RetryWatch} from '../../../shared/retry-watch';
2121
import {services} from '../../../shared/services';
2222
import {getResolvedTemplates} from '../../../shared/template-resolution';
@@ -473,18 +473,7 @@ export function WorkflowDetails({history, location, match}: RouteComponentProps<
473473
});
474474
}
475475

476-
function ensurePodName(wf: Workflow, node: NodeStatus, nodeID: string): string {
477-
if (workflow && node) {
478-
const annotations = workflow.metadata.annotations || {};
479-
const version = annotations[ANNOTATION_KEY_POD_NAME_VERSION];
480-
const templateName = getTemplateNameFromNode(node);
481-
return getPodName(wf.metadata.name, node.name, templateName, node.id, version);
482-
}
483-
484-
return nodeID;
485-
}
486-
487-
const podName = ensurePodName(workflow, selectedNode, nodeId);
476+
const podName = workflow && selectedNode ? getPodName(workflow, selectedNode) : nodeId;
488477

489478
const archived = isArchivedWorkflow(workflow);
490479

‎ui/src/app/workflows/components/workflow-logs-viewer/workflow-logs-viewer.tsx‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,13 @@ import {Observable} from 'rxjs';
66
import {map, publishReplay, refCount} from 'rxjs/operators';
77
import * as models from '../../../../models';
88
import {execSpec} from '../../../../models';
9-
import {ANNOTATION_KEY_POD_NAME_VERSION} from '../../../shared/annotations';
109
import {Button} from '../../../shared/components/button';
1110
import {ErrorNotice} from '../../../shared/components/error-notice';
1211
import {InfoIcon, WarningIcon} from '../../../shared/components/fa-icons';
1312
import {Links} from '../../../shared/components/links';
1413
import {Context} from '../../../shared/context';
1514
import {useLocalStorage} from '../../../shared/hooks/uselocalstorage';
16-
import {getPodName, getTemplateNameFromNode} from '../../../shared/pod-name';
15+
import {getPodName} from '../../../shared/pod-name';
1716
import {ScopedLocalStorage} from '../../../shared/scoped-local-storage';
1817
import {services} from '../../../shared/services';
1918
import {FullHeightLogsViewer} from './full-height-logs-viewer';
@@ -157,18 +156,14 @@ export function WorkflowLogsViewer({workflow, initialNodeId, initialPodName, con
157156
return () => clearTimeout(x);
158157
}, [logFilter]);
159158

160-
const annotations = workflow.metadata.annotations || {};
161-
const podNameVersion = annotations[ANNOTATION_KEY_POD_NAME_VERSION];
162-
163159
// map pod names to corresponding node IDs
164160
const podNamesToNodeIDs = new Map<string, string>();
165161
const podNames = [{value: '', label: 'All'}].concat(
166162
Object.values(workflow.status.nodes || {})
167163
.filter(x => x.type === 'Pod')
168164
.map(targetNode => {
169165
const {name, id, displayName} = targetNode;
170-
const templateName = getTemplateNameFromNode(targetNode);
171-
const targetPodName = getPodName(workflow.metadata.name, name, templateName, id, podNameVersion);
166+
const targetPodName = getPodName(workflow, targetNode);
172167
podNamesToNodeIDs.set(targetPodName, id);
173168
return {value: targetPodName, label: (displayName || name) + ' (' + targetPodName + ')'};
174169
})

‎ui/src/app/workflows/components/workflow-node-info/workflow-node-info.tsx‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,14 @@ import {useState} from 'react';
55

66
import * as models from '../../../../models';
77
import {Artifact, NodeStatus, Workflow} from '../../../../models';
8-
import {ANNOTATION_KEY_POD_NAME_VERSION} from '../../../shared/annotations';
98
import {Button} from '../../../shared/components/button';
109
import {ClipboardText} from '../../../shared/components/clipboard-text';
1110
import {DurationPanel} from '../../../shared/components/duration-panel';
1211
import {InlineTable} from '../../../shared/components/inline-table/inline-table';
1312
import {Links} from '../../../shared/components/links';
1413
import {Phase} from '../../../shared/components/phase';
1514
import {Timestamp} from '../../../shared/components/timestamp';
16-
import {getPodName, getTemplateNameFromNode} from '../../../shared/pod-name';
15+
import {getPodName} from '../../../shared/pod-name';
1716
import {ResourcesDuration} from '../../../shared/resources-duration';
1817
import {services} from '../../../shared/services';
1918
import {getResolvedTemplates} from '../../../shared/template-resolution';
@@ -100,12 +99,7 @@ const DisplayWorkflowTime = (props: {date: Date | string | number}) => {
10099

101100
const WorkflowNodeSummary = (props: Props) => {
102101
const {workflow, node} = props;
103-
104-
const annotations = workflow.metadata.annotations || {};
105-
const version = annotations[ANNOTATION_KEY_POD_NAME_VERSION];
106-
const templateName = getTemplateNameFromNode(node);
107-
108-
const podName = getPodName(workflow.metadata.name, node.name, templateName, node.id, version);
102+
const podName = getPodName(workflow, node);
109103

110104
const attributes = [
111105
{title: 'NAME', value: <ClipboardText text={props.node.name} />},

‎workflow/util/pod_name.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
const (
1414
maxK8sResourceNameLength = 253
1515
k8sNamingHashLength = 10
16+
maxPrefixLength = maxK8sResourceNameLength - k8sNamingHashLength
1617
)
1718

1819
// PodNameVersion stores which type of pod names should be used.
@@ -70,8 +71,6 @@ func GeneratePodName(workflowName, nodeName, templateName, nodeID string, versio
7071
}
7172

7273
func ensurePodNamePrefixLength(prefix string) string {
73-
maxPrefixLength := maxK8sResourceNameLength - k8sNamingHashLength
74-
7574
if len(prefix) > maxPrefixLength-1 {
7675
return prefix[0 : maxPrefixLength-1]
7776
}

0 commit comments

Comments
 (0)