Skip to content

Commit

Permalink
BuildsService: Start all jobs that match the scope exactly (v6) (#2487
Browse files Browse the repository at this point in the history
)

Backport of #2486
  • Loading branch information
thomasdax98 authored Aug 31, 2024
1 parent 763f2d7 commit 9194d9a
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 5 deletions.
7 changes: 7 additions & 0 deletions .changeset/tiny-books-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@comet/cms-api": patch
---

`BuildsService`: Start all jobs that match the scope exactly

Previously, the first job that matched the scope exactly would be started, and the rest would be ignored. This has been fixed so that all jobs that match the scope exactly are started.
14 changes: 14 additions & 0 deletions packages/api/cms-api/src/builds/builds.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ const jobMainEnglish = {
},
};

const jobMainEnglish2 = {
metadata: {
name: "main-en-2",
annotations: {
[CONTENT_SCOPE_ANNOTATION]: '{"domain":"main","language":"en"}',
},
},
};

const jobMainGerman = {
metadata: {
name: "main-de",
Expand Down Expand Up @@ -62,6 +71,11 @@ describe("BuildsService", () => {
await expect(service.getBuilderCronJobsToStart([{ domain: "main", language: "en" }])).resolves.toEqual([jobMainEnglish]);
});

it("should return two jobs if two jobs have the exact same scope", async () => {
mockedBuildTemplatesService.getAllBuilderCronJobs.mockResolvedValueOnce([jobMainEnglish, jobMainEnglish2]);
await expect(service.getBuilderCronJobsToStart([{ domain: "main", language: "en" }])).resolves.toEqual([jobMainEnglish, jobMainEnglish2]);
});

it("should return multiple jobs for multiple exact matches", async () => {
await expect(
service.getBuilderCronJobsToStart([
Expand Down
11 changes: 6 additions & 5 deletions packages/api/cms-api/src/builds/builds.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ export class BuildsService {
const builderCronJobs = await this.buildTemplatesService.getAllBuilderCronJobs();

const getMatchingBuilderCronJobs = (scope: ContentScope) => {
const matchingCronJobs: V1CronJob[] = [];
const partiallyMatchingCronJobs: V1CronJob[] = [];
const exactlyMatchingCronJobs: V1CronJob[] = [];

for (const cronJob of builderCronJobs) {
const cronJobScope = this.kubernetesService.getContentScope(cronJob);
Expand All @@ -169,22 +170,22 @@ export class BuildsService {

// Exact match between job's scope and the scope with changes.
if (Object.entries(cronJobScope).every(([key, value]) => (scope as Record<string, unknown>)[key] === value)) {
return [cronJob];
exactlyMatchingCronJobs.push(cronJob);
}

// Check if scopes match partially. For instance, a job's scope may be { "domain": "main" }, but the change was in
// { "domain": "main", "language": "en" }. Or the job's scope may be { "domain": "main", "language": "en" }, but the change
// was in { "domain": "main" }. In both cases, the job should still be started.
if (Object.entries(cronJobScope).some(([key, value]) => (scope as Record<string, unknown>)[key] === value)) {
matchingCronJobs.push(cronJob);
partiallyMatchingCronJobs.push(cronJob);
}
}

if (matchingCronJobs.length === 0) {
if (exactlyMatchingCronJobs.length === 0 && partiallyMatchingCronJobs.length === 0) {
throw new Error(`Found changes in scope ${JSON.stringify(scope)} but no matching builder cron job!`);
}

return matchingCronJobs;
return exactlyMatchingCronJobs.length > 0 ? exactlyMatchingCronJobs : partiallyMatchingCronJobs;
};

const uniqueMatchingCronJobs = new Set<V1CronJob>();
Expand Down

0 comments on commit 9194d9a

Please sign in to comment.