Mechanism to restart pods running outdated proxies

[hercynium]

Recently I was looking for better ways to manage our Istio upgrades at work, and a real pain point was identifying and restarting all the deployments that were still connected to the old Istio revision after we deployed a new revision and updated the webhook tags.

The sail operator looks really exciting, but it doesn't (yet) have this feature #74 ... so I decided to try creating it. I'm a beginner at golang, but here's what I came up with: https://github.com/hercynium/istio-fortsa

It's very much a work-in-progress, but feel free to use it for ideas to add this functionality to Sail. Even if it's not useful to the project, I'm always welcome to input on how to make my code better, too - issues, comments, PRs, etc!

[dgn]

hey @hercynium! First of all, sorry for the late response. I had a look at the repo, but can you maybe quickly outline how your implementation works? We're looking at simply restarting Deployments when there's a new version, but we need to investigate the details, e.g. how a phased rollout would work

[hercynium]

Before writing fortsa, we had a shell script that called istioctl proxy-status then scraped the output and did a bunch of other stuff before terminating pods. So it seemed natural to start with that mechanism, but try to get the data directly in code instead of parsing a CLI command's output.

Thinking about it, perhaps the pod controller could just check all the pods in the cluster for their annotation vs the annotation on the namespace they're in. Right now it only has to look at the ones that were marked as outdated when the mutatingwebhook or namespace controllers detected a change. I'll have to think about it some more on how to trigger a check of pods. I only worry that it could be a big load on the k8s API... but maybe that's not really a big problem? I'm still new to this stuff.

[luksa]

@hercynium The operator uses a local cache, so checking the annotations doesn't require any API calls.

[dgn]

thanks for the summary @hercynium! I agree with @luksa that using the injected annotation is probably simpler (and it's what we had in mind). do you have any logic to detect whether a migration was successful, and if not, perform rollbacks? another thing I'd like to include are phased rollouts, ie not migrate everything at once but have configurable limits on e.g. how many workloads are restarted at the same timec

[hercynium]

There's no logic to detect if the migration was successful, as it's the same logic as running kubectl rollout restart $deployment_name from the command-line. The controller could probably have logic to continuously check the status of the migration, similar to how kubectl rollout status works, though. As for checking if the new proxy is actually working correctly, the only way I know to do that is... via istioctl proxy-status.

For "phased rollouts" I think there would have to be a more complex process than what I've got, which is simply using the rate-limiting built into the operator framework (see: https://github.com/hercynium/istio-fortsa/blob/v0.0.30/internal/util/util.go#L71) and I'm not even sure that's working how I expected as I haven't yet observed it during an upgrade on a large cluster. However, I recently installed it to a medium-sized cluster (about 300 meshed pods) in our test env and I hope to see what happens during our next Istio upgrade in a few weeks.

[hercynium]

In case you're interested, I've re-written major portions for Fortsa based on your input... In particular, there's no longer any dependency on Istio XDS code! It seems to work well, and has been running stable in a dev cluster for a few days now where I've switched the Istio tags back and forth between different Istio revs. Each time it does the right thing.

I know I should have actual tests, but that will take more time to figure out how to do properly.