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

common: Better handling for missing/inaccessible ceph.conf files #14757

Merged
merged 9 commits into from Apr 25, 2017

Conversation

Projects
None yet
5 participants
@dmick
Member

dmick commented Apr 24, 2017

  • Note EINVAL as a specific Python exception type in rados.pyx
  • print the repr() of the exception so both the type and the message are shown
  • if the only error on conf search is ENOENT, report ENOENT
  • create/accumulate specific error messages for ENOENTs to report if all fail
  • log both warnings and errors to the client logger on failure to parse configs

I'm hopeful that this addresses many of the long-standing complaints about missing/inaccessible ceph.conf files being such a pain point, particularly for new Ceph users

Related Tracker issue: http://tracker.ceph.com/issues/19658

dmick added some commits Apr 18, 2017

ceph.in, pybind/rados/rados.pyx: Handle EINVAL better
Translate EINVAL to an exception type in rados.pyx
Print entire exception in ceph CLI

Signed-off-by: Dan Mick <dan.mick@redhat.com>
common/ConfUtils.cc: ConfFile:parse_file: warn about ENOENT
Consider ENOENT an error; it may or may not be reported, but
accumulate it as if it might be

Signed-off-by: Dan Mick <dan.mick@redhat.com>
common/ConfUtils.cc parse_file: fix function name in errors
Signed-off-by: Dan Mick <dan.mick@redhat.com>
common/config.cc md_config_t::parse_config_files: return ENOENT
If the only error we get is ENOENT, pass that back instead of
EINVAL, as it more-accurately reflects the error(s)

Signed-off-by: Dan Mick <dan.mick@redhat.com>
librados/librados.cc rados_conf_read_file: report errors to client
Previously all the error text was hidden; log it.
Note: ENOENT is noted as a 'parse error'

Signed-off-by: Dan Mick <dan.mick@redhat.com>
if (ret) {
if (warnings.str().length())

This comment has been minimized.

@badone

badone Apr 25, 2017

Contributor

Should we use "warnings.tellp() > 0" to avoid the string copy here?

This comment has been minimized.

@dmick

dmick Apr 25, 2017

Member

I suppose. I did a quick scan and missed that. Is it idiomatic?

This comment has been minimized.

@dmick

dmick Apr 25, 2017

Member

Done.

@badone

This comment has been minimized.

Contributor

badone commented Apr 25, 2017

@dmick looks like some tests may need attention. I remember talking to you about this some time ago and have had it in the back of my mind since so thanks for taking this on.

@@ -101,6 +101,9 @@ parse_file(const std::string &fname, std::deque<std::string> *errors,
char *buf = NULL;
FILE *fp = fopen(fname.c_str(), "r");
if (!fp) {
ostringstream oss;
oss << "parse_file: cannot open " << fname << ": " << cpp_strerror(errno);

This comment has been minimized.

@tchaikov

tchaikov Apr 25, 2017

Contributor

might want to use __func__ here.

This comment has been minimized.

@dmick

dmick Apr 25, 2017

Member

could. would save a little data space.

This comment has been minimized.

@dmick

dmick Apr 25, 2017

Member

Done

@dmick

This comment has been minimized.

Member

dmick commented Apr 25, 2017

Clearly I need to fix the tests, and I will

dmick added some commits Apr 25, 2017

global/global_init.cc global_pre_init: look for ENOENT
EINVAL for no conf file found changes to ENOENT

Signed-off-by: Dan Mick <dan.mick@redhat.com>
test/librados_test_stub/LibradosTestStub.cc: accept ENOENT
Missing ceph.conf now returns ENOENT rather than EINVAL

Signed-off-by: Dan Mick <dan.mick@redhat.com>
test/cli/ceph-conf/env-vs-args.t: accept new error messages
Failing to load ceph.conf now dumps more errors for the user

Signed-off-by: Dan Mick <dan.mick@redhat.com>
squashme: review comments
Signed-off-by: Dan Mick <dan.mick@redhat.com>

@liewegas liewegas changed the title from Better handling for missing/inaccessible ceph.conf files to common: Better handling for missing/inaccessible ceph.conf files Apr 25, 2017

@dmick

This comment has been minimized.

Member

dmick commented Apr 25, 2017

The tests pass. @tchaikov @badone if you're happy with the review fixes I will squash

@liewegas

lgtm!

@badone

badone approved these changes Apr 25, 2017

@liewegas liewegas merged commit 4ba4567 into ceph:master Apr 25, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
@bevinm

This comment has been minimized.

bevinm commented Jul 3, 2018

when I put
ssh mon1 sudo ceph health
I got this error
Error initializing cluster client: Error('error calling conf_read_file: error code 22',)

how I solve this,? why it is happened?

@dmick

This comment has been minimized.

Member

dmick commented Jul 3, 2018

@bevinm: this is an old closed pull request, and not the forum to ask questions; try on the ceph users list at ceph-users@lists.ceph.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment