Fix Kafka cleaner & Azure Location creation flakies #2097

williamlardier · 2024-06-24T07:52:31Z

Fix Kafkacleaner test, with dynamic sleep based on the partitions, and comparing the low property evolution to be more robust.
Add kubernetes clients for observing zenko status and data service deployments (cloudserver & backbeat)
In 2.8+ branches, fix the azure tests creating locations by using these new functions.

Change in 2.8+: 032cb7e

bert-e · 2024-06-24T07:52:35Z

Hello williamlardier,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2024-06-24T07:52:43Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

williamlardier · 2024-06-24T07:52:49Z

/create_integration_branches

bert-e · 2024-06-24T07:53:55Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/ZENKO-4833-fix into
development/2.6
w/2.7/bugfix/ZENKO-4833-fix will be merged into development/2.7
w/2.8/bugfix/ZENKO-4833-fix will be merged into development/2.8
w/2.9/bugfix/ZENKO-4833-fix will be merged into development/2.9

The following branches will NOT be impacted:

development/2.5

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: create_integration_branches

bert-e · 2024-06-24T07:53:58Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

The following options are set: create_integration_branches

KillianG · 2024-06-24T09:49:56Z

tests/ctst/common/common.ts

-
-                        // Topic is cleaned, we don't need to check it anymore
+                        // Ensure we're accessing the correct partition details
+                        const lowOffsetIncreased = parseInt(partition.low) >


why do we need to parseInt here and not below?

I am not done with the pr yet, jira transitioned it in review because I created the PR (not in draft to have the integration branches), but I still have some parts to improve, especially on the 2.8 branch
I will let you know when it's done

tests/ctst/common/common.ts

KillianG · 2024-06-24T09:52:09Z

tests/ctst/steps/utils/kubernetes.ts

+    // So the status might not be updated immediately after the overlay is applied.
+    // So, this function will first wait till we detect a reconciliation
+    // (deploymentInProgress = true), and then wait for the status to be available
+    const timeout = 15 * 60 * 1000;


why 15 minutes ? can we make it configurable ? The default TO in cucumber is 100 seconds, should we make it 100 seconds as well by default ?

I think we can access the timeout from the test by default, by passing the test object in the function, so that we wait just what's needed

So, to answer the question: the functions will have the timeout as a parameter, but the default remains 15min; the reason is that the service restart can take up to 3~4min as per my observations, and sometimes longer, if some tests are running at high frequency, blocking kube from restarting the pods quickly (as they are still handling requests).
So, the steps reyling on these functions will need specific timeouts.

tests/ctst/common/common.ts

bert-e · 2024-06-24T15:52:32Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

The following options are set: create_integration_branches

williamlardier · 2024-06-25T07:04:47Z

/force_reset

bert-e · 2024-06-25T07:04:57Z

Reset complete

I have successfully deleted this pull request's integration branches.

The following options are set: create_integration_branches

bert-e · 2024-06-25T07:05:51Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/ZENKO-4833-fix into
development/2.6
w/2.7/bugfix/ZENKO-4833-fix will be merged into development/2.7
w/2.8/bugfix/ZENKO-4833-fix will be merged into development/2.8
w/2.9/bugfix/ZENKO-4833-fix will be merged into development/2.9

The following branches will NOT be impacted:

development/2.5

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: create_integration_branches

bert-e · 2024-06-25T07:05:52Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

The following options are set: create_integration_branches

tests/ctst/common/common.ts

francoisferrand · 2024-06-28T14:01:12Z

tests/ctst/common/common.ts

