Skip to content

add test for running container as non-default user that is missing#904

Merged
anmaxvl merged 2 commits intomicrosoft:masterfrom
anmaxvl:tests/run-lcow-non-default-user
Dec 10, 2020
Merged

add test for running container as non-default user that is missing#904
anmaxvl merged 2 commits intomicrosoft:masterfrom
anmaxvl:tests/run-lcow-non-default-user

Conversation

@anmaxvl
Copy link
Copy Markdown
Contributor

@anmaxvl anmaxvl commented Dec 8, 2020

refector tests to use getRunPodSandboxRequest where possible

Signed-off-by: Maksim An [email protected]

@anmaxvl anmaxvl requested a review from a team as a code owner December 8, 2020 09:50
Comment thread test/cri-containerd/container_downlevel_test.go
Comment thread test/cri-containerd/run_container_non_default_user_test.go Outdated
Config: &runtime.PodSandboxConfig{
Metadata: &runtime.PodSandboxMetadata{
Name: t.Name(),
Uid: "0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No idea what this is used for 🤣 but we aren't setting it anymore for a couple tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

from what I saw, it's used to construct sandbox name. so having it set to the same value doesn't accomplish much... we could randomize it I guess.

@dcantah
Copy link
Copy Markdown
Contributor

dcantah commented Dec 8, 2020

Maybe two separate commits would help here for better git history. First commit -> refactor and second commit -> add new test.

@anmaxvl anmaxvl force-pushed the tests/run-lcow-non-default-user branch from 9a2bc9e to bd2844c Compare December 8, 2020 18:12
@anmaxvl anmaxvl force-pushed the tests/run-lcow-non-default-user branch from bd2844c to dca168e Compare December 8, 2020 18:20
@ambarve
Copy link
Copy Markdown
Contributor

ambarve commented Dec 8, 2020

Thanks for adding this test!
Can we also add a similar test which uses a username instead of a uid?

@anmaxvl
Copy link
Copy Markdown
Contributor Author

anmaxvl commented Dec 8, 2020

Thanks for adding this test!
Can we also add a similar test which uses a username instead of a uid?

yeah, sure.

@anmaxvl anmaxvl force-pushed the tests/run-lcow-non-default-user branch from dca168e to c4f4fbe Compare December 8, 2020 19:12
@anmaxvl
Copy link
Copy Markdown
Contributor Author

anmaxvl commented Dec 8, 2020

Thanks for adding this test!
Can we also add a similar test which uses a username instead of a uid?

yeah, sure.

and done

@ambarve
Copy link
Copy Markdown
Contributor

ambarve commented Dec 8, 2020

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants