⚠ Migrate to ginkgo v2

[schrej]

Revives #1792, thanks to @acumino for all your work on this.

Fixes: #1545

Important Note:
-> Per node timeout has been removed to prevent the wrongly marking a test as failure which is caused by some other test. Read more about it here
-> Custom reporters have been deprecated in ginkgo v2 and will be removed in future releases.

Note: This is a breaking change for two reasons:

• (we're not affected by this) ginkgo v1 and v2 have conflicting flags, so you can't import them both at the same time (see here). Thanks to @mboersma for pointing this out
• The PR removes two helpers in pkg/envtest/printer that were used with ginkgo v1. Since you can't use both versions at the same time, this isn't relevant though.

Upstream upgraded already: kubernetes/kubernetes#109111

/cc @matteoolivi @camilamacedo86

[k8s-ci-robot]

@schrej: GitHub didn't allow me to request PR reviews from the following users: matteoolivi.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @matteoolivi @camilamacedo86

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[alvaroaleman]

ginkgo v1 and v2 have conflicting flags, so you can't import them both at the same time (see onsi/ginkgo#875 (comment)). Thanks to @mboersma for pointing this out

But we only import it in _test.go files and in the printer package, so this shouldn't matter for consumers, right?

[schrej]

But we only import it in _test.go files and in the printer package, so this shouldn't matter for consumers, right?

I just tested with the cluster-api project, manually replacing controller-runtime with this branch there. Tests can still run, even though CAPI is still on ginkgo v1. So you're right, it shouldn't matter to consumers in our case.

I'll revert the removal of the two helpers in pkg/envtest/printer, which should make this a non-breaking change then.

[schrej]

Ok. This is now using the ginkgo cli, which allows to remove the ginkgo helper to create unit test reports.
The apidiff is failing because of that.

I've adapted the test scripts to install the ginkgo cli automatically and to copy the unit test report to the correct location for prow to detect and visualise it.

One major drawback of the ginkgo cli is that it does not cache tests, and always runs the full suite.

[alvaroaleman]

@schrej the tests paniced but still passed: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-runtime/1977/pull-controller-runtime-test-master/1559847725640454144

[schrej]

@schrej the tests paniced but still passed: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-runtime/1977/pull-controller-runtime-test-master/1559847725640454144

Yeah. Apparently ginkgo detects and reports panics even when they are intentional ("injected panic").

•••E0817 10:24:49.659771   23457 runtime.go:77] Observed a panic: injected panic

The k8s.io/klog package logs panics it observes to stderr, and Ginkgo apparently prints that even when the test passes.

[pohly]

The k8s.io/klog package logs panics it observes to stderr, and Ginkgo apparently prints that even when the test passes.

It's the runtime which calls klog.Error, which then seems to go to stderr.

Does the suite call klog.SetOutput(ginkgo.GinkgoWriter)? Then I think you can suppress that extra output to stderr by reconfiguring the "stderrthreshold". I have a pending commit which does that for the in-tree e2e.test:
kubernetes/kubernetes@7d6af86

[pohly]

I've adapted the test scripts to install the ginkgo cli automatically and to copy the unit test report to the correct location for prow

FYI, it is possible to enable JUnit output without using the ginkgo cli. You can either do go test ./<test dir> -args -ginkgo.junit-report=<output file> or add some code like https://github.com/kubernetes/kubernetes/blob/a1128e380c2cf1c2d7443694673d9f1dd63eb518/test/e2e/framework/test_context.go#L558-L607.

[alvaroaleman]

FYI, it is possible to enable JUnit output without using the ginkgo cli. You can either do go test ./ -args -ginkgo.junit= or add some code like

The issue with that was that it made the tests always exit with rc0 even when there are failures

[pohly]

The issue with that was that it made the tests always exit with rc0 even when there are failures

Which of the two options? -ginkgo.junit or adding some custom GenerateJUnitReport invocation? Either way, that sounds like a bug that should be reported.

[pohly]

Hmm, both works for me (PMEM-CSI test/e2e, using ginkgo v2 v2.1.4).

FYI, I have to use this fork of the controller-runtime for that project because otherwise I would be stuck at ginkgo v1 and couldn't update to Kubernetes 1.25. I'll have to keep that in a WIP PR until this PR gets merged and released.

[alvaroaleman]

Which of the two options? -ginkgo.junit or adding some custom GenerateJUnitReport invocation? Either way, that sounds like a bug that should be reported.

It was reported here, maybe it doesn't happen anymore? onsi/ginkgo#706

FYI, I have to use this fork of the controller-runtime for that project because otherwise I would be stuck at ginkgo v1 and couldn't update to Kubernetes 1.25. I'll have to keep that in a WIP PR until this PR gets merged and released.

Why? Shouldn't we only depend on ginkgo in _test.go files? If anyhow possible, we shouldn't force users to use either version

[schrej]

Why? Shouldn't we only depend on ginkgo in _test.go files? If anyhow possible, we shouldn't force users to use either version

Probably because we're currently importing ginkgo v1 in pkg/envtest/printer.

[alvaroaleman]

Probably because we're currently importing ginkgo v1 in pkg/envtest/printer.

But v2 is imported as github.com/onsi/ginkgo/v2 or not?

[sbueringer]

/lgtm