Run unit tests 2 instead of 3 times via bazel

[spiffxp]

What type of PR is this?

/kind cleanup
/kind flake

What this PR does / why we need it:

Followup to #93605 which changed from retryng 3 times to avoid flakes, to requiring a pass 3 times.

In order to migrate the pull-kubernetes-bazel-test job to a communty owned cluster with dedicated resources, we need to stop using RBE. When we did so, we started seeing many more flakes. (ref: kubernetes/test-infra#19070 (comment))

RBE fans out to a large pool of compute, whereas now we're running in a single pod on a single node.

It may be that using 2 runs instead of 3 will relieve some resource contention, giving us an appropriate compromise between not ignoring flakes but not leaning all the way into causing them. (ref: kubernetes/test-infra#19070 (comment))

Which issue(s) this PR fixes:
n/a

Special notes for your reviewer:

/hold
I'd like to see if the behavior of pull-kubernetes-bazel-test improves now that we've given it 7 CPUs instead of 4. I'm teeing this up as a further mitigation if we decide that's not enough, or if 7 CPUs is excessive resource consumption.

NONE

/cc @liggitt @BenTheElder

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: spiffxp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• build/OWNERS [spiffxp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[spiffxp]

/sig testing
/priority important-soon

[liggitt]

7 CPUs is excessive resource consumption

7 CPUs to run unit tests feels excessive. If we are not actually caching unit test results from unaffected packages and are running all tests 3 times on every PR push, then reducing to 2 runs is reasonable.

[liggitt]

/lgtm

unhold at will

[BenTheElder]

7 CPUs to run unit tests feels excessive. If we are not actually caching unit test results from unaffected packages and are running all tests 3 times on every PR push, then reducing to 2 runs is reasonable.

cached-ness is easily confirmed in the log output.

a line like:

//pkg/kubelet/cm/cpumanager:go_default_test PASSED in 1.1s

means we ran it. it will say something like (cached) at the end otherwise.

[spiffxp]

/milestone v1.20
/hold cancel
Yeah I'm going to go ahead and drop us to 2. These are graphs for the CI job, which uses 7 cpu already. They're from a local grafana instance that queries against k8s-gubernator:builds.

It's pretty easy to see when #93605 landed. Makes me wonder if the retry_flake_attempts setting was added before the move to RBE

[spiffxp]

(Pulling this out of slack) and yes it turns out if we run multiple times, we can't cache, unless we want to accept potentially caching failures https://docs.bazel.build/versions/master/user-manual.html#flag--cache_test_results

If this option is set to 'auto' (the default) then Bazel will only rerun a test if any of the following conditions applies:

• Bazel detects changes in the test or its dependencies
• the test is marked as external
• multiple test runs were requested with --runs_per_test
• the test failed.

If 'no', all tests will be executed unconditionally.
If 'yes', the caching behavior will be the same as auto except that it may cache test failures and test runs with --runs_per_test.

[spiffxp]

Makes me wonder if the retry_flake_attempts setting was added before the move to RBE

#59283 added test:unit --flaky_test_attempts=3 on 2018-02-06

... but it was in build/root/Makefile as bazel flags before that
... before #45855 moved Makefile to build/root
... before #41280 moved it from .bazelrc to Makefile

So, yeah, it was in the first PR that added .bazelrc on 2017-01-20 (#40231). Definitely before we tried running bazel via RBE