Merge ErrorResponse and Status

[imp]

This addresses #40 and by extension #1862

Motivation

This is done in order to expose the entire API Status data as close as possible to the metav1::Status object.

Solution

The Status struct is used as a payload in kube::Error::Api variant effectively bringing all the data from the (failed) k8s API request.

Also a bunch of constants added for different values of reason field (the list of reasons is shamelessly stolen from the K8s Go code base) as well as a few Status methods for identifying a popular error situation.
The implementation is also inspired by that of the Kubernetes.

I'd like to solicit opinions if that feels like a correct approach?

For example I was not entirely sure whether to convert the reason field to Rust enum instead of String. Do folks think that would be a right thing to do? I mean usually it is a good thing in Rust world, but in this specific case it might be rather large addition of LoCs with relatively low value. Would love to hear your thoughts.

I didn't touch yet the original ErrorResponse struct, but it could be of course aliased to the Status or alternatively made From<Status> provided we agree on the rest.

[codecov]

Codecov Report

❌ Patch coverage is 65.38462% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.0%. Comparing base (14b5010) to head (4dd3ce9).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
kube-runtime/src/watcher.rs	0.0%	7 Missing ⚠️
kube-client/src/client/mod.rs	33.4%	6 Missing ⚠️
kube-core/src/response.rs	88.3%	4 Missing ⚠️
kube-client/src/api/core_methods.rs	50.0%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1875     +/-   ##
=======================================
+ Coverage   75.9%   76.0%   +0.2%     
=======================================
  Files         85      85             
  Lines       8237    8267     +30     
=======================================
+ Hits        6244    6276     +32     
+ Misses      1993    1991      -2     

Files with missing lines	Coverage Δ
kube-client/src/api/entry.rs	77.6% <ø> (ø)
kube-core/src/watch.rs	57.2% <ø> (ø)
kube-client/src/api/core_methods.rs	79.0% <50.0%> (ø)
kube-core/src/response.rs	64.0% <88.3%> (+30.7%)	⬆️
kube-client/src/client/mod.rs	76.5% <33.4%> (+1.1%)	⬆️
kube-runtime/src/watcher.rs	43.8% <0.0%> (ø)

... and 4 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]

Thanks a lot. I like this approach personally. Left some questions initially and my thoughts on enums/const strs here (tl;dr; i don't think it's worth making it more advanced, but we should document where we got the values from).

[imp]

So the question now is what to do with the original ErrorResponse?

I can think of three options:

• Drop it entirely
• Make it a type alias
• Keep it, mark as deprecated and provide impl From<Status> for ErrorResponse

Dropping it may be too drastic, type alias - I personally not a fan of it. My choice would be deprecating and removing in one of the followup releases.

Any feedback on that ?

[clux]

Keep it, mark as deprecated and provide impl From for ErrorResponse

You could do this, and it would slightly help users who use the struct directly in unexpected ways, but it wouldn't help the match cases though?. It's probably better than a blind removal since that would result in a "type not found" type error from the compiler, but unless people get to carry on using ErrorResponse in match statements, it would break anyway for the majority use case? If that's the case, I don't think it's providing enough benefit.

Make it a type alias

This would cause a small extra break for the users actually making an ErrorResponse if they are constructing it from scratch (it does not impl Default), but the compiler error for that (missing field) is pretty helpful here. I also expect the number of people constructing an ErrorResponse to be way lower than people doing matching on api errors.

If people have to change the match syntax anyway, then an alias feels better to me; it lessons the blow with a better compiler error (and a #[deprecated] warning on the type alias can help guide them towards the new match syntax).

[imp]

If people have to change the match syntax anyway, then an alias feels better to me; it lessons the blow with a better compiler error (and a #[deprecated] warning on the type alias can help guide them towards the new match syntax).

Agree. I'll make necessary changes.

[doxxx93]

Sorry, I accidentally clicked the "Update branch" button and created a merge commit (2910383).

If you prefer a clean rebase history, please feel free to force push to remove this merge commit. Apologies for the noise!

[imp]

Sorry, I accidentally clicked the "Update branch" button and created a merge commit (2910383).

If you prefer a clean rebase history, please feel free to force push to remove this merge commit. Apologies for the noise!

No worries

[clux]

Looks great. Thank you.

clippy is red, but that's because you pushed literally a minute before it got fixed so nw :D

happy to merge if you are

[imp]

Yay, all green!

[clux]

Ah, quick addon, i noticed this actually does* present errors when referencing ErrorResponse in one of my controllers;

error[E0422]: cannot find struct, variant or union type `ErrorResponse` in module `kube::error`
   --> src/controller/reconcile.rs:129:39
    |
129 |         kube::Error::Api(kube::error::ErrorResponse { code: 403, .. }) => {
    |                                       ^^^^^^^^^^^^^ not found in `kube::error`

For more information about this error, try `rustc --explain E0422`.
error: could not compile `xctrl` (lib) due to 1 previous error

I think the ErrorResponse type alias also need to exist in the places we export Status now.
(obv this example would still fail, but with the deprecation warning it should be easier to understand)

EDIT: doing this in #1883