Use environment from containers.conf #7355
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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
| @@ -43,6 +43,13 @@ type ContainerBasicConfig struct { | |||
| // image's configuration. | |||
| // Optional. | |||
| Command []string `json:"command,omitempty"` | |||
| // EnvHost indicates that the host environment should be added to container | |||
| // Optional. | |||
| EnvHost bool `json:"env_host,omitempty"` | |||
There was a problem hiding this comment.
Why are we sourcing this from the server, instead of the host? This doesn't make sense, IMO
There was a problem hiding this comment.
If you are on a MAC or Windows box you want this from the server. The Proxies are setup on the Server, Proxies on the Client might not work while you are on the server machines.
|
This does not solve the problem #7343 is trying to solve - ensuring that $PATH and $TERM are always populated in containers we make. |
|
PATH will always be set. If we need TERM set always the change should go into containers common. Not be hard coded in Podman. |
|
I don't see any guarantee that we set a default $PATH if one is not provided by the image. |
|
And IMO, we should just not support |
|
rebase on master >= 748e882 to fix CI |
|
Looking at the code --env-host and --http-proxy is already blocked from podman-Remote calls. |
rhatdan
force-pushed
the
env
branch
4 times, most recently
from
422f623 to
831ecae
Compare
|
LGTM |
podman needs to use the environment settings in containers.conf when setting up the containers. Also host environment variables should be relative to server side not the client. Signed-off-by: Daniel J Walsh <[email protected]>
|
@mheon @baude @giuseppe @vrothberg @QiWang19 @ashley-cui @jwhonce PTAL |
|
LGTM |
|
/lgtm |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
Wait a moment, I thought we agreed to drop |
|
env=host was dropped from podman-remote. |
github-actions
bot
added
the
locked - please file new issue/PR
podman needs to use the environment settings in containers.conf
when setting up the containers.
Also host environment variables should be relative to server side
not the client.
Signed-off-by: Daniel J Walsh [email protected]