Skip to content

kola/test: add bpf tests#233

Merged
tormath1 merged 2 commits intoflatcar-masterfrom
tormath1/bpf
Dec 7, 2021
Merged

kola/test: add bpf tests#233
tormath1 merged 2 commits intoflatcar-masterfrom
tormath1/bpf

Conversation

@tormath1
Copy link
Copy Markdown
Contributor

In this PR, we kick-off the writing of bpf tests.

To use it:

sudo ./bin/kola run -b cl -p qemu --qemu-image ./flatcar_production_qemu_image.img bpf.execsnoop

Related to: flatcar/Flatcar#89

@tormath1 tormath1 self-assigned this Sep 15, 2021
@tormath1 tormath1 marked this pull request as ready for review September 15, 2021 15:39
@tormath1 tormath1 requested a review from a team September 15, 2021 15:39
Copy link
Copy Markdown
Member

@krnowak krnowak left a comment

Choose a reason for hiding this comment

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

One question, otherwise looks alright.


// wait for the container and the `execsnoop` command to be correctly started before
// generating traffic.
if err := util.Retry(5, 2*time.Second, func() error {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Every loop of the retry runs docker ps so now its definitely possible to have more than 3 lines in the log (if the retry happens)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mm, I get your point but it should not:

in the iteration, we run docker ps then we check sudo cat $(docker inspect --format='{{.LogPath}}' %s):

  • empty: it means execsnoop is not correctly started (or does not filter correctly) -> we continue to iterate
  • not empty: execsnoop is correctly started because it generated some logs based on the docker ps "trigger" -> we continue the program

Even in the case we loop 5 times, it means that in the meantime not logs have been generated -> execsnoop was not ready. In the end, we should always get 3 lines into the logs.

@tormath1 tormath1 force-pushed the tormath1/bpf branch 3 times, most recently from f2f9f8e to 61d27cd Compare December 7, 2021 10:43
@tormath1
Copy link
Copy Markdown
Contributor Author

tormath1 commented Dec 7, 2021

@krnowak thanks for the review - comments have been addressed :)

Copy link
Copy Markdown
Member

@krnowak krnowak left a comment

Choose a reason for hiding this comment

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

Cool, looks good.

in this commit, we add a test for `execsnoop`

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1 tormath1 merged commit 615d5ec into flatcar-master Dec 7, 2021
@tormath1 tormath1 deleted the tormath1/bpf branch December 7, 2021 20:29
}

func init() {
register.Register(&register.Test{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On arm this fails with:

cluster.go:117: WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested

Could you disable this test for arm for the time being?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mm, Docker image has not yet arm64 support - let's open a tracking issue upstream and exclude ARM64 for this test then.
Thanks for the report !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants