CSR: fix a bug in GetClusterSize #2546
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/cc @losipiuk |
|
Looks reasonable but please confirm with someone else if we are not breaking some undocumented corner case with this change. |
| @@ -988,7 +988,9 @@ func (csr *ClusterStateRegistry) GetClusterSize() (currentSize, targetSize int) | |||
| for _, accRange := range csr.acceptableRanges { | |||
| targetSize += accRange.CurrentTarget | |||
| } | |||
| currentSize = csr.totalReadiness.Registered - csr.totalReadiness.NotStarted - csr.totalReadiness.LongNotStarted | |||
There was a problem hiding this comment.
Let's update the comment above to clarify this method only returns current and target number of nodes in autoscaled node groups.
While I don't particularly like long names, it also feels like this method's name could better reflect what it's doing - e.g. GetAutoscaledNodesCount?
Currently, GetClusterSize reports the target number for all autoscaled node groups, but the actual number for _all_ node groups, even those that are not autoscaled. This commit fixes that behavior so that both target and actual size reported are from autoscaled node groups only.
towca
force-pushed
the
jtuznik/get-cluster-size-fix
branch
from
425da26 to
f64b6cd
Compare
k8s-ci-robot
added
size/M
|
/lgtm Thanks! |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aleksandra-malinowska 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Currently, GetClusterSize reports the target number for all autoscaled
node groups, but the actual number for all node groups, even those
that are not autoscaled. This commit fixes that behavior so that both
target and actual size reported are from autoscaled node groups only.