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.

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

cephadm: check if cephadm is root after cli is parsed #44498

Merged
merged 2 commits into from
Jan 13, 2022

Conversation

phlogistonjohn
Copy link
Contributor

@phlogistonjohn phlogistonjohn commented Jan 7, 2022

This change avoids checking that the command is root until after the cli is parsed so that cli help can be displayed to regular non-root users. I opted not to try and refactor the logging setup call too much, but in the future it may be good to separate the setup of the logging module in python from the environmental setup actions that require root privs.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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 cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

Split the parsing and processing of the CLI from the logging
setup in the cephadm main func. Move logging setup to occur after
we've determined that there's a runnable command func.

This is preparatory work to allow running `cephadm --help` without being
root.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Fixes: https://tracker.ceph.com/issues/53572

Perform a check if cephadm is root after the CLI arguments are parsed
but before logging is configured. This allows a user to get help on
cephadm without requiring to be root or use sudo, etc.
The root check must be done before logging is configured because the
logging set up function creates dirs and files in system dirs.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
@phlogistonjohn phlogistonjohn requested a review from a team as a code owner January 7, 2022 15:47
@sebastian-philipp
Copy link
Contributor

jenkins test make check

@adk3798
Copy link
Contributor

adk3798 commented Jan 13, 2022

https://pulpito.ceph.com/melissali-2022-01-12_17:49:26-rados:cephadm-wip-swagner-testing-2022-01-11-1258-distro-basic-smithi/

Unrelated failures:


Command failed on smithi089 with status 1: 'CEPH_REF=master CEPH_ID="0" PATH=$PATH:/usr/sbin adjust-ulimits ceph-coverage /home/ubuntu/cephtest/archive/coverage /home/ubuntu/cephtest/virtualenv/bin/cram -v -- /home/ubuntu/cephtest/archive/cram.client.0/*.t'

Not aware of a tracker for this but it has also happened on other runs as well and has no correlation with this change


Multiple dead jobs testing ingress, unrelated to this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants