-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add a warning cosbench will be deprecated #317
base: master
Are you sure you want to change the base?
Conversation
…ass that cosbench support will be deprecated in a later PR. Signed-off-by: Lee Sanders <[email protected]>
@markhpc @perezjosibm please can you review my PR thanks! |
@lee-j-sanders I'm afraid you might have shoot yourself on the foot: you either misread or ignored the documentation of the unit test generator: https://github.com/ceph/cbt/blob/master/docs/AutomaticUnitTestGeneration.md section https://github.com/ceph/cbt/blob/master/docs/AutomaticUnitTestGeneration.md#workflow-recommeded Why do you need to generate a new unit test baseline .json on step 1) before doing changes? Does that make sense to you? |
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 am not sure if this PR is complete, do you need to specify somewhere the current version, and from which point you remove the actual CBT code that uses cosbench? I saw the PR for ceph/qa, I think might be helpful to link those somehown since are clearly dependent? Cheers
@@ -20,6 +20,8 @@ def __init__(self, archive_dir, cluster, config): | |||
|
|||
config = self.parse_conf(config) | |||
|
|||
logger.warning("Cosbench support within CBT will be deprecated in a later PR. Please use release tag v0.3 for continued use of cosbench.") |
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.
Is this warning appearing on any CBT version?
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.
v0.3 release tag was created before this PR.
Yes, since this PR will be merged after v0.3, if you pull this PR you will get this warning to indicate cosbench support will soon be deprecated everytime you run a cosbench test.
@@ -1,5 +1,9 @@ | |||
# CBT - The Ceph Benchmarking Tool | |||
|
|||
Release v0.3 will be the final version of CBT that contains support for cosbench. |
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 am guessing the current version is v0.3?
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.
Yes, the current version is v0.3, See the release tag history.
PR ceph/ceph#60169 on the ceph release implements the change to remove cosbench from Teuthology. There are QA tests using the cosbench CBT infrastructure I removed from PR60169. @perezjosibm Since ceph-ci only runs on releases Quincy and newer (as per the release page here: https://docs.ceph.com/en/latest/releases/) and Quincy and newer does not run any cosbench tests as part of the QA suite. Removal of cosbench from CBT will not affect the main Ceph release. Pacific and early uses cosbench, but no teuthology tests are run on this release. Therefore I don't believe there is any need to put a dependency link in. |
I mis-read your documentation and didn't realise that the baseline.json was already created so I didn't need to regenerate the baselne. I've re-run the tests here without running the baseline and confirmed that the test passes: |
@perezjosibm Thanks for the review, please see my replies above, no modifications made, please re-review, thanks |
Release v0.3 will be the final version of CBT that contains support for cosbench. | ||
There will be a PR soon that will deprecate support for cosbench. If you require | ||
support for cosbench please continue to use v0.3. | ||
|
||
## INTRODUCTION |
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.
Here is probably a good point where you can clarify the work surrounding this change (this is what I men for a link):
PR ceph/ceph#60169 on the ceph release implements the change to remove cosbench from Teuthology. There are QA tests using the cosbench CBT infrastructure I removed from PR60169.
It just makes things easier. Otherwise you need to jump across repos to track down everything is in place.
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.
@perezjosibm Added a new commit with the README.md change referencing the PR. Please re-review thanks!
…lease that removes cosbench CBT support from qa/tasks and teuthology. Signed-off-by: Lee Sanders <[email protected]>
@perezjosibm are you happy with the changes now? Thanks! |
This change adds a warning into the cosbench benchmark class to indicate that cosbench support will be deprecated in a later PR and to use the v0.3 release tag if you want to continue using cosbench with CBT.
A similar statement has been added to the README.md.
PR - ceph/ceph#60169 implements deprecation of Cosbench from Teuthology as part of the main Ceph release.
Quincy and newer releases do not run any cosbench tests. Teuthology/QA is only run for Quincy and newer releases.
Release tag v0.3 was created as a checkpoint to indicate this is the last version of CBT to support cosbench.
Once this PR is merged (after the release tag v0.3 was created), then cosbench may be removed in a future PR. This PR is a warning to indicate this may happen.