Skip to content

Conversation

@dcantah
Copy link
Contributor

@dcantah dcantah commented May 6, 2021

Add some new flags to the integration test suite that enables choosing what virtstack
to use for hypervisor isolated containers. This makes it so we can re-use our existing tests
and just pass in new flags to run them with a new stack.

To have this play nice with the test suite I changed getRunPodSandboxRequest to take in whatever
annotations we'd want to set on the pod config directly instead of us having to set them on the
returned object itself.

Signed-off-by: Daniel Canter dcanter@microsoft.com

Add some new flags to the integration test suite that enables choosing what virtstack
to use for hypervisor isolated containers. This makes it so we can re-use our existing tests
and just pass in new flags to run them with a new stack.

To have this play nice with the test suite I changed `getRunPodSandboxRequest` to take in whatever
annotations we'd want to set on the pod config directly instead of us having to set them on the
returned object itself.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah marked this pull request as ready for review May 6, 2021 23:07
@dcantah dcantah requested a review from a team as a code owner May 6, 2021 23:07
Copy link
Contributor

@anmaxvl anmaxvl left a comment

Choose a reason for hiding this comment

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

another option, I guess, would be to have a config for the service and pass path to it? but this version also LGTM

@dcantah
Copy link
Contributor Author

dcantah commented May 7, 2021

@anmaxvl Config for the service just for these tests or have the service use this config you mean? If the latter, as the service we're talking to is unknown (just anything that implements the ttrpc service) I guess we'd have to control what the config would look like and the server would have to play nice and conform.

For the most part we're going to be launching the server ourselves so we still need to know the path to the binary however to invoke it.

@anmaxvl
Copy link
Contributor

anmaxvl commented May 10, 2021

I meant a config for the tests, which is more of a nit.

@anmaxvl
Copy link
Contributor

anmaxvl commented May 18, 2021

is this good to go?

@dcantah
Copy link
Contributor Author

dcantah commented May 18, 2021

@anmaxvl Oop, yea should be. I wanted to try out something last week but never got around to it but I'll do that today

@dcantah dcantah merged commit 640d380 into microsoft:master May 19, 2021
dcantah added a commit to dcantah/hcsshim that referenced this pull request May 19, 2021
In this PR (microsoft#1019) I changed how we pass annotations to
the cri-containerd suite, but this PR (microsoft#1030) got in before
which added a new test. This caused the CI to fail on checkin of the first PR. Always rebase kids

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
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.

2 participants