Skip to content

Commit

Permalink
BC-7561 - separate job for failed batch deletion requests (#5448)
Browse files Browse the repository at this point in the history
  • Loading branch information
virgilchiriac authored and MartinSchuhmacher committed Feb 5, 2025
1 parent 01eb940 commit 2a00a52
Show file tree
Hide file tree
Showing 35 changed files with 829 additions and 284 deletions.
18 changes: 18 additions & 0 deletions ansible/roles/schulcloud-server-core/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,24 @@
tags:
- configmap

- name: Data deletion trigger failed CronJob
kubernetes.core.k8s:
kubeconfig: ~/.kube/config
namespace: "{{ NAMESPACE }}"
template: data-deletion-trigger-failed-cronjob.yml.j2
when: WITH_API_ADMIN is defined and WITH_API_ADMIN|bool
tags:
- cronjob

- name: Data deletion trigger failed CronJob ConfigMap
kubernetes.core.k8s:
kubeconfig: ~/.kube/config
namespace: "{{ NAMESPACE }}"
template: data-deletion-trigger-failed-cronjob-configmap.yml.j2
when: WITH_API_ADMIN is defined and WITH_API_ADMIN|bool
tags:
- configmap

- name: amqp files storage Deployment
kubernetes.core.k8s:
kubeconfig: ~/.kube/config
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
apiVersion: v1
kind: ConfigMap
metadata:
namespace: {{ NAMESPACE }}
name: data-deletion-trigger-failed-cronjob-configmap
labels:
app: data-deletion-trigger-failed-cronjob
data:
NODE_OPTIONS: "--max-old-space-size=3072"
NEST_LOG_LEVEL: "error"
EXIT_ON_ERROR: "true"
SC_DOMAIN: "{{ DOMAIN }}"
FEATURE_PROMETHEUS_METRICS_ENABLED: "true"
ETHERPAD__PAD_URI: "https://{{ DOMAIN }}/etherpad/p"

Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
apiVersion: batch/v1
kind: CronJob
metadata:
namespace: {{ NAMESPACE }}
labels:
app: data-deletion-trigger-failed
app.kubernetes.io/part-of: schulcloud-verbund
app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }}
app.kubernetes.io/name: data-deletion-trigger-failed
app.kubernetes.io/component: data-deletion
app.kubernetes.io/managed-by: ansible
git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }}
git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }}
name: data-deletion-trigger-failed-cronjob
spec:
concurrencyPolicy: Forbid
schedule: "{{ SERVER_DATA_DELETION_TRIGGER_FAILED_CRONJOB_SCHEDULE|default("@daily", true) }}"
jobTemplate:
metadata:
labels:
app: data-deletion-trigger-failed
app.kubernetes.io/part-of: schulcloud-verbund
app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }}
app.kubernetes.io/name: data-deletion-trigger-failed
app.kubernetes.io/component: data-deletion
app.kubernetes.io/managed-by: ansible
git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }}
git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }}
spec:
template:
spec:
securityContext:
runAsUser: 1000
runAsGroup: 1000
fsGroup: 1000
runAsNonRoot: true
containers:
- name: data-deletion-trigger-failed-cronjob
image: {{ SCHULCLOUD_SERVER_IMAGE }}:{{ SCHULCLOUD_SERVER_IMAGE_TAG }}
envFrom:
- configMapRef:
name: data-deletion-trigger-failed-cronjob-configmap
- secretRef:
name: admin-api-client-secret
- secretRef:
name: api-files-secret
command: ['/bin/sh', '-c']
args: ['npm run nest:start:deletion-console -- execution trigger -f true']
resources:
limits:
cpu: {{ API_CPU_LIMITS|default("2000m", true) }}
memory: {{ API_MEMORY_LIMITS|default("2Gi", true) }}
requests:
cpu: {{ API_CPU_REQUESTS|default("100m", true) }}
memory: {{ API_MEMORY_REQUESTS|default("150Mi", true) }}
restartPolicy: OnFailure
{% if AFFINITY_ENABLE is defined and AFFINITY_ENABLE|bool %}
affinity:
podAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- weight: 20
podAffinityTerm:
labelSelector:
matchExpressions:
- key: app.kubernetes.io/part-of
operator: In
values:
- schulcloud-verbund
topologyKey: "kubernetes.io/hostname"
namespaceSelector: {}
- weight: 10
podAffinityTerm:
labelSelector:
matchExpressions:
- key: git.repo
operator: In
values:
- {{ SCHULCLOUD_SERVER_REPO_NAME }}
topologyKey: "kubernetes.io/hostname"
namespaceSelector: {}
- weight: 10
podAffinityTerm:
labelSelector:
matchExpressions:
- key: git.branch
operator: In
values:
- {{ SCHULCLOUD_SERVER_BRANCH_NAME }}
topologyKey: "kubernetes.io/hostname"
namespaceSelector: {}
- weight: 10
podAffinityTerm:
labelSelector:
matchExpressions:
- key: app.kubernetes.io/version
operator: In
values:
- {{ SCHULCLOUD_SERVER_IMAGE_TAG }}
topologyKey: "kubernetes.io/hostname"
namespaceSelector: {}
{% endif %}
metadata:
labels:
app: data-deletion-trigger-failed
app.kubernetes.io/part-of: schulcloud-verbund
app.kubernetes.io/version: {{ SCHULCLOUD_SERVER_IMAGE_TAG }}
app.kubernetes.io/name: data-deletion-trigger-failed
app.kubernetes.io/component: data-deletion
app.kubernetes.io/managed-by: ansible
git.branch: {{ SCHULCLOUD_SERVER_BRANCH_NAME }}
git.repo: {{ SCHULCLOUD_SERVER_REPO_NAME }}
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
import { TriggerDeletionExecutionOptions } from '../interface';
import { TriggerDeletionExecutionOptionsBuilder } from './trigger-deletion-execution-options.builder';

