-
Notifications
You must be signed in to change notification settings - Fork 195
tests: add image service_offload feature test #4508
Conversation
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is some great work Wainer. The copyrights look good to me, so feel free to add me as a author/DCO sign-off.
I'd probably add another TODO of trying to reduce the copied code from ccv0.sh by updating it to use these methods, but that's probably an action on me and I'm fine with it not being part of this PR.
Thanks!
switch_image_service_offload on | ||
} | ||
|
||
@test "[cc][agent] Test can pull an unencrypted image inside the guest" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe specify that crictl is being used in this test description as I image we'll want to add a Kubernetes version (albeit in a different test file probably, but it might be helpful in reviewing the output to know quickly)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I will change it.
BTW, using "tags" on test description is an idea I got from the kubernetes e2e tests. Other Kata Containers tests doesn't use tags but I think they should.
local container_config="${FIXTURES_DIR}/container-config.yaml" | ||
|
||
# TODO: add a disable_full_debug to revert the changes on teardown. | ||
enable_full_debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be an argument that the debug config shouldn't be used in the pipeline, especially as it introduces potential security holes? It depends if we need it to grab required logs long term?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
Maybe I should break this enable_full_debug
into small enable_FOO_debug methods. For this test in particular, while running on CI (i.e. CI=true
) we only want the agent logs for debugging in case the test fails. However, if you as a developer wants to debug a problem locally, enabling other debug like runtime is really appreciated.
I will refactor that code and send a v2.
Added a test case to test the image offload feature of the agent. It was also introduce a library of function that can be used on other crictl+containerd tests for Confidential Containers. Fixes kata-containers#4509 Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
The new home for the crictl+containerd+confidential tests is in the integration/containerd directory. Fixes kata-containers#4509 Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
Added a new makefile target to run the confidential containers tests for containerd. Fixes kata-containers#4509 Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
Added the CC_CRI_CONTAINERD job type which will configure the kata containers build for running the Confidential Containers tests for containerd. Fixes kata-containers#4509 Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
92c2025
to
0678bab
Compare
/test Updated this PR:
Tested those changes in http://jenkins.katacontainers.io/job/wainer-test-CC/18 @stevenhorsman thanks for the review! and let me know if with those changes it is okay to merge as is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This introduces a test case for the image service_offload feature (i.e. the agent ability to pull the container image from inside the guest) when Kata Containers is built for Confidential Containers. My ultimate aim with this is to provide a backbone for more Confidential Containers tests be created by the community.
This is a adaptation of some methods and workflows implemented in @stevenhorsman 's
ccv0.sh
script. BTW, thanks @stevenhorsman for all the help and please let me know if I misplaced the copyright statements and also if I can add your signed-off-by.Also It was created a new CI job type (
CC_CRI_CONTAINERD
) to allow us add another Jenkins Job to run that and other cc + crictl + containerd tests. I added a job on Jenkins just to test this PR; and you can see a successful execution here.If you want to run this in your local host then you can use kata's vagrantfile (see instructions here). Do:
There are some work still to be done on the code (see the TODO markers) but I would prefer to address them in follow up PRs for two reasons:
Things that aren't TODO marked and I think they should be in focus next:
Fixes #4509
Cc @c3d @fidencio @sameo @Jakob-Naucke