deployment: carry over the securityContext from deployer to lifecycle hooks #12733
Conversation
|
There was some discussion upstream related to securityContext for contaienrs being broken but I can't remember if it was an issue or from the mailing list. |
|
@Kargakis if it was broken the deployer pod won't be working? |
|
Not necessarily. Found the discussion: kubernetes/kubeadm#107 |
|
@pweil- @php-coder can you confirm the securityContext is broken for SELinux? @Kargakis also it is possible to set |
looks like this was new to 1.12.5/1.13 |
| @@ -397,6 +397,7 @@ func makeHookPod(hook *deployapi.LifecycleHook, rc *kapi.ReplicationController, | |||
| Env: mergedEnv, | |||
| Resources: resources, | |||
| VolumeMounts: volumeMounts, | |||
| SecurityContext: baseContainer.SecurityContext, | |||
There was a problem hiding this comment.
this is a pointer, should we be making a copy?
There was a problem hiding this comment.
yes and should not be copied from baseContainer but from the deployment config...
There was a problem hiding this comment.
@pweil- fixed. finally ;)
mfojtik
force-pushed
the
lifecycle_security
branch
3 times, most recently
from
a67f146 to
a9b8931
Compare
|
[test] @pweil- updated, ptal |
| @@ -375,6 +375,11 @@ func makeHookPod(hook *deployapi.LifecycleHook, rc *kapi.ReplicationController, | |||
|
|
|||
| gracePeriod := int64(10) | |||
|
|
|||
| podSecurityContext, err := kapi.Scheme.DeepCopy(rc.Spec.Template.Spec.SecurityContext) | |||
There was a problem hiding this comment.
@pweil- if DC has RC with more than 1 container, which one will you pick for the hook pod? ;-)
There was a problem hiding this comment.
you have to tell the hook pod the container name so it knows the image to use. That would be unique
mfojtik
force-pushed
the
lifecycle_security
branch
2 times, most recently
from
8d9383e to
38576ca
Compare
… to lifecycle hook
mfojtik
force-pushed
the
lifecycle_security
branch
from
38576ca to
d026451
Compare
|
@pweil- updated, PTAL :) |
|
Evaluated for origin test up to d026451 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/569/) (Base Commit: 801b564) |
|
I'd also update the comment here: origin/pkg/deploy/strategy/support/lifecycle.go Lines 176 to 181 in 83e3250 |
|
good catch. (but will fix in next PR because I don't want to ruin the green test run of this ;-) |
|
[merge] |
|
Evaluated for origin merge up to d026451 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/569/) (Base Commit: ed285a1) (Image: devenv-rhel7_6119) |
Fixes: #12103