Improvement/zenko 4941 #2178
Improvement/zenko 4941 #2178
Conversation
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
|
/create_integration_branches |
williamlardier
force-pushed
the
improvement/ZENKO-4941
branch
2 times, most recently
from
5e9e1d6 to
6841f9b
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
williamlardier
force-pushed
the
improvement/ZENKO-4941
branch
from
6841f9b to
2281c03
Compare
williamlardier
force-pushed
the
improvement/ZENKO-4941
branch
2 times, most recently
from
522a2b7 to
70d130c
Compare
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
|
ping |
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. Use a file locking mechanism to ensure no concurrent job runs - Watching the resource or using kube api is prone to race condition, as their is a delay between the request to create a job, and its run, plus, the logic is not easy to centralize if we consider job already completed or not yet started, as we delete the old jobs upon completion - Using alock mechanism is similar to what we already have for quotas, plus, it ensures no race condition and tests being able to restart as soon as possible. Issue: ZENKO-4941
williamlardier
force-pushed
the
improvement/ZENKO-4941
branch
from
29a2c25 to
22d65a2
Compare
History mismatchMerge commit #fbf25db87d2669b6b7877eaf2adf29f4c2075d0b on the integration branch It is likely due to a rebase of the branch Please use the The following options are set: create_integration_branches |
|
/force_reset |
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
ConflictA conflict has been raised during the update of Please resolve the conflict on the integration branch ( Here are the steps to resolve this conflict: $ git fetch
$ git checkout w/2.8/improvement/ZENKO-4941
$ git pull # or "git reset --hard origin/w/2.8/improvement/ZENKO-4941"
$ git merge origin/development/2.8
$ # <intense conflict resolution>
$ git commit
$ git merge origin/w/2.7/improvement/ZENKO-4941
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.8/improvement/ZENKO-4941The following options are set: create_integration_branches |
- Ensure no scenario requiring count items to run at the same time, as they were overriding each other: if count items completes between 2 checks of a quota scenario, the "current" may be cleaned, leading to wrong test results. - Incerase the count items lock stale duration: if we have too many buckets, we might fail to get the lock in time. - Put the count items scenario in a dedicated folder, as it is not related to quotas. Issue: ZENKO-4941
williamlardier
force-pushed
the
improvement/ZENKO-4941
branch
from
29a7622 to
e479cb4
Compare
History mismatchMerge commit #29a7622583ed2a0044029a19e3ce3b7e1065d667 on the integration branch It is likely due to a rebase of the branch Please use the The following options are set: create_integration_branches |
|
/force_reset |
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
- It may be prone to periodic instabilities - Not sopping won't change the result and make the step more close to what the sentence says Issue: ZENKO-4941
- We use 2s but the scuba frequency is 2s, so if we are unlucky, we may not be able to have it flushed, leading to flakies Issue: ZENKO-4941
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the The following options are set: create_integration_branches |
|
|
||
| if (!fs.existsSync(lockFilePath)) { | ||
| fs.writeFileSync(lockFilePath, ''); | ||
| } |
There was a problem hiding this comment.
if the lock file exists, it means another test is waiting for this job : in that case, should the function not simply wait (polling) until the lock is removed? (i.e. the "first" test has completed its wait, and the job has indeed completed)
There was a problem hiding this comment.
The problem with polling like that is that two tests may detect that the file does not exist at the same time, hence the use of the lock file, that internally relies on directory creation (which is atomic) and last modified dates, to ensure the whole approach is safe.
There was a problem hiding this comment.
instead of if (!fs.existsSync(lockFilePath)) { fs.writeFileSync(lockFilePath, '');, we can do fs.writeFileSync(path, data, { overwrite: false } : to atomically write the file without overwriting, so only one will ever succeed.
--> depending on the "outcome" of the creation, the code can decide if it should proceed with reading the cron/... or if it should simply wait because something else is already doing it.
Won't change the test, but the sentences are now more meaningful. Issue: ZENKO-4941
| @@ -353,8 +353,7 @@ When('the user tries to perform the current S3 action on the bucket {int} times | |||
| } | |||
| await runActionAgainstBucket(this, this.getSaved<ActionPermissionsType>('currentAction').action); | |||
| if (this.getResult().err) { | |||
There was a problem hiding this comment.
maybe better to keep failing on error, except for "retriable" errors : which can be safey ignore as this step is indeed about making multiple attempts
Complete tests of Quota feature in CTST.
-> custom images to be deleted
Another test to review starting the 2.8 branch:
development/2.8...w/2.8/improvement/ZENKO-4941#diff-0f9a8cc11d2560e9e9ea8668d53be732997ae7cecb4ec7201c836bc8367fc5c9R182
-> All tests are passing: