Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs/DEVELOPMENT: document the CI #224

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

wainersm
Copy link
Member

@wainersm wainersm commented Jul 7, 2023

Created a new "Continuous Integration (CI)" section in the developer's guide to explain the e2e CI jobs types, how/who can execute them as well as how to run some of them locally.

Fixes #113
Signed-off-by: Wainer dos Santos Moschetta [email protected]

@wainersm wainersm added documentation Improvements or additions to documentation technical debt Issue releated with a technical debt in the project testing labels Jul 7, 2023
```

Notice that the `-r` parameter passed to `run-local.sh` above specifies the runtimeClass to be tested. You can switch to, for example, `kata-clh` to test Cloud Hypervisor. See the script's help (`run-local.sh -h`) for further details.

Choose a reason for hiding this comment

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

Suggested change
### Running e2e test locally with Vagrant


We recommend that you run the e2e Non-TEE tests on your local machine before opening a PR to check your changes will not break the CI so to avoid wasting resources. You can also use the approach describe below to debug and fix failures, or test changes on the scripts themselves.

The entry point script is [tests/e2e/run-local.sh/run-local.sh](../tests/e2e/run-local.sh). It is going to install softwares and change the system's configuration, so we recommend that you run the e2e tests on VMs.

Choose a reason for hiding this comment

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

Suggested change
The entry point script is [tests/e2e/run-local.sh/run-local.sh](../tests/e2e/run-local.sh). It is going to install softwares and change the system's configuration, so we recommend that you run the e2e tests on VMs.
The entry point script is [tests/e2e/run-local.sh](../tests/e2e/run-local.sh). It is going to install software and change the system's configuration, so we recommend that you run the e2e tests on VMs.

Choose a reason for hiding this comment

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

Maybe we could add what the requirements are for the underlying system/VM, like min. CPU/Mem and nested virtualization support for VMs.

sudo apt-get install -y ansible python-is-python3
cd tests/e2e
export PATH="$PATH:/usr/local/bin"
./run-local.sh -r "kata-qemu"

Choose a reason for hiding this comment

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

Suggested change
./run-local.sh -r "kata-qemu"
./run-local.sh -u -r "kata-qemu"

Copy link
Contributor

Choose a reason for hiding this comment

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

With the recent addition it should IMO stay without the -u

@wainersm
Copy link
Member Author

hey @katexochen , thanks for your review!

Updated this PR to address his comments:

  • Added mininum requirements for the VM
  • Explained the '-u' flag
  • Added the 'Running e2e test locally with Vagrant' section

Copy link

@katexochen katexochen left a comment

Choose a reason for hiding this comment

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

Great, thanks! :)

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

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

Looks good. I think there are two things missing.

First, when you run the tests locally, how do you point them to your changes? If you follow the instructions here you will just run the tests against the upstream code which isn't super useful (although it's probably a good idea to do that before trying your changes). So a section about how to update the Kustomize files to point to your own bundle (either a diffrent one from Kata upstream or one you built yourself) would be nice. Also maybe a note about how to point the tests-runner.sh script to a different version of the tests.

Second, one tip that I might add is that when you run run-local.sh it will setup your node and run things. Once you've done this you can run tests-runner.sh (with the -r flag) as many times as you want to rerun the tests (as long as you didn't use the -u flag when you set things up). Ofc this won't pickup any changes you make to the bundle, but it can pickup changes to the tests and you can make changes locally. In fact my main way of debugging serious problems is to run tests-runner.sh again and again while adjust components manually on the node.

docs/DEVELOPMENT.md Outdated Show resolved Hide resolved
docs/DEVELOPMENT.md Outdated Show resolved Hide resolved
@fidencio
Copy link
Member

@wainersm, aren't there secrets stored on jenkins that are needed for running kata-containers related tests?
If so, shall we expose those here so those tests do not fail when running locally?


## Continuous Integration (CI)

In order to be merged your opened pull request (PR) should pass the static analysis checks and end-to-end (e2e) tests.
Copy link
Contributor

@ldoktor ldoktor Nov 28, 2023

Choose a reason for hiding this comment

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

The e2e testing around kubernetes is often referred to kubernetes e2e conformance tests. How about using the term coco operator functional testing or such? Anyway not as part of this PR... (it would require changing the references in tests/e2e but I think it could improve the first impression for newcomers)


```shell
export RUNTIMECLASS="kata-qemu"
vagrant up tests-e2e-ubuntu2004
Copy link
Contributor

Choose a reason for hiding this comment

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

This works quite well, syncs everything, performs all the steps and reports correct exit status (so, unlike when executed manually, there is a big fat warning it failed), but it takes forever to execute. Still I think it'd be better to move this above the manual steps to give users single entrypoint they could run. Anyway there should be a way to speedup the deployment, perhaps with the good old deploy/snapshot/run/run/run/delete fashion.

Copy link
Contributor

Choose a reason for hiding this comment

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

43m on my machine, perhaps worth mentioning here as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Mentioned that on the guide.

@wainersm
Copy link
Member Author

@ldoktor @fitzthum update the CI guide to address most of the comments you made. The name of the jobs currently executed on pull requests will be updated alongside #260 as well as more cases (e.g. rebuild the pre-install image) can be added on following PRs.

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

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

LGTM

Seems like a few things here might be referring to our old jenkins things, but I guess this will be updated separately.

sudo chown "$USER" ~/.kube/config
```

Then use the `tests_runner.sh` script to re-run the tests like shown below (`-r` has the same meaning of in `run-local.sh`, i.e., use to set the runtimeclass):
Copy link
Contributor

Choose a reason for hiding this comment

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

has the same meaning as in

Copy link
Contributor

@ldoktor ldoktor Dec 12, 2023

Choose a reason for hiding this comment

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

or even shorter "similarly to run-local the -r sets the runtimeclass"

sudo -E make docker-build
sudo -E make docker-push
./tests/e2e/tests_runner.sh -r kata-qemu
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to promote the tests/e2e/operator.sh -h for available targets, rather than individual commands? On the other hands developers probably prefer the individual commands but both can be useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea!

Created a new "Continuous Integration (CI)" section in the developer's
guide to explain the e2e CI jobs types, how/who can execute them as well
as how to run some of them locally.

Fixes confidential-containers#113
Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
@wainersm
Copy link
Member Author

Updates:

  • removed the old jenkins jobs from the table of run jobs
  • promoting operator.sh as @ldoktor suggested

Copy link
Contributor

@ldoktor ldoktor left a comment

Choose a reason for hiding this comment

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

lgtm

@portersrc
Copy link
Member

LGTM, thanks for adding this

Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@wainersm wainersm merged commit f1a4ec5 into confidential-containers:main Dec 13, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation technical debt Issue releated with a technical debt in the project testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document the operator CI
7 participants