search

fluxcd / flux

Successor: https://github.com/fluxcd/flux2
https://fluxcd.io
Apache License 2.0
6.9k stars 1.08k forks source link

Suppress deprecation warnings #3558

Closed dimbleby closed 2 years ago

dimbleby commented 3 years ago

Fixes #3554

kingdonb commented 2 years ago

Thank you! This looks like what we discussed at the dev meeting. Let me test it out now that I am done traveling for a while.

kingdonb commented 2 years ago

We are going to get some feedback from @squaremo before this can be merged, thanks for your patience.

squaremo commented 2 years ago

I had to remind myself how this stuff worked :-S It checks out though, with the (very reasonable, and possibly already verified?) assumption that the warnings are coming from trying to list resources during garbage collection. This happens in https://github.com/fluxcd/flux/blob/07948b22b0574f1342bc19ae8799ce735f1c5e9f/pkg/cluster/kubernetes/sync.go#L336 and it uses the dynamic client. This is the only use of the dynamic client so far as I can tell.

So this is indeed an incisive fix -- nicely done @dimbleby and @kingdonb.

kingdonb commented 2 years ago

I would like to understand why this one deprecation warning still comes through, when all the others are suppressed:

W1101 19:44:22.571730       7 warnings.go:70] batch/v1beta1 CronJob is deprecated in v1.21+, unavailable in v1.25+; use batch/v1 CronJob

I think these errors must be emitted from the getWorkload method on *cronJobKind, which calls the batch/v1beta1 CronJob client explicitly, (and that would need to be addressed some day, if Flux v1 should actually live long enough to see Kubernetes v1.25, but that'll still be quite some time down the road...) :

https://github.com/fluxcd/flux/blob/07948b22b0574f1342bc19ae8799ce735f1c5e9f/pkg/cluster/kubernetes/resourcekinds.go#L368

This happens only because Flux understands CronJob as a workload type that can be subject to automated upgrades.

These are probably the particular "real important deprecation warnings" that I was actually looking to see, so they are indeed preserved by this change, and the overall volume of deprecation warning spam has gone down to practically zero as desired. I'm satisfied after testing the change and will approve this, then merge it to create a new release this week. 👍

Thanks all, for the help you've provided to resolve this issue!