-
Notifications
You must be signed in to change notification settings - Fork 84
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.
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
Cleanup created accounts and buckets after tests #2152
base: development/2.6
Are you sure you want to change the base?
Conversation
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
6162a8c
to
8f4cd25
Compare
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command:
Alternatively, the |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command:
Alternatively, the |
241c291
to
eea904c
Compare
/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 |
|
||
MONGODB_COLLECTIONS=$(kubectl exec -n ${NAMESPACE} data-db-mongodb-sharded-mongos-0 -- mongo ${ZENKO_MONGODB_DATABASE} -u ${MONGODB_ROOT_USERNAME} -p ${MONGODB_ROOT_PASSWORD} --authenticationDatabase admin --quiet --eval "db.getCollectionNames().join(' ')") | ||
|
||
for collection in ${MONGODB_COLLECTIONS}; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to keep only the special collections?
For now, I see ~400 collections kept (due to object lock or test failing), they all have less than 10 objects anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what we want to do...
it takes ~1min20, which is significant (maybe not after 2h of test, but we need to reduce this 2h anyway...)
- we should try to reduce the time. Probably a large time is related to kubectl kube setup & I/O transfers out of the container... May be much faster using mongo dump ; and possibly "copying" the file from the container after the tool has finished, or creating a new container with a host-volume to directly "save" the file to the host...
- if we can't make it significantly faster, we may need some filtering indeed AND/OR do this only if the previous job has failed...
tests/ctst/steps/utils/utils.ts
Outdated
await uploadSetup(world, 'PutObject'); | ||
world.addCommandParameter({ key: world.getSaved<string>('objectName') }); | ||
world.addCommandParameter({ bucket: world.getSaved<string>('bucketName') }); | ||
const result = await S3.putObject(world.getCommandParameters()); | ||
const versionId = extractPropertyFromResults<string>(result, 'VersionId'); | ||
world.saveCreatedObject(world.getSaved<string>('objectName'), versionId || ''); | ||
|
||
world.addToSaved('versionId', extractPropertyFromResults( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saving versionId
seems redundant with world.saveCreatedObject
; even if we keep it to avoid too wide-ranging changes to the tests, should it not be done inside saveCreatedObject
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the lines, and updated the function: it was storing always the last version id, but we may need to store all the known version IDs... We can later add a function in the world to delete the object if removed from the bucket, but I would like to do it in a dedicated ticket, where we centralize all the duplicated logic (so far I see that tests usually require some common functions to manage objects, that should be part of the World and not re-implemented in different places).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> ZENKO-4904
MONGODB_COLLECTIONS=$(kubectl exec -n ${NAMESPACE} data-db-mongodb-sharded-mongos-0 -- mongo ${ZENKO_MONGODB_DATABASE} -u ${MONGODB_ROOT_USERNAME} -p ${MONGODB_ROOT_PASSWORD} --authenticationDatabase admin --quiet --eval "db.getCollectionNames().join(' ')") | ||
|
||
for collection in ${MONGODB_COLLECTIONS}; do | ||
kubectl exec -n ${NAMESPACE} data-db-mongodb-sharded-mongos-0 -- mongo ${ZENKO_MONGODB_DATABASE} -u ${MONGODB_ROOT_USERNAME} -p ${MONGODB_ROOT_PASSWORD} --authenticationDatabase admin --quiet --eval "db.getCollection((\"${collection}\")).find().toArray()" > /tmp/artifacts/data/${STAGE}/mongodb-${collection}.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we use mongodump
instead?
it allows dumping the whole DB or specific collection, may be faster. However, it exports a BSON: more compact, but maybe less practical to use (for analysis, may need to pass it through bsondump
or restore it into another mongo)
otherwise, we may use mongoexport
to dump a collection to JSON, one at a time...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated with mongodump & bsondump, let's see if my approach is faster...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New logic takes 10s to complete, I think we are good...
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.7/bugfix/ZENKO-4898
$ git pull # or "git reset --hard origin/w/2.7/bugfix/ZENKO-4898"
$ git merge origin/development/2.7
$ # <intense conflict resolution>
$ git commit
$ git merge origin/bugfix/ZENKO-4898
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/2.7/bugfix/ZENKO-4898 The following options are set: create_integration_branches |
febf8e3
to
7eb9161
Compare
7eb9161
to
da7994a
Compare
Issue: ZENKO-4898
da7994a
to
f9d31df
Compare
History mismatchMerge commit #c2e65127c6f4162bc8e06aa73caeb351601707fc 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 |
/reset |
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: create_integration_branches |
- Remove duplicated logic - Use a world-managed function to keep track of created objects - Clean the buckets at the end of any test, unless it failed, to ease debugging. Issue: ZENKO-4898
- Assume role tests create additional accounts that we must clean at the end of any (successful) scenario. - This ensures that the GetRolesForWebIdentity calls are not impacted by the high number of accounts. Issue: ZENKO-4898
- Quotas are not needed, plus, they cause errors when using the env variable in a kubectl command. Issue: ZENKO-4898
- Tests should cleanup resources, unless a scenario failed - Use of mongodump and bsondump for fast speed Issue: ZENKO-4898
77045d6
to
7ab8dce
Compare
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 |
Without cleanup, the GetRolesForWebIdentity API ends up being very slow, eventually causing Vault to be unresponsive and thus causing timeouts, and tests failures. This API is not supposed to work with more than 100 accounts (as per requirements), and here, we end up with >1000, meaning at least this number of internal (and sequential) DB calls per "GetRolesForWebIdentity" API call.
The logic detects that we created an account (during cross-account tests), and delete all resources, before deleting the account.
Issue: ZENKO-4898