search

bookingcom / shipper

Kubernetes native multi-cluster canary or blue-green rollouts using Helm
Apache License 2.0
733 stars 39 forks source link

Clustrerclientstore embeds shipper clientset and informer factory refs #276 #290

Closed osdrv closed 4 years ago

osdrv commented 4 years ago

This commit changes the signature of clusterclientstore interface and arms it with GetApplicationClusterClientset/2 method which returns an instance of clusterclientstore.ClientsetInterface. The new composite interface is supposed to embed kubernetes and shipper clientsets and informer factories. The aforementioned method is the one that can return an error, which is opposite to the former clusterclientset.Interface behavior where every independent method call to GetConfig/1, GetClient/1 and GetInformerFactory/1 could return an error independently, leaving the client with a responsibility if it's ok to proceed forward.

Fixes #276

osdrv commented 4 years ago

@juliogreff this is a draft version of this PR and I'd be super happy to hear your thoughts and suggestions. Thanks!

juliogreff commented 4 years ago

I can take another harder look and nitpick, but so far LGTM. What's needed to take it out of draft?

osdrv commented 4 years ago

@juliogreff I mainly wanted to make sure I followed your guidance correctly and left no elephants in the room. Commit management is the only todo so far.