Api<PartialObjectMeta<K>> should opportunistically degrade to metadata requests

[doxxx93]

Motivation

Currently Api<PartialObjectMeta<K>>::get (and list, watch, patch) requests the whole
object from the API server and discards everything except metadata during deserialization.
This requires every downstream API to maintain separate metadata variants (e.g.
metadata_watcher, and Controller::watches has no metadata path at all).

Solution

Add fn metadata_api() -> bool to the Resource trait (default false), overridden to
true for PartialObjectMeta<K>. Api methods get, list, watch, and patch branch
on this flag to automatically use metadata-optimized Accept headers.

• Add metadata_api() to Resource trait with doctest
• Branch in Api's get/list/watch/patch based on K::metadata_api()
• Deprecate metadata_watcher in favor of watcher(Api::<PartialObjectMeta<K>>::all(client), config)

Closes #1614

[codecov]

Codecov Report

❌ Patch coverage is 91.76471% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.8%. Comparing base (67e4671) to head (f0ae3dd).

Files with missing lines	Patch %	Lines
kube-core/src/metadata.rs	60.0%	2 Missing ⚠️
kube-core/src/resource.rs	0.0%	2 Missing ⚠️
kube-runtime/src/controller/mod.rs	0.0%	2 Missing ⚠️
kube-client/src/api/mod.rs	75.0%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1952     +/-   ##
=======================================
+ Coverage   75.6%   75.8%   +0.2%     
=======================================
  Files         89      89             
  Lines       8666    8745     +79     
=======================================
+ Hits        6544    6621     +77     
- Misses      2122    2124      +2     

Files with missing lines	Coverage Δ
kube-client/src/api/core_methods.rs	81.9% <100.0%> (+2.9%)	⬆️
kube-client/src/client/mod.rs	82.9% <100.0%> (+5.3%)	⬆️
kube-runtime/src/watcher.rs	58.9% <ø> (+0.6%)	⬆️
kube-client/src/api/mod.rs	66.1% <75.0%> (+0.8%)	⬆️
kube-core/src/metadata.rs	71.8% <60.0%> (+4.2%)	⬆️
kube-core/src/resource.rs	60.0% <0.0%> (-1.3%)	⬇️
kube-runtime/src/controller/mod.rs	31.6% <0.0%> (+0.3%)	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[clux]

This is a great idea. Simplifies watcher complexity a bit and lits the logic into kube-core which is always great to see.

It is a breaking change so one for the next major, but i like this a lot.

[clux]

Looking a bit more around, I am noticing that this technically breaks the intention of the Lookup trait introduced in #1393 since now special user types (described in #1389 ) have to impl Resource anyway.

Maybe we should also extend Lookup to be able to determine if it's a metadata_api type (with a default impl), and maybe we could rely on Lookup as the watcher constraint instead since it is lighter than full Resource?

[doxxx93]

sorry for late reply I'll check it tmrw and update it

[doxxx93]

Sorry for the long delay!

Digging into where the K: Resource actually crept in: it was really just one line in core_methods.rs (calling K::metadata_api() per request), which then forced the same bound onto both ApiMode impls in watcher.rs.

So the fix is to cache metadata_api on Api<K> itself, populated in the existing impl<K: Resource> Api<K> constructors where the bound was already required. The per-method paths just read self.metadata_api now, and I could drop all three Resource additions. No method bound is tighter than before this PR.

On extending Lookup with metadata_api() and switching the watcher to a Lookup bound: I'd love to do it but it feels like a bigger refactor than this PR should swallow. Api<K> itself needs K: Resource for URL building, and watcher takes Api<K>, so even with the flag on Lookup a Lookup-only K can't reach the watcher path today. The reflector::Store Lookup relaxation from #1393 still works for the #1389 use case (they bring their own watch source).

Happy to open a follow-up issue for "loosen watcher's data source so Lookup-only K can use it" if that's a direction you want to keep alive. Or if you'd rather have Lookup::metadata_api() default-imp landed here too as a pure declarative move, just say the word and I'll add it.

[clux]

Very nice. I agree you don't you need to extend Lookup as it is already quite custom. I just don't want it to be completely incompatible with even the Api.

Notifying @SOF3 as a heads up about this change though.

[clux]

Did some quick associated doc updates in kube-rs/website#95 as a sanity check to myself.

A slight downside of the deprecation here is that users do have to bring PartialObjectMeta into scope to select the metadata api at an earlier stage than before (usually people only would notice when they had to use it), but they would generally have to import this anyway, and the deprecation notice is clear.