open-cluster-management-io / registration

hub / spoke registration controllers
Apache License 2.0
42 stars 58 forks source link

exclude the cordoned nodes when calc node allocatable #174

Closed skeeey closed 3 years ago

skeeey commented 3 years ago

Signed-off-by: liuwei liuweixa@redhat.com

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: skeeey

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: - ~~[OWNERS](https://github.com/open-cluster-management-io/registration/blob/main/OWNERS)~~ [skeeey] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
skeeey commented 3 years ago

refer to https://github.com/open-cluster-management-io/community/issues/77

skeeey commented 3 years ago

/assign @qiujian16

skeeey commented 3 years ago

why do we separate this to another controller? It will double the update call, right?

I think separate this make the code logic more clear, currently, we maintain this in the cluster joining controller, but collecting the node resource and handling cluster join event may be two irrelevant tasks

when the cluster starts to register, this will add an update call, but if we use cluster Available as the judgement condition, I think we can stagger this update from others

what's your opinion? do you think we should keep this in the joining controller?

qiujian16 commented 3 years ago

I think we can move it as part of health check (available) controller

skeeey commented 3 years ago

agree with you, and I think we can also put the get version in health check controller, thus the joining controller will only handle the cluster joining

qiujian16 commented 3 years ago

/lgtm