gha: Enable nydus snapshotter in CoCo ci tests #8953
Conversation
ChengyuZhu6
force-pushed
the
ci-guest-pull
branch
from
22cb6b3
to
d61d8c6
Compare
katacontainersbot
added
size/medium
There was a problem hiding this comment.
@ChengyuZhu6, I like a lot how you're splitting this changes knowing that those will only take action after the PR is merged, kudos!
IMHO, this is good to go apart from a very little nitpick that could lead to an unbound variable.
|
Hi @ChengyuZhu6 , just to ensure I understood the strategy you are following:
|
There was a problem hiding this comment.
Hi @ChengyuZhu6, a few comments:
I think you need to explain in the commit message what this is doing. You say "This commit describes how to enable snapshot....", But in fact your code/changes are not describing. Is actually changing coco tests to deploy/remove the snapshotter. Because of the commit message I was looking to some short of documentation describing as you mentioned.
Also, this commit message says it fixes #8584, but we have another PR (#8585) that says the same. Which one will close the issue?
Besides that, thanks a lot for the huge effort and appreciate your patience here.
ChengyuZhu6
force-pushed
the
ci-guest-pull
branch
2 times, most recently
from
46a124e
to
0257d82
Compare
Yes, exactly. |
Apologies for any confusion. I'll update the commit message to clarify the changes for the reviewers. As @wainersm said, I just want to make the changes on the Github workflows, and the skeleton to deploy_snapshotter() and cleanup_snapshotter() in tests/integration/kubernetes/gha-run.sh in this PR. |
ChengyuZhu6
force-pushed
the
ci-guest-pull
branch
from
0257d82
to
68bac04
Compare
|
hi @ChengyuZhu6 , some CI jobs are failing on provision the environment on AKS. Days ago it was merged some fix to that problem. Could you please rebase this PR so that I can re-trigger the jobs? |
ChengyuZhu6
force-pushed
the
ci-guest-pull
branch
from
68bac04
to
d183a56
Compare
Done. |
Appreciate rewritting the commit message. Makes sense now. However, this commit and the commit on the #8585 also mentions will fix the issue. Either this PR or #8585 needs to remove the "Fixes". If the PR was split, I'm assuming we must split the issue as well, otherwise we will not have consistency. Currently we have "May be fixed by #8585 or #8953" and this seems wrong to me. Does that makes sense to you? If so, do you mind providing more details on the issue it self, and fixing the "Fixes" lines accordingly? |
Thanks for your comments, I have fixed. |
|
/test |
|
@ChengyuZhu6 finally all required CI jobs are passing \o/ @beraldoleal @fidencio can we have one more approval to get it merged? This will allow us to cherry-pick commits from #8585 to #8484 , in order to test the later PR. |
ChengyuZhu6
force-pushed
the
ci-guest-pull
branch
3 times, most recently
from
6e15f19
to
88a8a34
Compare
This PR is a split of kata-containers#8585. make the changes on the Github workflows, and the skeleton to deploy_snapshotter() and cleanup_snapshotter() in tests/integration/kubernetes/gha-run.sh in this commit. After initially merging this patch to trigger CI jobs for CoCo, which will begin executing the dummy functions deploy_snapshotter() and cleanup_snapshotter(), the implementation details for these functions remain in kata-containers#8585. Our subsequent step involves transferring this logic to the PR kata-containers#8484, enabling the PR to undergo CI testing prior to its merge. Fixes: kata-containers#8997 Signed-off-by: ChengyuZhu6 <chengyu.zhu@intel.com>
ChengyuZhu6
force-pushed
the
ci-guest-pull
branch
from
88a8a34
to
a214bd8
Compare
|
/test |
There was a problem hiding this comment.
LGTM. Thanks @ChengyuZhu6! cc @BbolroC incase the new environment variables need mirroring across to the SE nightly test run?
Thanks for the notice. I will update the nightly test accordingly when this PR is merged. 😉 |
Fixed. |
This patch describes how to enable snapshotter and pull-type for Confidential Containers (coco) tests. Coco supports two methods to pull images: pulling images in the guest and sharing images on the host. Both methods ensure data confidentiality. To pull images in the guest, we use the nydus snapshotter, which passes image information from containerd to the agent. To share images on the host, we use either the nydus-snapshotter or the tardev snapshotter, which share images by block devices from the host to the guest.
Therefore, we need to set the snapshotter type and the pull type to allow ci to prepare the environment. This will affect the kata-agent build in kata-deploy.
I noticed that kata-coco-test did not trigger the deployment/cleanup of nydus snapshotter PR, such as run-k8s-tests-on-sev. Therefore, I created this PR to implement the deployment/cleanup process first, and then PR will add the full logic.
Fixes #8997