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

rgw: Correct the return codes for the health check feature #13557

Merged
merged 1 commit into from Mar 1, 2017

Conversation

Projects
None yet
3 participants
@prallabh
Contributor

prallabh commented Feb 21, 2017

Fixes: http://tracker.ceph.com/issues/19025
Signed-off-by: Pavan Rallabhandi PRallabhandi@walmartlabs.com

@cbodley cbodley requested a review from rzarzynski Feb 22, 2017

@cbodley

This comment has been minimized.

Contributor

cbodley commented Feb 22, 2017

looks correct to me. what do you think @rzarzynski ?

@prallabh

This comment has been minimized.

Contributor

prallabh commented Feb 28, 2017

@rzarzynski can you please take a look

if (::access(g_conf->rgw_healthcheck_disabling_path.c_str(), F_OK) == 0) {
/* path exists */
op_ret = -ERR_SERVICE_UNAVAILABLE;
} else {

This comment has been minimized.

@rzarzynski

rzarzynski Feb 28, 2017

Contributor

Is there a reason for duplicating the 200 OK case? Maybe something like below?

  if (! g_conf->rgw_healthcheck_disabling_path.empty()) &&
      ::access(g_conf->rgw_healthcheck_disabling_path.c_str(), F_OK ) == 0) {
    /* Disabling path specified & existent in the filesystem. */
    op_ret = -ERR_SERVICE_UNAVAILABLE; /* 503 */
  } else {
    op_ret = 0; /* 200 OK */
  }

This comment has been minimized.

@prallabh

prallabh Mar 1, 2017

Contributor

No reason but for to be more explicit on the path existence and access failures. Corrected it now.

This comment has been minimized.

@rzarzynski

rzarzynski Mar 1, 2017

Contributor

Thanks!

root
rgw: Correct the return codes for the health check feature
Fixes: http://tracker.ceph.com/issues/19025
Signed-off-by: Pavan Rallabhandi <PRallabhandi@walmartlabs.com>
@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Mar 1, 2017

jenkins test this please

@rzarzynski rzarzynski self-assigned this Mar 1, 2017

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Mar 1, 2017

jenkins test this please (unrelated failure in ceph_objectstore_tool.py)

@rzarzynski rzarzynski merged commit d40a318 into ceph:master Mar 1, 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

@prallabh prallabh deleted the prallabh:healthcheck branch Mar 2, 2017

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