Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend bucket notification timeout and map to the test timeout #2135

Draft
wants to merge 5 commits into
base: development/2.6
Choose a base branch
from
Draft

Extend bucket notification timeout and map to the test timeout #2135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a2ec22a
Extend bucket notification timeout and map to the test timeout
williamlardier Aug 28, 2024
1b2f5f3
Ensure the KafkaCleaner test completes
williamlardier Sep 3, 2024
0257dca
Extend the duration checks for the DMF volume
williamlardier Sep 3, 2024
e22a9de
Lint code
williamlardier Sep 3, 2024
0125893
Increase frequency and checks of kafkacleaner
williamlardier Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions tests/ctst/common/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,15 @@ Then('i {string} be able to add user metadata to object {string}',
}
});

Then('kafka consumed messages should not take too much place on disk', { timeout: -1 },
const kafkaCleanerTimeout = 900000; // 15min
Then('kafka consumed messages should not take too much place on disk', { timeout: kafkaCleanerTimeout },
async function (this: Zenko) {
const kfkcIntervalSeconds = parseInt(this.parameters.KafkaCleanerInterval);
const checkInterval = kfkcIntervalSeconds * (1000 + 5000);
const checkInterval = kfkcIntervalSeconds * 1000 + 5000;

const timeoutID = setTimeout(() => {
assert.fail('Kafka cleaner did not clean the topics within the expected time');
}, checkInterval * 10); // Timeout after 10 Kafka cleaner intervals
}, Math.min(kafkaCleanerTimeout, checkInterval * 25)); // Timeout after 25 Kafka cleaner intervals

try {
const ignoredTopics = ['dead-letter'];
Expand Down Expand Up @@ -307,6 +308,9 @@ Then('kafka consumed messages should not take too much place on disk', { timeout

// If a topic remains in this array, it means it has not been cleaned
assert(topics.length === 0, `Topics ${topics.join(', ')} still have not been cleaned`);
} catch (err: unknown) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this not done in cucumber/ctst?
should it not be?

Copy link
Contributor Author

@williamlardier williamlardier Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a "finally" that clears the timeout even if we got an error, which mean we never stop the test as its timeout was -1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example: https://github.com/scality/Zenko/actions/runs/10668356308

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I mean is that (even with the catch) the exception should be propagated: and eventually reach cucumber/ctst where it shoudl be caught.... and immediately fail the test (even with no timeout)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the timeout was not due to an error... At least this ensures we log the error with a message

Errors not catched by the tests are sent to cucumber directly, it'll mark the test as failed. But in the build above, it didn't work

this.logger.error('Error while checking Kafka cleaner', { err });
assert.fail(err as Error);
} finally {
clearTimeout(timeoutID);
}
Expand Down
6 changes: 5 additions & 1 deletion tests/ctst/steps/dmf.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
import { Then, After } from '@cucumber/cucumber';
import assert from 'assert';
import { Utils } from 'cli-testing';
import { execShellCommand } from 'common/utils';

async function cleanDmfVolume() {
await execShellCommand('rm -rf /cold-data/*');
}

Then('dmf volume should contain {int} objects', async (objectCount: number) => {
Then('dmf volume should contain {int} objects', { timeout: 5 * 60 * 1000 }, async (objectCount: number) => {
let conditionOk = false;
while (!conditionOk) {
// Getting the number of objects inside the volume used
// by the mock dmf to store transitioned objects
const outStr = await execShellCommand('find /cold-data -type f | wc -l');
// we store two files per object (content and manifest.json)
conditionOk = Number(outStr) === objectCount * 2;
if (!conditionOk) {
await Utils.sleep(500);
}
}
assert(conditionOk);
});
Expand Down
5 changes: 3 additions & 2 deletions tests/ctst/steps/notifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Then, Given, When, After } from '@cucumber/cucumber';
import { strict as assert } from 'assert';
import { S3, Utils, KafkaHelper, AWSVersionObject, NotificationDestination } from 'cli-testing';
import { S3, Utils, KafkaHelper, AWSVersionObject, NotificationDestination, Constants } from 'cli-testing';
import { Message } from 'node-rdkafka';
import { cleanS3Bucket } from 'common/common';
import Zenko from 'world/Zenko';

const KAFKA_TESTS_TIMEOUT = Number(process.env.KAFKA_TESTS_TIMEOUT) || 60000;
const KAFKA_TESTS_TIMEOUT = Number(process.env.KAFKA_TESTS_TIMEOUT) || Constants.DEFAULT_TIMEOUT * 1.5;

const allNotificationTypes = [
's3:ObjectCreated:Put',
Expand Down Expand Up @@ -308,6 +308,7 @@ Then('notifications should be enabled for {string} event in destination {int}',
});

Then('i should {string} a notification for {string} event in destination {int}',
{ timeout: Constants.DEFAULT_TIMEOUT * 2 },
async function (this: Zenko, receive: string, notificationType: string, destination: number) {

const receivedNotification = await KafkaHelper.consumeTopicUntilCondition(
Expand Down
Loading