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

Support different Kubernetes versions #3

Closed
Arnavion opened this issue Jun 2, 2018 · 6 comments
Closed

Support different Kubernetes versions #3

Arnavion opened this issue Jun 2, 2018 · 6 comments

Comments

@Arnavion
Copy link
Owner

Arnavion commented Jun 2, 2018

I don't think there's a need to specialize on patch versions (1.7.2 vs 1.7.3), so just separate minor versions will be good enough (1.7.x vs 1.8.x).


Options for source organization:

  1. Separate git branches for every version, including the code generator code.

    • 👎 Fixes to the code generator itself would need to be cherry-picked to every branch constantly. Easy to forget to update all of them and possibly introduce regressions.
  2. Separate git branches for every version, and a separate branch for just the code generator code.

    • 👎 Loses the association of code generator changes with corresponding codegen changes. Makes it easy to forget to update all of them and possibly introduce regressions.
  3. Separate directories in the same branch.

    • 👍 Can't forget to update any codegen for the corresponding code generator changes.
    • 👍 Every code generator change is clearly associated with the corresponding codegen change.
    • 👎 Lots of checked-in files with lots of duplication.

Options for publishing:

  1. Multiple releases of a single crate - k8s-openapi@1.7.*, k8s-openapi@1.8.*

    • 👎 Only space for a single number in the version to increment for releases, so can't follow semver for breaking changes (like changing the root namespace of all types). Would have to teach users to use = constraints with a specific patch version in their Cargo.toml.
  2. Separate crate for every version - k8s-openapi-1-7@*, k8s-openapi-1-8@*

    • 👍 Access to a full crate versions allows semver-major changes, like changing the root namespace.

    • 👍 For websites like crates.io or docs.rs that always default to showing the latest version, this makes it clear that older Kubernetes versions are supported.

    • 👎 Lots of crate names.


cc @anowell

@anowell
Copy link

anowell commented Jun 3, 2018

Thanks for the copy. I don't know that I've thought enough about this have a strong opinion yet, but some combinations appeal more than others at the moment.

What I find most tempting is a slight variation of your options: If you organized source in separate directories AND used cargo features to conditionally compile a particular version directory, then you could publish a single k8s-openapi crate with semver versioning completely independent of the underlying kubernetes versions. I could simply depend on k8s-openapi with something like features = ["k8s-1-8"]. If k8s-openapi took that approach, I'd probably use the same features in kubeclient with the goal of having the latest kubeclient work across multiple recent k8s versions.

Downsides of that approach: still lots of duplicated code checked in, and cargo doesn't really have a way to enforce mutually exclusive features (other than, compile errors for name collisions). Would there be a default version bumped with every k8s release? That would be a semver breaking change almost every 3-4 months. I think this approach would work best if NOT specifying a default feature version and only removing old versions when you have sufficient cause for a semver breaking change or letting old versions live for many months as #[deprecated] before quietly removing them in minor versions. It certainly adds complexity.

If you don't take a feature-flagged approach like that, I'd probably tend toward a branch-per-version (no preference on how to version the generator) and still publishing a single crate approach despite needing the clear warning for "not following semver". I think you could get by with promoting wildcard constraints like 1.7.* if you can limit breaking changes to when you add a new k8s version (e.g. 1.8.0). I'd rather have kubeclient depend on k8s-openapi 1.7.* instead of =1.7.0 as the latter makes it harder for kubeclient users to get an essential k8s-openapi 1.7.1 bugfix.

I struggle with the separate crate for every version. I see the semver advantage, and ~4 crates per year probably isn't too bad, but would you also name squat every possible k8s-openapi-x.y crate name for the foreseeable future? And it also messes with stats since someone might be looking at the brand new k8s-openapi-1.10 crate and only see like 85 total downloads even if the previous 2 versions have thousands of downloads each.

@Arnavion
Copy link
Owner Author

Arnavion commented Jun 3, 2018

cargo doesn't really have a way to enforce mutually exclusive features

Actually you don't want mutually exclusive features, because you do want to be able to support more than one target Kubernetes version in the same deps tree. Eg the user's application wants to use a Kubernetes 1.10 feature so it enables the v1_10 feature, but it also uses an operators helper crate that supports any version >= 1.7 so that crate enables the v1_7 feature.

You could have features support this scenario by having the v1_10 feature depend on the v1_7 feature, and making the codegen annotate only the new types and fields added in 1.10 with #[cfg(v1_10)].

Eg 1.10 added ConfigMap::binary_data, so ConfigMap would have to look like

pub struct ConfigMap {
    #[serde(rename = "apiVersion")]
    #[serde(skip_serializing_if = "Option::is_none")]
    pub api_version: Option<String>,

    #[cfg(feature = "v1_10")]
    #[serde(rename = "binaryData")]
    #[serde(skip_serializing_if = "Option::is_none")]
    pub binary_data: Option<::std::collections::BTreeMap<String, ::ByteString>>,

    #[serde(skip_serializing_if = "Option::is_none")]
    pub data: Option<::std::collections::BTreeMap<String, String>>,
}

But even this has a problem. The operators crate that only enables the v1_7 feature has code like

let cm = ConfigMap { api_version, data };

and now it doesn't compile because the compiler complains it doesn't initialize the binary_data field.

It also has the problem of being impossibly complex to codegen automatically, especially given diffs like this between api::core::v1::Endpoints in 1.9 and 1.10:

-    pub subsets: Vec<::api::core::v1::EndpointSubset>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub subsets: Option<Vec<::api::core::v1::EndpointSubset>>,

which would need to emitted like

    #[cfg(all(feature = "v1_7", not(any(feature = "v1_10"))))]
    pub subsets: Vec<::api::core::v1::EndpointSubset>,

    #[cfg(feature = "v1_10")]
    #[serde(skip_serializing_if = "Option::is_none")]
    pub subsets: Option<Vec<::api::core::v1::EndpointSubset>>,

and would still break when 1.11 got released.

So I don't think features can be made to work with this.


I think you could get by with promoting wildcard constraints like 1.7.* if you can limit breaking changes to when you add a new k8s version (e.g. 1.8.0).

The kind of semver-breaking change I'm worried about is one that's unavoidably pushed down from upstream because it isn't a breaking change for them. That's why I used the example of changing the root namespace. Today the code generator strips the io.k8s. prefix from all types because all types are under than namespace. But if Kubernetes 1.7.1 adds a type under io.foo then it would need a breaking change in the k8s-openapi@1.7.1 compared to the k8s-openapi@1.7.0 crate where all existing types like api::core::v1::PodSpec have to move to k8s::api::core::v1::PodSpec.

I can also imagine upstream changing the spec file in ways that doesn't affect the JSON serialization but does affect the Rust representation, eg by renaming a definition name, or adding or removing type aliases.

So if such a backward-incompatible change can sneak into a 1.7.* release, then it forces kubeclient and other users to use strict =1.7.0 constraints if they want their crate to continue compiling tomorrow. And that has the problems you said.

~4 crates per year probably isn't too bad, but would you also name squat every possible k8s-openapi-x.y crate name for the foreseeable future?

I wouldn't reserve them up-front, but yes, having to make four new subdirectories / branches + crates per year is definitely something I'm not enthused about.

@anowell
Copy link

anowell commented Jun 4, 2018

Actually you don't want mutually exclusive features, because you do want to be able to support more than one target Kubernetes version in the same deps tree.

Great point I completely overlooked! I assume this is about allowing you to switch target k8s version at runtime. I struggled thinking through how I'd actually use this. Parsing a PodSpec based on runtime version or passing around a PodSpec irrespective of version ends up adding complications, especially when you end up with repeating this for every k8s resource type. But I think I could just codegen copies of my kubeclient types to live behind modules like mod v1_7. Essentially, I'd have a 1.7 client, 1.8 client, etc.. all in the same crate.

But I'm not yet fully persuaded that I'd use multi-target versioning it in kubeclient. I might even point to the fact that there doesn't seem to be a guarantee that any particular version of kubectl works with a different version of the API (I've been burned trying to use kubectl 1.9 on a 1.5 cluster). Still, I am intrigued by the idea that kubeclient could support multiple versions chosen at runtime, so definitely something here for me to keep chewing on.

