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.

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

qa/mgr/test_progress: add _get_osd_in_out_events to account for osd marked in/out events #38107

Merged
merged 1 commit into from Nov 18, 2020

Conversation

kamoltat
Copy link
Member

@kamoltat kamoltat commented Nov 16, 2020

Fixes a failing test case regarding osd coming back
after being marked out. The old test case wasn't accounting
for a specific event, therefore this resulted in the failure.
The fix basically accounts for a specific event of osd being
marked in/out.

Fixes: https://tracker.ceph.com/issues/48217

Signed-off-by: Kamoltat ksirivad@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

Copy link
Member

@neha-ojha neha-ojha left a comment

Choose a reason for hiding this comment

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

can you please follow https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#fixes-lines to add the "Fixes" line and update the commit title to explain what the fix does

Fixes a failing test case regarding osd coming back
after being marked out. The old test case wasn't accounting
for a specific event, therefore this resulted in the failure.
The fix basically accounts for a specific event of osd being
marked in/out.

Fixes: https://tracker.ceph.com/issues/48217

Signed-off-by: Kamoltat <ksirivad@redhat.com>
@kamoltat
Copy link
Member Author

@neha-ojha I ran 18 tests on this and it passed successfully. Let me know if I should run more tests: https://pulpito.ceph.com/ksirivad-2020-11-17_04:43:46-rados:mgr-wip-mgr-progress-fix-48217-distro-basic-smithi/

@neha-ojha
Copy link
Member

@neha-ojha I ran 18 tests on this and it passed successfully. Let me know if I should run more tests: https://pulpito.ceph.com/ksirivad-2020-11-17_04:43:46-rados:mgr-wip-mgr-progress-fix-48217-distro-basic-smithi/

If there aren't any other tests exercising the progress module, then this is good.

Copy link
Member

@neha-ojha neha-ojha left a comment

Choose a reason for hiding this comment

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

nit: how about changing the title to qa/mgr/test_progress: add _get_osd_in_out_events to account for osd marked in/out events?

@neha-ojha
Copy link
Member

@epuertat @tchaikov does the auto labeler remove manually added labels? Seeing it in this PR.

@epuertat
Copy link
Member

epuertat commented Nov 18, 2020

@epuertat @tchaikov does the auto labeler remove manually added labels? Seeing it in this PR.

@neha-ojha this is the issue: actions/labeler#104

"Hacked" (until the real fix happens) in #38158

epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Nov 18, 2020
Fixes: ceph#38107
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Nov 18, 2020
Fixes: ceph#38107 (comment)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Nov 18, 2020
Yaml syntax cleaned too.

Fixes: ceph#38107 (comment)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Nov 18, 2020
Yaml syntax cleaned too.

Fixes: ceph#38107 (comment)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
@kamoltat kamoltat changed the title qa/mgr/test_progress: fix bug 48217 qa/mgr/test_progress: add _get_osd_in_out_events to account for osd marked in/out events Nov 18, 2020
@neha-ojha neha-ojha added the core label Nov 18, 2020
@neha-ojha neha-ojha merged commit 1523bf9 into master Nov 18, 2020
@neha-ojha neha-ojha deleted the wip-mgr-progress-fix-48217 branch November 18, 2020 15:15
@tchaikov
Copy link
Contributor

@kamoltat please use your own repo for creating topic branches. and also please note the title of a pull request is not the title of a commit, in future, i'd suggest use git commit --amend to update your commit and repush to update your change.

@smithfarm
Copy link
Contributor

smithfarm commented Dec 10, 2020

Just to supplement what @tchaikov said: here we have a descriptive PR title ("qa/mgr/test_progress: add _get_osd_in_out_events to account for osd marked in/out events") coupled with a bad, non-descriptive commit title ("qa/mgr/test_progress: fix bug 48217"), yet it's the commit title that enters the git history. The git history is what gets downloaded to each developer's computer, and the commit title is what is shown in git log. The github PR title, by contrast, only resides on github's servers and can (effectively) only be seen via a web browser.

https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#describe-your-changes has more guidance on this subject.

@kamoltat
Copy link
Member Author

@smithfarm I agree, my apologies for the bad commit title will try to improve for the next PRs/commits

jmolmo pushed a commit to jmolmo/ceph that referenced this pull request Dec 14, 2020
Yaml syntax cleaned too.

Fixes: ceph#38107 (comment)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
tchaikov pushed a commit to tchaikov/ceph that referenced this pull request Mar 8, 2021
Yaml syntax cleaned too.

Fixes: ceph#38107 (comment)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
(cherry picked from commit 59702b6)
tchaikov pushed a commit to tchaikov/ceph that referenced this pull request Mar 8, 2021
Yaml syntax cleaned too.

Fixes: ceph#38107 (comment)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
(cherry picked from commit 59702b6)
ideepika pushed a commit to ideepika/ceph that referenced this pull request Apr 5, 2021
Yaml syntax cleaned too.

Fixes: ceph#38107 (comment)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
(cherry picked from commit 59702b6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants