crd: add label with CRD version

[MrFreezeex]

Add a CRD version label that must be updated each time we update the corresponding CRD to facilitate install from go.

In cilium we are doing this for our internal CRDs with some logic that check this label so that we don't accidentally downgrade the version and it would be super useful if we can have directly in the mcs-api repo for MCS-API CRDs to facilitate that check.

When we would be updating the CRD we would basically need to bump the version in config/crd-base and launch make manifest to regenerate the label in the config/crd folder (or just update directly there).

Followup to #115

[MrFreezeex]

/cc @skitt @tpantelis

WDYT of this?

[mikemorris]

Gateway API does something similar to this, including the "bundle version" (GitHub release version for the spec, different from the Kubernetes versioning like v1/v1alpha1) and release channel in CRD annotations https://github.com/kubernetes-sigs/gateway-api/blob/main/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml#L6-L7

cc @robscott @youngnick I think maybe one of y'all have mentioned this in a talk about why we do this and how it can be useful?

[youngnick]

Yes, the apiVersion field is a bit coarse-grained and only really tracks breaking changes. So having a schema version that tracks if compatible, additive changes have been made is very useful. That's why we include a bundle version in Gateway API CRDs.

[youngnick]

I'd also recommend noting the procedure down in some release instructions somewhere - otherwise it's easy to forget.

[MrFreezeex]

I'd also recommend noting the procedure down in some release instructions somewhere - otherwise it's easy to forget.

Hmm well there's not really a properly defined release so far, but maybe this could be the same as the git tag which would make it an actual release? It would also mean that we would want to make a new release/git tag almost every-time we change the CRD. WDYT @skitt?

[lauralorenz]

Triage notes:

• This changes our release machinery because we would have another label to update; when/how do we do that exactly we ask ourselves? Mention that more frequent release is ok. We think this is not necessarily blocking and we want to talk to @skitt about this who has the most release experience here
• This proposal is based on how cillium does it today, so that is also prior art
• There is a specific annotation that Gateway API uses and we could copy that and there is some positivity about that - though the Gateway API "bundle" is not required by vendors to be implemented and may have a slightly different semantic meaning than what Arthur is suggesting. This is a naming bikeshed conversation :)

[skitt]

Since we want to track changes, I think the version should be bumped whenever there’s a change — so the bump should be part of the PR making the change, not part of the release. This also means that the CRD version won’t be the same as the project tag, but that’s fine.

We’ll use a version triplet in the same style as Gateway API.

[MrFreezeex]

I updated the PR to start from v0.1.0 since it's not correlated with this repo version (and also adding the v prefix like GW-API). It's very similar to GW-API the only thing is that GW-API use the term "bundle-version" and this is proposing the term "crd-schema-version".

I feel like since it's not part of the release and that GW-API has different bundles (standard, experimental) the term "crd-schema-version" is better suited since I think we are in slighly different scenario than GW-API.

But I don't feel strongly about it so I'm happy to change the term if this means landing this faster and/or that "bundle-version" (or something else) is better liked (cc @mikemorris).

[ryanzhang-oss]

/lgtm

[skitt]

This doesn’t affect this PR, but we should add a check on PRs to make sure that any change does result in a version bump.

[MrFreezeex]

This doesn’t affect this PR, but we should add a check on PRs to make sure that any change does result in a version bump.

@skitt I added a small script that integrate with the verify-all script executed by prow, it should enforce this I believe 🎉

[skitt]

/approve

Thanks!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrFreezeex, skitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [skitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[skitt]

/sig multicluster

[mikemorris]

This also means that the CRD version won’t be the same as the project tag, but that’s fine.

@skitt FWIW I would caution that a mismatch here (in addition to the understanding how this relates to the v1alpha|beta Kubernetes versioning) will quickly get confusing. If you want to add some kind of automation, I would suggest something like setting a NEEDS_PATCH_BUMP=1 or NEEDS_MINOR_BUMP=1 in some file in a release/ directory, then checking and resetting those as part of a release process when creating a new git version tag.

I don't think we really need a corresponding release for every individual change (batching a few small updates together should be fine) as long as we're diligent enough to avoid letting merged PRs sit in an unreleased state for too long, but I do see the desire for minimizing the risk that somebody creating a new release might miss important changes if the release happens much later than the change.

There are more structured approaches to this that some projects have adopted like creating changelog entry files to accompany each PR that include if it's a major|minor|patch change which can be compiled and inspected by release machinery, but I don't think this particular project is moving at a pace that necessitates setting up a system like that.

[MrFreezeex]

@mikemorris TBH I don't think it would that confusing, in practice this is more supposed to be used by imlementors to auto install (and prevent downgrades) than users. And implementors can just import whatever hash they want and the CRD installer from their project could just do its thing as the version is bumped on every CRD change.

While instead if we do only bump version while we release project auto installing the CRDs, implementors won't be able to use version not part of a release anymore as they most likely require the tag to be bumped to install the new CRD. So it might even create more confusion to bump this only on a release.

For instance on Cilium, we always bump the patch of the CRDs on any change to one of our CRD so that we are able to always upgrade the CRD correctly (even from a dev version to another) and on every new minor version (the release we have every 6 months) we always bump the minor version. I am not sure we need to do the same here as the mcs-api repo definition is a significantly simpler project though.

Also BTW this is not something that we won't be able to change anymore, if we decide to in the future we could still resync the project version and CRD version at the expense of skipping a few versions for one side.

[mikemorris]

If you want an auto-incrementing number for every change, I'd suggest avoiding any potential confusion (and complexity) with semver, and using something simpler like revision: with an unsigned integer type.

[lauralorenz]

Triage notes:

• Recent conversation about release tag and its machinery vs the CRD version string bump and how they should be synced or not
• 10/14 SIG-MC meeting mentioned that that should not block this since it seems that we can choose to resync them later per last comment in crd: add label with CRD version #116 (comment)
• Missing lgtm at this point

[lauralorenz]

MORE triage notes!:

• Is being able to "branch" past "releases" of the CRD version a requirement? This makes the idea of having a single number incrementing (like a generation number) encoding the CRD version not sufficient
  • There is an idea that "revision can be forever-incrementing, or can reset to 0 on every semver bump", "Like version: 1.1.0, revision: 3, version: 1.2.0, revision: 0", more like build number. We like this!
• Is being able to track the relationship between the CRD version / generation number and the release tag semver-style version important? There is some yes (being able to tell on sight / automation seems nice), there is some no (too slow of a moving target)

[MrFreezeex]

And this is now updated to distinguish between a release version and the revision (incremental update made to a CRD "within" a version). 🎉

[MrFreezeex]

@mikemorris if this new version looks reasonable to you would you be able to add your / lgtm here 🙏?

[mikemorris]

Seems like a good approach to me now after discussion in last SIG meeting!

/lgtm