volumesnapshots CRD has invalid property: anyOf

[ialidzhikov]

What happened:
volumesnapshots CRD has invalid property: anyOf

How to reproduce it (as minimally and precisely as possible):

1. Apply the volumesnapshots.snapshot.storage.k8s.io CRD.

$ k apply -f https://raw.githubusercontent.com/kubernetes-csi/external-snapshotter/master/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml

2. Ensure the following log entry in the kube-apiserver logs:

$ k -n kube-system logs kube-apiserver-m01

E0408 10:21:28.449071       1 customresource_handler.go:655] error building openapi models for volumesnapshots.snapshot.storage.k8s.io: ERROR $root.definitions.io.k8s.storage.snapshot.v1beta1.VolumeSnapshot.properties.status.properties.restoreSize has invalid property: anyOf

Environment:

• Kubernetes version (use kubectl version): v1.16.8

[xing-yang]

This is automatically generated by controller-gen. Maybe because it does not know how to interpret resource.Quantity.

            restoreSize:
              anyOf:
              - type: integer
              - type: string

@yuxiangqian should we manually remove a type after it is generated automatically?
https://github.com/kubernetes-csi/external-snapshotter/blob/master/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml#L160

[humblec]

Yeah.. looks like that. we could add resource.Quantity . Let me give a try.

[humblec]

Yeah.. looks like that. we could add resource.Quantity . Let me give a try.

@xing-yang @yuxiangqian this kubernetes-sigs/controller-tools#328 has some details on what we are facing. for now, adjusting the CRD looks to be the way. I can send a PR for the same if we are in agreement.

[yuxiangqian]

c3beea5#diff-f816aba0495ad43b0e3b8b097e94a8d2
this is the PR which introduced the breakage. we should NOT use anyof according to (details)

[xing-yang]

The existing code was generated automatically by controller-gen. @yuxiangqian did you have to manually set this to "type: string" in 2.0? Otherwise, this is a change in behavior in controller-gen.

This is what we have in release-2.0:
https://github.com/kubernetes-csi/external-snapshotter/blob/release-2.0/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml#L133

Should we just manually modify this and set to "type: string" instead?

[yuxiangqian]

@humblec did you follow the instructions here?
I saw this field has been removed from that PR: preserveUnknownFields: false
which likely will cause problem.

[humblec]

/assign humblec

[humblec]

@humblec did you follow the instructions here?
I saw this field has been removed from that PR: preserveUnknownFields: false
which likely will cause problem.

@yuxiangqian Yes, I did and once I have a commit to update the controller gen to v0.3.0, if I run update-crd.sh, below is the only change:

[root@localhost external-snapshotter]# git diff
diff --git a/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml b/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml
index 4aa980cc..984ce42b 100644
--- a/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml
+++ b/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml
@@ -5,7 +5,6 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     controller-gen.kubebuilder.io/version: v0.2.5
-    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/260"
   creationTimestamp: null
   name: volumesnapshotclasses.snapshot.storage.k8s.io
 spec:
@@ -62,6 +61,9 @@ spec:
             object represents. Servers may infer this from the endpoint the client
             submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
           type: string
+        metadata:
+          description: 'Standard object''s metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata'
+          type: object
         parameters:
           additionalProperties:
             type: string
diff --git a/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml b/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml
index 34c51ad6..a8c56e24 100644
--- a/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml
+++ b/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml
@@ -5,7 +5,6 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     controller-gen.kubebuilder.io/version: v0.2.5
-    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/260"
   creationTimestamp: null
   name: volumesnapshotcontents.snapshot.storage.k8s.io
 spec:
@@ -66,6 +65,9 @@ spec:
             object represents. Servers may infer this from the endpoint the client
             submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
           type: string
+        metadata:
+          description: 'Standard object''s metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata'
+          type: object
         spec:
           description: spec defines properties of a VolumeSnapshotContent created
             by the underlying storage system. Required.
diff --git a/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml b/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml
index 483706f1..74d31ae9 100644
--- a/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml
+++ b/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml
@@ -5,7 +5,6 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     controller-gen.kubebuilder.io/version: v0.2.5
-    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/260"
   creationTimestamp: null
   name: volumesnapshots.snapshot.storage.k8s.io
 spec:
@@ -70,6 +69,9 @@ spec:
             object represents. Servers may infer this from the endpoint the client
             submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
           type: string
+        metadata:
+          description: 'Standard object''s metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata'
+          type: object
         spec:
           description: 'spec defines the desired characteristics of a snapshot requested

The document says about, removing the newly added metadata section, so we can ignore that part. Second thing what the current document says is replacing the api-approved.kubernetes.io which actually got removed once we ran the script. but I can bring back . I dont see the preserveUnknownFields: false in my git diff at all. Are you sure thats happening with update-crd.sh ?

[ialidzhikov]

Ping, is there any update?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[xing-yang]

This is fixed.
https://github.com/kubernetes-csi/external-snapshotter/blob/v3.0.1/client/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml#L159

[vishutrilio]

Hi @xing-yang , can you tell how did you fix it? I just wanted to understand how resource.Quantity resulted in string in CRD. Thanks

[xing-yang]

We made a manual change to remove "integer" and keep "string". It is documented in the README:
https://github.com/kubernetes-csi/external-snapshotter/tree/master/client/hack#update-crdsh

[vishutrilio]

Thank you @xing-yang