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

clarify release support #88

Merged
merged 2 commits into from
Dec 12, 2023
Merged

clarify release support #88

merged 2 commits into from
Dec 12, 2023

Conversation

dplore
Copy link
Member

@dplore dplore commented Jun 26, 2023

This update is in response to requests for clarification on expectations for versioning of a gNSI server.

@earies
Copy link
Contributor

earies commented Jun 27, 2023

@dplore @marcushines

I was actually just about to post a similar item but would like to draw attention to protobuf/API versioning in general as various umbrella projects under OpenConfig are inconsistent and propose a more cohesive approach to this

gNSI

  • IDL specifies a package name which contains a version (v1 today) - this affects the URI in the HTTP header
  • IDL leverages gNOI types to encode a semver option (1.2.0 today)
  • all gNSI services import a common version so any changes need to increment the global version

gNOI

  • All distinct gNOI services specify their own package name - no versioning in the package
  • All distinct gNOI services encode a unique semver option from gNOI types
  • No common versioning

gNMI

  • No versioning in the package name
  • Redefinition of the semver option (vs. reuse from gNOI types)

gRIBI

  • No versioning whatsoever but a directory structure (v1) that started to think about versioning

Now, some request for comments here as there are multiple approaches (some of which are more friendly to transition paths than others)

  • Does gNSI need to encode the version in the package name? We now have 2 versions at play here - the semver in version.proto and as soon as it breaches 2.0.0 means that the package really should be bumped to v2
  • Does gNSI need to be globally versioned? A change in one API signals a change in all
  • To the above point, if that is the case then why don't we make all proto IDL part of the same package (gnsi) w/ the service blocks making the differentiation?
  • If gNSI is globally versioned then what is the justification vs. say gNOI? (And it could very well be that it is all or nothing from an implementation standpoint)
  • Should types.proto be more generic, removed from gNOI and reused for all sub-projects rather than create the dependence -> gNOI (or complete redefinition in gNMI)
  • Thoughts on consistency across all OpenConfig projects implementing gRPC/GPB APIs (all projects must be versioned)

@earies
Copy link
Contributor

earies commented Jun 27, 2023

Also note, that it appears the release numbering is now also misaligned w/ the proto versioning (release 1.2.1 vs. proto 1.2.0)

https://github.com/openconfig/gnsi/releases/tag/v1.2.1

IMO a release that fixes something like stub regen should be a patch build off 1.2.0 (e.g. 1.2.0-20230605.00)

@dplore
Copy link
Member Author

dplore commented Jun 29, 2023

Thanks for the comments!

A version name is needed in the package so a client can support multiple major versions simultaneously. This means the packages need to have different names. Clients are expected to carry this burden so a given gNSI server will only need to support a single version. In a large network, there will be many devices with multiple vendors and some rollout strategy that results in different g* services versions running on different devices. So the client needs to take on this burden.

It's a good idea to consider using a semver build annotation if code regeneration is updated and the proto file is not changed. But probably we should endeavor to update the proto and generated code together in one release so we don't need this extra annotation.

I'll put together a bigger proposal to make the versioning strategy consistent across the g* services and share in the near future.

@earies
Copy link
Contributor

earies commented Jun 30, 2023

A version name is needed in the package so a client can support multiple major versions simultaneously. This means the packages need to have different names. Clients are expected to carry this burden so a given gNSI server will only need to support a single version. In a large network, there will be many devices with multiple vendors and some rollout strategy that results in different g* services versions running on different devices. So the client needs to take on this burden.

Having the version in the package name is imo a good thing but one that has not been seen in any OpenConfig API work to date (common in many other gRPC/HTTP APIs). Essentially when the package name is altered, you can consider this an entirely new specification for the API + endpoint. In theory, you could not even consider the previous version however a protocol should be put in place of what is actually allowed/disallowed between say v1 -> v2 of a set of service specifications (e.g. being mindful of a transition path for the client + server)

It's a good idea to consider using a semver build annotation if code regeneration is updated and the proto file is not changed. But probably we should endeavor to update the proto and generated code together in one release so we don't need this extra annotation.

Agreed and this applies directly to my last comment where the release was updated to 1.2.1 diverging the proto IDL/specifications

I'll put together a bigger proposal to make the versioning strategy consistent across the g* services and share in the near future.

Thx - a few considerations worth noting

  • Within a backwards compatible major version, abide by protobuf language rules for compatibility
  • The package version is only uprev'd on major versions v1 -> v2
  • The semantic version within the IDL (or set of IDL under a common package) continues as-is. (e.g. a change in the IDL from 1.2.0 -> 1.3.0 -> 1.3.1 all iterates the version extension but the package name remains at v1 throughout) - The published releases follow this scheme + any build annotations (to accommodate non-API changes)
  • For the semantic version extension, consider one "version" extension option that is leveraged for all APIs vs. each g* project redefining and/or needing their own imports (gNMI, gNOI + gNSI all reimplement the same concept today)
  • Where a set of services is to share a common version, consider making these all part of the same package. For instance, if gNOI continues to be "per service" versioned under the umbrella project, that is fine and disaggregated in that one implementation can support a subset at a time easier. For gNSI, since the goal is that all services come in tandem and are mandatory dependencies, all import and use a common version then I see no reason why these need separate package names (they could continue to just implement separate service blocks and share a common package/version)

@marcushines marcushines merged commit d5abc2e into main Dec 12, 2023
5 checks passed
nmahabaleshwar pushed a commit to nmahabaleshwar/gnsi that referenced this pull request Jan 3, 2024
* clarify release support

* trim whitespace
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

Successfully merging this pull request may close these issues.

None yet

3 participants