Fix status comparison in Openshift #1136
Conversation
| @@ -75,7 +75,11 @@ func UpdateRemoteCluster( | |||
| } | |||
| } | |||
| // Update state | |||
| reconcileState.UpdateRemoteClusters(currentRemoteClusters) | |||
| // If the map is empty then it must be stored as nil, not an empty map. It is retrieved as a nil value from the API | |||
| // and it could lead to some reconcile loops that never converge on Openshift. | |||
There was a problem hiding this comment.
I find the comment a bit misleading. We say it must be stored as nil, but that's not what we actually do.
What we do here IIUC is update the state only if there are some remote clusters. We ignore all cases where the map is either nil or empty.
What if we have 3 remote clusters at some point, then do some updates and get to 0 remote clusters: does it mean the reconcileState never gets updated to match we now have 0 remote clusters?
|
We are going to remove the remote cluster feature which also solves the problem this PR addresses. |
Yes, since it is a blocker for Openshift and I don't know when this code will be removed I just wanted to fix it now so I can make some progress on Openshift. |
|
superseded by #1164 |
Openshift et Kubernetes does not behave the same regarding the comparison of the fields in the status.
While Kubernetes seems to treat empty map and
nilin the same way Openshift does not.This very small PR fixes this problem and allows the operator to converge to the desired state with Openshift.