Add HasNodeGroupStartedScaleUp to cluster state registry. #6169
Conversation
|
/LGTM |
|
/assign |
| // Difference between IsNodeGroupScalingUp() is, this function doesn't check whether | ||
| // there are any upcoming nodes for the node group. | ||
| func (csr *ClusterStateRegistry) HasNodeGroupStartedScaleUp(nodeGroupName string) bool { | ||
| _, found := csr.scaleUpRequests[nodeGroupName] |
There was a problem hiding this comment.
Node group requests are cleared in
, what semantics exactly are you looking for here?There was a problem hiding this comment.
This is required in the forked CA to see whether a node group has started scale up regardless of any upcoming nodes. In forked CA, node group's target size can change before the next call to UpdateNodes (and in turn to updateScaleRequests).
| @@ -501,6 +501,14 @@ func (csr *ClusterStateRegistry) IsNodeGroupScalingUp(nodeGroupName string) bool | |||
| return found | |||
| } | |||
|
|
|||
| // HasNodeGroupStartedScaleUp returns true if the node group has started scale up. | |||
| // Difference between IsNodeGroupScalingUp() is, this function doesn't check whether | |||
There was a problem hiding this comment.
This comment would be more useful if it explained why this is useful rather than describing how it is implemented.
There was a problem hiding this comment.
Reworded the comment
There was a problem hiding this comment.
Please add a test case that would illustrate the difference betwen IsNodeGroupScalingUp and HasNodeGroupStartedScaleUp. As far as unit testing goes, they seem to be identical.
There was a problem hiding this comment.
Added a new test demonstrating the case where two methods differ.
| @@ -501,6 +501,17 @@ func (csr *ClusterStateRegistry) IsNodeGroupScalingUp(nodeGroupName string) bool | |||
| return found | |||
| } | |||
|
|
|||
| // HasNodeGroupStartedScaleUp returns true if the node group has started scale up regarless | |||
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hbostan, x13n 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 |
- HasNodeGroupStartedScaleUp checks wheter a scale up request exists without checking any upcoming nodes.
|
/lgtm |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds a new function to
ClusterStateRegistrycalledHasNodeGroupStartedScaleUp()which returns whether the node group has a scale up request without checking if there are any upcoming nodes in the node group.Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: