Skip to content

k8s: clarify CRD schema versioning and its update process#12493

Closed
aanm wants to merge 1 commit intomasterfrom
pr/docs-schema-version
Closed

k8s: clarify CRD schema versioning and its update process#12493
aanm wants to merge 1 commit intomasterfrom
pr/docs-schema-version

Conversation

@aanm
Copy link
Copy Markdown
Member

@aanm aanm commented Jul 10, 2020

Add steps how and when the CRD validation version should be updated.

Signed-off-by: André Martins andre@cilium.io

@aanm aanm added needs-backport/1.6 release-note/misc This PR makes changes that have no direct user impact. labels Jul 10, 2020
@aanm aanm requested a review from joestringer July 10, 2020 13:10
@aanm aanm requested review from a team as code owners July 10, 2020 13:10
Comment thread Documentation/concepts/kubernetes/compatibility.rst Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 10, 2020

Coverage Status

Coverage increased (+0.02%) to 36.98% when pulling e035d4c on pr/docs-schema-version into 7e6a7bb on master.

Add steps on and when the CRD validation version should be updated.

Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm requested a review from nebril July 10, 2020 14:45
@aanm aanm force-pushed the pr/docs-schema-version branch from 8f2c7ec to e035d4c Compare July 10, 2020 14:45
Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying these steps.

I'd like to minimize the extra steps during the release process if possible, couple of comments below towards that aim.

Comment on lines +42 to +46
| 1.6.0-rc1 | 1.14 |
+-----------------+----------------+
| 1.6.0-rc2 | 1.14 |
+-----------------+----------------+
| 1.6.0-rc3 | 1.14 |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if we can come up with a workflow that doesn't require us to update this doc as well whenever we create a new release. Could we group these by version instead, perhaps with a link to the relevant variable in the code to pkg.go.dev in case we forget to update?

But we shouldn't forget to update if we put clear instructions above the schema version definition to outline the expected steps, including "if you change the schema version, update this ____ docs page".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From out-of-band discussion and the item below, it makes sense to increment the version for each release to provide a number space for potential bugfix bumps to the schema, like the one that provoked this PR.

We discussed that this table can be auto-generated to a separate file then imported directly into this page. This will resolve my concern around adding manual steps to the release process by automating it.

Comment thread pkg/k8s/apis/cilium.io/v2/register.go
joestringer added a commit that referenced this pull request Jul 10, 2020
As per the discussions from #12493,
we should not increment the v1.6 schema version to 1.16 as this would
conflict with early v1.7 cycle versions. Fix it to 1.15.1 instead.

The 1.16 version was never part of a released v1.6.x version of Cilium.

Fixes: bd5bc5b ("Fix small CRD issue with toGroups")
Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer
Copy link
Copy Markdown
Member

@aanm by the way, this will definitely not backport cleanly to 1.6 as not all of those release maintenance docs exist on the older branches. Also, I don't think we'll want to backport the schema changes will we?

@christarazi christarazi self-requested a review July 10, 2020 19:58
Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should address #12493 (comment) first.

qmonnet pushed a commit that referenced this pull request Jul 14, 2020
As per the discussions from #12493,
we should not increment the v1.6 schema version to 1.16 as this would
conflict with early v1.7 cycle versions. Fix it to 1.15.1 instead.

The 1.16 version was never part of a released v1.6.x version of Cilium.

Fixes: bd5bc5b ("Fix small CRD issue with toGroups")
Signed-off-by: Joe Stringer <joe@cilium.io>
aanm added a commit that referenced this pull request Jul 23, 2020
As per the discussions from #12493,
we should not increment the v1.8 schema version to 1.22 as this would
conflict with early currently v1.9 cycle. Fix it to 1.21.1 instead.

Fixes: 336791f ("Fix small CRD issue with toGroups")
Signed-off-by: André Martins <andre@cilium.io>
@aanm
Copy link
Copy Markdown
Member Author

aanm commented Jul 23, 2020

1.7 backported in #12622

christarazi pushed a commit that referenced this pull request Jul 23, 2020
As per the discussions from #12493,
we should not increment the v1.8 schema version to 1.22 as this would
conflict with early currently v1.9 cycle. Fix it to 1.21.1 instead.

Fixes: 336791f ("Fix small CRD issue with toGroups")
Signed-off-by: André Martins <andre@cilium.io>
@nathanjsweet
Copy link
Copy Markdown
Member

retest-net-next

@nathanjsweet
Copy link
Copy Markdown
Member

retest-4.9

@nathanjsweet
Copy link
Copy Markdown
Member

retest-4.19

@aanm aanm marked this pull request as draft July 28, 2020 16:45
@stale
Copy link
Copy Markdown

stale Bot commented Aug 28, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Aug 28, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Sep 11, 2020

This issue has not seen any activity since it was marked stale. Closing.

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

Labels

release-note/misc This PR makes changes that have no direct user impact. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants