check for nil pointer before invoking rollout controller

[nirvanagit]

During startup time, the virtual service controller and the rollout controller get initialized successfully, but the remote registry cache (which stores all the controllers) is initialized only after all the controllers have been initialized, as seen in https://github.com/istio-ecosystem/admiral/blob/bdfcbae2337a6314dbd8c4a2fc9b23db3509abcb/admiral/pkg/clusters/registry.go#L237-L238

Meanwhile the controllers are getting initialized, there might be a virtual service event which gets sent from the kubernetes API server. When handling a virtual service event, admiral checks for corresponding argorollout. But until this https://github.com/istio-ecosystem/admiral/blob/bdfcbae2337a6314dbd8c4a2fc9b23db3509abcb/admiral/pkg/clusters/registry.go#L237-L238 code is not called, the remote registry will not be cached, hence when this statement is executed: https://github.com/istio-ecosystem/admiral/blob/05fc3dbbba4757c03be28a52f67c7316402b32fc/admiral/pkg/clusters/handler.go#L390, it causes a nil pointer exception.

In order to prevent the nil pointer, we explicitly check if the remote registry cache has the controller for the given cluster, and if it exist, we continue. Else the sync will happen in the next full sync cycle.

[codecov-commenter]

Codecov Report

Merging #256 (72a672d) into master (05fc3db) will decrease coverage by 1.27%. The diff coverage is 11.11%.

@@            Coverage Diff             @@
##           master     #256      +/-   ##
==========================================
- Coverage   73.02%   71.74%   -1.28%     
==========================================
  Files          31       31              
  Lines        3129     3136       +7     
==========================================
- Hits         2285     2250      -35     
- Misses        700      746      +46     
+ Partials      144      140       -4     

Impacted Files	Coverage Δ
admiral/pkg/controller/common/config.go	75.43% <ø> (ø)
admiral/pkg/clusters/handler.go	57.09% <11.11%> (-7.28%)	:arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[nirvanagit]

Looks good. I am worried this this can happen for other events like DestinationRule etc when they are added to customer namespaces. Should we implement this in the controllers one level up, do we have access to RemoteRegistry there?

Totally missed this one, did you mean in Handlers of each type? If so, yes, each handler has remote registry as its property.