Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow merging multi-version CRDs into a single schema #889

Merged
merged 12 commits into from May 3, 2022
Merged

Conversation

clux
Copy link
Member

@clux clux commented Apr 24, 2022

Implementation for #569. Follows official docs.

  • example showing how upgrades work in practice without conversion
  • tests doing a simple yaml schema merge
  • docs; how to maintain/upgrade versions with kube-derive

Docs touch on #865 because it's hard to recommend multi-versioning without it, but users can hack around it.
Tests add some dependencies use elsewhere in project (assert_json_diff from examples).

The example is the most interesting, found out this with 2 versions:

  • while v1 is active (storage: true); writes to v2 will truncate schema as per v1 defn
  • while v2 is active (storage: true); writes to v1 will truncate schema as per v2 defn

this means that you can end up:

  • not being able to read from the v1 api if v2 is not backwards compat
  • not being able to read from v2 api if v1 is not forwards compat

tried to clarify this as well as possible in the example and docs.

@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2022

Codecov Report

Merging #889 (cc3c4ad) into master (98c0ef2) will increase coverage by 0.09%.
The diff coverage is 82.92%.

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
+ Coverage   70.25%   70.35%   +0.09%     
==========================================
  Files          61       62       +1     
  Lines        4223     4264      +41     
==========================================
+ Hits         2967     3000      +33     
- Misses       1256     1264       +8     
Impacted Files Coverage Δ
kube-derive/src/lib.rs 0.00% <ø> (ø)
kube-core/src/crd.rs 82.92% <82.92%> (ø)
kube-runtime/src/wait.rs 68.00% <0.00%> (-2.00%) ⬇️

@clux
Copy link
Member Author

clux commented Apr 24, 2022

This should be pretty ready now. I have made it a separate function rather than implementing it on CustomResourceExt because I'm not doing this for v1beta1, and it's probably not great to add an unimplemented!() on that side.

We could also remove v1beta1 now. Can't find any cloud providers supporting Kubernetes < 1.16 anymore.
EDIT: done in #890

@clux clux requested a review from a team April 24, 2022 16:58
@clux clux linked an issue Apr 24, 2022 that may be closed by this pull request
@clux clux added the changelog-add changelog added category for prs label Apr 24, 2022
@clux clux added this to the 0.72.0 milestone Apr 24, 2022
@clux
Copy link
Member Author

clux commented Apr 27, 2022

@kazk you able to have a look at this?

kube-core/src/crd.rs Outdated Show resolved Hide resolved
clux added 12 commits May 3, 2022 07:02
Sketch for #569

Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
- while v1 is active; you cannot write to v2 and simultaneously maintain v2 schema
- while v2 is active; you cannot write to v1 and simultaneously maintain v1 schema

this means that you can end up:

- not being able to from the v1 api if v2 is not backwards compat
- not being able to read from v2 api if v1 is not forwards compat

Signed-off-by: clux <sszynrae@gmail.com>
changing to &str

Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented May 3, 2022

This should be pretty ready now. I have made it a separate function rather than implementing it on CustomResourceExt because I'm not doing this for v1beta1, and it's probably not great to add an unimplemented!() on that side.

Even with v1beta1 removed, I don't think the api is nicer with a static CustomResourceExt::merge_crds so am going to leave this as is as a separate merge_crds fn.

@clux clux merged commit e8a4b70 into master May 3, 2022
@clux clux deleted the crd-merge branch May 3, 2022 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-add changelog added category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CRDs for multiple resource versions
3 participants