-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Sync ClusterDeployment Unreachable condition to managedcluster for UI/Console to leverage. #452
base: main
Are you sure you want to change the base?
WIP: Sync ClusterDeployment Unreachable condition to managedcluster for UI/Console to leverage. #452
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xuezhaojun 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 |
/assign @qiujian16 Request for the review to confirm the direction of this solution is right. |
…/Console to leverage. Signed-off-by: xuezhaojun <[email protected]>
806e034
to
a35eccc
Compare
Quality Gate failedFailed conditions |
@xuezhaojun: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
I have two comments in general:
|
Thanks for the review! I checked the The most close one is under the Comparing with the import-controller, the path of the controller only has a the That's why I believe import-controller is more relate to this feature. Because the CA update happens on the agent side, so to check the valid of the admin-kubeconfig secret, we need to implement the In hive, they use a DelayingReconciler to check valid status every 1000 seconds. And hive also ensure the performance at scale by using the MaxConcurrentReconciles and some hard code retry logic e.g. maxUnreachableDuration If we want to do the same valid check, we need to introduce these complixities to our repo and consider the performance issue as well. So to me, I prefer, for the first step, we only do syncing the condition to managed cluster to let console have something to display. If customer like the feature and use it a lot, then we put effort to enhance it. |
Jira: https://issues.redhat.com/browse/ACM-16340
The source condition is from: https://github.com/openshift/hive/blob/a428d219e9fb59937d9b7e847f8d921ffb7b59a8/apis/hive/v1/clusterdeployment_types.go#L426
The controller in hive maintain this condition: https://github.com/openshift/hive/blob/a428d219e9fb59937d9b7e847f8d921ffb7b59a8/pkg/controller/unreachable/unreachable_controller.go#L277