annotate fakeNodes so that cloudprovider implementations can identify them #4119
Conversation
k8s-ci-robot
added
cncf-cla: yes
marwanad
force-pushed
the
annotate-unregistered-nodes
branch
2 times, most recently
from
8bffb5a to
92bba5b
Compare
marwanad
force-pushed
the
annotate-unregistered-nodes
branch
from
92bba5b to
36460df
Compare
|
|
||
| // FakeNodeReasonAnnotation is an annotation added to the fake placeholder nodes CA has created | ||
| // Note that this don't map to real nodes in k8s and are merely used for error handling | ||
| FakeNodeReasonAnnotation = "k8s.io/cluster-autoscaler/fake-node-reason" |
There was a problem hiding this comment.
Given that the stated goal is to use those annotations in cloudprovider, won't declaring them here risk cyclic dependency problems? I think it's much cleaner to aim at situation where cloudprovider doesn't need to import core CA modules.
Maybe we can declare those consts in cloudprovider.go instead? This has an added benefit of making them easier to discover for anyone implementing new provider.
There was a problem hiding this comment.
yeah - good call on that!
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MaciekPytel, marwanad 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
This adds an annotation to the "fakeNode" used by CA for unregistered node. This can be useful for cloudproviders in the
DeleteNode()method to have some custom error handling logic and also identify the reason for thefakeNode. (For now, it seems only GCE would make use of the nodes created with error path)An example of how we want to leverage that: we don't want to decrement the cached NodeGroup capacity in Azure because the capacity reported by APIs won't include Failed instances so the cache will end up drifting unnecessarily from the real capacity and can inadvertently lead to a scale down if some conditions align.
I've noticed the packet provider tries to identify some via providerId but I don't think this is a reliable way. Similar with the magnum provider via the object UID. I think a namespaced annotation would be a convenient way for identification.