Cluster-autoscaler: add NodeReadyPredicate and allow unready nodes in CA #2314
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
This change is |
| } | ||
|
|
||
| p2 := BuildTestPod("p2", 800, 0) | ||
| p1.Spec.NodeName = "n1" |
There was a problem hiding this comment.
nit: move 2 lines up
| }) | ||
|
|
||
| provider := testprovider.NewTestCloudProvider(nil, func(nodeGroup string, node string) error { | ||
| deletedNodes <- node |
There was a problem hiding this comment.
Where those channels get verified?
There was a problem hiding this comment.
they are unused, removed.
| } | ||
|
|
||
| func TestScaleDownNoMove(t *testing.T) { | ||
| deletedPods := make(chan string, 10) |
There was a problem hiding this comment.
Where those pipes are verified?
There was a problem hiding this comment.
they are unused, removed.
mwielgus
force-pushed
the
enable-unready
branch
from
e67c15e to
f72b526
Compare
|
[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files: We suggest the following people: |
| return true, nil, fmt.Errorf("Wrong node: %v", getAction.GetName()) | ||
| }) | ||
| fakeClient.Fake.AddReactor("delete", "pods", func(action core.Action) (bool, runtime.Object, error) { | ||
| panic(fmt.Errorf("no delete is expected")) |
There was a problem hiding this comment.
nit: wouldn't t.FailNow() be more appropriate?
| } | ||
|
|
||
| // TODO: remove once all of the unready node handling elements are in place. | ||
| if err := CheckGroupsAndNodes(readyNodes, autoscalingContext.CloudProvider); err != nil { |
There was a problem hiding this comment.
Isn't method CheckGroupsAndNodes unused? If yes, can you remove it?
| if condition.Type == apiv1.NodeReady { | ||
| if condition.Status == apiv1.ConditionTrue { | ||
| return true, condition.LastTransitionTime.Time, nil | ||
| canNodeBeReady, readyFound := true, false |
There was a problem hiding this comment.
Please merge GetReadinessState and IsNodeReadyAndSchedulable into one method as they are very similar.
mwielgus
force-pushed
the
enable-unready
branch
from
f72b526 to
271929d
Compare
|
LGTM |
…le-unready Cluster-autoscaler: add NodeReadyPredicate and allow unready nodes in CA
…le-unready Cluster-autoscaler: add NodeReadyPredicate and allow unready nodes in CA
5 participants
Ref: #2228
cc: @jszczepkowski @fgrzadkowski