Skip to content

Commit 577facf

Browse files
elai-shalevelai-shalev
authored
feat(x2a): real K8s jobs for analyze and migrate (#2304)
* First draft: Real Job creation * job template is able to git pull * Working full init job including git * init phase coomplete, skeleton for other phases * With fixes tests, yarn full * app-config reset * rbac * reverting aap and rbac to main * prettier * Working Analyze phase + owner reference delete secret * analyze job works * migrate changes * remove callback url call * Working analyze + migration steps * api report * After rebase * Error handling on bash script * Tests error
1 parent 34f9da5 commit 577facf

13 files changed

Lines changed: 285 additions & 87 deletions

File tree

‎workspaces/x2a/plugins/x2a-backend/src/router/common.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
x2aAdminWritePermission,
3333
} from '@red-hat-developer-hub/backstage-plugin-x2a-common';
3434

35+
import type { RouterDeps } from './types';
36+
3537
const isUserOfAdminViewPermission = async (
3638
request: Request,
3739
permissionsSvc: PermissionsService,
@@ -108,10 +110,44 @@ const removeSensitiveFromJob = (job?: UnsecureJob): Job | undefined => {
108110
return newJob;
109111
};
110112

113+
// TODO: Remove once collectArtifacts (or the `report` command) is implemented
114+
// and jobs update their own status on completion. Until then this is the only
115+
// mechanism that syncs stale DB records with actual K8s job state.
116+
async function reconcileJobStatus(
117+
job: Job,
118+
deps: Pick<RouterDeps, 'kubeService' | 'x2aDatabase' | 'logger'>,
119+
): Promise<Job> {
120+
if (!['pending', 'running'].includes(job.status)) {
121+
return job;
122+
}
123+
if (!job.k8sJobName) {
124+
return job;
125+
}
126+
127+
const k8sStatus = await deps.kubeService.getJobStatus(job.k8sJobName);
128+
129+
if (k8sStatus.status === 'success' || k8sStatus.status === 'error') {
130+
const log = (await deps.kubeService.getJobLogs(job.k8sJobName)) as string;
131+
const updated = await deps.x2aDatabase.updateJob({
132+
id: job.id,
133+
status: k8sStatus.status,
134+
finishedAt: new Date(),
135+
log,
136+
});
137+
deps.logger.info(
138+
`Reconciled job ${job.id}: DB had '${job.status}', K8s reports '${k8sStatus.status}'`,
139+
);
140+
return updated ?? job;
141+
}
142+
143+
return job;
144+
}
145+
111146
export {
112147
isUserOfAdminViewPermission,
113148
isUserOfAdminWritePermission,
114149
authorize,
115150
getUserRef,
151+
reconcileJobStatus,
116152
removeSensitiveFromJob,
117153
};

‎workspaces/x2a/plugins/x2a-backend/src/router/modules.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ import { InputError, NotFoundError } from '@backstage/errors';
2121
import { Module } from '@red-hat-developer-hub/backstage-plugin-x2a-common';
2222

2323
import type { RouterDeps } from './types';
24-
import { getUserRef, removeSensitiveFromJob } from './common';
24+
import {
25+
getUserRef,
26+
reconcileJobStatus,
27+
removeSensitiveFromJob,
28+
} from './common';
2529

2630
export function registerModuleRoutes(
2731
router: express.Router,
@@ -236,7 +240,16 @@ export function registerModuleRoutes(
236240
projectId,
237241
moduleId,
238242
});
239-
const hasActiveJob = existingJobs.some(job =>
243+
244+
// Reconcile jobs that appear active against K8s
245+
const reconciledJobs = await Promise.all(
246+
existingJobs
247+
.filter(job => ['pending', 'running'].includes(job.status))
248+
.map(job =>
249+
reconcileJobStatus(job, { kubeService, x2aDatabase, logger }),
250+
),
251+
);
252+
const hasActiveJob = reconciledJobs.some(job =>
240253
['pending', 'running'].includes(job.status),
241254
);
242255

‎workspaces/x2a/plugins/x2a-backend/src/router/projects.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
getUserRef,
3131
isUserOfAdminViewPermission,
3232
isUserOfAdminWritePermission,
33+
reconcileJobStatus,
3334
} from './common';
3435
import { ProjectsGet, ProjectsPost } from '../schema/openapi';
3536

@@ -255,10 +256,18 @@ export function registerProjectRoutes(
255256

256257
// Check for existing running init job
257258
const existingJobs = await x2aDatabase.listJobsForProject({ projectId });
258-
const hasActiveInitJob = existingJobs.some(
259+
const activeInitJobs = existingJobs.filter(
259260
job =>
260261
job.phase === 'init' && ['pending', 'running'].includes(job.status),
261262
);
263+
const reconciledInitJobs = await Promise.all(
264+
activeInitJobs.map(job =>
265+
reconcileJobStatus(job, { kubeService, x2aDatabase, logger }),
266+
),
267+
);
268+
const hasActiveInitJob = reconciledInitJobs.some(job =>
269+
['pending', 'running'].includes(job.status),
270+
);
262271

263272
if (hasActiveInitJob) {
264273
return res.status(409).json({

‎workspaces/x2a/plugins/x2a-backend/src/schema/openapi.yaml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,6 @@ paths:
293293
$ref: '#/components/schemas/AAPCredentials'
294294
required:
295295
- phase
296-
- sourceRepoAuth
297-
- targetRepoAuth
298296
responses:
299297
'200':
300298
description: Migration job created successfully

‎workspaces/x2a/plugins/x2a-backend/src/schema/openapi/generated/models/ProjectsProjectIdModulesModuleIdRunPostRequest.model.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { ModulePhase } from '../models/ModulePhase.model';
2626
*/
2727
export interface ProjectsProjectIdModulesModuleIdRunPostRequest {
2828
phase: ModulePhase;
29-
sourceRepoAuth: GitRepoAuth;
30-
targetRepoAuth: GitRepoAuth;
29+
sourceRepoAuth?: GitRepoAuth;
30+
targetRepoAuth?: GitRepoAuth;
3131
aapCredentials?: AAPCredentials;
3232
}

‎workspaces/x2a/plugins/x2a-backend/src/schema/openapi/generated/router.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -447,9 +447,7 @@ export const spec = {
447447
}
448448
},
449449
"required": [
450-
"phase",
451-
"sourceRepoAuth",
452-
"targetRepoAuth"
450+
"phase"
453451
]
454452
}
455453
}

‎workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.test.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ describe('JobResourceBuilder', () => {
368368
describe('buildJobSecret', () => {
369369
const jobId = 'job-456';
370370
const projectId = 'proj-123';
371+
const phase = 'init';
371372
const gitCredentials = {
372373
sourceRepo: {
373374
url: 'https://github.com/org/source',
@@ -385,6 +386,7 @@ describe('JobResourceBuilder', () => {
385386
const secret = JobResourceBuilder.buildJobSecret(
386387
jobId,
387388
projectId,
389+
phase,
388390
gitCredentials,
389391
);
390392

@@ -399,6 +401,7 @@ describe('JobResourceBuilder', () => {
399401
const secret = JobResourceBuilder.buildJobSecret(
400402
jobId,
401403
projectId,
404+
phase,
402405
gitCredentials,
403406
);
404407

@@ -413,6 +416,7 @@ describe('JobResourceBuilder', () => {
413416
const secret = JobResourceBuilder.buildJobSecret(
414417
jobId,
415418
projectId,
419+
phase,
416420
gitCredentials,
417421
);
418422

@@ -427,6 +431,7 @@ describe('JobResourceBuilder', () => {
427431
const secret = JobResourceBuilder.buildJobSecret(
428432
jobId,
429433
projectId,
434+
phase,
430435
gitCredentials,
431436
);
432437

@@ -444,16 +449,18 @@ describe('JobResourceBuilder', () => {
444449
const secret = JobResourceBuilder.buildJobSecret(
445450
jobId,
446451
projectId,
452+
phase,
447453
gitCredentials,
448454
);
449455

450-
expect(secret.metadata?.name).toBe(`x2a-job-secret-${jobId}`);
456+
expect(secret.metadata?.name).toBe(`x2a-job-secret-${phase}-${jobId}`);
451457
});
452458

453459
it('should include description annotation', () => {
454460
const secret = JobResourceBuilder.buildJobSecret(
455461
jobId,
456462
projectId,
463+
phase,
457464
gitCredentials,
458465
);
459466

@@ -468,6 +475,7 @@ describe('JobResourceBuilder', () => {
468475
const secret = JobResourceBuilder.buildJobSecret(
469476
jobId,
470477
projectId,
478+
phase,
471479
gitCredentials,
472480
);
473481

@@ -646,7 +654,7 @@ describe('JobResourceBuilder', () => {
646654
},
647655
{
648656
secretRef: {
649-
name: 'x2a-job-secret-job-123',
657+
name: 'x2a-job-secret-init-job-123',
650658
},
651659
},
652660
]);
@@ -820,7 +828,7 @@ describe('JobResourceBuilder', () => {
820828

821829
const container = job.spec?.template.spec?.containers![0];
822830
// Script handles unknown phases with exit 1
823-
expect(container!.args![0]).toContain('ERROR: Unknown phase');
831+
expect(container!.args![0]).toContain('Unknown phase');
824832
});
825833
});
826834
});

‎workspaces/x2a/plugins/x2a-backend/src/services/JobResourceBuilder.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,8 @@ export class JobResourceBuilder {
132132
* Builds a Kubernetes Secret for a specific job containing ephemeral Git credentials
133133
*
134134
* Note: ownerReferences are NOT set here because the job doesn't exist yet at secret
135-
* creation time. The job secret will be cleaned up by Kubernetes TTL after the job
136-
* completes (via ttlSecondsAfterFinished setting on the job). For explicit cleanup,
137-
* the deleteJobSecret method can be called.
135+
* creation time. After job creation, KubeService.createJob() sets the ownerReference
136+
* on this secret so it is automatically garbage-collected when the Job is deleted.
138137
*
139138
* @param jobId - The job UUID
140139
* @param projectId - The project UUID
@@ -144,12 +143,13 @@ export class JobResourceBuilder {
144143
static buildJobSecret(
145144
jobId: string,
146145
projectId: string,
146+
phase: string,
147147
gitCredentials: {
148148
sourceRepo: GitRepo;
149149
targetRepo: GitRepo;
150150
},
151151
): V1Secret {
152-
const secretName = `x2a-job-secret-${jobId}`;
152+
const secretName = `x2a-job-secret-${phase}-${jobId}`;
153153

154154
return {
155155
apiVersion: 'v1',
@@ -196,7 +196,7 @@ export class JobResourceBuilder {
196196
const shortId = crypto.randomBytes(4).toString('hex');
197197
const jobName = `job-x2a-${params.phase}-${shortId}`;
198198
const projectSecretName = `x2a-project-secret-${params.projectId}`;
199-
const jobSecretName = `x2a-job-secret-${params.jobId}`;
199+
const jobSecretName = `x2a-job-secret-${params.phase}-${params.jobId}`;
200200

201201
return {
202202
apiVersion: 'batch/v1',
@@ -247,7 +247,9 @@ export class JobResourceBuilder {
247247
spec: {
248248
restartPolicy: 'Never',
249249
// Init container: Clone source and target repositories
250-
initContainers: [this.buildGitFetchInitContainer(params.jobId)],
250+
initContainers: [
251+
this.buildGitFetchInitContainer(params.jobId, params.phase),
252+
],
251253
containers: [
252254
{
253255
name: 'x2a',
@@ -421,8 +423,11 @@ export class JobResourceBuilder {
421423
* @param jobId - The job UUID (for secret reference)
422424
* @returns V1Container for the init container
423425
*/
424-
private static buildGitFetchInitContainer(jobId: string): V1Container {
425-
const jobSecretName = `x2a-job-secret-${jobId}`;
426+
private static buildGitFetchInitContainer(
427+
jobId: string,
428+
phase: string,
429+
): V1Container {
430+
const jobSecretName = `x2a-job-secret-${phase}-${jobId}`;
426431

427432
return {
428433
name: 'git-fetch',

‎workspaces/x2a/plugins/x2a-backend/src/services/KubeService.test.ts‎

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { X2AConfig } from '../../config';
2222
const mockCoreV1Api = {
2323
createNamespacedSecret: jest.fn(),
2424
readNamespacedSecret: jest.fn(),
25+
replaceNamespacedSecret: jest.fn(),
2526
deleteNamespacedSecret: jest.fn(),
2627
listNamespacedPod: jest.fn(),
2728
readNamespacedPodLog: jest.fn(),
@@ -251,6 +252,16 @@ describe('KubeService', () => {
251252
beforeEach(() => {
252253
// Mock createProjectSecret and createJobSecret to succeed
253254
mockCoreV1Api.createNamespacedSecret.mockResolvedValue({});
255+
// Mock readNamespacedSecret and replaceNamespacedSecret for ownerReference patching
256+
mockCoreV1Api.readNamespacedSecret.mockResolvedValue({
257+
apiVersion: 'v1',
258+
kind: 'Secret',
259+
metadata: {
260+
name: 'x2a-job-secret-init-job-123',
261+
namespace: 'test-namespace',
262+
},
263+
});
264+
mockCoreV1Api.replaceNamespacedSecret.mockResolvedValue({});
254265
});
255266

256267
it('should create both project and job secrets before creating job', async () => {
@@ -278,7 +289,7 @@ describe('KubeService', () => {
278289
namespace: 'test-namespace',
279290
body: expect.objectContaining({
280291
metadata: expect.objectContaining({
281-
name: 'x2a-job-secret-job-123',
292+
name: 'x2a-job-secret-init-job-123',
282293
}),
283294
stringData: expect.objectContaining({
284295
SOURCE_REPO_URL: 'https://github.com/org/source',
@@ -319,7 +330,7 @@ describe('KubeService', () => {
319330
name: 'x2a',
320331
envFrom: [
321332
{ secretRef: { name: 'x2a-project-secret-proj-123' } },
322-
{ secretRef: { name: 'x2a-job-secret-job-123' } },
333+
{ secretRef: { name: 'x2a-job-secret-init-job-123' } },
323334
],
324335
}),
325336
]),
@@ -330,6 +341,53 @@ describe('KubeService', () => {
330341
});
331342
expect(result.k8sJobName).toBeDefined();
332343
});
344+
345+
it('should set ownerReference on job secret after job creation', async () => {
346+
mockBatchV1Api.createNamespacedJob.mockResolvedValue({
347+
metadata: { name: 'job-x2a-init-abc123', uid: 'uid-456' },
348+
});
349+
350+
await kubeService.createJob(params);
351+
352+
// Should read the job secret
353+
expect(mockCoreV1Api.readNamespacedSecret).toHaveBeenCalledWith({
354+
name: 'x2a-job-secret-init-job-123',
355+
namespace: 'test-namespace',
356+
});
357+
358+
// Should replace the secret with ownerReference set
359+
expect(mockCoreV1Api.replaceNamespacedSecret).toHaveBeenCalled();
360+
const replaceCall =
361+
mockCoreV1Api.replaceNamespacedSecret.mock.calls[0][0];
362+
expect(replaceCall.name).toBe('x2a-job-secret-init-job-123');
363+
expect(replaceCall.namespace).toBe('test-namespace');
364+
const ownerRefs = replaceCall.body.metadata.ownerReferences;
365+
expect(ownerRefs).toHaveLength(1);
366+
expect(ownerRefs[0]).toEqual(
367+
expect.objectContaining({
368+
apiVersion: 'batch/v1',
369+
kind: 'Job',
370+
uid: 'uid-456',
371+
blockOwnerDeletion: true,
372+
}),
373+
);
374+
expect(ownerRefs[0].name).toMatch(/^job-x2a-init-/);
375+
});
376+
377+
it('should succeed even if ownerReference patching fails', async () => {
378+
mockBatchV1Api.createNamespacedJob.mockResolvedValue({
379+
metadata: { name: 'job-x2a-init-abc123', uid: 'uid-456' },
380+
});
381+
mockCoreV1Api.readNamespacedSecret.mockRejectedValue(
382+
new Error('Secret read failed'),
383+
);
384+
385+
const result = await kubeService.createJob(params);
386+
387+
// Job creation should still succeed
388+
expect(result.k8sJobName).toBeDefined();
389+
expect(mockBatchV1Api.createNamespacedJob).toHaveBeenCalled();
390+
});
333391
});
334392

335393
describe('getJobStatus', () => {

0 commit comments

Comments
 (0)