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: remove privileged security context from mgr sidecar container #13741

Merged
merged 1 commit into from Feb 13, 2024

Conversation

rkachach
Copy link
Contributor

@rkachach rkachach commented Feb 9, 2024

Mgr sidecar was using a privileged security context because it needed
to create the /var/lib/rook directory, generate the config on it, and
then copy it to the /etc/ceph directory. This change creates empty
directories on /var/lib/rook and /etc/ceph so the mgr sidecar can use
them, removing the need for using a privileged security context.

Issue resolved by this Pull Request:
Fixes: #13719

Manual testing:

k get pod | grep mgr | awk '{print $1}' | (while read x; do echo "======= $x =====" ; k get pod $x -o=json | jq -r '.spec.containers[] | "\(.name) - \(.securityContext)"'; done)

======= rook-ceph-mgr-a-74f9fd45d4-lpdhl =====
mgr - {"privileged":false}
watch-active - {"privileged":false}
======= rook-ceph-mgr-b-6549f7bcd6-wz9r7 =====
mgr - {"privileged":false}
watch-active - {"privileged":false}

Mgr sidecar watch-active is running without issues both on the active and on the mgr standby pods:

k logs rook-ceph-mgr-a-74f9fd45d4-lpdhl -c watch-active | tail -10                                         ✔   
2024-02-12 09:52:00.872858 I | cephcmd: successfully checked mgr_role label. checking again in 15s
2024-02-12 09:52:15.887326 I | op-mgr: Checking mgr_role label value of daemon a (prev active mgr was a)
2024-02-12 09:52:16.352018 I | op-mgr: active mgr is still the same (a). No need to update mgr_role label on daemon a.
2024-02-12 09:52:16.352034 I | cephcmd: successfully checked mgr_role label. checking again in 15s
2024-02-12 09:52:31.365846 I | op-mgr: Checking mgr_role label value of daemon a (prev active mgr was a)
2024-02-12 09:52:31.828295 I | op-mgr: active mgr is still the same (a). No need to update mgr_role label on daemon a.
2024-02-12 09:52:31.828314 I | cephcmd: successfully checked mgr_role label. checking again in 15s
2024-02-12 09:52:46.840292 I | op-mgr: Checking mgr_role label value of daemon a (prev active mgr was a)
2024-02-12 09:52:47.259273 I | op-mgr: active mgr is still the same (a). No need to update mgr_role label on daemon a.
2024-02-12 09:52:47.259293 I | cephcmd: successfully checked mgr_role label. checking again in 15s
k logs rook-ceph-mgr-b-6549f7bcd6-wz9r7 -c watch-active | tail -10                                         ✔   
2024-02-12 09:52:32.478736 I | cephcmd: successfully checked mgr_role label. checking again in 15s
2024-02-12 09:52:47.478856 I | op-mgr: Checking mgr_role label value of daemon b (prev active mgr was a)
2024-02-12 09:52:47.872795 I | op-mgr: active mgr is still the same (a). No need to update mgr_role label on daemon b.
2024-02-12 09:52:47.872813 I | cephcmd: successfully checked mgr_role label. checking again in 15s
2024-02-12 09:53:02.886029 I | op-mgr: Checking mgr_role label value of daemon b (prev active mgr was a)
2024-02-12 09:53:03.371399 I | op-mgr: active mgr is still the same (a). No need to update mgr_role label on daemon b.
2024-02-12 09:53:03.371469 I | cephcmd: successfully checked mgr_role label. checking again in 15s
2024-02-12 09:53:18.381855 I | op-mgr: Checking mgr_role label value of daemon b (prev active mgr was a)
2024-02-12 09:53:18.789335 I | op-mgr: active mgr is still the same (a). No need to update mgr_role label on daemon b.
2024-02-12 09:53:18.789353 I | cephcmd: successfully checked mgr_role label. checking again in 15s

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@rkachach rkachach requested review from travisn and a team February 9, 2024 15:59
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.

The integration tests are passing, so if manual testing looks good, this should be good to go after a couple other small CI issues are fixed.

pkg/operator/ceph/controller/spec.go Outdated Show resolved Hide resolved
@rkachach rkachach marked this pull request as ready for review February 12, 2024 09:13
@rkachach rkachach changed the title [WIP] mgr: remove privileged security context from mgr sidecar container mgr: remove privileged security context from mgr sidecar container Feb 12, 2024
pkg/operator/ceph/cluster/mgr/spec.go Outdated Show resolved Hide resolved
@rkachach rkachach force-pushed the fix_issue_13719 branch 3 times, most recently from 821a5d0 to e60b9c3 Compare February 13, 2024 13:02
Mgr sidecar was using a privileged security context because it needed
to create the /var/lib/rook directory, generate the config on it, and
then copy it to the /etc/ceph directory. This change creates empty
directories on /var/lib/rook and /etc/ceph so the mgr sidecar can use
them, removing the need for using a privileged security context.

Fixes: rook#13719

Signed-off-by: Redouane Kachach <rkachach@redhat.com>
@travisn travisn merged commit 932400a into rook:master Feb 13, 2024
51 checks passed
travisn added a commit that referenced this pull request Feb 13, 2024
mgr: remove privileged security context from mgr sidecar container (backport #13741)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mgr sidecar container is running in privileged mode
3 participants