rollouts: add ready condition to remote root sync #3737
Conversation
| generationChanged := rrs.Generation == rrs.Status.ObservedGeneration | ||
| readyStatusChanged := len(rrs.Status.Conditions) == 0 || rrs.Status.Conditions[0].Type != "Ready" || rrs.Status.Conditions[0].Status != rssReady | ||
|
|
||
| if !generationChanged && !syncStatusChanged && !readyStatusChanged { |
There was a problem hiding this comment.
Another way to do this will be: compute the new status and compare the new status with current using reflect.DeepEqual (that compares the entire struct recursively). That will future proof this code as we evolve the status.
There was a problem hiding this comment.
Looking into this. I will need to exclude the Status.Conditions[].LastTransitionTime field from the comparison as this can vary from the existing status and any status I compute from scratch.
| @@ -136,42 +141,81 @@ func (r *RemoteRootSyncReconciler) Reconcile(ctx context.Context, req ctrl.Reque | |||
|
|
|||
| gkeCluster, err := r.store.GetCluster(ctx, cr.Name) | |||
| if err != nil { | |||
| r.updateStatus(ctx, &remoterootsync, clusterConnectivityFailed, nil) | |||
There was a problem hiding this comment.
I need to double check the underlying code but just getting the cluster shouldn't require reaching out to the target cluster so connectivityFailure may not be applicable.
|
@ChristopherFry Do we want to do anything here or close this out ? |
|
Closing this pull request in favor of #3849. |
This pull request updates the rollouts proof of concept, adding a ready condition to the remote root sync status. This condition allows us to understand if the remote root sync has connectivity to the target cluster.