postpone connecting to remote cluster until when needed #2402
Conversation
| okToRemoveFinalizers := false | ||
| var err error | ||
|
|
||
| remoteClient, err := util.GetClusterClient(r.context, r.Client, r.Scheme, clusterapiutil.ObjectKey(cluster)) | ||
| if err != nil { | ||
| log.Error(err, "Error getting remote cluster client") |
There was a problem hiding this comment.
suggestion: Don't log errors unless you're adding extra debug info. Instead, wrap it and return it like:
return ctrl.Result{Requeue: true}, fmt.Errorf("failed to get remote cluster client: %w", err)There was a problem hiding this comment.
Thanks for the comment. I addressed this one, but noticed there are about 50 other cases in this file to fix. I'll add an issue and do those after the testings as it is of less priority
| @@ -213,6 +208,11 @@ func (r *ClusterBootstrapReconciler) reconcileNormal(cluster *clusterapiv1beta1. | |||
| return ctrl.Result{}, err | |||
| } | |||
|
|
|||
| remoteClient, err := util.GetClusterClient(r.context, r.Client, r.Scheme, clusterapiutil.ObjectKey(cluster)) | |||
| if err != nil { | |||
| return ctrl.Result{Requeue: true}, fmt.Errorf("failed to get remote cluster client: %w", err) | |||
There was a problem hiding this comment.
Add a delay with RequeueAfter: constants.RequeueAfterDuration. At this point the cluster is "ready" based on cluster api but the api-server may need a while to accept requests.
| remoteClient, err := util.GetClusterClient(r.context, r.Client, r.Scheme, clusterapiutil.ObjectKey(cluster)) | ||
| if err != nil { | ||
| log.Error(err, "Error getting remote cluster client") | ||
| return ctrl.Result{Requeue: true}, err |
There was a problem hiding this comment.
Same comment, add a delay.
addons/controllers/errorfile
Outdated
| @@ -0,0 +1,173 @@ | |||
| Running Suite: Addon Controller Suite | |||
There was a problem hiding this comment.
Omg, how this one has been committed! Definitely, thanks for actually noticing it
|
@vijaykatam @rajathagasthya Could I please get a merge approval label for this PR? Thanks |
What this PR does / why we need it
This is to postpone connecting to remote cluster until when needed
Which issue(s) this PR fixes
Fixes #2401
Describe testing done for PR
Existing unit/env tests
Release note
PR Checklist
Additional information
Special notes for your reviewer