Incorporate image inspect data in play kube #2696
Conversation
|
fixes: #2665 |
|
/test images |
before, when an image signified a user, play kube ignored it. Incorporate that information. Signed-off-by: Peter Hunt <[email protected]>
haircommander
force-pushed
the
play-kube-dash-u
branch
from
2871d4c to
ce81ca3
Compare
|
LGTM |
|
|
||
| imageData, _ := newImage.Inspect(ctx) | ||
|
|
||
| containerConfig.User = "0" |
There was a problem hiding this comment.
Is this OK in a rootless scenario? But having said that, I'm not sure what else to set it to.
There was a problem hiding this comment.
rootless is actually disabled in play kube right now. This is also the default in pkg/spec/createconfig.go 's ParseCreateConfig. So I think it's safe even when rootless is enabled
There was a problem hiding this comment.
Yeah, rootless uses a user namespace to map UIDs, so we want to maintain the UID 0 default there too
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, rhatdan 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 |
openshift-ci-robot
added
the
approved
| @@ -280,6 +289,7 @@ func kubeContainerToCreateConfig(containerYAML v1.Container, runtime *libpod.Run | |||
| for _, e := range containerYAML.Env { | |||
| envs[e.Name] = e.Value | |||
| } | |||
| containerConfig.Env = envs | |||
There was a problem hiding this comment.
Wait, were we just... not using the environment variables before?
There was a problem hiding this comment.
nope I just scooted it up because an older PR I did split the for loop for envs and setting Env
|
LGTM |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
before, when an image signified a user, play kube ignored it. Incorporate that information.
Signed-off-by: Peter Hunt [email protected]