From 97a12fb877a5d6d070ea873898b55d84fff3662f Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 17 Mar 2026 15:28:14 -0700 Subject: [PATCH 1/2] Canonicalize deploy job names --- .../deploymentManager/deploymentManager.ts | 7 +- .../lib/kubernetes/__tests__/jobNames.test.ts | 52 ++++++++ src/server/lib/kubernetes/jobNames.ts | 47 +++++++ .../lib/kubernetesApply/applyManifest.ts | 7 +- .../lib/nativeHelm/__tests__/helm.test.ts | 120 +++++++++++++++++- src/server/lib/nativeHelm/helm.ts | 25 ++-- .../lib/tests/deploymentManager.test.ts | 60 +++++++++ 7 files changed, 302 insertions(+), 16 deletions(-) create mode 100644 src/server/lib/kubernetes/__tests__/jobNames.test.ts create mode 100644 src/server/lib/kubernetes/jobNames.ts diff --git a/src/server/lib/deploymentManager/deploymentManager.ts b/src/server/lib/deploymentManager/deploymentManager.ts index 0320930..92a3557 100644 --- a/src/server/lib/deploymentManager/deploymentManager.ts +++ b/src/server/lib/deploymentManager/deploymentManager.ts @@ -23,6 +23,7 @@ import DeployService from 'server/services/deploy'; import { getLogger, withLogContext } from 'server/lib/logger'; import { ensureServiceAccountForJob } from '../kubernetes/common/serviceAccount'; import { waitForDeployPodReady } from '../kubernetes'; +import { buildDeployJobName } from '../kubernetes/jobNames'; const generateJobId = customAlphabet('abcdefghijklmnopqrstuvwxyz0123456789', 6); @@ -164,7 +165,11 @@ export class DeploymentManager { }); const shortSha = deploy.sha?.substring(0, 7) || 'unknown'; - const jobName = `${deploy.uuid}-deploy-${jobId}-${shortSha}`; + const jobName = buildDeployJobName({ + deployUuid: deploy.uuid, + jobId, + shortSha, + }); const result = await monitorKubernetesJob(jobName, deploy.build.namespace); if (!result.success) { diff --git a/src/server/lib/kubernetes/__tests__/jobNames.test.ts b/src/server/lib/kubernetes/__tests__/jobNames.test.ts new file mode 100644 index 0000000..b72fdbb --- /dev/null +++ b/src/server/lib/kubernetes/__tests__/jobNames.test.ts @@ -0,0 +1,52 @@ +/** + * Copyright 2025 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { buildDeployJobName, KUBERNETES_NAME_MAX_LENGTH } from '../jobNames'; + +describe('buildDeployJobName', () => { + it('preserves deploy job names that already fit', () => { + const jobName = buildDeployJobName({ + deployUuid: 'api-crimson-tooth-697165', + jobId: 'k4hlde', + shortSha: '28e350a', + }); + + expect(jobName).toBe('api-crimson-tooth-697165-deploy-k4hlde-28e350a'); + }); + + it('truncates only the prefix and preserves the full suffix', () => { + const jobName = buildDeployJobName({ + deployUuid: 'cyclerx-cosmosdb-emulator-crimson-tooth-697165', + jobId: 'k4hlde', + shortSha: '28e350a', + }); + + expect(jobName).toHaveLength(KUBERNETES_NAME_MAX_LENGTH); + expect(jobName).toBe('cyclerx-cosmosdb-emulator-crimson-tooth-6-deploy-k4hlde-28e350a'); + expect(jobName.endsWith('deploy-k4hlde-28e350a')).toBe(true); + }); + + it('removes trailing separators after truncation', () => { + const jobName = buildDeployJobName({ + deployUuid: 'service-ending-with-dash------crimson-tooth-697165', + jobId: 'job123', + shortSha: 'abcdef0', + }); + + expect(jobName).not.toContain('--deploy-'); + expect(jobName.endsWith('-')).toBe(false); + }); +}); diff --git a/src/server/lib/kubernetes/jobNames.ts b/src/server/lib/kubernetes/jobNames.ts new file mode 100644 index 0000000..0eba1b4 --- /dev/null +++ b/src/server/lib/kubernetes/jobNames.ts @@ -0,0 +1,47 @@ +/** + * Copyright 2025 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export const KUBERNETES_NAME_MAX_LENGTH = 63; + +interface BuildDeployJobNameOptions { + deployUuid: string; + jobId: string; + shortSha: string; + maxLength?: number; +} + +export function buildDeployJobName({ + deployUuid, + jobId, + shortSha, + maxLength = KUBERNETES_NAME_MAX_LENGTH, +}: BuildDeployJobNameOptions): string { + const suffix = `deploy-${jobId}-${shortSha}`; + const fullName = `${deployUuid}-${suffix}`; + + if (fullName.length <= maxLength) { + return fullName.replace(/-+$/g, ''); + } + + const maxPrefixLength = maxLength - suffix.length - 1; + + if (maxPrefixLength <= 0) { + return suffix.substring(0, maxLength).replace(/-+$/g, ''); + } + + const truncatedPrefix = deployUuid.substring(0, maxPrefixLength).replace(/-+$/g, ''); + return truncatedPrefix ? `${truncatedPrefix}-${suffix}` : suffix; +} diff --git a/src/server/lib/kubernetesApply/applyManifest.ts b/src/server/lib/kubernetesApply/applyManifest.ts index 6b49405..a5e5ad5 100644 --- a/src/server/lib/kubernetesApply/applyManifest.ts +++ b/src/server/lib/kubernetesApply/applyManifest.ts @@ -19,6 +19,7 @@ import { HttpError } from '@kubernetes/client-node'; import { Deploy } from 'server/models'; import { getLogger } from 'server/lib/logger'; import GlobalConfigService from 'server/services/globalConfig'; +import { buildDeployJobName } from 'server/lib/kubernetes/jobNames'; export interface KubernetesApplyJobConfig { deploy: Deploy; @@ -35,7 +36,11 @@ export async function createKubernetesApplyJob({ kc.loadFromDefault(); const batchApi = kc.makeApiClient(k8s.BatchV1Api); const shortSha = deploy.sha?.substring(0, 7) || 'unknown'; - const jobName = `${deploy.uuid}-deploy-${jobId}-${shortSha}`.substring(0, 63); + const jobName = buildDeployJobName({ + deployUuid: deploy.uuid, + jobId, + shortSha, + }); const serviceName = deploy.deployable?.name || deploy.service?.name || ''; getLogger().info(`Job: creating name=${jobName} service=${serviceName}`); diff --git a/src/server/lib/nativeHelm/__tests__/helm.test.ts b/src/server/lib/nativeHelm/__tests__/helm.test.ts index 41d96d5..6baef82 100644 --- a/src/server/lib/nativeHelm/__tests__/helm.test.ts +++ b/src/server/lib/nativeHelm/__tests__/helm.test.ts @@ -14,14 +14,48 @@ * limitations under the License. */ -import { shouldUseNativeHelm, createHelmContainer } from '../helm'; +import { shouldUseNativeHelm, createHelmContainer, nativeHelmDeploy } from '../helm'; +import * as nativeHelmUtils from '../utils'; import { determineChartType, constructHelmCommand, ChartType, constructHelmCustomValues } from '../utils'; import { RegistryAuthConfig } from '../registryAuth'; import Deploy from 'server/models/Deploy'; import GlobalConfigService from 'server/services/globalConfig'; +import { buildDeployJobName } from 'server/lib/kubernetes/jobNames'; +import { waitForJobAndGetLogs } from 'server/lib/nativeBuild/utils'; +import { shellPromise } from 'server/lib/shell'; +import { getLogArchivalService } from 'server/services/logArchival'; jest.mock('server/services/globalConfig'); +jest.mock('../utils', () => { + const originalModule = jest.requireActual('../utils'); + return { + ...originalModule, + determineChartType: jest.fn(originalModule.determineChartType), + getHelmConfiguration: jest.fn(originalModule.getHelmConfiguration), + mergeHelmConfigWithGlobal: jest.fn(originalModule.mergeHelmConfigWithGlobal), + resolveHelmReleaseConflicts: jest.fn(originalModule.resolveHelmReleaseConflicts), + }; +}); jest.mock('server/lib/kubernetes'); +jest.mock('server/lib/random', () => ({ + randomAlphanumeric: jest.fn().mockReturnValue('k4hlde'), +})); +jest.mock('server/lib/shell', () => ({ + shellPromise: jest.fn(), +})); +jest.mock('server/lib/nativeBuild/utils', () => { + const originalModule = jest.requireActual('server/lib/nativeBuild/utils'); + return { + ...originalModule, + waitForJobAndGetLogs: jest.fn(), + }; +}); +jest.mock('server/services/logArchival', () => ({ + getLogArchivalService: jest.fn(), +})); +jest.mock('server/lib/kubernetes/common/serviceAccount', () => ({ + ensureServiceAccountForJob: jest.fn().mockResolvedValue('deploy-sa'), +})); jest.mock('server/lib/helm/utils', () => { const originalModule = jest.requireActual('server/lib/helm/utils'); return { @@ -894,4 +928,88 @@ describe('Native Helm', () => { expect(result.args[0]).toContain('helm-app'); }); }); + + describe('nativeHelmDeploy', () => { + it('uses the canonical deploy job name for monitoring and archival', async () => { + const deploy = { + uuid: 'cyclerx-cosmosdb-emulator-crimson-tooth-697165', + sha: '28e350a123456789', + branchName: 'main', + id: 42, + deployableId: 99, + deployable: { + name: 'cyclerx-cosmosdb-emulator', + repository: { fullName: 'GoodRx/example' }, + }, + build: { + uuid: 'crimson-tooth-697165', + namespace: 'testns', + isStatic: false, + pullRequest: { repository: { fullName: 'GoodRx/example' } }, + }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + $query: jest.fn().mockReturnValue({ + patch: jest.fn().mockResolvedValue(undefined), + }), + } as unknown as Deploy; + + const archiveLogs = jest.fn().mockResolvedValue(undefined); + + mockGetAllConfigs.mockResolvedValue({ + logArchival: { enabled: true }, + }); + + (nativeHelmUtils.resolveHelmReleaseConflicts as jest.Mock).mockResolvedValue(undefined); + (nativeHelmUtils.getHelmConfiguration as jest.Mock).mockResolvedValue({ + chartType: ChartType.PUBLIC, + customValues: [], + valuesFiles: [], + chartPath: 'prometheus-community/prometheus', + releaseName: deploy.uuid, + helmVersion: '3.12.0', + }); + (nativeHelmUtils.determineChartType as jest.Mock).mockResolvedValue(ChartType.PUBLIC); + (nativeHelmUtils.mergeHelmConfigWithGlobal as jest.Mock).mockResolvedValue({ + chart: { name: 'prometheus-community/prometheus' }, + nativeHelm: {}, + }); + + (shellPromise as jest.Mock).mockResolvedValue(''); + (waitForJobAndGetLogs as jest.Mock).mockResolvedValue({ + logs: 'helm logs', + success: true, + status: 'succeeded', + startedAt: '2026-03-17T00:00:00.000Z', + completedAt: '2026-03-17T00:01:00.000Z', + duration: 60, + }); + (getLogArchivalService as jest.Mock).mockReturnValue({ + archiveLogs, + }); + + const setTimeoutSpy = jest.spyOn(global, 'setTimeout').mockImplementation(((fn: TimerHandler) => { + if (typeof fn === 'function') { + fn(); + } + return 0 as any; + }) as typeof setTimeout); + + const result = await nativeHelmDeploy(deploy, { namespace: 'testns' }); + const expectedJobName = buildDeployJobName({ + deployUuid: deploy.uuid, + jobId: 'k4hlde', + shortSha: '28e350a', + }); + + expect(waitForJobAndGetLogs).toHaveBeenCalledWith(expectedJobName, 'testns', `[HELM ${deploy.uuid}]`); + expect(archiveLogs).toHaveBeenCalledWith(expect.objectContaining({ jobName: expectedJobName }), 'helm logs'); + expect(result).toEqual({ + completed: true, + logs: 'helm logs', + status: 'succeeded', + }); + + setTimeoutSpy.mockRestore(); + }); + }); }); diff --git a/src/server/lib/nativeHelm/helm.ts b/src/server/lib/nativeHelm/helm.ts index 205ac74..7fc622d 100644 --- a/src/server/lib/nativeHelm/helm.ts +++ b/src/server/lib/nativeHelm/helm.ts @@ -44,6 +44,7 @@ import { } from './utils'; import { detectRegistryAuth, RegistryAuthConfig } from './registryAuth'; import { HELM_IMAGE_PREFIX } from './constants'; +import { buildDeployJobName } from 'server/lib/kubernetes/jobNames'; import { createCloneScript, waitForJobAndGetLogs, @@ -110,7 +111,11 @@ export async function createHelmContainer( }; } -export async function generateHelmManifest(deploy: Deploy, jobId: string, options: HelmDeployOptions): Promise { +export async function generateHelmManifest( + deploy: Deploy, + jobName: string, + options: HelmDeployOptions +): Promise { await deploy.$fetchGraph('deployable.repository'); await deploy.$fetchGraph('build'); @@ -164,12 +169,6 @@ export async function generateHelmManifest(deploy: Deploy, jobId: string, option ], }; - const shortSha = deploy.sha ? deploy.sha.substring(0, 7) : 'no-sha'; - let jobName = `${deploy.uuid}-deploy-${jobId}-${shortSha}`.substring(0, 63); - if (jobName.endsWith('-')) { - jobName = jobName.slice(0, -1); - } - const deployMetadata = { sha: deploy.sha || '', branch: deploy.branchName || '', @@ -211,13 +210,13 @@ export async function nativeHelmDeploy(deploy: Deploy, options: HelmDeployOption await new Promise((resolve) => setTimeout(resolve, 2000)); - const manifest = await generateHelmManifest(deploy, jobId, options); - const shortSha = deploy.sha ? deploy.sha.substring(0, 7) : 'no-sha'; - let jobName = `${deploy.uuid}-deploy-${jobId}-${shortSha}`.substring(0, 63); - if (jobName.endsWith('-')) { - jobName = jobName.slice(0, -1); - } + const jobName = buildDeployJobName({ + deployUuid: deploy.uuid, + jobId, + shortSha, + }); + const manifest = await generateHelmManifest(deploy, jobName, options); const localPath = `${MANIFEST_PATH}/helm/${deploy.uuid}-helm-${shortSha}`; await fs.promises.mkdir(`${MANIFEST_PATH}/helm/`, { recursive: true }); diff --git a/src/server/lib/tests/deploymentManager.test.ts b/src/server/lib/tests/deploymentManager.test.ts index 228bc6f..231a9b8 100644 --- a/src/server/lib/tests/deploymentManager.test.ts +++ b/src/server/lib/tests/deploymentManager.test.ts @@ -16,11 +16,29 @@ import { DeploymentManager } from '../deploymentManager/deploymentManager'; import { Deploy } from 'server/models'; +import { buildDeployJobName } from '../kubernetes/jobNames'; // import { deployHelm } from '../helm'; jest.mock('../helm', () => ({ deployHelm: jest.fn().mockResolvedValue(void 0), })); +jest.mock('../kubernetesApply/applyManifest', () => ({ + createKubernetesApplyJob: jest.fn().mockResolvedValue(void 0), + monitorKubernetesJob: jest.fn().mockResolvedValue({ success: true, message: 'ok' }), +})); +jest.mock('../kubernetes/common/serviceAccount', () => ({ + ensureServiceAccountForJob: jest.fn().mockResolvedValue(void 0), +})); +jest.mock('../kubernetes', () => ({ + waitForDeployPodReady: jest.fn().mockResolvedValue(true), +})); +jest.mock('server/services/deploy', () => { + return jest.fn().mockImplementation(() => ({ + patchAndUpdateActivityFeed: jest.fn().mockResolvedValue(void 0), + })); +}); + +import { createKubernetesApplyJob, monitorKubernetesJob } from '../kubernetesApply/applyManifest'; // todo: add more tests for the below scenarios // let deploysWithoutDependencies: Deploy[]; @@ -173,4 +191,46 @@ describe('DeploymentManager', () => { // expect(deployHelm).toHaveBeenCalledTimes(2); // }); // }); + + describe('deployManifests', () => { + it('monitors the canonical truncated deploy job name for long deploy uuids', async () => { + const deploy = { + uuid: 'cyclerx-cosmosdb-emulator-crimson-tooth-697165', + sha: '28e350a123456789', + manifest: 'apiVersion: v1\nkind: ConfigMap', + runUUID: 'run-1', + build: { + namespace: 'testns', + }, + deployable: { + name: 'cyclerx-cosmosdb-emulator', + type: 'github', + deploymentDependsOn: [], + }, + service: { + type: 'github', + }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + } as unknown as Deploy; + + deploymentManager = new DeploymentManager([deploy]); + + await deploymentManager['deployManifests'](deploy); + + expect(createKubernetesApplyJob).toHaveBeenCalledWith({ + deploy, + namespace: 'testns', + jobId: expect.any(String), + }); + + const createdJobId = (createKubernetesApplyJob as jest.Mock).mock.calls[0][0].jobId; + const expectedJobName = buildDeployJobName({ + deployUuid: deploy.uuid, + jobId: createdJobId, + shortSha: '28e350a', + }); + + expect(monitorKubernetesJob).toHaveBeenCalledWith(expectedJobName, 'testns'); + }); + }); }); From 7223e8ef2689b952dc647bfcf320e208546b9816 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 17 Mar 2026 15:42:04 -0700 Subject: [PATCH 2/2] add additional test --- .../lib/kubernetes/__tests__/jobNames.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/server/lib/kubernetes/__tests__/jobNames.test.ts b/src/server/lib/kubernetes/__tests__/jobNames.test.ts index b72fdbb..baa38fd 100644 --- a/src/server/lib/kubernetes/__tests__/jobNames.test.ts +++ b/src/server/lib/kubernetes/__tests__/jobNames.test.ts @@ -49,4 +49,18 @@ describe('buildDeployJobName', () => { expect(jobName).not.toContain('--deploy-'); expect(jobName.endsWith('-')).toBe(false); }); + + it('returns a truncated suffix when suffix length alone exceeds maxLength', () => { + // suffix = 'deploy-k4hlde-28e350a' (21 chars); maxLength=14 → maxPrefixLength = 14-21-1 = -8 + // falls back to suffix.substring(0, 14) = 'deploy-k4hlde-' → trailing dash stripped + const jobName = buildDeployJobName({ + deployUuid: 'some-service', + jobId: 'k4hlde', + shortSha: '28e350a', + maxLength: 14, + }); + + expect(jobName).toBe('deploy-k4hlde'); + expect(jobName.endsWith('-')).toBe(false); + }); });