Skip to content

mon/MonMap: fix unconditional failure for init_with_hosts#37758

Merged
tchaikov merged 4 commits intoceph:masterfrom
batrick:i47951
Oct 26, 2020
Merged

mon/MonMap: fix unconditional failure for init_with_hosts#37758
tchaikov merged 4 commits intoceph:masterfrom
batrick:i47951

Conversation

@batrick
Copy link
Copy Markdown
Member

@batrick batrick commented Oct 22, 2020

Fixes: https://tracker.ceph.com/issues/47951
Signed-off-by: Patrick Donnelly pdonnell@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

@batrick batrick force-pushed the i47951 branch 2 times, most recently from 1c5ecf6 to 31dbd86 Compare October 22, 2020 19:53
@tchaikov

This comment has been minimized.

Sign in to view
@batrick
Copy link
Copy Markdown
Member Author

batrick commented Oct 22, 2020

Oops, thanks @tchaikov .

@wido
Copy link
Copy Markdown
Member

wido commented Oct 23, 2020

This fixes: 2f07570

dillaman
dillaman approved these changes Oct 23, 2020
View reviewed changes
Copy link
Copy Markdown

@dillaman dillaman left a comment

Choose a reason for hiding this comment

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

lgtm -- can we get a quick unit test to resolve ceph.io (or something) by hostname to prevent this from getting broken in the future?

@wido
Copy link
Copy Markdown
Member

wido commented Oct 23, 2020

lgtm -- can we get a quick unit test to resolve ceph.io (or something) by hostname to prevent this from getting broken in the future?

That would be great. I know many clusters out there using Round Robin DNS entries for Monitor discovery.

@batrick
Copy link
Copy Markdown
Member Author

batrick commented Oct 23, 2020

I'm working on adding a test.

@batrick
test/mon: build MonMap unittest
8408b63 
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick
test/mon: fix compiler errors in MonMap unittest
06f44cb 
The code atrophied. Clean this up.

The tests are disabled because they SIGSEGV during SetUp.

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick
Copy link
Copy Markdown
Member Author

batrick commented Oct 23, 2020

lgtm -- can we get a quick unit test to resolve ceph.io (or something) by hostname to prevent this from getting broken in the future?

Done!

@batrick
test/mon: add tests for mon_host build by hostname
4022c1f 
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick
mon/MonMap: fix unconditional failure for init_with_hosts
21d9f83 
This bug prevents setting mon_host to a DNS name.

Fixes: https://tracker.ceph.com/issues/47951
Fixes: 7a1f02a
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick batrick requested a review from rjfd October 24, 2020 02:20
@batrick
Copy link
Copy Markdown
Member Author

batrick commented Oct 24, 2020

@rjfd I added to your MonMap.cc tests but found out they weren't actually getting built or run. I fixed some compiler errors but the tests SIGSEGV during SetUp. You might want to have another look at those after this gets merged?

@batrick
Copy link
Copy Markdown
Member Author

batrick commented Oct 24, 2020 

        Start 139: unittest_mon_monmap
        Start 140: unittest_mon_pgmap
        Start 141: unittest_mon_montypes
        Start 142: unittest_mon_election
119/209 Test #126: unittest_erasure_code_shec ................   Passed    6.61 sec
120/209 Test #141: unittest_mon_montypes .....................   Passed    0.81 sec
121/209 Test #140: unittest_mon_pgmap ........................   Passed    1.22 sec
122/209 Test #139: unittest_mon_monmap .......................   Passed    1.52 sec
123/209 Test #133: unittest_journal ..........................   Passed    5.91 sec

@tchaikov
Copy link
Copy Markdown
Contributor

tchaikov commented Oct 24, 2020

failures tracked by

@smithfarm
Copy link
Copy Markdown
Contributor

smithfarm commented Oct 24, 2020

@batrick Is the nautilus backport of this fix something we should fast-track?

@batrick
Copy link
Copy Markdown
Member Author

batrick commented Oct 25, 2020

@batrick Is the nautilus backport of this fix something we should fast-track?

Yes, I think so.

@batrick
Copy link
Copy Markdown
Member Author

batrick commented Oct 25, 2020

https://pulpito.ceph.com/?branch=wip-pdonnell-testing-20201024.032205

failures unrelated.

@tchaikov tchaikov merged commit 07019be into ceph:master Oct 26, 2020
@batrick batrick deleted the i47951 branch October 26, 2020 13:47
@smithfarm smithfarm mentioned this pull request Oct 26, 2020
Merged
@smithfarm
Copy link
Copy Markdown
Contributor

smithfarm commented Oct 26, 2020

@batrick Nautilus backport is up

@smithfarm smithfarm mentioned this pull request Oct 26, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tchaikov tchaikov tchaikov approved these changes

@wido wido wido approved these changes

@neha-ojha neha-ojha neha-ojha approved these changes

@ShyamsundarR ShyamsundarR Awaiting requested review from ShyamsundarR

@rjfd rjfd Awaiting requested review from rjfd

+1 more reviewer

@dillaman dillaman dillaman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug-fix core mon needs-qa

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@batrick @tchaikov @wido @smithfarm @dillaman @neha-ojha