Skip to content

Add a metadata field to Status#1891

Merged
clux merged 1 commit into
kube-rs:mainfrom
ryanpbrewster:add-metadata-to-error
Jan 9, 2026
Merged

Add a metadata field to Status#1891
clux merged 1 commit into
kube-rs:mainfrom
ryanpbrewster:add-metadata-to-error

Conversation

@ryanpbrewster
Copy link
Copy Markdown
Contributor

This is a fresh attempt at #1809

The motivation is summarized in #1890

Reviewer notes:

  • I opted to grab the ListMeta field directly from k8s-openapi. This...kind of feels weird, since the kube_core::Status is now extremely similar to the k8s-openapi Status. Should these just be the same struct?
  • ListMeta does not impl Eq, so I removed it from Status. As such, this PR is a breaking change in at least 2 ways (it adds a new field, and it removes an impl).

I can manually impl Eq (which is safe right now, but may become unsafe in the future). I can also use a separate struct that is wire-compatible with the k8s-openapi struct (I'd probably only define the continue token initially). Preferences?

///
/// ```
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This struct contains a Status so the Eq change propagates up

@clux clux linked an issue Jan 7, 2026 that may be closed by this pull request
Signed-off-by: Ryan Brewster <[email protected]>
@ryanpbrewster ryanpbrewster force-pushed the add-metadata-to-error branch from d535020 to 221479e Compare January 7, 2026 14:23
@clux
Copy link
Copy Markdown
Member

clux commented Jan 7, 2026

I opted to grab the ListMeta field directly from k8s-openapi. This...kind of feels weird, since the kube_core::Status is now extremely similar to the k8s-openapi Status. Should these just be the same struct?

ahhh. lol. i don't remember that being there. yeah, maybe they should be the same. however it's needed right now for a deprecation note so don't want to change it right now for the next version. as long as they are the same, it's not a problem for now.

maybe in the next version we can change our Status to a re-export of k8s-openapi's generated Status.

@clux
Copy link
Copy Markdown
Member

clux commented Jan 7, 2026

ListMeta does not impl Eq, so I removed it from Status. As such, this PR is a breaking change in at least 2 ways (it adds a new field, and it removes an impl).

i think this is fine, we are already touching Status for this version. I don't think this Eq derive was introduced conciously. I've said no to Eq in the past.

@clux clux added the changelog-change changelog change category for prs label Jan 7, 2026
@clux clux added this to the 3.0.0 milestone Jan 7, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.0%. Comparing base (178e4ae) to head (221479e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1891     +/-   ##
=======================================
+ Coverage   76.0%   76.0%   +0.1%     
=======================================
  Files         85      85             
  Lines       8329    8336      +7     
=======================================
+ Hits        6325    6334      +9     
+ Misses      2004    2002      -2     
Files with missing lines Coverage Δ
kube-core/src/admission.rs 61.2% <ø> (ø)
kube-core/src/conversion/types.rs 71.5% <ø> (ø)
kube-core/src/response.rs 67.2% <100.0%> (+3.3%) ⬆️

... 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.

Copy link
Copy Markdown
Member

@clux clux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you! (I'll leave it for a day in case other comments pop up, but otherwise will merge).

@clux clux merged commit 3012d9c into kube-rs:main Jan 9, 2026
17 checks passed
@clux clux mentioned this pull request Jan 12, 2026
hugoponthieu pushed a commit to hugoponthieu/kube that referenced this pull request Jan 29, 2026
cchndl pushed a commit to cchndl/kube that referenced this pull request Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-change changelog change category for prs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kube_core::Status missing ListMeta

2 participants