Skip to content

Upgrade CRDs (apiextensions) from v1beta1 to v1#11477

Merged
aanm merged 13 commits intomasterfrom
pr/upgrade-crd-schema
Sep 16, 2020
Merged

Upgrade CRDs (apiextensions) from v1beta1 to v1#11477
aanm merged 13 commits intomasterfrom
pr/upgrade-crd-schema

Conversation

@aanm
Copy link
Copy Markdown
Member

@aanm aanm commented May 11, 2020

This PR is the final piece of the CRD work to fully automate CRD generation and validation.

Depends on #11476

This is required for support of K8s 1.19 1.22:

Related: #13155
Fixes: #11142
Fixes: #12516

@maintainer-s-little-helper

This comment has been minimized.

@aanm aanm mentioned this pull request Aug 14, 2020
12 tasks
@christarazi christarazi force-pushed the pr/upgrade-crd-schema branch from 8646a39 to 45b4703 Compare September 4, 2020 20:03
@maintainer-s-little-helper maintainer-s-little-helper Bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 4, 2020
@christarazi christarazi force-pushed the pr/upgrade-crd-schema branch from 45b4703 to 9f4bac9 Compare September 4, 2020 20:06
@christarazi christarazi added area/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. kind/enhancement This would improve or streamline existing functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. upgrade-impact This PR has potential upgrade or downgrade impact. and removed dont-merge/needs-release-note dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 4, 2020
@christarazi

This comment has been minimized.

@christarazi christarazi force-pushed the pr/upgrade-crd-schema branch from 9f4bac9 to cccf294 Compare September 7, 2020 04:59
@christarazi

This comment has been minimized.

1 similar comment
@christarazi

This comment has been minimized.

@christarazi christarazi force-pushed the pr/upgrade-crd-schema branch from f179c99 to 4314e82 Compare September 8, 2020 16:28
@christarazi

This comment has been minimized.

@christarazi

This comment has been minimized.

@christarazi

This comment has been minimized.

@christarazi
Copy link
Copy Markdown
Member

CI has passed! 🎉

This provides coverage for both v1 and v1beta1 variants.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit fixes occurrences of the wrong policy file referenced in the
expectations, resulting in misleading error messages in the test output.

Also, this fixes up an incorrect error message when we expect a policy
to fail to apply.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Our policies were written under an incorrect format, where we were
setting a top-level description instead of using the already existing
spec.description field. Since upgrading our CRDs to v1, this causes
validation errors when applying these YAMLs:

```
$ kubectl apply -f test/k8sT/manifests/cnp-update-allow-all.yaml
error: error validating "test/k8sT/manifests/cnp-update-allow-all.yaml":
error validating data: ValidationError(CiliumNetworkPolicy): unknown
field "description" in io.cilium.v2.CiliumNetworkPolicy; if you choose
to ignore these errors, turn validation off with --validate=false
```

This commit migrates our use of the incorrect field to the correct
location. Note that policies can still be created with a top-level
description field due to support from our fork of controller-tools, see
commit:
christarazi/controller-tools@e0828e0

Support for the top-level description field will likely be removed in
future versions of Cilium.

Related: #13155

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi
Copy link
Copy Markdown
Member

christarazi commented Sep 14, 2020

Added the following to d592e8c:

diff --git a/pkg/k8s/apis/cilium.io/v2/client/register.go b/pkg/k8s/apis/cilium.io/v2/client/register.go
index ee24493cb..b2d7adc2f 100644
--- a/pkg/k8s/apis/cilium.io/v2/client/register.go
+++ b/pkg/k8s/apis/cilium.io/v2/client/register.go
@@ -336,6 +336,12 @@ func updateV1CRD(
 				clusterCRD.ObjectMeta.Labels = crd.ObjectMeta.Labels
 				clusterCRD.Spec = crd.Spec
 
+				// Even though v1 CRDs omit this field by default (which also
+				// means it's false) it is still carried over from the previous
+				// CRD. Therefore, we must set this to false explicitly because
+				// the apiserver will carry over the old value (true).
+				clusterCRD.Spec.PreserveUnknownFields = false
+
 				_, err := client.CustomResourceDefinitions().Update(
 					context.TODO(),
 					clusterCRD,

@christarazi
Copy link
Copy Markdown
Member

test-me-please

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.

LGTM. I only looked at my code owners. I did not look at the CRD definition examples.

Copy link
Copy Markdown
Member Author

@aanm aanm left a comment

Choose a reason for hiding this comment

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

Approved

@aanm aanm merged commit abf62e6 into master Sep 16, 2020
@aanm aanm deleted the pr/upgrade-crd-schema branch September 16, 2020 09:49
christarazi added a commit to christarazi/cilium that referenced this pull request Sep 23, 2020
This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
cilium#11477 (comment)

Updates: cilium#12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
qmonnet pushed a commit that referenced this pull request Sep 24, 2020
This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
#11477 (comment)

Updates: #12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
vadorovsky pushed a commit that referenced this pull request Sep 25, 2020
[ upstream commit 5c6aad6 ]

This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
#11477 (comment)

Updates: #12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
vadorovsky pushed a commit that referenced this pull request Sep 25, 2020
[ upstream commit 5c6aad6 ]

This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
#11477 (comment)

Updates: #12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
vadorovsky pushed a commit that referenced this pull request Sep 29, 2020
[ upstream commit 5c6aad6 ]

This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
#11477 (comment)

Updates: #12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
vadorovsky pushed a commit that referenced this pull request Sep 29, 2020
[ upstream commit 5c6aad6 ]

This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
#11477 (comment)

Updates: #12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
aanm pushed a commit that referenced this pull request Sep 29, 2020
[ upstream commit 5c6aad6 ]

This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
#11477 (comment)

Updates: #12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
joestringer pushed a commit that referenced this pull request Sep 29, 2020
[ upstream commit 5c6aad6 ]

This commit removes the ability to delete CRDs from Cilium because that
would delete all the CRs in the cluster.

Follow-up from:
#11477 (comment)

Updates: #12737

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. kind/enhancement This would improve or streamline existing functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. upgrade-impact This PR has potential upgrade or downgrade impact.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reject Cilium network policies with unknown fields Add schema validation for all Cilium CRDs

3 participants