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

mgr: run the watch side car config on /etc/ceph #9386

Merged
merged 2 commits into from Dec 15, 2021

Conversation

leseb
Copy link
Member

@leseb leseb commented Dec 10, 2021

Description of your changes:

We don't need to use /var/lib/rook for the ceph config and keyring, so
let's use /etc/ceph which has the right permissions too. Also, we need
to run with the ceph user since the rook image defaults to the "rook"
user.

Signed-off-by: Sébastien Han seb@redhat.com

Which issue is resolved by this Pull Request:
Resolves #9385

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Skip Tests for Docs: Add the flag for skipping the build if this is only a documentation change. See here for the flag.
  • Skip Unrelated Tests: Add a flag to run tests for a specific storage provider. See test options.
  • Reviewed the developer guide on Submitting a Pull Request
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.
  • Pending release notes updated with breaking and/or notable changes, if necessary.
  • Upgrade from previous release is tested and upgrade user guide is updated, if necessary.
  • Code generation (make codegen) has been run to update object specifications, if necessary.

@BlaineEXE
Copy link
Member

Just checking:
since /etc/ceph is mounted with ceph.conf from the rook-config-override configmap, will this be writing any data to the /etc/ceph/ceph.conf file that would mess with the override?

@leseb
Copy link
Member Author

leseb commented Dec 13, 2021

Just checking: since /etc/ceph is mounted with ceph.conf from the rook-config-override configmap, will this be writing any data to the /etc/ceph/ceph.conf file that would mess with the override?

Good point, I need to check and merge, if not done already.

For now, we must run the container with UID 0 and privileged for
multiple reasons:

* the rook binary writes ceph config to /var/lib/rook which is owned by
  root
* it's difficult to use /etc/ceph since it will conflict with the
  rook-ceph-override configmap AND is also owned by root since it's a
  mounted configmap.
* using /etc/ceph might be possible but has other issues with rook's
  exec package since the ceph config is built from /var/lib/rook

Closes: rook#9385
Signed-off-by: Sébastien Han <seb@redhat.com>
The context was not initialized and thus the configmap fetch will fail
with a nil pointer.

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb leseb force-pushed the mgr-active-side-car-permissions branch from d370654 to 892c9a2 Compare December 13, 2021 13:17
@leseb leseb marked this pull request as ready for review December 13, 2021 13:18
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

LGTM, if you could just confirm that it's been tested on OpenShift where the privs would be more critical.

Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

Testing on openshift looks good for both 1 or 2 mgrs, and the watch-active sidecar is running successfully. The OSDs also were created without any issues.

@travisn travisn merged commit 7a6a62f into rook:master Dec 15, 2021
mergify bot added a commit that referenced this pull request Dec 15, 2021
mgr: run the watch side car config on /etc/ceph (backport #9386)
@leseb leseb deleted the mgr-active-side-car-permissions branch December 16, 2021 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

watch-active sidecar /var/lib/rook: permission denied on 1.8.0
3 participants