Run manila services using manila user/group #317
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fmount The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fmount
force-pushed
the
manila_user
branch
2 times, most recently
from
d3eed61
to
aab8d4f
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/b5937352a4704a908391e903b953f0da ❌ openstack-k8s-operators-content-provider FAILURE in 18m 36s |
recheck |
|
recheck |
|
Code lgtm, thank you! |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/c7238555a0b441cd924ec4a5f1a57ba1 ❌ openstack-k8s-operators-content-provider FAILURE in 11m 36s |
|
mm looks like there are issues cloning the repo [1] (/cc @viroel ) |
... which was probably a flaky issue |
fmount
force-pushed
the
manila_user
branch
2 times, most recently
from
6ec22e5
to
6e9ffd5
Compare
pkg/manila/dbsync.go
Outdated
| VolumeMounts: dbSyncMounts, | ||
| Args: args, | ||
| Image: instance.Spec.ManilaAPI.ContainerImage, | ||
| SecurityContext: dbSyncSecurityContext(), |
There was a problem hiding this comment.
@gouthampacha FYI now this run with its own security context, which is different from cronjobs but it's what we did for glance as well [1].
pkg/manila/funcs.go
Outdated
| RunAsGroup: &runAsGroup, | ||
| Capabilities: &corev1.Capabilities{ | ||
| Drop: []corev1.Capability{ | ||
| "MKNOD", |
There was a problem hiding this comment.
Did dropping "ALL" fail here? I wonder what Caps are needed..
There was a problem hiding this comment.
Yes, I tried added both SETUID and SETGID caps (and removing ALL), however kolla_start fails to run the init script.
One thing we can do, as an alternative solution, is to remove kolla and run the command /usr/bin/manila-manage --config-dir /etc/manila/manila.conf.d db sync" directly. This way we shouldn't need any additional capability and we can Drop: All`.
I'm going to update the patch and try that.
| @@ -19,6 +19,7 @@ LogFormat "%{X-Forwarded-For}i %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-A | |||
| SetEnvIf X-Forwarded-For "^.*\..*\..*\..*" forwarded | |||
| CustomLog /dev/stdout combined env=!forwarded | |||
| CustomLog /dev/stdout proxy env=forwarded | |||
| ErrorLog /dev/stdout | |||
There was a problem hiding this comment.
Did you discover this when testing capabilities? :)
There was a problem hiding this comment.
correct, w/o this it fails when using manilaUser
There was a problem hiding this comment.
Thank you @fmount
This looks good; i have one question regarding the dbsync caps..
fmount
force-pushed
the
manila_user
branch
2 times, most recently
from
426fe46
to
b8353e3
Compare
This patch represents an improvment of the existing code to make sure we run manila services using the manila user instead of root. It also set the right SecurityContext on both dbsync and cronjobs. Jira: https://issues.redhat.com/browse/OSPRH-9115 Signed-off-by: Francesco Pantano <[email protected]>
|
/test manila-operator-build-deploy-kuttl |
| @@ -71,6 +79,8 @@ const ( | |||
| ShortDuration = time.Duration(5) * time.Second | |||
| // NormalDuration - | |||
| NormalDuration = time.Duration(10) * time.Second | |||
| //DBSyncCommand - | |||
| DBSyncCommand = "/usr/bin/manila-manage --config-dir /etc/manila/manila.conf.d db sync" | |||
There was a problem hiding this comment.
++ Thanks; i do think this is a good improvement even in isolation!
|
/lgtm This looks great; thanks @fmount |
openshift-merge-bot
bot
merged commit f733ae7
into
openstack-k8s-operators:main
This patch represents an improvment of the existing code to make sure we run manila services using the manila user instead of root.
Jira: https://issues.redhat.com/browse/OSPRH-10142
Jira: https://issues.redhat.com/browse/OSPRH-9409