Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

Add back default UNIX env to container config #1280

Merged
merged 1 commit into from
Sep 19, 2019

Conversation

estesp
Copy link
Member

@estesp estesp commented Sep 19, 2019

Fixes: #1279

Due to changes to the defaults in containerd, the CRI path to creating a
container OCI config needs to add back in the default UNIX $PATH (and
any other defaults) as that is the expected behavior from other
runtimes.

This is a miss from our fix released in 1.2.9 after breaking in 1.2.8 with the standard path in containerd via the API and ctr client (reported in containerd issue #3597 and fixed in containerd PR #3599)

Signed-off-by: Phil Estes estesp@linux.vnet.ibm.com

@estesp estesp mentioned this pull request Sep 19, 2019
@estesp
Copy link
Member Author

estesp commented Sep 19, 2019

/test pull-cri-containerd-verify

@Random-Liu Random-Liu added this to the v1.2 milestone Sep 19, 2019
Due to changes to the defaults in containerd, the CRI path to creating a
container OCI config needs to add back in the default UNIX $PATH (and
any other defaults) as that is the expected behavior from other
runtimes.

Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
@estesp
Copy link
Member Author

estesp commented Sep 19, 2019

/test pull-cri-containerd-verify

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

testID := "test-id"
testSandboxID := "sandbox-id"
testPid := uint32(1234)
expectedDefault := "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably export Containerd default PATH, and use it here. We can do that in a following PR.

@Random-Liu
Copy link
Member

/lgtm

@Random-Liu
Copy link
Member

@estesp Can you please also send cherry-picks to 1.2 and 1.3? Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cri path issue
4 participants