describe(TriggerDeletionExecutionOptionsBuilder.name, () => {
describe(TriggerDeletionExecutionOptionsBuilder.build.name, () => {
describe('when called with proper arguments', () => {
const setup = () => {
const limit = 1000;
const runFailed = false;

const expectedOutput: TriggerDeletionExecutionOptions = { limit };

return { limit, expectedOutput };
return { limit, runFailed };
};

it('should return valid object with expected values', () => {
const { limit, expectedOutput } = setup();
const { limit, runFailed } = setup();

const output = TriggerDeletionExecutionOptionsBuilder.build(limit);
const output = TriggerDeletionExecutionOptionsBuilder.build(limit, runFailed);

expect(output).toStrictEqual(expectedOutput);
expect(output).toStrictEqual({ limit, runFailed });
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { TriggerDeletionExecutionOptions } from '../interface';

export class TriggerDeletionExecutionOptionsBuilder {
static build(limit: number): TriggerDeletionExecutionOptions {
return { limit };
static build(limit: number, runFailed: boolean): TriggerDeletionExecutionOptions {
return { limit, runFailed };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -249,5 +249,28 @@ describe(DeletionClient.name, () => {
await expect(client.executeDeletions()).rejects.toThrow(Error);
});
});

describe('when runFailed is true', () => {
const setup = () => {
setupConfig();

const response: AxiosResponse<DeletionRequestOutput> = axiosResponseFactory.build({
status: 204,
});

httpService.post.mockReturnValueOnce(of(response));

const fullUrl = 'http://api-admin:4030/admin/api/v1/deletionExecutions?runFailed=true';

return { fullUrl };
};
it('should call endpoint with runFailed param', async () => {
const { fullUrl } = setup();

await client.executeDeletions(undefined, true);

expect(httpService.post).toHaveBeenCalledWith(fullUrl, null, expect.any(Object));
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ export class DeletionClient {
}
}

public async executeDeletions(limit?: number): Promise<void> {
public async executeDeletions(limit?: number, runFailed?: boolean): Promise<void> {
try {
const response = await this.postDeletionExecutionRequest(limit);
const response = await this.postDeletionExecutionRequest(limit, runFailed);
this.checkResponseStatusCode(response, HttpStatusCode.NoContent);
} catch (err) {
throw this.createError(err, 'executeDeletions');
Expand All @@ -46,13 +46,25 @@ export class DeletionClient {
return response;
}

private async postDeletionExecutionRequest(limit?: number): Promise<AxiosResponse<any, any>> {
private async postDeletionExecutionRequest(limit?: number, runFailed?: boolean): Promise<AxiosResponse<any, any>> {
const defaultHeaders = this.createDefaultHeaders();
const headers = this.isLimitGeaterZero(limit) ? { ...defaultHeaders, params: { limit } } : defaultHeaders;

const baseUrl = this.configService.get('ADMIN_API_CLIENT_BASE_URL', { infer: true });
const postDeletionExecutionsEndpoint = new URL('/admin/api/v1/deletionExecutions', baseUrl).toString();
const endpoint = '/admin/api/v1/deletionExecutions';

const postDeletionExecutionsEndpoint = new URL(endpoint, baseUrl);

const params = new URLSearchParams(postDeletionExecutionsEndpoint.search);
if (limit && this.isLimitGreaterZero(limit)) {
params.append('limit', limit.toString());
}
if (runFailed) {
params.append('runFailed', runFailed.toString());
}

const fullUrl = `${postDeletionExecutionsEndpoint.toString()}?${params.toString()}`;

const request = this.httpService.post(postDeletionExecutionsEndpoint, null, headers);
const request = this.httpService.post(fullUrl, null, defaultHeaders);
const response = await firstValueFrom(request);

return response;
Expand Down Expand Up @@ -87,7 +99,7 @@ export class DeletionClient {
}
}

private isLimitGeaterZero(limit?: number): boolean {
private isLimitGreaterZero(limit?: number): boolean {
return typeof limit === 'number' && limit > 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { DeletionExecutionConsole } from './deletion-execution.console';
import { DeletionExecutionTriggerResultBuilder, TriggerDeletionExecutionOptionsBuilder } from './builder';
import { DeletionExecutionUc } from './uc';
import { DeletionConsoleModule } from './deletion-console.app.module';
import { TriggerDeletionExecutionOptions } from './interface';

describe(DeletionExecutionConsole.name, () => {
let module: TestingModule;
Expand Down Expand Up @@ -38,7 +39,7 @@ describe(DeletionExecutionConsole.name, () => {
const setup = () => {
const limit = 1000;

const options = TriggerDeletionExecutionOptionsBuilder.build(1000);
const options = TriggerDeletionExecutionOptionsBuilder.build(1000, false);

return { limit, options };
};
Expand All @@ -50,13 +51,13 @@ describe(DeletionExecutionConsole.name, () => {

await console.triggerDeletionExecution(options);

expect(spy).toBeCalledWith(limit);
expect(spy).toBeCalledWith(limit, false);
});
});

describe(`when ${DeletionExecutionUc.name}'s triggerDeletionExecution() method doesn't throw an exception`, () => {
const setup = () => {
const options = TriggerDeletionExecutionOptionsBuilder.build(1000);
const options = TriggerDeletionExecutionOptionsBuilder.build(1000, false);

deletionExecutionUc.triggerDeletionExecution.mockResolvedValueOnce(undefined);
const spy = jest.spyOn(DeletionExecutionTriggerResultBuilder, 'buildSuccess');
Expand All @@ -75,7 +76,7 @@ describe(DeletionExecutionConsole.name, () => {

describe(`when ${DeletionExecutionUc.name}'s triggerDeletionExecution() method throws an exception`, () => {
const setup = () => {
const options = TriggerDeletionExecutionOptionsBuilder.build(1000);
const options = TriggerDeletionExecutionOptionsBuilder.build(1000, false);
const err = new Error('some error occurred...');

deletionExecutionUc.triggerDeletionExecution.mockRejectedValueOnce(err);
Expand All @@ -92,5 +93,29 @@ describe(DeletionExecutionConsole.name, () => {
expect(spy).toHaveBeenCalledWith(err);
});
});

describe('when runFailed is given as string "true"', () => {
it('should convert runFailed to boolean', async () => {
const options = { limit: 1000, runFailed: 'true' } as unknown as TriggerDeletionExecutionOptions;

const spy = jest.spyOn(deletionExecutionUc, 'triggerDeletionExecution').mockResolvedValueOnce(undefined);

await console.triggerDeletionExecution(options);

expect(spy).toHaveBeenCalledWith(1000, true);
});
});

describe('when limit is a string "5"', () => {
it('should convert limit to number', async () => {
const options = { limit: '5', runFailed: false } as unknown as TriggerDeletionExecutionOptions;

const spy = jest.spyOn(deletionExecutionUc, 'triggerDeletionExecution').mockResolvedValueOnce(undefined);

await console.triggerDeletionExecution(options);

expect(spy).toHaveBeenCalledWith(5, false);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,36 @@ export class DeletionExecutionConsole {
options: [
{
flags: '-l, --limit <value>',
// TODO implemnt fn with proper command testing
// fn: (value: string) => (value ? Number(value) : undefined),
description: 'Limit of the requested deletion executions that should be performed.',
required: false,
},
{
flags: '-f, --runFailed <value>',
// TODO implemnt fn with proper command testing
// fn: (value: string) => /^(true|yes|1)$/i.test(value),
description: 'Limit of the requested deletion executions that should be performed.',
required: false,
},
],
})
async triggerDeletionExecution(options: TriggerDeletionExecutionOptions): Promise<void> {
public async triggerDeletionExecution(options: TriggerDeletionExecutionOptions): Promise<void> {
// Try to trigger the deletion execution(s) via Deletion API client,
// return successful status in case of a success, otherwise return
// a result with a failure status and a proper error message.
let result: DeletionExecutionTriggerResult;

try {
await this.deletionExecutionUc.triggerDeletionExecution(options.limit ? Number(options.limit) : undefined);
if (typeof options.limit === 'string') {
options.limit = Number(options.limit);
}

if (typeof options.runFailed === 'string') {
options.runFailed = /^(true|yes|1)$/i.test(options.runFailed);
}

try {
await this.deletionExecutionUc.triggerDeletionExecution(options.limit, options.runFailed);
result = DeletionExecutionTriggerResultBuilder.buildSuccess();
} catch (err) {
result = DeletionExecutionTriggerResultBuilder.buildFailure(err as Error);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export interface TriggerDeletionExecutionOptions {
limit: number;
limit: string | number;
runFailed: string | boolean;
}
Loading

0 comments on commit 2a00a52

Please sign in to comment.