Add HiveD scheduler config adapter for CA #4868
Conversation
Add HiveD scheduler config patcher for Cluster Autoscaler.
|
maybe adapter is a more appropriate name than patcher? |
| self.__init_hived_config() | ||
| self.__init_kube_client() | ||
| self.__nodes = [] | ||
| self.__fake_nodes = ["fake{}".format(self.base36(i)) for i in range(max_nodes)] |
There was a problem hiding this comment.
Why not just i instead of base36(i), for easy read and parse
There was a problem hiding this comment.
to keep consistent with vmss naming format, six digits base 36
There was a problem hiding this comment.
ok, but seems no benifts to keep this consistent
|
|
||
| def watch_nodes(self): | ||
| w = watch.Watch() | ||
| for event in w.stream(self.__client.list_node, _request_timeout=30): |
There was a problem hiding this comment.
What is the _request_timeout?
If no events during the timeout, will the for loop exit?
Shall we increase it?
There was a problem hiding this comment.
What is the _request_timeout?
It's used in single list_node api, reference.
If no events during the timeout, will the for loop exit?
No, there's another timeout_seconds parameter.
Shall we increase it?
I think it's fine, the example uses 60.
There was a problem hiding this comment.
So let's make timeout_seconds longer ?
See kubernetes-client/python#124
Update HiveD config patcher.
| class HiveDConfigPatcher(object): | ||
| def __init__( | ||
| self, | ||
| min_nodes=None, |
There was a problem hiding this comment.
min_nodes [](start = 12, length = 9)
Seems we do not use min_nodes
There was a problem hiding this comment.
it's an attribute of CA like max_nodes, included here in case we need it in the future
Rename to adapter and update deployment.
Workaround for resource calculation.
Add HiveD scheduler config adapter for Cluster Autoscaler.