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.

Sign up for GitHub

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

Jump to bottom

hammer: mon: MonmapMonitor should return success when MON will be removed #12006

Merged
merged 1 commit into from Nov 21, 2016
Merged

hammer: mon: MonmapMonitor should return success when MON will be removed #12006

merged 1 commit into from Nov 21, 2016

Conversation

smithfarm
Copy link
Contributor

@jecluis @smithfarm
mon: MonmapMonitor: return success when monitor will be removed
0ac7f2f 
Fixes: http://tracker.ceph.com/issues/17725

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
(cherry picked from commit c9d46cf)

Conflicts:
	src/mon/MonmapMonitor.cc (just inserted the "err = 0" assignment
        immediately after the error message line)
@smithfarm smithfarm self-assigned this Nov 15, 2016
@smithfarm smithfarm added this to the hammer milestone Nov 15, 2016
@smithfarm
Copy link
Contributor Author

@jecluis Please review. I did not cherry-pick the second commit - the one removing the goto statements - because the hammer code does not seem to have goto statements.

@jecluis
Copy link
Member

jecluis commented Nov 16, 2016

@smithfarm lgtm

and the second commit isn't required as part of the bugfix.

@smithfarm smithfarm changed the title hammer: Error EINVAL: removing mon.a at 172.21.15.16:6789/0, there will be 1 monitors hammer: mon: MonmapMonitor should return success when MON will be removed Nov 16, 2016
smithfarm added a commit that referenced this pull request Nov 18, 2016
@smithfarm
Merge pull request #12006: hammer: mon: MonmapMonitor should return s…
87f5c54 
…uccess when MON will be removed

Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Nov 20, 2016
@smithfarm
Merge pull request #12006: hammer: mon: MonmapMonitor should return s…
a19857b 
…uccess when MON will be removed

Reviewed-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

@liewegas This backport passed a rados suite at http://tracker.ceph.com/issues/17151#note-7 with failures that I believe have been addressed (except for http://tracker.ceph.com/issues/15139 which is caused by the build system no longer providing dumpling-era packages).

I have rebuilt the integration branch to include the two fixes and scheduled a new run at http://tracker.ceph.com/issues/17151#note-14

Do you think it's OK to merge provided the second run succeeds?

smithfarm added a commit that referenced this pull request Nov 21, 2016
@smithfarm
Merge pull request #12006: hammer: mon: MonmapMonitor should return s…
22bbf9e 
…uccess when MON will be removed

Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Nov 21, 2016
@smithfarm
Merge pull request #12006: hammer: mon: MonmapMonitor should return s…
67d9465 
…uccess when MON will be removed

Reviewed-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

Second run succeeded; third run here: http://pulpito.front.sepia.ceph.com/smithfarm-2016-11-21_11:52:41-rados-hammer-backports---basic-smithi/

@liewegas liewegas merged commit 708c549 into ceph:hammer Nov 21, 2016
@smithfarm smithfarm deleted the wip-17905-hammer branch November 21, 2016 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smithfarm smithfarm

Labels
bug-fix core
Projects
None yet
Milestone
hammer
3 participants
@smithfarm @jecluis @liewegas