It also has the problem of being impossibly complex to codegen automatically,

I don't think you'd want to codegen it the way you described even if you could. Using your Endpoints example, if I build a tool that supports both 1.7 and 1.10 with a runtime switch that determines which version I'm using, and I run it in 1.7 mode, it would still skip serialization of subsets which is required by the 1.7 API. I need to have 2 different Endpoints types.

That said, you could simply version module namespaces. use k8s_openapi::v1_7::api::core::v1::PodSpec would make it possible to create runtime logic for choosing between different versions. In fact, this feels a lot like multiple crates idea except that the namespace is at the module-level instead of crate-level. Extending the comparison of these 2 approaches:

Separate crate per k8s version
👍 access to full crate version
👍 clear that old k8s versions are supported
👍 allows you the flexibility to choose between maintaining source in multiple directories or multiple branches
👎 lots of crate names, most of them really only intended to get support for 9-12 months.
👎 crate metrics/ranking/popularity effectively reset for every new k8s version (may affect discoverability)
👎 risk that someone squats a name that follows your crate naming pattern
👎 multiple crates to version. Do you republish them all when the generator changes? Will they have similar versions? When a 2.0.0 is needed for an older crate, does the next new crate start from 2.0.0?
👎 Subtle transitive dep issues mixing k8s API versions. There might be crate version pairings of k8s-openapi-v1-07 and k8s-openapi-v1-10 that result in an unsolvable dep tree. You'd probably want to require that mixing k8s-openapi crates requires also keeping them in the same semver range.

Separate module per k8s version
👍 access to full crate version
👍 clear that old k8s versions are supported
👍 single crate (all metrics, 1 to publish, 1 place to discover)
❓ forces you to use multiple directories for maintaining source (only a negative if you'd prefer to use branches)
👎 removing an old k8s version requires a semver breaking change or a #[deprecation] strategy
👎 do you default to building all versions (bloat) or no versions (unusable until users specify a feature)?


The kind of semver-breaking change I'm worried about is one that's unavoidably pushed down from upstream

Yeah, I couldn't think of a concrete example. I feel like the example you gave could be worked around to avoid a breaking change until the next minor release, but it might be harder to maintain and there may be some other upstream changes that are harder to work around. Anyhow, it seems like this idea has less traction anyway.

I find myself drawn to a single crate solution using modules to namespace different k8s API versions..

@Arnavion
Copy link
Owner Author

Arnavion commented Jun 4, 2018

Actually you don't want mutually exclusive features, because you do want to be able to support more than one target Kubernetes version in the same deps tree.

I assume this is about allowing you to switch target k8s version at runtime.

No, the situation is like the example I gave after that sentence. If a crate works with k8s version X or higher, it'll enable only that version's feature in its k8s-openapi dependency. So two libraries with different minimum version requirements will enable two different features. That's why I said that the only way this can work is if each higher-version feature automatically enables all lower-version features, and the codegen annotates changes with all(not(lower_version), higher_version) and all(higher_version).

I might even point to the fact that there doesn't seem to be a guarantee that any particular version of kubectl works with a different version of the API.

You are always able to use an older API client with a newer apiserver, per their backwards-compatibility design. Fields added to existing objects are always optional for input (ConfigMap::binary_data), and breaking changes to objects are made behind a version change (v1beta1 -> v1beta2 -> v1).

That said, you could simply version module namespaces.

That's a good idea. The modules should probably still be behind version-specific feature-gates so that the crate finishes compiling before the heat death of the universe. Let me think about this.

@Arnavion
Copy link
Owner Author

Arnavion commented Jun 7, 2018

@anowell You probably want to pin kubeclient-rs's dep of this repo to the previous commit (475b5b2) instead of master if you're not going to implement your own version-based features yet.

@anowell
Copy link

anowell commented Jun 7, 2018

Thx. I'll try to pin that soon, and then start working with your versions - hopefully in a couple weeks when I get past some work deadlines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants