tests/cri-containerd: split between cri-containerd and crictl suites #8096

wainersm · 2023-10-03T01:48:00Z

This split the cri-containerd in two suites of tests:

cri-containerd: keep running containerd's cri-integration tests
crictl: new home for the crictl-based tests

Then the crictl tests were convert to use the bats framework. I didn't want to improve the tests, depict many opportunities, so I tried to keep the original code and logic intact as much as possible. In the future, when we start pulling tests from CCv0 I think it will be the right time for improvements.

A github workflow was created to run the crictl suite of tests. The workflow won't fail due to failures on tests, because I will be able to fully test the tests on CI after the workflow is merged (right?) and then I don't want a broken workflow blocking unrelated pull requests.

Fixes #8160

fidencio · 2023-10-03T12:55:55Z

@wainersm, I don't understand 9c1a213

What exactly are you trying to gain with that? integration/cri-containerd should be clear that this is cri-containerd integration tests ... making it integration/cri-containerd-integration is redundant.

wainersm · 2023-10-03T13:23:40Z

Hi @fidencio !

@wainersm, I don't understand 9c1a213

What exactly are you trying to gain with that? integration/cri-containerd should be clear that this is cri-containerd integration tests ... making it integration/cri-containerd-integration is redundant.

Once upon a time PR #8061, that @beraldoleal asked me to help understand why some CI jobs failed. I took look at https://github.com/kata-containers/kata-containers/actions/runs/6356069723/job/17266102296?pr=8061 first, when I realized that our cri-containerd suite of tests is actually running two types of tests:

The containerd's cri-integration suite. External to kata's code base, writing in Golang...etc
Custom shell script tests within kata's code base, using crictl/cri/containerd

So my goal is to split those two types of tests in different suites to easy the interpretation of the logs when the CI fail. The tests 2) (cri-containerd) could even growth in the future with the addition of https://github.com/kata-containers/tests/tree/CCv0/integration/containerd/confidential tests.

Let me know if I'm missing something and/or this is an unwanted/worthless split.

wainersm · 2023-10-03T13:26:41Z

Hi @fidencio !

@wainersm, I don't understand 9c1a213
What exactly are you trying to gain with that? integration/cri-containerd should be clear that this is cri-containerd integration tests ... making it integration/cri-containerd-integration is redundant.

Once upon a time PR #8061, that @beraldoleal asked me to help understand why some CI jobs failed. I took look at https://github.com/kata-containers/kata-containers/actions/runs/6356069723/job/17266102296?pr=8061 first, when I realized that our cri-containerd suite of tests is actually running two types of tests:
1. The containerd's cri-integration suite. External to kata's code base, writing in Golang...etc

2. Custom shell script tests within kata's code base, using crictl/cri/containerd
So my goal is to split those two types of tests in different suites to easy the interpretation of the logs when the CI fail. The tests 2) (cri-containerd) could even growth in the future with the addition of https://github.com/kata-containers/tests/tree/CCv0/integration/containerd/confidential tests.

Let me know if I'm missing something and/or this is an unwanted/worthless split.

ah, and the tests 2) (cri-containerd) I'd like to convert to bats... :)

fidencio · 2023-10-03T14:30:02Z

Aha, okay, it makes sense.

I'd just not name it as cri-containerd-integration :-)

wainersm · 2023-10-03T15:12:14Z

Aha, okay, it makes sense.

I'd just not name it as cri-containerd-integration :-)

hmm.. what do you suggest? Keep containerd's cri-integration tests called cri-containerd and naming our custom tests something else (e.g. crictl-containerd) ? Naming is an art my friend...

wainersm · 2023-10-03T17:51:52Z

run-cri-containerd (lts, qemu) is also failing locally and I still don´t know why.

wainersm · 2023-10-04T12:18:38Z

run-cri-containerd (lts, qemu) is also failing locally and I still don´t know why.

Fixed. Added an instruction to pull the pause image before creating the pod.

wainersm · 2023-10-04T22:40:58Z

Re-done my initial PR to move the crictl tests to its own suite (guess what, named crictl). So the cri-containerd suite is kept running the containerd's cri-integration tests.

Once CI passing I should be working on:

convert crictl to use bats
create a new CI job to run crictl only

wainersm · 2023-10-06T21:38:01Z

@fidencio this PR is ready to review. The three commits that convert to bats I should squash after the reviews (keeping them separated to ease the reviewer's life).
/ ----
/ ------
@stevenhorsman tentative to build a | [] | (house) for the crictl tests that we will (or not) be pulling to main...

wainersm · 2023-10-06T21:38:41Z

@fidencio this PR is ready to review. The three commits that convert to bats I should squash after the reviews (keeping them separated to ease the reviewer's life). / ---- / ------ @stevenhorsman tentative to build a | [] | (house) for the crictl tests that we will (or not) be pulling to main...

yeah, my tentative to build a house in ascii didn't work out....

stevenhorsman · 2023-10-09T12:28:09Z

 / ---- \
/ ------ \
  | [] |

stevenhorsman

Looks good to me, one minor comment...

tests/integration/crictl/lib.sh

Created the tests/integration/cri-containerd/lib.sh file to host code that can be shared with other tests suites. The original code wasn't touched, it just moved of place. In preparation to create the crictl suite of tests. Fixes kata-containers#8160 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

The tests/integration/cri-containerd currently runs the containerd's cri-integration tests plus a couple of kata's specific tests using crictl. Splited in two suites of tests: * cri-containerd - keep running the containerd's cri-integration tests * crictl - new home for crictl based tests The code is essentially a copy of * cri-containerd/integration-tests.sh -> crictl/run_tests.sh * cri-containerd/gha-run.sh -> crictl/gha-run.sh On crictl/run_tests.sh, `testContainerStart()` was added a handler to pull the pause image before creating the pod. Fixes kata-containers#8160 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

Re-implemented the tests to use the bats framework. Fixes kata-containers#8160 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

Added a new workflow to run the tests/integration/crictl suite of tests with same containerd/hypervisor matrix as cri-containerd workflow. Important: at this point the workflow won't fail due to failing tests. Fixes kata-containers#8160 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

wainersm · 2023-10-10T15:03:24Z

Rebase this to main, added a header to tests/integration/crictl/lib.sh and squashed the commits that convert tests to bats.

stevenhorsman · 2024-01-02T14:56:54Z

@wainersm - Do we still want to get this in? I think it needs a rebase and re-test and then we can have another attempt?

katacontainersbot added the size/large Task of significant size label Oct 3, 2023

wainersm added the ok-to-test label Oct 3, 2023

wainersm force-pushed the refactor_cri-containerd branch from 12dffe6 to d94d30c Compare October 3, 2023 12:04

wainersm force-pushed the refactor_cri-containerd branch from d94d30c to df1633a Compare October 3, 2023 21:39

wainersm force-pushed the refactor_cri-containerd branch from df1633a to 57f78c6 Compare October 4, 2023 22:38

katacontainersbot added size/huge Largest and most complex task (probably needs breaking into small pieces) and removed size/large Task of significant size labels Oct 4, 2023

wainersm mentioned this pull request Oct 5, 2023

Port agent API end-point restrictions test to the main branch #8101

Open

wainersm force-pushed the refactor_cri-containerd branch 2 times, most recently from 911c12c to 9e05f86 Compare October 6, 2023 20:46

wainersm removed the ok-to-test label Oct 6, 2023

wainersm force-pushed the refactor_cri-containerd branch from 9e05f86 to be5b9f4 Compare October 6, 2023 21:09

wainersm changed the title ~~[WIP] Refactor cri-containerd tests suite~~ tests/cri-containerd: split between cri-containerd and crictl suites Oct 6, 2023

wainersm force-pushed the refactor_cri-containerd branch from be5b9f4 to f870fb5 Compare October 6, 2023 21:32

wainersm added no-backport-needed area/testing Issues related to testing labels Oct 6, 2023

stevenhorsman approved these changes Oct 9, 2023

View reviewed changes

tests/integration/crictl/lib.sh Show resolved Hide resolved

wainersm added 2 commits October 10, 2023 11:40

tests/crictl: convert from shell scripts to bats

25f39e8

Re-implemented the tests to use the bats framework. Fixes kata-containers#8160 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

wainersm force-pushed the refactor_cri-containerd branch from f870fb5 to f81722b Compare October 10, 2023 14:46

wainersm mentioned this pull request Oct 13, 2023

tests: bring the CCv0 cri-containerd tests to main #8227

Closed

stevenhorsman removed the no-backport-needed label Apr 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests/cri-containerd: split between cri-containerd and crictl suites #8096

tests/cri-containerd: split between cri-containerd and crictl suites #8096

wainersm commented Oct 3, 2023 •

edited

Loading

fidencio commented Oct 3, 2023

wainersm commented Oct 3, 2023

wainersm commented Oct 3, 2023

fidencio commented Oct 3, 2023

wainersm commented Oct 3, 2023

wainersm commented Oct 3, 2023

wainersm commented Oct 4, 2023

wainersm commented Oct 4, 2023

wainersm commented Oct 6, 2023

wainersm commented Oct 6, 2023

stevenhorsman commented Oct 9, 2023

stevenhorsman left a comment

wainersm commented Oct 10, 2023

stevenhorsman commented Jan 2, 2024

tests/cri-containerd: split between cri-containerd and crictl suites #8096

Are you sure you want to change the base?

tests/cri-containerd: split between cri-containerd and crictl suites #8096

Conversation

wainersm commented Oct 3, 2023 • edited Loading

fidencio commented Oct 3, 2023

wainersm commented Oct 3, 2023

wainersm commented Oct 3, 2023

fidencio commented Oct 3, 2023

wainersm commented Oct 3, 2023

wainersm commented Oct 3, 2023

wainersm commented Oct 4, 2023

wainersm commented Oct 4, 2023

wainersm commented Oct 6, 2023

wainersm commented Oct 6, 2023

stevenhorsman commented Oct 9, 2023

stevenhorsman left a comment

Choose a reason for hiding this comment

wainersm commented Oct 10, 2023

stevenhorsman commented Jan 2, 2024

wainersm commented Oct 3, 2023 •

edited

Loading