.github: Capture hubble flows when smoke test fails

[christarazi]

This is useful when debugging to understand why pod connectivity may be
broken.

Opened while working on #16608

[joestringer]

The deployment pieces of this definitely make sense to me, hubble-relay will help us get flows from the entire cluster.

The part where we gather hubble flows outside of sysdump seems broken to me: sysdump should be gathering everything we need all at once rather than requiring a separate additional step. I believe that @tklauser and/or @bmcustodio may be looking into this particular aspect for the new sysdump tool though so maybe we can just go with this for now.

One minor comment below (applies to both workflows).

Were you able to test this already? I see that it is opened from your own branch, and therefore these workflow changes are not being tested in the runs that GitHub reports on this PR. There was a PSA during the last community meeting during the testing section around what's necessary to test workflow changes.

[christarazi]

@joestringer I was definitely able to test these new changes as I forced it to fail to make sure that capturing hubble flows worked and included in the sysdump. And I think this is in line with the community meeting PSA as it says to use pull_request trigger , which these smoke tests are under :).

Re: the sysdump tool including this already. I looked into that and saw that it wasn't implemented yet.

[joestringer]

Hmm, true, that's what it says. I guess given that we limit permissions to "read-only" and that github only runs workflows for contributors who have had PRs merged, and that these are only smoke tests running stuff locally in the workflow, that's why these particular tests are OK to run under pull_request and run whatever logic is submitted in PRs from external branches.

[joestringer]

There's probably no point in holding back on getting this information in these CI tests since the work is already done, but this shouldn't stop us from working on getting these flows in sysdumps as well.

[nbusseneau]

Hmm, true, that's what it says. I guess given that we limit permissions to "read-only" and that github only runs workflows for contributors who have had PRs merged, and that these are only smoke tests running stuff locally in the workflow, that's why these particular tests are OK to run under pull_request and run whatever logic is submitted in PRs from external branches.

The read-only scope applies to the GITHUB_TOKEN and "only" forbids a potential attacker getting hold of the token to read the repository, and not write to it (i.e. no malicious git push). However it does not forbid the workflow from reading the repo's secrets.

Since we are using pull_request here, the workflow itself runs in the context of the PR branch. If the PR branch comes from a fork, the workflow only has access to the fork's secrets, not cilium repo's secrets, so we are good :)

[joestringer]

I removed @nebril from assignment since @nbusseneau provided ci-structure review. Looks like the tests are failing though so not ready-to-merge.

[aanm]

LGTM except the last commit

[christarazi]

Moving to draft until we figure out why hubble observe is not printing any flows even though hubble status indicates that it's connected (as well as the hubble relay logs in the sysdump): https://github.com/cilium/cilium/runs/3147403555?check_suite_focus=true#step:10:29

[christarazi]

This was the change that was required to resolve #16968 (comment).

[joestringer]

🤯

[christarazi]

All reviews are in and the change has been validated manually by forcing it to fail and inspecting the sysdump, and the relevant jobs are passing now. Marking ready to merge.