addConditionAndDeletePod returned nil both after deleting the pod and
when the pod already had a DeletionTimestamp, so maybeDeletePod counted
pods that someone else was deleting in the deletion metrics and in the
DeviceTaintRule evicted pod count.
* Fix StatefulSet maxUnavailable budget to ignore pods below partition
Unavailable pods outside the update range were consuming the rolling-update
budget and could stall Parallel rolls even when maxUnavailable was unset.
* Keep UnavailableReplicas as whole-STS while budget stays partition-scoped
Split the counters so the metric still reports all unavailable replicas, and add a metrics case with unavailable pods below partition.
* Use max() to clamp expected deleted pods in STS test
Satisfies the golangci modernize/minmax hint on the OrderedReady invariant expectation.
* Scale STS maxUnavailable percent from partition range and fix API docs
Percent MaxUnavailable is computed from Replicas-Partition so it matches
the partition-scoped update budget; apps/v1 godoc now states the same.
* Use new() instead of ptr.To in STS maxUnavailable tests
Satisfy golangci modernize/newexpr on the rewritten table.
* Regenerate API docs after MaxUnavailable comment update
Sync generated openapi, swagger, proto, and applyconfiguration
comments with the apps/v1 RollingUpdateStatefulSetStrategy docs.
* Update openapi-spec for STS MaxUnavailable docs
Regenerate swagger and apps/v1 OpenAPI descriptions to match the
partition-scoped MaxUnavailable comment.
The updated logcheck now warns about passing a pointer as value where String
could crash if nil. The new findings are false positives, but there have been
real issues before and adding klog.SafePtr is cheap mitigation for false
positives.
The HandleError* wrappers around contextual logging are now annotated such that
the new logcheck knows that they do contextual logging. This revealed some real
issues (expecting printf-style formatting or missing keys, passing a *function*
instead of the function result).
A raw rest.Config lets a controller build a client that bypasses the
client configuration and transport wrappers the controller manager
applies to the clients it builds, such as the leader election write gate
of KEP-6361. Deprecate Config and ConfigOrDie on ControllerClientBuilder
and on the cloud-provider copy of the interface; out-of-tree cloud
providers and controller managers that call them keep working, and the
configs they get still carry the controller manager's wrappers.
Add TypedClientBuilder, implemented by both in-tree builders, with the
clients controllers were building from raw configs: metadata, dynamic
and scale clients, WithRateLimits for controllers that need more QPS and
burst than the defaults, and WithDropManagedFields for the informer
clients. The controller managers hand the builder to their controllers,
and a forbidigo rule keeps in-tree code off the deprecated methods. Unit
tests cover the derived builders on both implementations.
DiscoveryClient's discovery-tuned limits become a floor rather than an
override, so a builder configured with higher limits keeps them; the
storage version migrator relies on that for its discovery client.
The HPA metrics and apiextensions clients still need a config and are
built in one lint-exempted place in the controller context, since
k8s.io/controller-manager does not depend on those client packages.
DynamicControllerClientBuilder cached the composed WrapTransport, so the
base config's wrapper was applied twice to every client after the first
one built under a service account name. Cache only the token wrapper.
* Fix PDB controller to enqueue all affected budgets on pod changes
Wake previously matching PDBs when pod labels change, and enqueue every
overlapping PDB so status stays correct instead of picking one arbitrarily.
* PDB: warn on multiple budgets only for the current pod
Skip the MultiplePodDisruptionBudgets warning for old labels on update,
and list matching PDB names in the event message.
* PDB: extract multi-budget warning from getPdbsForPod
Move the MultiplePodDisruptionBudgets event out of the lookup helper,
compare pod labels with reflect.DeepEqual, and quote matching PDB names
in the warning message.
* PDB: use controller.KeyFunc for updatePod sync map keys
Match enqueuePdb and use namespace/name keys when deduping PDBs to sync
on pod label updates.
DeviceToleration.effect documents that an empty value matches all taint
effects, and resourceclaim.ToleratesTaint implements that. The eviction
controller however skipped every toleration whose effect was not exactly
NoExecute before calling it, so a toleration without effect did not
protect the pod from a NoExecute taint.
Accept an empty effect as well, and add a test case.
Signed-off-by: yogeshbendre <07.yogesh@gmail.com>
Iterating over containers using `append(pod.Spec.Containers, pod.Spec.InitContainers...)`
allocates a new backing slice and copies large Container structs by value, which
adds unnecessary heap allocations and GC overhead.
Switch callers across kubelet, scheduler plugins, and controllers to iterate
in-place via podutil.ContainerIter.
* hpa: eliminate implicit float64 divide-by-zero in per-pod metric functions
GetObjectPerPodMetricReplicas and GetExternalPerPodMetricReplicas both
compute:
usageRatio := float64(usage) / (float64(target) * float64(replicaCount))
where replicaCount = statusReplicas. When statusReplicas == 0 this
produces +Inf or NaN. The code accidentally works today because
isWithin(+Inf) and isWithin(NaN) both return false, falling through to
the ceilToInt32 path. However this is fragile:
- a future int conversion of usageRatio before comparison would panic
or produce a wrong result
- a change to the isWithin implementation could silently break
scale-from-zero for these metric types
Replace the implicit +Inf/NaN path with an explicit statusReplicas == 0
guard that skips the ratio check and goes directly to the desired count,
matching the intent of the existing comment in getUsageRatioReplicaCount
which already handles currentReplicas == 0 for other metric types.
Existing test cases for statusReplicas == 0 continue to pass unchanged.
Signed-off-by: Samiksha Agarwal <agarwalsamiksha88@gmail.com>
* Update pkg/controller/podautoscaler/replica_calculator.go
Co-authored-by: Adrian Moisey <adrian@changeover.za.net>
* hpa: trim verbose comment in GetExternalPerPodMetricReplicas
Keep the comment concise, matching the simpler style of neighboring
functions.
Signed-off-by: Samiksha Agarwal <agarwalsamiksha88@gmail.com>
* Removed trailing spaces in comments
Signed-off-by: Samiksha Agarwal <agarwalsamiksha88@gmail.com>
* Extract calcPerPodReplicas and guard ratio check with statusReplicas != 0
Move the duplicated per-pod replica calculation from GetObjectPerPodMetricReplicas and GetExternalPerPodMetricReplicas into a shared calcPerPodReplicas helper.
Replace the early return for statusReplicas == 0 with a guard around the usageRatio and tolerance check only, so the desired-count calculation appears once as the fall-through. No behavior change.
Signed-off-by: Samiksha Agarwal <agarwalsamiksha88@gmail.com>
* Rename helper to getPerPodReplicas and add explanatory comments
Explain the return value and why the tolerance check is skipped when no pods are running.
Signed-off-by: Samiksha Agarwal <agarwalsamiksha88@gmail.com>
---------
Signed-off-by: Samiksha Agarwal <agarwalsamiksha88@gmail.com>
Co-authored-by: Adrian Moisey <adrian@changeover.za.net>
The migration runner trimmed a CRD's status.storedVersions to the
storage version whenever the CRD generation was unchanged, including
after a failed migration. A failed migration can leave objects in an
old version, and once that version is gone from storedVersions it can
be removed from the CRD, leaving those objects undecodable.
Only update storedVersions after a successful migration.
* Deployment rollouts fail to report timeouts after old ReplicaSet scale-down errors
Signed-off-by: weizhou blue <weizhou.lan@daocloud.io>
* Consolidate deployment rollout status and error handling
Signed-off-by: weizhou blue <weizhou.lan@daocloud.io>
---------
Signed-off-by: weizhou blue <weizhou.lan@daocloud.io>
* Fix StatefulSet bug of matching leading-zero pod name
* Fix CI error: return value of is not checked
* Avoid duplicated code by making identityMatches call isMemberOf
* Add more test cases
* Simplify the test and add comment to skip lint check
* Update comments of isMemberOf
deleteActivePods and deleteJobPods counted ready pods as deleted even when
DeletePod returned an error, undercounting Job status.ready until the next
successful sync.
* Deployment: surface old RS scale-down errors for workqueue retry
reconcileOldReplicaSets was returning nil when cleanupUnhealthyReplicas
or scaleDownOldReplicaSetsForRollingUpdate failed, so sync looked
successful and did not AddRateLimited. Propagate the error like the
new-RS path, and add a unit test that injects an RS update failure.
* Deployment: fold old-RS scale error cases into table tests
Address review: inject RS update failures in reconcileOldReplicaSets
table (ready and unavailable old replicas) and assert with errors.Is.
processNextWorkItem had its error check inverted when it was
generalised to serve both the PodGroup and CompositePodGroup queues.
Successful syncs were requeued with backoff forever and logged as
failures, while failed syncs (for example a conflict while removing
the finalizer) were dropped and never retried.
Restore the err == nil check and add a test for processNextWorkItem,
which the existing tests did not exercise.
Two in-tree places looked for entries with a literal string lookup. This
compared a QualifiedName from a current slice against a QualifiedName from an
allocation result, which is based on some older slice. The literal lookup
breaks if the slices changed from fully-qualified to un-qualified or vice
versa.
The correct approach is to do what lookupAttribute for constraints does: check
both forms. Following the example set by lookupAttribute, fully-qualified is
given precedence in case of a conflict.
kubernetes-prow[bot]