kubernetes
kind jobs: stop relying on implicit SKIP=[Serial]
All jobs using e2e-k8s.sh with PARALLEL=true automatically skipped serial tests even though technically they now could run in the job: https://github.com/kubernetes-sigs/kind/blame/d1eecc46e30cac9d35cd32dc52677ef75ec22e18/hack/ci/e2e-k8s.sh#L226-L234
https://github.com/kubernetes-sigs/kind/pull/4015 is changing that mandatory skip for jobs using LABEL_FILTER because it may be desirable to include serial tests, depending on the job. It's also better to be explicit about it in each job's LABEL_FILTER to avoid confusion and potential mistakes (not running tests that were expected to run).
To prepare for that change, jobs get updated based on the following principles:
- If a presubmit runs infrequently and is only invoked to test certain aspects, then including serial tests is desirable to get full test coverage of that aspect. Example: pull-kubernetes-e2e-kind-beta-features
- If a presubmit runs always, serial tests should be excluded. Example: pull-kubernetes-e2e-kind
- Periodic jobs should always run all supported tests, including the serial ones. Example: ci-kubernetes-e2e-kind
- Canary jobs match the behavior of the non-canary variant. Example: pull-kubernetes-e2e-kind-canary
- Jobs for release branches should better not change for the sake of preserving the old behavior (but that is debatable). Example: ci-kubernetes-e2e-kind-1-34
To make it more obvious where the upcoming e2e-k8s.sh will change test selection, "Includes serial tests for the sake of completeness." comments get added. Those are not true right now, but will be once the script is changed.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: pohly Once this PR has been reviewed and has the lgtm label, please assign madhavjivrajani for approval. For more information see the Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/hold
Needs to be reviewed together with https://github.com/kubernetes-sigs/kind/pull/4015. This PR then needs to be merged first.
/assign @BenTheElder
@dims: this changes some of your jobs. Do you agree that including serial tests makes sense for them?
@aojea: same with your network jobs. You expressed a desired to exclude serial tests from presubmits, so I leaned in that direction for them.
Maintaining two variants is more work and makes the testgrid dashboards larger.
I think that's done for jobs not using LABEL_FILTER yet. There is a ci-kubernetes-kube-network-policies-conformance-parallel but no corresponding ci-kubernetes-kube-network-policies-conformance-serial. With this change there is no need for it.
I was worried that that tests like test/e2e/apimachinery/garbage_collector.go: framework.ConformanceIt("should orphan pods created by rc if delete options say so", framework.WithSerial() might not have run as often as desired. But
ci-kubernetes-conformance-kind-ga-only runs everything sequentially, so at least serial conformance tests are covered as release blocking.
The same cannot be said for other tests like test/e2e/storage/testsuites/volumelimits.go: f.It("should support volume limits", f.WithSerial(). That's not in kind-master, nor is there a serial variant of it. It is in gce-cos-master-serial though as release informing.
PR needs rebase.
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-sigs/prow repository.