Skip to content

cephadm: Add "--format" option to "ceph orch status"#35805

Merged
sebastian-philipp merged 1 commit intoceph:masterfrom
ricardoasmarques:ceph-orch-status-format
Jul 1, 2020
Merged

cephadm: Add "--format" option to "ceph orch status"#35805
sebastian-philipp merged 1 commit intoceph:masterfrom
ricardoasmarques:ceph-orch-status-format

Conversation

@ricardoasmarques
Copy link
Contributor

@ricardoasmarques ricardoasmarques commented Jun 26, 2020

With this PR, it's now possible to specify --format on ceph orch status command, e.g.:

master:~ # ceph orch status
Backend: cephadm
Available: False (SSH keys not set. Use `ceph cephadm set-priv-key` and `ceph cephadm set-pub-key` or `ceph cephadm generate-key`)

master:~ # ceph orch status --format=json

{"available": false, "backend": "cephadm", "reason": "SSH keys not set. Use `ceph cephadm set-priv-key` and `ceph cephadm set-pub-key` or `ceph cephadm generate-key`"}

master:~ # ceph orch status --format=json-pretty

{
  "available": false,
  "backend": "cephadm",
  "reason": "SSH keys not set. Use `ceph cephadm set-priv-key` and `ceph cephadm set-pub-key` or `ceph cephadm generate-key`"
}

master:~ # ceph orch status --format=yaml
available: false
backend: cephadm
reason: SSH keys not set. Use `ceph cephadm set-priv-key` and `ceph cephadm set-pub-key`
  or `ceph cephadm generate-key`

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

Signed-off-by: Ricardo Marques rimarques@suse.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 dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@ricardoasmarques ricardoasmarques requested a review from a team as a code owner June 26, 2020 17:49
sebastian-philipp
sebastian-philipp suggested changes Jun 26, 2020
View reviewed changes
Copy link
Contributor

@sebastian-philipp sebastian-philipp left a comment

Choose a reason for hiding this comment

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

I don't see a reason to only fix the format here.

src/pybind/mgr/orchestrator/module.py
output += "\nAvailable: {0}".format(result['available'])
if 'reason' in result:
output += ' ({0})'.format(result['reason'])
return HandleCommandResult(stdout=output)
Copy link
Contributor

@sebastian-philipp sebastian-philipp Jun 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
return HandleCommandResult(stdout=output)
return HandleCommandResult(retval=0 if avail else errno.EINVAL, stdout=output)

Copy link
Contributor Author

@ricardoasmarques ricardoasmarques Jun 30, 2020

Choose a reason for hiding this comment

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

If we return a non zero exit code, we will always see the Error EINVAL: prefix, which for instance, makes the following output an invalid json:

# ceph orch status --format=json
Error EINVAL: {"available": false, "backend": "cephadm", "reason": "SSH keys not set. Use `ceph cephadm set-priv-key` and `ceph cephadm set-pub-key` or `ceph cephadm generate-key`"}

For this reason, let's keep exit code zero for now.

@sebastian-philipp
Copy link
Contributor

sebastian-philipp commented Jun 29, 2020

If you want, you can add a call to ceph orch status --format ... in https://github.com/ceph/ceph/blob/749da74e2d9af075c2f365516cf9ea4370b58ec0/qa/tasks/cephadm_cases/test_cli.py to verify this.

@sebastian-philipp sebastian-philipp added the wip-swagner-testing My Teuthology tests label Jun 29, 2020
@ricardoasmarques ricardoasmarques force-pushed the ceph-orch-status-format branch from f8c0996 to 1fc6091 Compare June 30, 2020 17:38
@ricardoasmarques
Copy link
Contributor Author

ricardoasmarques commented Jun 30, 2020

If you want, you can add a call to ceph orch status --format ... in https://github.com/ceph/ceph/blob/749da74e2d9af075c2f365516cf9ea4370b58ec0/qa/tasks/cephadm_cases/test_cli.py to verify this.

Done

@ricardoasmarques
cephadm: Add "--format" option to "ceph orch status"
83ff831 
Fixes: https://tracker.ceph.com/issues/46233

Signed-off-by: Ricardo Marques <rimarques@suse.com>
@ricardoasmarques ricardoasmarques force-pushed the ceph-orch-status-format branch from 1fc6091 to 83ff831 Compare July 1, 2020 09:06
@sebastian-philipp
Copy link
Contributor

sebastian-philipp commented Jul 1, 2020 •
edited
Loading

https://pulpito.ceph.com/swagner-2020-07-01_09:26:21-rados:cephadm-wip-swagner-testing-2020-07-01-0956-distro-basic-smithi/

This was referenced Jul 1, 2020
Merged
Merged
@sebastian-philipp sebastian-philipp merged commit 7efae08 into ceph:master Jul 1, 2020
@sebastian-philipp sebastian-philipp mentioned this pull request Jul 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgfritch mgfritch mgfritch approved these changes

+1 more reviewer

@sebastian-philipp sebastian-philipp sebastian-philipp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cephadm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ricardoasmarques @sebastian-philipp @mgfritch