add status subresources for volumeSnapshot #121

zhucan · 2019-05-07T06:15:36Z

/kind bug

Does this PR introduce a user-facing change?:

Add Status subresource for VolumeSnapshot.

What this PR does / why we need it:
add status subresources for volumeSnapshot so when update status of volumeSnapshot , it will ignore the spec filed of volumeSnapshot

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

k8s-ci-robot · 2019-05-07T06:15:49Z

Hi @zhucan. Thanks for your PR.

I'm waiting for a kubernetes-csi or kubernetes 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.

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 · 2019-05-08T02:38:19Z

/ok-to-test

xing-yang · 2019-05-08T03:12:10Z

cmd/csi-snapshotter/create_crd.go

+	_, err := clientset.ApiextensionsV1beta1().CustomResourceDefinitions().Get(crd.Name, metav1.GetOptions{})
+	if err == nil {
+		if res, err := clientset.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd); err != nil {
+			klog.Fatalf("failed to update VolumeSnapshotResource: %#v, err: %#v",


Start external-snapshotter for the first time. It starts without errors. Stop it and start it again. It failed here with the following error:

F0508 02:46:44.816579 1 create_crd.go:50] failed to update VolumeSnapshotResource: &v1beta1.CustomResourceDefinition{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:v1.ObjectMeta{Name:"", GenerateName:"", Namespace:"", SelfLink:"", UID:"", ResourceVersion:"", Generation:0, CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}, DeletionTimestamp:(*v1.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Initializers:(*v1.Initializers)(nil), Finalizers:[]string(nil), ClusterName:"", ManagedFields:[]v1.ManagedFieldsEntry(nil)}, Spec:v1beta1.CustomResourceDefinitionSpec{Group:"", Version:"", Names:v1beta1.CustomResourceDefinitionNames{Plural:"", Singular:"", ShortNames:[]string(nil), Kind:"", ListKind:"", Categories:[]string(nil)}, Scope:"", Validation:(*v1beta1.CustomResourceValidation)(nil), Subresources:(*v1beta1.CustomResourceSubresources)(nil), Versions:[]v1beta1.CustomResourceDefinitionVersion(nil), AdditionalPrinterColumns:[]v1beta1.CustomResourceColumnDefinition(nil), Conversion:(*v1beta1.CustomResourceConversion)(nil)}, Status:v1beta1.CustomResourceDefinitionStatus{Conditions:[]v1beta1.CustomResourceDefinitionCondition(nil), AcceptedNames:v1beta1.CustomResourceDefinitionNames{Plural:"", Singular:"", ShortNames:[]string(nil), Kind:"", ListKind:"", Categories:[]string(nil)}, StoredVersions:[]string(nil)}}, err: &errors.StatusError{ErrStatus:v1.Status{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ListMeta:v1.ListMeta{SelfLink:"", ResourceVersion:"", Continue:""}, Status:"Failure", Message:"customresourcedefinitions.apiextensions.k8s.io "volumesnapshotclasses.snapshot.storage.k8s.io" is invalid: metadata.resourceVersion: Invalid value: 0x0: must be specified for an update", Reason:"Invalid", Details:(*v1.StatusDetails)(0xc0000a8360), Code:422}}

I will check it again

xing-yang · 2019-05-08T03:13:06Z

This PR does not work. I tried to create a snapshot but it failed with the following error:

E0508 03:10:03.696019 1 goroutinemap.go:150] Operation for "create-default/new-snapshot-demo[c616fd63-713e-11e9-86c5-000c29e70439]" failed. No retries permitted until 2019-05-08 03:10:04.19600577 +0000 UTC m=+151.703720599 (durationBeforeRetry 500ms). Error: "snapshot controller failed to update default/new-snapshot-demo on API server: the server could not find the requested resource (put volumesnapshots.snapshot.storage.k8s.io new-snapshot-demo)"

xing-yang · 2019-05-08T20:57:56Z

pkg/controller/snapshot_controller.go

@@ -831,7 +831,7 @@ func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSn
 			status.RestoreSize = resource.NewQuantity(size, resource.BinarySI)
 		}
 		snapshotClone.Status = status
-		newSnapshotObj, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshots(snapshotClone.Namespace).UpdateStatus(snapshotClone)
+		newSnapshotObj, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshots(snapshotClone.Namespace).Update(snapshotClone)


UpdateStatus was called at two places in the original PR.

when using "UpdateStatus" , the error: snapshot_controller.go:649] failed to update snapshot snapshotting-8419/snapshot-dnvgd creation timestamp: snapshot controller failed to update snapshotting-8419/snapshot-dnvgd on API server: the server could not find the requested resource (put volumesnapshots.snapshot.storage.k8s.io snapshot-dnvgd)

This should not be changed to Update(). It should use UpdateStatus(). I think you need to add the following in volumesnapshot.yaml file for subresource status update to work. See this PR: https://github.com/kubernetes/kubernetes/pull/60021/files

subresources:
status: {}

ok, I will try it.

xing-yang · 2019-05-09T20:46:08Z

Please add the release note info to the problem description on top. This is required.

Does this PR introduce a user-facing change?:

NONE

xing-yang · 2019-05-29T02:23:59Z

Release note needs to be in the following format:

Does this PR introduce a user-facing change?:
```release-note
NONE
```

It will look like this:

NONE

xing-yang · 2019-06-12T18:02:01Z

Tested creating a snapshot using example yaml files in the repo, but got the following error in the snapshot controller:

E0612 17:58:00.571119       1 snapshot_controller.go:370] createSnapshot [create-default/new-snapshot-demo[9e08154a-8d3b-11e9-b0aa-000c29e70439]]: error occurred in createSnapshotOperation: snapshot controller failed to update default/new-snapshot-demo on API server: the server could not find the requested resource (put volumesnapshots.snapshot.storage.k8s.io new-snapshot-demo)
E0612 17:58:00.571176       1 goroutinemap.go:150] Operation for "create-default/new-snapshot-demo[9e08154a-8d3b-11e9-b0aa-000c29e70439]" failed. No retries permitted until 2019-06-12 17:58:01.071150315 +0000 UTC m=+59.357142407 (durationBeforeRetry 500ms). Error: "snapshot controller failed to update default/new-snapshot-demo on API server: the server could not find the requested resource (put volumesnapshots.snapshot.storage.k8s.io new-snapshot-demo)"
I0612 17:58:02.876452       1 snapshot_controller.go:615] createSnapshot: Creating snapshot default/new-snapshot-demo through the plugin ...
E0612 17:58:02.893121       1 snapshot_controller.go:370] createSnapshot [create-default/new-snapshot-demo[9e08154a-8d3b-11e9-b0aa-000c29e70439]]: error occurred in createSnapshotOperation: snapshot controller failed to update default/new-snapshot-demo on API server: the server could not find the requested resource (put volumesnapshots.snapshot.storage.k8s.io new-snapshot-demo)
E0612 17:58:02.893212       1 goroutinemap.go:150] Operation for "create-default/new-snapshot-demo[9e08154a-8d3b-11e9-b0aa-000c29e70439]" failed. No retries permitted until 2019-06-12 17:58:03.893184781 +0000 UTC m=+62.179176863 (durationBeforeRetry 1s). Error: "snapshot controller failed to update default/new-snapshot-demo on API server: the server could not find the requested resource (put volumesnapshots.snapshot.storage.k8s.io new-snapshot-demo)"

xing-yang · 2019-06-12T18:26:05Z

Never mind. Tested again on a clean setup and it worked for me.

xing-yang · 2019-06-12T18:26:21Z

/lgtm
/approve

k8s-ci-robot · 2019-06-12T18:26:33Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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~~ [xing-yang]

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

fe1f284 Merge pull request kubernetes-csi#121 from kvaps/namespace-check 8fdf0f7 Merge pull request kubernetes-csi#128 from fengzixu/master 1c94220 fix: fix a bug of csi-sanity a4c41e6 Merge pull request kubernetes-csi#127 from pohly/fix-boilerplate ece0f50 check namespace for snapshot-controller dbd8967 verify-boilerplate.sh: fix path to script 9289fd1 Merge pull request kubernetes-csi#125 from sachinkumarsingh092/optional-spelling-boilerplate-checks ad29307 Make the spelling and boilerplate checks optional 5f06d02 Merge pull request kubernetes-csi#124 from sachinkumarsingh092/fix-spellcheck-boilerplate-tests 48186eb Fix spelling and boilerplate errors 71690af Merge pull request kubernetes-csi#122 from sachinkumarsingh092/include-spellcheck-boilerplate-tests 981be3f Adding spelling and boilerplate checks. 2bb7525 Merge pull request kubernetes-csi#117 from fengzixu/master 4ab8b15 use the tag to replace commit of csi-test 5d74e45 change the csi-test import path to v4 7dcd0a9 upgrade csi-test to v4.0.2 git-subtree-dir: release-tools git-subtree-split: fe1f284

1d60e77 Merge pull request kubernetes-csi#131 from pohly/kubernetes-1.20-tag 9f10459 prow.sh: support building Kubernetes for a specific version fe1f284 Merge pull request kubernetes-csi#121 from kvaps/namespace-check 8fdf0f7 Merge pull request kubernetes-csi#128 from fengzixu/master 1c94220 fix: fix a bug of csi-sanity a4c41e6 Merge pull request kubernetes-csi#127 from pohly/fix-boilerplate ece0f50 check namespace for snapshot-controller dbd8967 verify-boilerplate.sh: fix path to script 9289fd1 Merge pull request kubernetes-csi#125 from sachinkumarsingh092/optional-spelling-boilerplate-checks ad29307 Make the spelling and boilerplate checks optional 5f06d02 Merge pull request kubernetes-csi#124 from sachinkumarsingh092/fix-spellcheck-boilerplate-tests 48186eb Fix spelling and boilerplate errors 71690af Merge pull request kubernetes-csi#122 from sachinkumarsingh092/include-spellcheck-boilerplate-tests 981be3f Adding spelling and boilerplate checks. 2bb7525 Merge pull request kubernetes-csi#117 from fengzixu/master 4ab8b15 use the tag to replace commit of csi-test 5d74e45 change the csi-test import path to v4 7dcd0a9 upgrade csi-test to v4.0.2 git-subtree-dir: release-tools git-subtree-split: 1d60e77

bc0504a Merge pull request kubernetes-csi#140 from jsafrane/remove-unused-k8s-libs 5b1de1a go-get-kubernetes.sh: remove unused k8s libs 49b4269 Merge pull request kubernetes-csi#120 from pohly/add-kubernetes-release a1e1127 Merge pull request kubernetes-csi#139 from pohly/kind-for-kubernetes-latest 1c0fb09 prow.sh: use KinD main for latest Kubernetes 1d77cfc Merge pull request kubernetes-csi#138 from pohly/kind-update-0.10 bff2fb7 prow.sh: KinD 0.10.0 95eac33 Merge pull request kubernetes-csi#137 from pohly/fix-go-version-check 437e431 verify-go-version.sh: fix check after removal of travis.yml 1748b16 Merge pull request kubernetes-csi#136 from pohly/go-1.16 ec844ea remove travis.yml, Go 1.16 df76aba Merge pull request kubernetes-csi#134 from andyzhangx/add-build-arg e314a56 add build-arg ARCH for building multi-arch images, e.g. ARG ARCH FROM k8s.gcr.io/build-image/debian-base-${ARCH}:v2.1.3 7bc70e5 Merge pull request kubernetes-csi#129 from pohly/squash-documentation e0b02e7 README.md: document usage of --squash 316cb95 Merge pull request kubernetes-csi#132 from yiyang5055/bugfix/boilerplate 26e2ab1 fix: default boilerplate path 1add8c1 Merge pull request kubernetes-csi#133 from pohly/kubernetes-1.20-tag 3e811d6 prow.sh: fix "on-master" prow jobs 1d60e77 Merge pull request kubernetes-csi#131 from pohly/kubernetes-1.20-tag 9f10459 prow.sh: support building Kubernetes for a specific version f7e7ee4 docs: steps for adding testing against new Kubernetes release fe1f284 Merge pull request kubernetes-csi#121 from kvaps/namespace-check 8fdf0f7 Merge pull request kubernetes-csi#128 from fengzixu/master 1c94220 fix: fix a bug of csi-sanity a4c41e6 Merge pull request kubernetes-csi#127 from pohly/fix-boilerplate ece0f50 check namespace for snapshot-controller dbd8967 verify-boilerplate.sh: fix path to script 9289fd1 Merge pull request kubernetes-csi#125 from sachinkumarsingh092/optional-spelling-boilerplate-checks ad29307 Make the spelling and boilerplate checks optional 5f06d02 Merge pull request kubernetes-csi#124 from sachinkumarsingh092/fix-spellcheck-boilerplate-tests 48186eb Fix spelling and boilerplate errors 71690af Merge pull request kubernetes-csi#122 from sachinkumarsingh092/include-spellcheck-boilerplate-tests 981be3f Adding spelling and boilerplate checks. 2bb7525 Merge pull request kubernetes-csi#117 from fengzixu/master 4ab8b15 use the tag to replace commit of csi-test 5d74e45 change the csi-test import path to v4 7dcd0a9 upgrade csi-test to v4.0.2 git-subtree-dir: release-tools git-subtree-split: bc0504a

Check namespace for snapshot-controller

…ncy-openshift-4.15-ose-csi-snapshot-controller OCPBUGS-24329: Updating ose-csi-snapshot-controller-container image to be consistent with ART

wackxu and others added 3 commits November 21, 2018 14:30

add status Subresources for volumeSnapshot

ad5dd2b

update cluster role for snapshot status

9485506

add status subresources for volumeSnapshot & solute conflict

51a8bd8

k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 7, 2019

k8s-ci-robot requested review from jingxu97 and lpabon May 7, 2019 06:15

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 7, 2019

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 May 8, 2019

xing-yang reviewed May 8, 2019

View reviewed changes

restart external-snapshotter failed

dd61cbe

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 8, 2019

zhucan force-pushed the status-52 branch from f0d5f3e to dd61cbe Compare May 8, 2019 06:50

xing-yang added this to To do in Volume Snapshot May 8, 2019

xing-yang moved this from To do to In progress in Volume Snapshot May 8, 2019

xing-yang reviewed May 8, 2019

View reviewed changes

xing-yang mentioned this pull request May 9, 2019

add status subresources for volumeSnapshot #52

Closed

xing-yang changed the title ~~add status subresources for volumeSnapshot & solute conflict~~ add status subresources for volumeSnapshot May 10, 2019

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 12, 2019

update snapshot yaml

95fc4bc

zhucan force-pushed the status-52 branch from 3423399 to 95fc4bc Compare May 14, 2019 10:19

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2019

Merge branch 'master' into status-52

10d6437

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2019

zhucan added 2 commits May 14, 2019 18:41

run 'make -k all test' failed

4f82209

use 'UpdateStatus' failed

2db3a1e

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 29, 2019

zhucan mentioned this pull request May 31, 2019

New membership for zhucan kubernetes/org#870

Closed

6 tasks

clean useless key 'subresources' in snapshot yaml

a23be2d

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jun 12, 2019

k8s-ci-robot assigned xing-yang Jun 12, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 12, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 12, 2019

k8s-ci-robot merged commit d3243e0 into kubernetes-csi:master Jun 12, 2019

msau42 moved this from In progress to Done in Volume Snapshot Jun 14, 2019

k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 19, 2019

pohly mentioned this pull request Jan 26, 2021

master: update release-tools #464

Merged

ggriffiths mentioned this pull request Apr 16, 2021

Add latest release-tools to release-4.0 branch #498

Merged

xing-yang pushed a commit to xing-yang/external-snapshotter that referenced this pull request Jul 26, 2021

Merge pull request kubernetes-csi#121 from kvaps/namespace-check

fe1f284

Check namespace for snapshot-controller

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add status subresources for volumeSnapshot #121

add status subresources for volumeSnapshot #121

zhucan commented May 7, 2019 •

edited by xing-yang

Loading

k8s-ci-robot commented May 7, 2019

xing-yang commented May 8, 2019

xing-yang May 8, 2019

zhucan May 8, 2019

xing-yang commented May 8, 2019

xing-yang May 8, 2019

zhucan May 9, 2019

xing-yang May 9, 2019

zhucan May 12, 2019

xing-yang commented May 9, 2019 •

edited

Loading

xing-yang commented May 29, 2019 •

edited

Loading

xing-yang commented Jun 12, 2019

xing-yang commented Jun 12, 2019

xing-yang commented Jun 12, 2019

k8s-ci-robot commented Jun 12, 2019

add status subresources for volumeSnapshot #121

add status subresources for volumeSnapshot #121

Conversation

zhucan commented May 7, 2019 • edited by xing-yang Loading

k8s-ci-robot commented May 7, 2019

xing-yang commented May 8, 2019

xing-yang May 8, 2019

Choose a reason for hiding this comment

zhucan May 8, 2019

Choose a reason for hiding this comment

xing-yang commented May 8, 2019

xing-yang May 8, 2019

Choose a reason for hiding this comment

zhucan May 9, 2019

Choose a reason for hiding this comment

xing-yang May 9, 2019

Choose a reason for hiding this comment

zhucan May 12, 2019

Choose a reason for hiding this comment

xing-yang commented May 9, 2019 • edited Loading

xing-yang commented May 29, 2019 • edited Loading

xing-yang commented Jun 12, 2019

xing-yang commented Jun 12, 2019

xing-yang commented Jun 12, 2019

k8s-ci-robot commented Jun 12, 2019

zhucan commented May 7, 2019 •

edited by xing-yang

Loading

xing-yang commented May 9, 2019 •

edited

Loading

xing-yang commented May 29, 2019 •

edited

Loading