@@ -342,8 +339,13 @@ Then('kafka consumed messages should not take too much place on disk', { timeout
                .filter(t => (t.includes(this.parameters.InstanceID) &&
                !ignoredTopics.some(e => t.includes(e))));

+            timeoutID = setTimeout(() => {
+                assert.fail('Kafka cleaner did not clean the topics within the expected time');
+            }, (topics.length || 1) * checkInterval * 5); // Timeout after 5 Kafka cleaner intervals


why do we increase the timeout by the number of topics?

the timeout is based on kafka-cleaner period, and we check every (remaining) topic on each iteration: so number of topics should not increase the timeout...

move the timeout here prevents catching issues with kafka connection (listTopics() call above) which could get stuck...

I adapted the logic, it's true we do not need to wait for each topic, but for sure we needed to wait more, as the cleaning might be slower in the CI due to other tests jeopardizing resources temporarily...

williamlardier · 2024-07-05T16:12:34Z

/wait

williamlardier · 2024-07-05T16:12:38Z

/approve

williamlardier marked this pull request as ready for review June 24, 2024 07:52

scality deleted a comment from bert-e Jun 24, 2024

KillianG reviewed Jun 24, 2024

View reviewed changes

tests/ctst/common/common.ts Outdated Show resolved Hide resolved

KillianG reviewed Jun 24, 2024

View reviewed changes

tests/ctst/common/common.ts Outdated Show resolved Hide resolved

KillianG reviewed Jun 24, 2024

View reviewed changes

tests/ctst/common/common.ts Outdated Show resolved Hide resolved

williamlardier force-pushed the bugfix/ZENKO-4833-fix branch 2 times, most recently from 9538041 to dd79b07 Compare June 25, 2024 07:01

williamlardier added 4 commits June 25, 2024 09:03

ZENKO-4833: rename quota teardown function

66c36d4

ZENKO-4833: add kube client for reconciliation and deploy watchers

c3fc72a

ZENKO-4833: bump ctst to 1.0.1

b04b791

ZENKO-4833: rework kafkacleaner test

746a5c6

williamlardier force-pushed the bugfix/ZENKO-4833-fix branch from dd79b07 to 746a5c6 Compare June 25, 2024 07:04

scality deleted a comment from bert-e Jun 25, 2024

williamlardier commented Jun 26, 2024

View reviewed changes

tests/ctst/common/common.ts Outdated Show resolved Hide resolved

williamlardier requested a review from KillianG June 26, 2024 08:37

williamlardier requested review from francoisferrand and benzekrimaha June 28, 2024 10:20

francoisferrand reviewed Jun 28, 2024

View reviewed changes

williamlardier added 2 commits July 1, 2024 09:12

ZENKO-4833: wait once for all topics

bbd4e40

ZENKO-4833: improve waiting duration

e331443

williamlardier requested a review from francoisferrand July 1, 2024 11:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Kafka cleaner & Azure Location creation flakies #2097

Fix Kafka cleaner & Azure Location creation flakies #2097

williamlardier commented Jun 24, 2024 •

edited

Loading

bert-e commented Jun 24, 2024

bert-e commented Jun 24, 2024

williamlardier commented Jun 24, 2024

bert-e commented Jun 24, 2024

bert-e commented Jun 24, 2024

KillianG Jun 24, 2024 •

edited

Loading

williamlardier Jun 24, 2024

KillianG Jun 24, 2024

williamlardier Jun 24, 2024

williamlardier Jun 25, 2024

bert-e commented Jun 24, 2024

williamlardier commented Jun 25, 2024

bert-e commented Jun 25, 2024

bert-e commented Jun 25, 2024

bert-e commented Jun 25, 2024

francoisferrand Jun 28, 2024

williamlardier Jul 1, 2024

williamlardier commented Jul 5, 2024

williamlardier commented Jul 5, 2024

Fix Kafka cleaner & Azure Location creation flakies #2097

Are you sure you want to change the base?

Fix Kafka cleaner & Azure Location creation flakies #2097

Conversation

williamlardier commented Jun 24, 2024 • edited Loading

bert-e commented Jun 24, 2024

Hello williamlardier,

bert-e commented Jun 24, 2024

Request integration branches

williamlardier commented Jun 24, 2024

bert-e commented Jun 24, 2024

Integration data created

bert-e commented Jun 24, 2024

Waiting for approval

KillianG Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

williamlardier Jun 24, 2024

Choose a reason for hiding this comment

KillianG Jun 24, 2024

Choose a reason for hiding this comment

williamlardier Jun 24, 2024

Choose a reason for hiding this comment

williamlardier Jun 25, 2024

Choose a reason for hiding this comment

bert-e commented Jun 24, 2024

Waiting for approval

williamlardier commented Jun 25, 2024

bert-e commented Jun 25, 2024

Reset complete

bert-e commented Jun 25, 2024

Integration data created

bert-e commented Jun 25, 2024

Waiting for approval

francoisferrand Jun 28, 2024

Choose a reason for hiding this comment

williamlardier Jul 1, 2024

Choose a reason for hiding this comment

williamlardier commented Jul 5, 2024

williamlardier commented Jul 5, 2024

williamlardier commented Jun 24, 2024 •

edited

Loading

KillianG Jun 24, 2024 •

edited

Loading