istio-cni code coverage

[bleggett]

(This is used to request new product features, please visit https://github.com/istio/istio/discussions for questions on using Istio)

Describe the feature request
We don't have a good ongoing sense of what chunks have poor unit test coverage, and we don't have a good ongoing sense of where adding more cheap unit tests would bring the most value, and we generally want to encourage more, cheaper, easier to debug/maintain unit tests, and fewer, expensive, harder to debug/maintain integ/e2e tests.

We should generate code coverage stats (per branch/per func) as part of PR CI, and compare it with the current high-watermark in mainline, and warn/fail the CI check if coverage drops below the current high-watermark.

(usual caveat - code coverage is not the only proof of good code coverage or useful tests, but it is the easiest to automate, and hard to casually cheat)

We could do this for other components too , but istio-cni seems like a decent place to start as it is both relatively critical, relatively complex, and relatively self-contained.

Unofficial requirements (subject to change)

• Must work locally and in CI without extra configuration
• Must be something that can be invoked as a simple makefile target locally and in CI
• May not rely on external services being available
• Should dump coverage report as a CI artifact

When developing the inpod CNI internally we used overcover which is a very basic/simple thing that just parses go test coverage reports. It worked well enough and doesn't require any magic. Can always dump the coverage report as an artifact if people wanna see it, it doesn't need to be fancy.

overcover --coverprofile cover.out ./... --threshold $(COVERAGE_THRESH_PCT)

tl;dr: I really don't want to use anything that can't work locally AND as a simple makefile target.

EDIT: After 5/15 WG, following are the requirements:

• Start with one component at a time (istio-cni in this case)
• Simple, language-specific tooling only, must run locally and in CI the same way (makefile target, no out of band setup)
• CI should publish coverage report artifacts.
• An overall number for all the packages in the component should be generated (go test needs help to do this IIRC or it will only report per-go-pkg coverages, which is not as useful - something like overcover can help with that)
• Stretch goal -> figure out how to get integration tests to count towards coverage. Istio currently has an (IMO excessively) topheavy inverted test pyramid, so being able to include integ tests in coverage would be nice too).

[JayKaku]

Hey @bleggett, I work with Istio frequently and was looking to pick this up as a way to start contributing, is there any specific section of cni that should be considered first? I'd like to work on it!

[bleggett]

ztunnel side of the same issue: istio/ztunnel#1049

@JayKaku thanks for volunteering - could you hold off a bit? We are still discussing how we want to approach this.

[JayKaku]

Sure! lemme take a look at ztunnel unit test and do lemme know where can I watch the discussions for this! Thanks!

[bleggett]

5/15 WG consensus was that we do want to add codecoverage, but not CI gates for it.

@JayKaku I updated the issue with what we're looking for - if you want to take a stab a this, feel free.

What we want here is to report overall coverage for all of istio-cni, versus just Go package-level reports.

[bleggett]

FWIW branch code coverage can help us identify tests we don't need as well: #51192 (review)