Evict pods at the end of install command
This PR tries to make the install command more gentle with a production cluster. Instead of deleting all unmanaged pods, we try to evict them, which essentially achieves a pod deletion but it does so by honoring Pod Disruption Budgets. This way deletions won't cause service outages.
Fixes: #239
Signed-off-by: Lorenzo Fundaró [email protected]
⚠️ No tests yet, waiting for Authors input first.
@lfundaro thanks! The PR LGTM.
warning No tests yet, waiting for Authors input first.
I think we can start address testing now. I guess it would help if the logic was split into a dedicated function (for readability if not for testing).
@lfundaro thanks! The PR LGTM.
warning No tests yet, waiting for Authors input first.
I think we can start address testing now. I guess it would help if the logic was split into a dedicated function (for readability if not for testing).
Hi @kaworu ! I refactored the code and split functions. Also, I started testing some of the newly introduced functions. I'm delaying the test of runEvictions as it's a bit more involved and thought I might discuss with you before. For that function I see two ways of testing:
- Mocking the eviction API. A bit involved I think.
- Not testing the function per se but rather use CI to run a
cilium installand verify that restarting pods honors PDBs.
Those are the ones off the top of my head. What do you think ?
btw, I'm gonna be off the radar for a week, but as soon as I'm back, I can pick up any comments left.
Cheers !
@lfundaro yes using CI make sense to me. /cc @cilium/ci-structure
Quick high-level question before I look at the code, was this an optional behaviour already, or was pod deletion done unconditionally?
Quick high-level question before I look at the code, was this an optional behaviour already, or was pod deletion done unconditionally?
@errordeveloper
cilium install has the option --restart-unmanaged-pods which defaults to true.
basically, you can opt-out from this behavior, otherwise, restart happens.
The build for this PR is failing and there seem to be review comments which haven't been addressed yet. I'll thus convert the PR to draft status. Please mark it as ready for review once everything is addressed.