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.

Sign up for GitHub

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

Jump to bottom

Default version need to follow official Version Priority #23

Closed
imp opened this issue Dec 8, 2021 · 6 comments · Fixed by #27
Closed

Default version need to follow official Version Priority #23

imp opened this issue Dec 8, 2021 · 6 comments · Fixed by #27

Comments

@imp
Copy link
Collaborator

imp commented Dec 8, 2021 •
edited
Loading

When multiple versions are described in the CRD and no --api-version flag is given it may be better to use the one that is specified in storedVersion of the status, rather than just the first one.
@imp
Copy link
Collaborator Author

imp commented Dec 8, 2021

Well, after giving this more thoughts - no, it is not better.

@imp imp closed this as completed Dec 8, 2021
@nightkr
Copy link
Member

nightkr commented Dec 8, 2021

IMO, this should either align with kubectl (whatever its behaviour is, I'll admit I haven't tried this yet) or always be required.

@imp
Copy link
Collaborator Author

imp commented Dec 8, 2021

Ah, here we go - Version Priority

@imp imp reopened this Dec 8, 2021
@imp imp changed the title storedVersion may be better default when multiple versions are present Default version need to follow official Version Priority Dec 8, 2021
@imp
Copy link
Collaborator Author

imp commented Dec 8, 2021 •
edited
Loading

And here seems to be the Golang implementation. Although apparently it doesn't follow the description above strictly, as it doesn't handle versions like v1alpha and v4beta properly.

@imp imp mentioned this issue Dec 8, 2021
Closed
@clux
Copy link
Member

clux commented Dec 10, 2021

As a side-note, we do have code for the version policy in kube-client/discovery, but it's private: https://github.com/kube-rs/kube-rs/blob/master/kube-client/src/discovery/version.rs

@imp
Copy link
Collaborator Author

imp commented Dec 10, 2021

Oh, had no idea. Looks pretty similar and doesn't use regex, which is advantage. So let me know what you want to do with that.

clux added a commit to kube-rs/kube that referenced this issue Dec 21, 2021
@clux
Export Version parser with Ord impl in kube_core
9c6cdcd 
doing the cleanup steps described to make this worthy of exporting.
needed for kube-rs/kopium#23

Signed-off-by: clux <sszynrae@gmail.com>
@clux clux mentioned this issue Dec 21, 2021
Merged
clux added a commit to kube-rs/kube that referenced this issue Dec 22, 2021
@clux
Export Version parser with Ord impl in kube_core
c6781c2 
doing the cleanup steps described to make this worthy of exporting.
needed for kube-rs/kopium#23

Signed-off-by: clux <sszynrae@gmail.com>
clux added a commit to kube-rs/kube that referenced this issue Dec 22, 2021
@clux
Export Version parser with Ord impl in kube_core
f476cb4 
doing the cleanup steps described to make this worthy of exporting.
needed for kube-rs/kopium#23

Signed-off-by: clux <sszynrae@gmail.com>
clux added a commit to kube-rs/kube that referenced this issue Dec 27, 2021
@clux @nightkr @kazk
Export Version priority parser with Ord impls in kube_core (#764)
4017a3f 
* Export Version parser with Ord impl in kube_core

doing the cleanup steps described to make this worthy of exporting.
needed for kube-rs/kopium#23

Signed-off-by: clux <sszynrae@gmail.com>

* remove leftover TODO + improve doc links between discovery and core

Signed-off-by: clux <sszynrae@gmail.com>

* import more tests, fix one, better docs

Signed-off-by: clux <sszynrae@gmail.com>

* a bit less Reverse and less double negatives

Signed-off-by: clux <sszynrae@gmail.com>

* Version export - explicitly request ordering (#767)

* Explicitly request which ordering to use

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>

* Add `Version::latest` that includes considers v2 > v2beta1 > v1

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>

* Rename Version::latest_stable to priority

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>

* Also rename LatestStable accordingly

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>

* rename latest to semantic for language consistency

we had two orders ::latest and ::priority, but v.latest() was a
ambiguous. v.latest could mean:

- we are sorting by age (but that's not true)
- we are sorting by latest version in semver semantics (true

so think v.semantic() is slightly more clear. v2.semantic() >
v1.semantic() feels a little easier to comprehend than v2.latest() >
v1.latest()

Signed-off-by: clux <sszynrae@gmail.com>

* fix self-references and rename semantic to distance

Signed-off-by: clux <sszynrae@gmail.com>

* rename to generation

Signed-off-by: clux <sszynrae@gmail.com>

* We need to split on byte lengths, not code point counts (#772)

See #764 (comment)

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>

* Apply suggestions from code review

Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>

* Update kube-core/src/version.rs

Co-authored-by: kazk <kazk.dev@gmail.com>

Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>
Co-authored-by: kazk <kazk.dev@gmail.com>
@clux clux mentioned this issue Dec 27, 2021
Merged
@clux clux linked a pull request Jan 16, 2022 that will close this issue
select version by priority rather than picking first #27
Merged
@clux clux closed this as completed in #27 Jan 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

select version by priority rather than picking first kube-rs/kopium
3 participants
@imp @clux @nightkr