From fbf25db87d2669b6b7877eaf2adf29f4c2075d0b Mon Sep 17 00:00:00 2001 From: williamlardier Date: Thu, 28 Nov 2024 14:54:52 +0100 Subject: [PATCH 1/9] Complete quota tests - Test that we properly account for deletions between count items runs. - Test that we can expire objects when a bucket has a quota enabled. Issue: ZENKO-4941 --- tests/ctst/features/quotas/Quotas.feature | 40 +++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/ctst/features/quotas/Quotas.feature b/tests/ctst/features/quotas/Quotas.feature index e9de9ba341..8fca68a8c0 100644 --- a/tests/ctst/features/quotas/Quotas.feature +++ b/tests/ctst/features/quotas/Quotas.feature @@ -96,3 +96,43 @@ Feature: Quota Management for APIs | 100 | 200 | 0 | IAM_USER | | 100 | 0 | 200 | IAM_USER | | 100 | 200 | 200 | IAM_USER | + + @2.6.0 + @PreMerge + @Quotas + @CronJob + @DataDeletion + @NonVersioned + Scenario Outline: Quotas are affected by deletion operations between count items runs + Given an action "DeleteObject" + And a permission to perform the "PutObject" action + And a STORAGE_MANAGER type + And a bucket quota set to 10000 B + And an account quota set to 10000 B + And an upload size of 1000 B for the object "obj-1" + And a bucket quota set to B + And an account quota set to B + And a type + And an environment setup for the API + And an "existing" IAM Policy that "applies" with "ALLOW" effect for the current API + When I wait 3 seconds + And I PUT an object with size + Then the API should "fail" with "QuotaExceeded" + When the "count-items" cronjobs completes without error + When I wait 3 seconds + # At this point if negative inflights are not supported, write should + # not be possible, as the previous inflights are now part of the current + # metrics. + And i delete object "obj-1" + And I wait 3 seconds + And I PUT an object with size + Then the API should "succeed" with "" + + Examples: + | uploadSize | bucketQuota | accountQuota | userType | + | 100 | 200 | 0 | ACCOUNT | + | 100 | 0 | 200 | ACCOUNT | + | 100 | 200 | 200 | ACCOUNT | + | 100 | 200 | 0 | IAM_USER | + | 100 | 0 | 200 | IAM_USER | + | 100 | 200 | 200 | IAM_USER | From 6682d5fea76f57dcee73f1907c22e70c83bb5d2a Mon Sep 17 00:00:00 2001 From: williamlardier Date: Fri, 29 Nov 2024 16:44:09 +0100 Subject: [PATCH 2/9] Test that negative inflights do not allow to bypass the quota Issue: ZENKO-4941 --- tests/ctst/features/quotas/Quotas.feature | 48 ++++++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/tests/ctst/features/quotas/Quotas.feature b/tests/ctst/features/quotas/Quotas.feature index 8fca68a8c0..a61b6cf9a9 100644 --- a/tests/ctst/features/quotas/Quotas.feature +++ b/tests/ctst/features/quotas/Quotas.feature @@ -107,8 +107,8 @@ Feature: Quota Management for APIs Given an action "DeleteObject" And a permission to perform the "PutObject" action And a STORAGE_MANAGER type - And a bucket quota set to 10000 B - And an account quota set to 10000 B + And a bucket quota set to 1000 B + And an account quota set to 1000 B And an upload size of 1000 B for the object "obj-1" And a bucket quota set to B And an account quota set to B @@ -119,11 +119,13 @@ Feature: Quota Management for APIs And I PUT an object with size Then the API should "fail" with "QuotaExceeded" When the "count-items" cronjobs completes without error + # Wait for inflights to be read by SCUBA When I wait 3 seconds # At this point if negative inflights are not supported, write should # not be possible, as the previous inflights are now part of the current # metrics. And i delete object "obj-1" + # Wait for inflights to be read by SCUBA And I wait 3 seconds And I PUT an object with size Then the API should "succeed" with "" @@ -136,3 +138,45 @@ Feature: Quota Management for APIs | 100 | 200 | 0 | IAM_USER | | 100 | 0 | 200 | IAM_USER | | 100 | 200 | 200 | IAM_USER | + + @2.6.0 + @PreMerge + @Quotas + @CronJob + @DataDeletion + @NonVersioned + Scenario Outline: Negative inflights do not allow to bypass the quota + Given an action "DeleteObject" + And a permission to perform the "PutObject" action + And a STORAGE_MANAGER type + And a bucket quota set to 1000 B + And an account quota set to 1000 B + And an upload size of 1000 B for the object "obj-1" + And a bucket quota set to B + And an account quota set to B + And a type + And an environment setup for the API + And an "existing" IAM Policy that "applies" with "ALLOW" effect for the current API + When I wait 3 seconds + And I PUT an object with size + Then the API should "fail" with "QuotaExceeded" + When the "count-items" cronjobs completes without error + # Wait for inflights to be read by SCUBA + When I wait 3 seconds + # At this point if negative inflights are not supported, write should + # not be possible, as the previous inflights are now part of the current + # metrics. + And i delete object "obj-1" + # Wait for inflights to be read by SCUBA + And I wait 3 seconds + And I PUT an object with size 1000 + Then the API should "fail" with "QuotaExceeded" + + Examples: + | uploadSize | bucketQuota | accountQuota | userType | + | 100 | 200 | 0 | ACCOUNT | + | 100 | 0 | 200 | ACCOUNT | + | 100 | 200 | 200 | ACCOUNT | + | 100 | 200 | 0 | IAM_USER | + | 100 | 0 | 200 | IAM_USER | + | 100 | 200 | 200 | IAM_USER | \ No newline at end of file From 53a1dd962a88debc7586962778b3d49a3797fa9e Mon Sep 17 00:00:00 2001 From: williamlardier Date: Mon, 2 Dec 2024 11:00:10 +0100 Subject: [PATCH 3/9] Bump scuba Issue: ZENKO-4941 --- solution/deps.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solution/deps.yaml b/solution/deps.yaml index 9ec3b33adf..105e1c6bc0 100644 --- a/solution/deps.yaml +++ b/solution/deps.yaml @@ -106,7 +106,7 @@ scuba: sourceRegistry: ghcr.io/scality dashboard: scuba/scuba-dashboards image: scuba - tag: 1.0.8 + tag: 1.0.9 envsubst: SCUBA_TAG sorbet: sourceRegistry: ghcr.io/scality From 7c05b0e4d5c21e6b8419c3cb9d4ec617ad450181 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Mon, 2 Dec 2024 14:46:33 +0100 Subject: [PATCH 4/9] Update how zenko-ctst account is created It seems that we are not robust against parallel account creations in Vault/Pensieve, leading in some cases to two accounts in the __account collection, but also in the pensieve overlay. However, when the overlay is updated (adding a new location, or a bucket configuration), ZKOP will thus fail the overlay validation and the reconciliations will all fail. It was affecting the bucket tests, but may affect more tests later. Issue: ZENKO-4941 --- tests/ctst/world/Zenko.ts | 54 ++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/tests/ctst/world/Zenko.ts b/tests/ctst/world/Zenko.ts index 4421348cdc..1005442fe0 100644 --- a/tests/ctst/world/Zenko.ts +++ b/tests/ctst/world/Zenko.ts @@ -4,6 +4,8 @@ import { AccessKey } from '@aws-sdk/client-iam'; import { Credentials } from '@aws-sdk/client-sts'; import { aws4Interceptor } from 'aws4-axios'; import qs from 'qs'; +import fs from 'fs'; +import lockFile from 'proper-lockfile'; import Werelogs from 'werelogs'; import { CacheHelper, @@ -630,24 +632,36 @@ export default class Zenko extends World { if (!Identity.hasIdentity(IdentityEnum.ACCOUNT, accountName)) { Identity.useIdentity(IdentityEnum.ADMIN, site.adminIdentityName); - + const filePath = `/tmp/account-init-${accountName}.json`; + if (!fs.existsSync(filePath)) { + fs.writeFileSync(filePath, JSON.stringify({ + ready: false, + })); + } let account = null; - CacheHelper.logger.debug('Creating account', { - accountName, - adminIdentityName: site.adminIdentityName, - credentials: Identity.getCurrentCredentials(), - }); - // Create the account if already exist will not throw any error + let releaseLock: (() => Promise) | null = null; try { - await SuperAdmin.createAccount({ accountName }); - /* eslint-disable */ - } catch (err: any) { - CacheHelper.logger.debug('Error while creating account', { - accountName, - err, + releaseLock = await lockFile.lock(filePath, { + stale: Constants.DEFAULT_TIMEOUT / 2, + retries: { + retries: 5, + factor: 3, + minTimeout: 1000, + maxTimeout: 5000, + } }); - if (!err.EntityAlreadyExists && err.code !== 'EntityAlreadyExists') { - throw err; + + try { + await SuperAdmin.createAccount({ accountName }); + /* eslint-disable */ + } catch (err: any) { + if (!err.EntityAlreadyExists && err.code !== 'EntityAlreadyExists') { + throw err; + } + } + } finally { + if (releaseLock) { + await releaseLock(); } } /* eslint-enable */ @@ -690,7 +704,7 @@ export default class Zenko extends World { const accountName = this.sites['source']?.accountName || CacheHelper.parameters.AccountName!; const accountAccessKeys = Identity.getCredentialsForIdentity( IdentityEnum.ACCOUNT, this.sites['source']?.accountName - || CacheHelper.parameters.AccountName!) || { + || CacheHelper.parameters.AccountName!) || { accessKeyId: '', secretAccessKey: '', }; @@ -862,7 +876,7 @@ export default class Zenko extends World { } async awsS3Request(method: Method, path: string, - userCredentials: AWSCredentials, headers: object = {}, payload: object = {}) : Promise { + userCredentials: AWSCredentials, headers: object = {}, payload: object = {}): Promise { const interceptor = aws4Interceptor({ options: { region: 'us-east-1', @@ -888,7 +902,7 @@ export default class Zenko extends World { statusCode: response.status, data: response.data as unknown, }; - /* eslint-disable */ + /* eslint-disable */ } catch (err: any) { return { stdout: '', @@ -964,7 +978,7 @@ export default class Zenko extends World { } } - async addWebsiteEndpoint(this: Zenko, endpoint: string) : + async addWebsiteEndpoint(this: Zenko, endpoint: string): Promise<{ statusCode: number; data: object } | { statusCode: number; err: unknown }> { return await this.managementAPIRequest('POST', `/config/${this.parameters.InstanceID}/website/endpoint`, @@ -974,7 +988,7 @@ export default class Zenko extends World { `"${endpoint}"`); } - async deleteLocation(this: Zenko, locationName: string) : + async deleteLocation(this: Zenko, locationName: string): Promise<{ statusCode: number; data: object } | { statusCode: number; err: unknown }> { return await this.managementAPIRequest('DELETE', `/config/${this.parameters.InstanceID}/location/${locationName}`); From 821726ab39a1077364b4b892d8596fbd73b4631c Mon Sep 17 00:00:00 2001 From: williamlardier Date: Mon, 2 Dec 2024 15:28:08 +0100 Subject: [PATCH 5/9] Set scuba log level as debug in CI Issue: ZENKO-4941 --- .github/scripts/end2end/configs/zenko.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/scripts/end2end/configs/zenko.yaml b/.github/scripts/end2end/configs/zenko.yaml index 9465ee2013..00ad3aaaf8 100644 --- a/.github/scripts/end2end/configs/zenko.yaml +++ b/.github/scripts/end2end/configs/zenko.yaml @@ -108,6 +108,9 @@ spec: command-timeout: "60s" pending-job-poll-after-age: "10s" pending-job-poll-check-interval: "10s" + scuba: + logging: + logLevel: debug ingress: workloadPlaneClass: 'nginx' controlPlaneClass: 'nginx' From 5fa95012587d05fa3279dc1ddee3eb5c5d4bb7c6 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Mon, 2 Dec 2024 18:26:30 +0100 Subject: [PATCH 6/9] Check that quota is set before conttunuing test Issue: ZENKO-4941 --- tests/ctst/steps/quotas/quotas.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/ctst/steps/quotas/quotas.ts b/tests/ctst/steps/quotas/quotas.ts index cd7ce1b022..850119f908 100644 --- a/tests/ctst/steps/quotas/quotas.ts +++ b/tests/ctst/steps/quotas/quotas.ts @@ -6,6 +6,7 @@ import { Scality, Command, Utils, AWSCredentials, Constants, Identity, IdentityE import { createJobAndWaitForCompletion } from '../utils/kubernetes'; import { createBucketWithConfiguration, putObject } from '../utils/utils'; import { hashStringAndKeepFirst20Characters } from 'common/utils'; +import assert from 'assert'; export async function prepareQuotaScenarios(world: Zenko, scenarioConfiguration: ITestCaseHookParameter) { /** @@ -136,6 +137,16 @@ Given('a bucket quota set to {int} B', async function (this: Zenko, quota: numbe result, }); + // Ensure the quota is set + const resultGet: Command = await Scality.getBucketQuota( + this.parameters, + this.getCommandParameters()); + this.logger.debug('GetBucketQuota result', { + resultGet, + }); + + assert(resultGet.stdout.includes(`${quota}`)); + if (result.err) { throw new Error(result.err); } @@ -158,6 +169,9 @@ Given('an account quota set to {int} B', async function (this: Zenko, quota: num result, }); + // Ensure the quota is set + assert(JSON.parse(result.stdout).quota === quota); + if (result.err) { throw new Error(result.err); } From d9d2427d6fda2e35a0ab8a5e7d93dc21380f1ae0 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 3 Dec 2024 11:26:49 +0100 Subject: [PATCH 7/9] Ensure no cleaning of quota upon test failure Issue: ZENKO-4941 --- tests/ctst/common/hooks.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/ctst/common/hooks.ts b/tests/ctst/common/hooks.ts index ed19aea392..1ce8d7ee4f 100644 --- a/tests/ctst/common/hooks.ts +++ b/tests/ctst/common/hooks.ts @@ -54,7 +54,13 @@ After(async function (this: Zenko, results) { ); }); -After({ tags: '@Quotas' }, async function () { +After({ tags: '@Quotas' }, async function (this: Zenko, results) { + if (results.result?.status === 'FAILED') { + this.logger.warn('quota was not cleaned for test', { + bucket: this.getSaved('bucketName'), + }); + return; + } await teardownQuotaScenarios(this as Zenko); }); From dc6eaa1b275560b764189ca03f9277063bcf22c3 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 3 Dec 2024 14:49:37 +0100 Subject: [PATCH 8/9] Simplify quota test scenario No need to reset the quota Issue: ZENKO-4941 --- tests/ctst/features/quotas/Quotas.feature | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/ctst/features/quotas/Quotas.feature b/tests/ctst/features/quotas/Quotas.feature index a61b6cf9a9..212c6cadff 100644 --- a/tests/ctst/features/quotas/Quotas.feature +++ b/tests/ctst/features/quotas/Quotas.feature @@ -72,11 +72,9 @@ Feature: Quota Management for APIs Given an action "DeleteObject" And a permission to perform the "PutObject" action And a STORAGE_MANAGER type - And a bucket quota set to 10000 B - And an account quota set to 10000 B - And an upload size of 1000 B for the object "obj-1" And a bucket quota set to B And an account quota set to B + And an upload size of 200 B for the object "obj-1" And a type And an environment setup for the API And an "existing" IAM Policy that "applies" with "ALLOW" effect for the current API From 6a8731ccab9dbd15d6ae88dd926731e0679a69e4 Mon Sep 17 00:00:00 2001 From: williamlardier Date: Tue, 3 Dec 2024 14:52:31 +0100 Subject: [PATCH 9/9] Ensure count items is not already running A race condition where a count items was running when the new one started at the beggining of the quota tests, and the old one completing after the new one, would lead to an override of the infostore collection, in turn leading to missing metrics for the quota buckets/accounts, making the quota tests fail till a new count items starts. Issue: ZENKO-4941 --- tests/ctst/steps/utils/kubernetes.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/ctst/steps/utils/kubernetes.ts b/tests/ctst/steps/utils/kubernetes.ts index 32e9aceb91..5bafe5f8ae 100644 --- a/tests/ctst/steps/utils/kubernetes.ts +++ b/tests/ctst/steps/utils/kubernetes.ts @@ -71,9 +71,34 @@ export function createKubeCustomObjectClient(world: Zenko): CustomObjectsApi { return KubernetesHelper.customObject; } +// Do not check job result, only wait till it completes +export async function waitForExistingJobCompletion(world: Zenko, jobName: string) { + const watchClient = createKubeWatchClient(world); + try { + await new Promise(resolve => { + void watchClient.watch( + '/apis/batch/v1/namespaces/default/jobs', + {}, + (type: string, apiObj, watchObj) => { + if ((watchObj.object?.metadata?.name as string)?.startsWith?.(jobName)) { + if (watchObj.object?.status?.succeeded || watchObj.object?.status?.failed) { + resolve(); + } + } + }, () => resolve()); + }); + } catch (err: unknown) { + world.logger.error('error waiting for job completion', { + jobName, + err, + }); + } +} + export async function createJobAndWaitForCompletion(world: Zenko, jobName: string, customMetadata?: string) { const batchClient = createKubeBatchClient(world); const watchClient = createKubeWatchClient(world); + await waitForExistingJobCompletion(world, jobName); try { const cronJob = await batchClient.readNamespacedCronJob(jobName, 'default'); const cronJobSpec = cronJob.body.spec?.jobTemplate.spec;