K8SPSMDB-958 fix mongos metrics #1289
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
pkg/apis/psmdb/v1/psmdb_defaults.go
Outdated
| if platform == version.PlatformKubernetes { | ||
| var tp int64 = 1001 | ||
| fsgroup = &tp | ||
| } | ||
|
|
There was a problem hiding this comment.
[gofmt] reported by reviewdog 🐶
| if platform == version.PlatformKubernetes { | |
| var tp int64 = 1001 | |
| fsgroup = &tp | |
| } | |
| if platform == version.PlatformKubernetes { | |
| var tp int64 = 1001 | |
| fsgroup = &tp | |
| } | |
pkg/apis/psmdb/v1/psmdb_defaults.go
Outdated
| tvar := true | ||
| cr.Spec.Sharding.Mongos.ContainerSecurityContext = &corev1.SecurityContext{ | ||
| RunAsNonRoot: &tvar, | ||
| RunAsUser: fsgroup, | ||
| } | ||
| } | ||
|
|
||
| if cr.Spec.Sharding.Mongos.PodSecurityContext == nil { | ||
| cr.Spec.Sharding.Mongos.PodSecurityContext = &corev1.PodSecurityContext{ | ||
| FSGroup: fsgroup, | ||
| } | ||
| } |
There was a problem hiding this comment.
[gofmt] reported by reviewdog 🐶
| tvar := true | |
| cr.Spec.Sharding.Mongos.ContainerSecurityContext = &corev1.SecurityContext{ | |
| RunAsNonRoot: &tvar, | |
| RunAsUser: fsgroup, | |
| } | |
| } | |
| if cr.Spec.Sharding.Mongos.PodSecurityContext == nil { | |
| cr.Spec.Sharding.Mongos.PodSecurityContext = &corev1.PodSecurityContext{ | |
| FSGroup: fsgroup, | |
| } | |
| } | |
| tvar := true | |
| cr.Spec.Sharding.Mongos.ContainerSecurityContext = &corev1.SecurityContext{ | |
| RunAsNonRoot: &tvar, | |
| RunAsUser: fsgroup, | |
| } | |
| } | |
| if cr.Spec.Sharding.Mongos.PodSecurityContext == nil { | |
| cr.Spec.Sharding.Mongos.PodSecurityContext = &corev1.PodSecurityContext{ | |
| FSGroup: fsgroup, | |
| } | |
| } |
nmarukovich
requested review from
tplavcic,
hors,
egegunes,
inelpandzic and
pooknull
as code owners
pkg/apis/psmdb/v1/psmdb_defaults.go
Outdated
| @@ -176,6 +176,26 @@ func (cr *PerconaServerMongoDB) CheckNSetDefaults(platform version.Platform, log | |||
| } | |||
| } | |||
|
|
|||
| var fsgroup *int64 | |||
There was a problem hiding this comment.
I think we need to set it only for >- 1.15
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| } | ||
|
|
||
| if cr.Spec.Sharding.Mongos.ContainerSecurityContext == nil { | ||
| tvar := true |
There was a problem hiding this comment.
Why is it better to do it in sts? @inelpandzic
There was a problem hiding this comment.
I don't think we should touch CR if not necessary, it can be easily removed by the user. In the end, it is something internal that we need to ensure, just like we, for example, set init container or sidecar containers. We don't do it by updating the CR, but internally set it in our sts/deployment specs.
There was a problem hiding this comment.
As for me, we should either change this feature in CR or STS for all types of resources. Because for now we change it for cfg and rs in CR.
@egegunes @hors @inelpandzic
What do you think?
There was a problem hiding this comment.
Aha, I just saw that we do the same thing for other objects, like backup, and in this case, I think we should do it as you did it now @nmarukovich because of the consistency even though I think that generally it should be done where we create specs themselves rather than in CR.
There was a problem hiding this comment.
Aha, I just saw that we do the same thing for other objects, like backup, and in this case, I think we should do it as you did it now @nmarukovich because of the consistency even though I think that generally it should be done where we create specs themselves rather than in CR.
Despite the fact, that we already did it, I still want to discuss what we want to have ideally. In this case we can plan to refactor it or add a comment that decided to do like this (just avoid next time the same discussion )
There was a problem hiding this comment.
Ideally these should be (or close to) where we define sts for given component. Currently this place is the correct one IMO.
There was a problem hiding this comment.
Ok! So let's merge in this case (=
commit: b24a013 |
CHANGE DESCRIPTION
Problem:
Short explanation of the problem.
On mongos we do not have
Cause:
Short explanation of the root cause of the issue if applicable.
PMM fails to monitor mongoS due to lack of permission
Solution:
Short explanation of the solution we are providing with this PR.
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability