The device plugin e2e tests where failing lately and to unblock the
release a skip was added in the prow job configuration:
71cf119c84/config/jobs/kubernetes/sig-node/sig-node-presubmit.yaml (L401)
The problem here is not only the broken test which need to be
fixed, but also the fact that this is the only skip (for a specific
test) we do this way, which is surprising (xref:
https://github.com/kubernetes/kubernetes/issues/106635#issuecomment-1105627265)
As next step towards improvement, we add an explicit skip in the tests
proper. This makes at least more obvious these tests need more work,
and allow us to remove the edge case in the prow configuration.
Signed-off-by: Francesco Romani <fromani@redhat.com>
The cacher blocks requests until it is ready, however, the
ready variable doesn't differentiate if the cacher was stopped.
The cacher is using a condition variable based on sync.Cond to
handle the readiness, however, this was not taking into account
if it was not ready because it was waiting to be ready or it was
stopped.
Add a new condition to the condition variable to handle the
stop condition, and returning an error to signal the goroutines
that they should stop waiting and bail out.
the serviceAccountController controller used by the tests must wait for the caches to sync
since the tests don't check /readyz there is no way
the tests can tell it is safe to call the server and requests won't be rejected
In a number of tests, the underlying storage backend interaction will
return the revision (logical clock underpinning the MVCC implementation)
at the call-time of the RPC. Previously, the tests validated that this
returned revision was exactly equal to some previously seen revision.
This assertion is only true in systems where no other events are
advancing the logical clock. For instance, when using a single etcd
cluster as a shared fixture for these tests, the assertion is not valid
any longer. By checking that the returned revision is no older than the
previously seen revision, the validation logic is correct in all cases.
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
This is the first step towards being able to support a new plugin API version
in parallel with the existing one.
Signed-off-by: Kevin Klues <kklues@nvidia.com>
The following investigation occurred during development.
Add TimingHistogram impl that shares lock with WeightedHistogram
Benchmarking and profiling shows that two layers of locking is
noticeably more expensive than one.
After adding this new alternative, I now get the following benchmark
results.
```
(base) mspreitz@mjs12 kubernetes % go test -benchmem -run=^$ -bench ^BenchmarkTimingHistogram$ k8s.io/component-base/metrics/prometheusextension
goos: darwin
goarch: amd64
pkg: k8s.io/component-base/metrics/prometheusextension
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkTimingHistogram-16 22232037 52.79 ns/op 0 B/op 0 allocs/op
PASS
ok k8s.io/component-base/metrics/prometheusextension 1.404s
(base) mspreitz@mjs12 kubernetes % go test -benchmem -run=^$ -bench ^BenchmarkTimingHistogram$ k8s.io/component-base/metrics/prometheusextension
goos: darwin
goarch: amd64
pkg: k8s.io/component-base/metrics/prometheusextension
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkTimingHistogram-16 22190997 54.50 ns/op 0 B/op 0 allocs/op
PASS
ok k8s.io/component-base/metrics/prometheusextension 1.435s
```
and
```
(base) mspreitz@mjs12 kubernetes % go test -benchmem -run=^$ -bench ^BenchmarkTimingHistogramDirect$ k8s.io/component-base/metrics/prometheusextension
goos: darwin
goarch: amd64
pkg: k8s.io/component-base/metrics/prometheusextension
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkTimingHistogramDirect-16 28863244 40.99 ns/op 0 B/op 0 allocs/op
PASS
ok k8s.io/component-base/metrics/prometheusextension 1.890s
(base) mspreitz@mjs12 kubernetes %
(base) mspreitz@mjs12 kubernetes %
(base) mspreitz@mjs12 kubernetes % go test -benchmem -run=^$ -bench ^BenchmarkTimingHistogramDirect$ k8s.io/component-base/metrics/prometheusextension
goos: darwin
goarch: amd64
pkg: k8s.io/component-base/metrics/prometheusextension
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkTimingHistogramDirect-16 27994173 40.37 ns/op 0 B/op 0 allocs/op
PASS
ok k8s.io/component-base/metrics/prometheusextension 1.384s
```
So the new implementation is roughly 20% faster than the original.
Add overlooked exception, rename timingHistogram to timingHistogramLayered
Use the direct (one mutex) style of TimingHistogram impl
This is about a 20% gain in CPU speed on my development machine, in
benchmarks without lock contention. Following are two consecutive
trials.
(base) mspreitz@mjs12 prometheusextension % go test -benchmem -run=^$ -bench Histogram .
goos: darwin
goarch: amd64
pkg: k8s.io/component-base/metrics/prometheusextension
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkTimingHistogramLayered-16 21650905 51.91 ns/op 0 B/op 0 allocs/op
BenchmarkTimingHistogramDirect-16 29876860 39.33 ns/op 0 B/op 0 allocs/op
BenchmarkWeightedHistogram-16 49227044 24.13 ns/op 0 B/op 0 allocs/op
BenchmarkHistogram-16 41063907 28.82 ns/op 0 B/op 0 allocs/op
PASS
ok k8s.io/component-base/metrics/prometheusextension 5.432s
(base) mspreitz@mjs12 prometheusextension % go test -benchmem -run=^$ -bench Histogram .
goos: darwin
goarch: amd64
pkg: k8s.io/component-base/metrics/prometheusextension
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkTimingHistogramLayered-16 22483816 51.72 ns/op 0 B/op 0 allocs/op
BenchmarkTimingHistogramDirect-16 29697291 39.39 ns/op 0 B/op 0 allocs/op
BenchmarkWeightedHistogram-16 48919845 24.03 ns/op 0 B/op 0 allocs/op
BenchmarkHistogram-16 41153044 29.26 ns/op 0 B/op 0 allocs/op
PASS
ok k8s.io/component-base/metrics/prometheusextension 5.044s
Remove layered implementation of TimingHistogram
Previous work by liggitt in 01760927b8 improved the boilerplate
required to run an embedded etcd server for tests as well as set up the
`*etcd3.store{}` for testing. A number of tests were not ported to use the
new helpers, though, either due to custom setup or due to inconsistent
use of setup options. A follow-up by stevekuznetsov in 6aa37eb062
removed much of the inconsistency, meaning that most callers to
`newStore()` were simply using the default boilerplate and options that
`testSetup()` used.
This patch moves all users to testSetup(), adding options as necessary
to enable some fringe setup use-cases. With a unified setup, new tests
will not copy boilerplate they do not need and it will be immediately
obvious when reading a test if the client or storage setup is *not*
default, improving readability.
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Bump cAdvisor to v0.44.1 to pick up fix for containerd task timeout
which resulted in empty network metrics.
Signed-off-by: David Porter <david@porter.me>
A previous commit updated the containerd version used on Windows
nodes from 1.5.4 to 1.6.2. However, the folder structure of the
containerd releases changed since then from:
cni/$binary.exe
to:
cni/bin/$binary.exe
Because of this, the Windows nodes do not have the necessary CNI
binaries needed to setup the pod networks.
The test/e2e suite has never supported feature gates:
- it cannot discover at runtime how the cluster is configured
- its --feature-gates parameter had no effect
Despite that, tests were written that used
e2eskipper.SkipUnlessFeatureGateEnabled even though that function then only
checked the default feature gate state. To catch such mistakes, e2e tests
suites now must explicitly enable feature gate checking via
e2eskipper.InitFeatureGates. They also must register their own command line
flag. When that is not done, then using SkipUnlessFeatureGateEnabled or
SkipIfFeatureGateEnabled leads to a test failure.
test/e2e_node does both and therefore continues to work as before.
These SkipUnlessFeatureGateEnabled are useless because:
- the tests run in test/e2e where feature gates always
have their default state
- CSIMigration, SizeMemoryBackedVolumes and ExecProbeTimeout are
all enabled by default (beta resp. GA)
The readonly port could be disabled.
Since we are only using the /healthz endpoint,
we can use the healthz port for this.
Change-Id: Ie0e05a5ab4ec6f51e4d3c63226aa23c1b3a69956
It is not possible for the nil-check to ever return anything different
from what the explicit boolean used to, but this is only something that
a reader can come to the conclusion on if they very, very carefuly read
the code. Instead of having this implicit flow that is difficult to
follow, let's keep the boolean.
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
This commit cleans up references to the old kubernetes-node-e2e-images
project. In the process it removes the `LIST_IMAGES` mode as listing
large numbers of public cloud projects is not particularly useful, and
has been somewhat broken for a long period of time - as we defaulted
launching a VM to a different project than listing.
