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

minimum supported k8s version #684

Closed
olix0r opened this issue Oct 28, 2021 · 9 comments · Fixed by #924
Closed

minimum supported k8s version #684

olix0r opened this issue Oct 28, 2021 · 9 comments · Fixed by #924
Assignees
Labels
docs unclear documentation

Comments

@olix0r
Copy link
Contributor

olix0r commented Oct 28, 2021

From what I can tell, kube-runtime v0.63 does not work with k8s-openapi versions before v1.19 due to this line:

https://github.com/kube-rs/kube-rs/blob/16c4bf2389529ace6a2b784bfd71b91103be2198/kube-runtime/src/events.rs#L3

because the events API was not at v1 before v1.19. We get the following error:

error[E0432]: unresolved import `k8s_openapi::api::events::v1`
 --> /home/ver/.cargo/registry/src/github.com-1ecc6299db9ec823/kube-runtime-0.63.1/src/events.rs:3:46
  |
3 |     api::{core::v1::ObjectReference, events::v1::Event as CoreEvent},
  |                                              ^^ could not find `v1` in `events`

Practically, 1.19 is probably a perfectly reasonable minimum supported version at this point. But this was a surprise to us since Linkerd's minimum supported version is currently 1.17, so we either need to bump our minimum version or stay on v0.61.

Ignoring Linkerd's issues... I think kube needs to be explicit about its minimum supported version and communicate (i.e. in release notes, and ultimately semver) when support for a version is dropped. I'd even propose that kube re-export k8s-openapi with features for each of its supported versions so that this compatibility matrix is made explicit in kube's features.

@kazk kazk added the bug Something isn't working label Oct 28, 2021
@olix0r
Copy link
Contributor Author

olix0r commented Oct 28, 2021

As a suggestion, it's probably best for kube-rs to match the k8s support policy https://kubernetes.io/releases/patch-releases/ -- v1.19 is reaching its EOL in the next few hours, so it's probably perfectly fine to support v1.20+ (until February, when 1.20 reaches its EOL).

@kazk
Copy link
Member

kazk commented Oct 28, 2021

Thanks for reporting.

I think kube needs to be explicit about its minimum supported version and communicate (i.e. in release notes, and ultimately semver) when support for a version is dropped.

I don't think this was intentional. As far as I know, we try to support all versions supported by k8s-openapi which is based on Cloud offerings. But yeah, we should be explicit, and test against all of them. This doesn't happen too often, but it's not the first time either (#621). I believe this will happen more often as we add more (non-core) resource specific utils.

I'd even propose that kube re-export k8s-openapi with features for each of its supported versions so that this compatibility matrix is made explicit in kube's features.

Adding version features to kube is probably easier to maintain, and easier for the users as we add more resource-specific utilities. #621 was resolved by adding deprecated-crd-v1beta1. But then we'll need to keep track of which version supports what, and that's going to be a mess. Also, I don't think we can tell if the user has enabled k8s-openapi feature conflicting that.

@clux can you elaborate why you didn't want to introduce version features? (#621 (comment))

@olix0r
Copy link
Contributor Author

olix0r commented Oct 28, 2021

As a reference point, I think we're going to adopt a support policy that matches Kubernetes (linkerd/linkerd2#7171). At the very least, we want to be able to exercise the minimum supported version in k3d/kind, and that's difficult/impossible with versions like 1.17 (which reached EOL in January 2021).

@kazk kazk added the docs unclear documentation label Oct 28, 2021
@nightkr
Copy link
Member

nightkr commented Oct 28, 2021

Can't speak for @clux, but I don't like us having to maintain version features either:

  1. It blocks access to features added in new k8s versions until kube is updated (depending on how this is implemented, at least)
  2. It's redundant with k8s-openapi version features (or k8s-pb if we end up migrating to that)
  3. The same problem will appear for downstream crates as well

One immediate workaround would be to wrap the events module in a k8s_if_ge_1_19! block.

Definitely agree that we need to agree on what we actually support, and actually test against that.

@clux
Copy link
Member

clux commented Oct 28, 2021

Yeah, the duplication was my main reason there originally. Although I do think there are some good benefits here that can be architected from it if we think about it from a fresh k8s-pb perspective:

  • if we re-exported features we could also re-export types (avoiding the duplicate selection problem)
  • re-exporting versioning features lets us use k8s-openapi as a true dependency. Not a side-injected one that breaks dependabot upgrades with kube. Not a required dev dep that causes constant problems internally with CI (dev dep specificity in all crates, packaging features, docs build features, resolver 2 complaints)
  • we can run ci test suites against all supported versions without having to code in specific feature hack override (like we currently have in examples)
  • blocking access to new version features until kube was updated was always a problem anyway due to other minor changes in their api - if we manage the codegen then it should be easier to define a release process for this

@clux
Copy link
Member

clux commented Oct 28, 2021

As for what we support, I had suggested: min_supported(eks k8s version, aks k8s version, gke k8s version) which is currently min(1.17, 1.15, 1.19) if i am reading the AKS one correctly.

There's four categories of dates here we can concern ourselves with:

Now, we could go full kubernetes, version after it, and use skew policies, but I don't think we are set up to do that yet - too much flux to maintain several branches of releases. Maybe after our big projects like gold + pb codegen are sorted out.

I also don't want to make users choose to NOT upgrade kube because their infra team is overburdened and running in "trailing EKS mode", I think a little extra leeway on the EOL is helpful. It also doesn't cost us a lot since we don't currently integrate with a bunch of specific APIs (just admission, crds and now events IIRC).

I think the best we can do for now is something like kubernetes EOL date + N months:

N MK8SV
0 1.20 (today)
1 1.19 (until 2021-11-28)
4 1.19 (until 2022-02-28)
5 1.18 (until 2021-11-18)
9 1.18 (until 2022-02-18)
10 1.17 (until 2021-11-13)
12 1.17 (until 2022-01-13)

and decide on an N. E.g. 6 or 12 probably initially, then refine as we move towards full stability.

clux added a commit that referenced this issue Oct 28, 2021
Signed-off-by: clux <sszynrae@gmail.com>
clux added a commit that referenced this issue Oct 28, 2021
* deprecated events feature - for #684

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

* use if_ge_1_19 instead

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

* note about it in docs

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

clux commented Oct 28, 2021

Fix for this specific aspect of the kubernetes minimum version is published in 0.63.2 by hiding the broken module for older kubernetes versions.

We will probably also codify some MSK8SV policy somehow, but for now, didn't want to bump it from like 1.15 to 1.19 just because of an accident.

@clux clux removed the bug Something isn't working label Oct 28, 2021
@clux clux self-assigned this Oct 30, 2021
@clux
Copy link
Member

clux commented Oct 30, 2021

Had a go at creating a CI matrix job for this, and could not get anywhere sensible due to the version features. Thinking that this has to be tackled at the codegen level instead by avoiding version features.

@clux clux added docs unclear documentation and removed docs unclear documentation labels Nov 3, 2021
@clux clux removed their assignment Nov 21, 2021
@clux clux mentioned this issue Nov 21, 2021
33 tasks
clux added a commit that referenced this issue May 31, 2022
As part of Kubernetes version policies proposed in kube-rs/website#19
for #684

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

clux commented May 31, 2022

Been thinking more about this. Turns out we can define a minimum support and run simple tests against stable apis without changing k8s-openapi feature version at a start (because our tests don't rely on deprecated/alpha apis). This is done in #924 and policies are proposed in kube-rs/website#19.

@clux clux linked a pull request May 31, 2022 that will close this issue
9 tasks
@clux clux self-assigned this May 31, 2022
@clux clux closed this as completed in #924 Jun 19, 2022
clux added a commit that referenced this issue Jun 19, 2022
* Run e2e tests against supported kubernetes versions

As part of Kubernetes version policies proposed in kube-rs/website#19
for #684

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

* add integration tests as specific job to run it against mk8sv

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

* rename tests and try to fix integration

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

* fix k3d args?

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

* fix e2e

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

* test against v1.19

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

* badge update

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

* s/MINK8SV/MK8SV

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

* Big e2e rework (WIP)

Now two e2e tests, and more feature combinations.

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

* avoid rustls local due to the issues

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

* split e2e tests into own file and clean up when to matrix

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

* running bump-k8s also bumps mk8sv

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

* avoid illegal anchors and add badge bumping

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

* better version checks for mk8sv and msrv on ci

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

* move some ci around for consistency in naming

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

* stray command

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

* fix syntax errors?

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

* fix test warning from kube-derive after apiext removal

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

* evars from tests

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

* better run description on integration jobs

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

* better output for flaky integration test

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

* fmt + give test more time?

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

* sanity after timeout. seems likely cause.

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

* no need for an agent in k3d cmd

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

* how slow is this actions cluster?

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

* re-trigger once more to check

* apiserver, are you ok?

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

* another empty commit to retrigger

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

* remove duplicate changes from #924

(won't build until that is merged)

Signed-off-by: clux <sszynrae@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs unclear documentation
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants