Update volume snapshot CRDs to v1 version by prateekpandey14 · Pull Request #367 · kubernetes-csi/external-snapshotter

prateekpandey14 · 2020-08-21T13:47:42Z

Signed-off-by: prateekpandey14 prateekpandey14@gmail.com

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
Update the volume snapshot CRDs to apiextensions.k8s.io/v1.

FYI as per kubernetes/kubernetes#82022 (comment) removal of apiextensions.k8s.io/v1beta1 has been targeted for 1.22

Which issue(s) this PR fixes:
Fixes #364

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Update volume snapshot CRDs to apiextensions.k8s.io/v1 version

k8s-ci-robot · 2020-08-21T13:47:50Z

Welcome @prateekpandey14!

It looks like this is your first PR to kubernetes-csi/external-snapshotter 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-csi/external-snapshotter has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-08-21T13:47:50Z

Hi @prateekpandey14. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

xing-yang · 2020-08-21T13:51:29Z

/ok-to-test

xing-yang · 2020-08-21T13:53:33Z

Thanks @prateekpandey14 for the PR!
Please go through https://github.com/kubernetes-csi/external-snapshotter/blob/master/client/hack/README.md#update-crdsh and make sure that you have followed some manual steps documented there.

prateekpandey14 · 2020-08-21T17:44:37Z

@xing-yang pushed the changes, should i remove the step-4 about the metadata.description from the README as well (as suggested)

xing-yang · 2020-08-21T18:17:52Z

Yes, please remove the metadata section as described in README.

prateekpandey14 · 2020-08-24T08:10:39Z

Yes, please remove the metadata section as described in README.

Done @xing-yang

xing-yang · 2020-08-28T03:03:59Z

/assign @yuxiangqian

xing-yang · 2020-08-28T03:04:24Z

Hi @yuxiangqian, can you take a look of this?

yuxiangqian · 2020-08-28T04:31:04Z

@prateekpandey14 @xing-yang I'd like to hold-on this PR before #353 is merged. Any concerns?

Kartik494 · 2020-08-28T05:02:30Z

client/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml

+              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:


Hi @prateekpandey14 when i have tested this in my enviorment , then these crd not generated succesfully could you please remove this metadata section from all yaml files that are generated

Yes, that's right.

ahh, i forget to commit the changes, i have them locally, i will do that.

could you please commit the changes? and do we still need to manually remove this metadata after switching to use: crd:crdVersions=v1 in the controller-gen?

@yuxiangqian, I just read comments left by you in the README where you said we should no longer need to remove this metadata when we move to v1? However @Kartik494 seems to run into problems if these metadata sections are left there.

I was wondering whether that issue is a problem related to the controller-gen or v1beta1 apiextension. its better if we could remove as much manual editing to the generated yaml file as possible.

Agree that we should do as little manual editing as possible. Just need to verify that these manifests are working.

client/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml

client/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml

Kartik494 · 2020-08-28T06:26:01Z

Yes, please remove the metadata section as described in README.

Hi @xing-yang if i am not wrong here the idea is to remove metadata section from the yaml not from the README! Please suggest

xing-yang · 2020-08-28T12:27:23Z

Yes, please remove the metadata section as described in README.

Hi @xing-yang if i am not wrong here the idea is to remove metadata section from the yaml not from the README! Please suggest

Yes, don't remove from README! That's a documented step that needs to be done manually.
Remove the metadata section documented in README from all the generated YAMLs.

xing-yang · 2020-08-28T12:28:01Z

@prateekpandey14 @xing-yang I'd like to hold-on this PR before #353 is merged. Any concerns?

No problem.

xing-yang · 2020-08-28T12:28:10Z

/hold

prateekpandey14 · 2020-08-28T12:33:33Z

@prateekpandey14 @xing-yang I'd like to hold-on this PR before #353 is merged. Any concerns?

Sure @yuxiangqian make sense, it will be good to wait for validation webhook to get merge and then jump on this PR

xing-yang · 2020-08-29T12:29:29Z

#353 is merged.

/hold cancel

xing-yang · 2020-08-30T22:19:01Z

Hi @prateekpandey14, the webhook PR is merged. Can you please rebase and address the comments? Thanks.

yuxiangqian

@prateekpandey14 could you please push your latest version? WIP

client/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml

yuxiangqian · 2020-09-05T23:33:52Z

client/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml

+              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:


could you please commit the changes? and do we still need to manually remove this metadata after switching to use: crd:crdVersions=v1 in the controller-gen?

yuxiangqian · 2020-09-05T23:36:27Z

client/hack/update-crd.sh

 fi

-$CONTROLLER_GEN crd:trivialVersions=true,preserveUnknownFields=false paths=${SCRIPT_ROOT}/apis/volumesnapshot/v1beta1
+$CONTROLLER_GEN crd:crdVersions=v1,trivialVersions=true,preserveUnknownFields=false paths=${SCRIPT_ROOT}/apis/volumesnapshot/v1beta1


preserveUnknownFields=false is now redundant?

@yuxiangqian can you explain why it is redundant now? What does preserveUnknownFields=false mean?

preserveUnknownFields=false is there in v1beta1 apiextensions to prevent adding a CR with unknown/undefined fields persisted. However, that's no longer needed in v1 apiextension according to this.

removed the preserveUnknownFields=false flag

- bump controller-gen 0.4.0 version - update CRDs to use `apiextensions.k8s.io/v1` apis Signed-off-by: prateekpandey14 <prateekpandey14@gmail.com>

xing-yang · 2020-09-16T05:39:31Z

/lgtm
/approve

k8s-ci-robot · 2020-09-16T05:39:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: prateekpandey14, xing-yang

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [xing-yang]

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

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 21, 2020

k8s-ci-robot requested review from lpabon and yuxiangqian August 21, 2020 13:47

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 21, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 21, 2020

prateekpandey14 force-pushed the use-v1-crd branch from 8982b4d to 0cd30c2 Compare August 21, 2020 17:40

cyrilcros mentioned this pull request Aug 22, 2020

Snapshots are stucks as ‘waiting to be created by csi driver’ rook/rook#6139

Closed

prateekpandey14 force-pushed the use-v1-crd branch from 0cd30c2 to 88fd640 Compare August 24, 2020 08:09

k8s-ci-robot assigned yuxiangqian Aug 28, 2020

Kartik494 reviewed Aug 28, 2020

View reviewed changes

client/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml Show resolved Hide resolved

Kartik494 reviewed Aug 28, 2020

View reviewed changes

client/config/crd/snapshot.storage.k8s.io_volumesnapshots.yaml Outdated Show resolved Hide resolved

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 28, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 29, 2020

yuxiangqian reviewed Sep 5, 2020

View reviewed changes

prateekpandey14 force-pushed the use-v1-crd branch from 88fd640 to 44be5d1 Compare September 15, 2020 07:54

Update volume snapshot CRDs to v1 version

c1e1a1b

- bump controller-gen 0.4.0 version - update CRDs to use `apiextensions.k8s.io/v1` apis Signed-off-by: prateekpandey14 <prateekpandey14@gmail.com>

prateekpandey14 force-pushed the use-v1-crd branch from 44be5d1 to c1e1a1b Compare September 15, 2020 08:35

k8s-ci-robot assigned xing-yang Sep 16, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2020

k8s-ci-robot merged commit e562c2d into kubernetes-csi:master Sep 16, 2020

prateekpandey14 deleted the use-v1-crd branch September 16, 2020 05:41

Kartik494 mentioned this pull request Oct 15, 2020

Update Crd Link kubernetes-csi/docs#373

Merged

Kartik494 mentioned this pull request Dec 25, 2020

REQUEST: New membership for Kartik494 kubernetes/org#2397

Closed

6 tasks

prateekpandey14 mentioned this pull request Mar 15, 2021

REQUEST: New membership for prateekpandey14 kubernetes/org#2576

Closed

6 tasks

btkinghome mentioned this pull request Jul 13, 2021

CustomResourceDefinition apiextensions.k8s.io/v1beta change to apiextensions.k8s.io/v1 unsupport GoogleCloudPlatform/flink-on-k8s-operator#462

Closed

Conversation

prateekpandey14 commented Aug 21, 2020

Uh oh!

k8s-ci-robot commented Aug 21, 2020

Uh oh!

k8s-ci-robot commented Aug 21, 2020

Uh oh!

xing-yang commented Aug 21, 2020

Uh oh!

xing-yang commented Aug 21, 2020

Uh oh!

prateekpandey14 commented Aug 21, 2020

Uh oh!

xing-yang commented Aug 21, 2020

Uh oh!

prateekpandey14 commented Aug 24, 2020

Uh oh!

xing-yang commented Aug 28, 2020

Uh oh!

xing-yang commented Aug 28, 2020

Uh oh!

yuxiangqian commented Aug 28, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Kartik494 commented Aug 28, 2020

Uh oh!

xing-yang commented Aug 28, 2020

Uh oh!

xing-yang commented Aug 28, 2020

Uh oh!

xing-yang commented Aug 28, 2020

Uh oh!

prateekpandey14 commented Aug 28, 2020

Uh oh!

xing-yang commented Aug 29, 2020

Uh oh!

xing-yang commented Aug 30, 2020

Uh oh!

yuxiangqian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xing-yang commented Sep 16, 2020

Uh oh!

k8s-ci-robot commented Sep 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!