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

Alpha Beta GA designation for new capabilities #355

Closed
saad-ali opened this issue Mar 5, 2019 · 7 comments · Fixed by #417
Closed

Alpha Beta GA designation for new capabilities #355

saad-ali opened this issue Mar 5, 2019 · 7 comments · Fixed by #417
Milestone

Comments

@saad-ali
Copy link
Member

saad-ali commented Mar 5, 2019

Problem

Now that CSI is 1.0, we can no longer make backwards compatibility breaking changes. But we want to continue to add new functionality to CSI, and it is almost impossible to propose an API and have it be "perfect" in its initial release (which is why CSI remained in 0.x for a year).

Goal

We should define a process for introducing new official APIs as alpha then promote to beta and then GA.

Proposal

This can be as simple as a comment on a capability designating it alpha, beta, or GA. The tricky part is how alpha APIs can change without breaking clients. So we may need to say that fields and methods can be renamed/deprecated but never removed. Alpha features can be renamed or deprecated in any release. Beta features can be renamed or deprecated after 6 months or 2 releases (whichever is longer). And GA features can only be renamed or deprecated during major releases (e.g. between CSI v1.x to v2.x).

@saad-ali
Copy link
Member Author

saad-ali commented Mar 6, 2019

Proposal from @jieyu -- maintain multiple proto files: GA, alpha.

  • Initially they all start off as identical.
  • New functionality is added to alpha, and once graduated moves to the other files.
  • Guarantees for each file will differ as mentioned above.
  • Features must be added to alpha first. When they graduate, both files must be identical for that feature.
  • CI should be added to protect from making changes to mainline without making change in alpha as well.
  • Cons:
    • This may conflict with go module semantic versioning requirements -- if v1.1 has an alpha and v1.2 has a backwards in compatible version of alpha.
      • We should just document that very explicitly.

@saad-ali saad-ali added this to the v1.2 milestone Mar 6, 2019
@gnufied
Copy link
Contributor

gnufied commented Mar 6, 2019

I was thinking that we should have a csi-extension repo that possibly vendors-in the main CSI spec and then adds some extension on top of it. Some projects may choose to implement csi-extension spec and some other may adopt stable APIs.

The tricky thing perhaps will be most COs will only want to adopt CSI stable/main and even if a driver has support for csi-extension spec it may not be used immediately if CO does not support it.

Once changes in csi-extension become stable, they can be moved off to main repo.

@saad-ali
Copy link
Member Author

Simpler alternative from @julian-hj during meeting:

  • Accept that we will accumulate the cruft.
  • Upon deprecation: can mark field numbers as reserved.
  • Protobuf extension for alpha to programmatically detect if a field is alpha or not
  • Will make bar higher for adding alpha features.

What about RPCs?

  • Everyone has to implement them, and when deprecated they would remain optional to implement.

What about wild and crazy ideas?

  • Forks

@pohly
Copy link
Contributor

pohly commented Mar 22, 2019

What about RPCs?

  • Everyone has to implement them, and when deprecated they would remain optional to implement.

Is this about the API incompatibility in the Go bindings between CSI 1.0.0 and 1.1.0?

The problem is this: a CSI plugin that compiles with the Go bindings for 1.0.0 will not compile after updating to 1.1.0 unless the new gRPC calls are added to the source code of the plugin, because otherwise its implementation no longer matches the extended interface definition.

I consider this a violation of the semver contract that defines minor version bumps as "add functionality in a backwards-compatible manner" - its not backwards-compatible if users of the API have to be updated.

I'm not sure whether there is a good solution just with gRPC generated code. But this could work:

  • add a null implementation (empty struct plus methods) of all optional methods
  • declare that the public API is only stable if CSI plugins include that struct in their own implementation
  • when adding new optional methods, that null implementation gets updated together with re-generating the code

@jdef
Copy link
Member

jdef commented Mar 22, 2019

Thanks for your comments @pohly. I know that when I think of CSI spec versioning and backward compatibility, I normally don't ask myself "is the generated Go code honoring the semantic versioning contract?"

gRPC supports so many languages, and there's no spec for what gRPC compilers should generate. Consider that it's possible to write a gRPC compiler for Go that would not generate interface types to implement, but rather use reflection to discover and invoke struct methods. It would perform worse than today's de facto Go/gRPC-generator, but it would resolve the aforementioned Go interface compatibility problem independently of the changes to CSI's gRPC specification between v1.0 and v1.1. I think that strictly honoring the "semver contract" for generated code is directly impacted by per-language gRPC tooling (that is outside the control of this project), and IMO that's not a rabbit hole that we want to go down here.

I think the best that we can do w/ respect to semver guarantees is aim to be backward compatible "on the wire". Meaning that a CSI 1.0 CO should be able to interact successfully w/ a CSI 1.1 plugin, and vice-versa.

Your suggestion of a "null" implementation sounds like it would work, but I'm not 100% convinced that we should add it to this project. In the past we've drawn the line for per-language, library code in this repo at "it must be auto-generated from the spec". AFAIK the default protoc+grpc tooling doesn't have an option to generate null/adapter Go implementations of the Go interfaces - and so it would fall outside the scope as we've previously discussed.

All that said, it's also come up several times that various members of the community are interested in common libraries for CSI, above and beyond what's included in this repo. Null/adapter implementations (of each CSI service) sound like a good fit for something like that.

@pohly
Copy link
Contributor

pohly commented Mar 22, 2019

AFAIK the default protoc+grpc tooling doesn't have an option to generate null/adapter Go implementations of the Go interfaces - and so it would fall outside the scope as we've previously discussed.

Fair enough. I just wanted to know whether this had been considered.

All that said, it's also come up several times that various members of the community are interested in common libraries for CSI, above and beyond what's included in this repo. Null/adapter implementations (of each CSI service) sound like a good fit for something like that.

I'm worried that such additional common libraries will not gain critical mass and/or enough long-term support unless some existing group does them. Has it been considered to host such work in the github.com/container-storage-interface organization, i.e. owned by the same people who are also currently managing the spec?

@jdef
Copy link
Member

jdef commented Apr 4, 2019

Based on the "alpha API" discussion in the most recent CSI community sync I've drafted some mock ups of some fake experimental APIs so that we can all see how things might look: PTAL #365

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

Successfully merging a pull request may close this issue.

4 participants