bookingcom / shipper

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

shipper: disable resync for native k8s objects #208

Closed juliogreff closed 5 years ago

juliogreff commented 5 years ago

This is a fun little optimization that turns out to be much safer than it sounds :)

Some controllers, such as Capacity and Traffic controllers, subscribe to events in both the object they own (a CapacityTarget or TrafficTarget) as well as native kubernetes objects they're interested in (such as a Deployment or a Pod), so they can update the object they own accordingly whenever the state of the world changes.

This is all fine and good until resyncs enter the picture. If both the CRDs and the native objects get resynced at the same time (which they more or less do) and are processed by different workqueues (which they are), they'll race in a way that an update in one may trigger a new item in the workqueue for the other one, that just got processed, just because we can't take advantage of deduplication across different workqueues.

Disabling resyncs from native k8s objects in this case is safe because the CRDs will still keep resyncs (for now), and they're responsible for looking at all the objects they're interested in anyway, so we get better performance for no risk ^_^

As a bonus, the clusterclientstore completely loses its resync option, simplifying initialization a little bit.