When rapidly processing informer events it can happen that a pod gets scheduled
twice (seen only in the TestEviction/update unit test):
- Claim update observed, pod from informer cache with NodeName from update -> queue pod for eviction.
- Pod update observed, claim from informer cache -> queue pod again.
The effect is one additional Get call to the apiserver. We can avoid it by
maintaining an LRU cache with the UIDs of the pods which we have evicted and
thus don't need to do anything for.
In these cases it's certain that no time needs to pass, so Wait can
replace polling with Eventually. This also means that locking is
not necessary to prevent data races.
In particular with the builtin tCtx.Assert/Expect the assertions are also short
when using gomega and often more readable (no more confusion in Equal which one
is the expected and which the actual value).
We can observe the delay in the metric histogram. Because we run in a synctest
bubble, the delay is 100% predictable.
Unfortunately we cannot use the reactor mechanism of the fake client: that
delays while holding the fake's mutex. When some other goroutine (in this case,
the event recorder) calls the client, it gets blocked without being considered
durably blocked by synctest, so time does not advance and the test gets stuck.
Thanks for waiting for cache sync via channels the random delays caused by
polling are gone, making the initial setup including cache sync happen
"immediately" when a test starts (= same virtual time). This makes the tests
more predictable and simplifies making further assertions about when something
happens or how long it takes.
While at it, restore previous performance by setting feature gates once and
running tests in parallel again.
Added podToVolumes reverse index to optimize DeletePod.
Currently we simply iterate through all the volumes and remove the pod
being deleted from there. This is inefficient and takes longer the
longer the volume list becomes.
Keeping a map pod -> volumes makes removing a pod fast. We can just jump
to the relevant volumes directly and remove the pod from there.
When calling ControllerSELinuxTranslator.Conflicts(), the SELinux label
is repeatedly split into []string to detect conflicts. This causes a huge
number of allocations when there are many comparisons.
This is now made more efficient by pre-parsing the SELinux label and
storing it in podInfo as [4]string for fast comparison when needed.
Replace all imports of k8s.io/apimachinery/pkg/util/dump with
k8s.io/utils/dump across the repo. The apimachinery dump package
now contains deprecated wrapper functions that delegate to
k8s.io/utils/dump for backwards compatibility.
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
The daemonset controller already has handling for NotFound errors.
Right now if the statefulset controller is attempting to scale down a
statefulset and its informer cache is stale, it can get hard-blocked on a
missing pod.
This issue will eventually self-resolve once the informer cache "catches
up", but in the process of exploring this issue I realized that
404s during pod deletions don't strictly need to abort the entire sync;
we can continue.
This is especially impactful for large statefulsets with
podManagementStrategy: Parallel, where a single "phantom" pod (actually
missing, but still present in the informer cache) can block thousands of
other pods from being cleaned up.
* DRA: support non-pod references in ReservedFor
Signed-off-by: MohammedSaalif <salifud2004@gmail.com>
* Expand reservation validation comment in syncClaim as suggested by mortent
* Address feedback: rename valid to remaining and remove obsolete TODO
---------
Signed-off-by: MohammedSaalif <salifud2004@gmail.com>
5644850607 added support for List+Watch to a fake client-go instance.
However, that support was not quite working yet as seen when analyzing a test
flake:
- List returned early when there were no objects, without adding the
ResourceVersion. The ResourceVersion should have been "0" instead.
- When encountering "" as ResourceVersion, Watch didn't deliver
any objects. That was meant to preserve compatibility with clients
which don't expect objects from a Watch, but the right semantic of
"" is "Start at most recent", which includes delivering existing
objects.
Tests which meddle with the List implementation via a reactor (like
clustertrustbundlepublisher) have to be aware that Watch now may
return objects when given an empty ResourceVersion.
This initially was attempted in 5f083e3b9f
but it caused e2e failing heavily (see
https://github.com/kubernetes/kubernetes/issues/135222 for more details).
The changes proposed in e6641cd290
clarified the naming of the two existing scale methods but they still
leave both of them as is.
This change goes a step further by combining both into a single method
with a parameter `forceUpdate` which is responsible for driving the mode
of execution between lazy update and a forced update.
Signed-off-by: Maciej Szulik <soltysh@gmail.com>
