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: adding an initial CI documentation #8988

Merged
merged 1 commit into from Apr 9, 2024

Conversation

beraldoleal
Copy link
Member

@beraldoleal beraldoleal commented Feb 1, 2024

This is actually a first attempt to document our CI, and all this content was based on the document created by @fidencio (kudos to him). We are just moving the content and discussion from Google Docs to here.

I used the "poetic license" to add some notes on what I believe our CI will look like in the future.

TIP: I suggest viewing the file in preview mode and also enabling the TOC feature in Github for better navigation.

Copy link
Contributor

@portersrc portersrc left a comment

Choose a reason for hiding this comment

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

awesome readme for the CI; thanks for porting parts of the google doc and consolidating here

ci/README.md Show resolved Hide resolved
Copy link
Member

@gkurz gkurz left a comment

Choose a reason for hiding this comment

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

Thanks for making this more official @beraldoleal ! We can then work to improve it with the community.

docs/ci.md Outdated Show resolved Hide resolved
Copy link
Member

@fidencio fidencio 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 @beraldoleal!

@gkurz
Copy link
Member

gkurz commented Feb 2, 2024

Hi @beraldoleal ! This still requires the usual Fixes ##### to pass CI.

@beraldoleal beraldoleal changed the title RFC: docs: adding an initial CI documentation docs: adding an initial CI documentation Feb 2, 2024
@beraldoleal
Copy link
Member Author

/test

docs/ci.md Outdated Show resolved Hide resolved
ci/README.md Outdated
Comment on lines 144 to 166
### Running the tests as part of the CI

If you're a maintainer of the project, you'll be able to kick in the tests by
yourself. With the current approach, you just need to add the `"ok-to-test"`
label and the tests will automatically start. We're moving, though, to use a
`/test` command as part of a GitHub review comment, which will simplify this
process.

If you're not a maintainer, please, send a message on Slack or wait till one of
the maintainers reviews your PR. Maintainers will then kick in the tests on
your behalf.

In case a test fails and there's the suspicion it happens due to flakiness in
the test itself, please, create an issue for us, and then re-run the tests
following these steps:

- Locate which tests is failing
- Click in "details"
- In the top right corner, click in "Re-run jobs"
- And then in "Re-run failed jobs"
- And finally click in the green "Re-run jobs" button

> [!NOTE]
> TODO: We need figures here
Copy link
Member

Choose a reason for hiding this comment

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

I wrote a script to find failed tests in one click. Maybe it is worth listing here for a quick check. See: #9030.

ci/README.md Outdated Show resolved Hide resolved
@beraldoleal
Copy link
Member Author

Sorry for the late reply here. Fixed with force-push.

@wainersm
Copy link
Contributor

/test

@beraldoleal beraldoleal force-pushed the ci-docs branch 3 times, most recently from 9205350 to 4452f43 Compare April 3, 2024 18:09
@gkurz
Copy link
Member

gkurz commented Apr 8, 2024

@beraldoleal CI fails with :

ERROR: Document docs/ci.md is not referenced

which is indeed the case AFAICT.

Also, do we really need a symbolic link ?

Cannot you just add a [Kata Containers CI](../ci/README.md) line somewhere in docs/README.md ?

Or maybe even [ci](ci/README.md) at https://github.com/kata-containers/kata-containers/blob/main/README.md?plain=1#L142 ?

@beraldoleal beraldoleal force-pushed the ci-docs branch 2 times, most recently from 1f3675b to ef60126 Compare April 8, 2024 17:26
@gkurz
Copy link
Member

gkurz commented Apr 9, 2024

/test

@gkurz
Copy link
Member

gkurz commented Apr 9, 2024

/retest-arm-main

- Run the following tests:
- Tests depending on the generated tarball
- Metrics (runs on bare-metal)
- `docker` (runs on Azure small instances)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it'd be harder and more prone to changes but it'd be nice to put links to the workflow files at least for the main pipelines so people can quickly find them and see the details.

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely helpful, but I will leave this to others in following PRs. This is just an attempt to move the content from google docs here, because of annoying spam/bots there.

ci/README.md Outdated Show resolved Hide resolved
ci/README.md Outdated Show resolved Hide resolved
- And finally click in the green "Re-run jobs" button

> [!NOTE]
> TODO: We need figures here
Copy link
Contributor

@ldoktor ldoktor Apr 9, 2024

Choose a reason for hiding this comment

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

Do you want to mention the re-run all button? From my experience that's what people use the most anyway... (which is sad)

Copy link
Member Author

Choose a reason for hiding this comment

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

It is mentioned already on line 156. I would keep like that and improve the workflow later.

ci/README.md Outdated Show resolved Hide resolved
## Known limitations

As the GitHub actions are structured right now we cannot: Test the addition of a
GitHub action that's not triggered by a pull_request event as part of the PR.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about discussing the workaround (sending a PR with always-passing version and then follow-up with the real code?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Given this is a current limitation, I would suggest creating an issue to discuss the workaround there.

@gkurz gkurz marked this pull request as draft April 9, 2024 10:10
@gkurz
Copy link
Member

gkurz commented Apr 9, 2024

@beraldoleal I've made this a draft to be sure it doesn't get merged before you had time to look at @ldoktor 's comments.
This doesn't invalidate CI. If you want to proceed without additional changes, you just need to press the Ready for review button.

This is actually a first attempt to document our CI, and all this
content was based on the document created by Fabiano Fidencio (kudos to
him). We are just moving the content and discussion from Google Docs to
here.

I used the "poetic license" to add some notes on what I believe our CI
will look like in the future.

Fixes kata-containers#9006

Co-authored-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Signed-off-by: Beraldo Leal <bleal@redhat.com>
@gkurz
Copy link
Member

gkurz commented Apr 9, 2024

/test

@beraldoleal beraldoleal marked this pull request as ready for review April 9, 2024 13:49
@gkurz
Copy link
Member

gkurz commented Apr 9, 2024

Unrelated flaky CI blocking the way again. I'll force merge this PR.

@gkurz gkurz merged commit 8935324 into kata-containers:main Apr 9, 2024
282 of 296 checks passed
@ldoktor
Copy link
Contributor

ldoktor commented Apr 9, 2024

Unrelated flaky CI blocking the way again. I'll force merge this PR.

Nice, sudo merge :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants