Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove volumesnapshot.spec.source.snapshotHandle field. #246

Closed
bells17 opened this issue Feb 2, 2020 · 2 comments
Closed

Remove volumesnapshot.spec.source.snapshotHandle field. #246

bells17 opened this issue Feb 2, 2020 · 2 comments

Comments

@bells17
Copy link
Contributor

bells17 commented Feb 2, 2020

It seems that volumesnapshot.spec.source.snapshotHandle is the same field that has meaning as volumesnapshot.spec.snapshotHandle.

And I noticed volumesnapshot.spec.source.snapshotHandle never had any value set.
So now the GetSnapshotStatus method never calls.

if content.Spec.Source.SnapshotHandle != nil {
klog.V(5).Infof("checkandUpdateContentStatusOperation: call GetSnapshotStatus for snapshot which is pre-bound to content [%s]", content.Name)
readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content)
if err != nil {
klog.Errorf("checkandUpdateContentStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err)
return nil, err
}
driverName = content.Spec.Driver
snapshotID = *content.Spec.Source.SnapshotHandle

So I think that should remove volumesnapshot.spec.source.snapshotHandle field.
(if volumesnapshot.spec.source.snapshotHandle and volumesnapshot.spec.snapshotHandle has another role, please teach me about it 🙏 )

@xing-yang
Copy link
Collaborator

Spec.Source.SnapshotHandle in VolumeSnapshotContent is only specified for pre-provisioned snapshots. It indicates a desired state. If you are testing dynamically creating a snapshot, it will be empty. See "Importing an existing volume snapshot with Kubernetes" section in the blog https://kubernetes.io/blog/2019/12/09/kubernetes-1-17-feature-cis-volume-snapshot-beta/.

Status.SnapshotHandle in VolumeSnapshotContent indicates actual state. This field should be populated in both dynamic provisioning and pre-provisioned cases.

@bells17
Copy link
Contributor Author

bells17 commented Feb 3, 2020

Thank you for your describe. I understand.

@bells17 bells17 closed this as completed Feb 3, 2020
andyzhangx added a commit to andyzhangx/external-snapshotter that referenced this issue Dec 16, 2023
b54c1ba Merge pull request kubernetes-csi#246 from xing-yang/go_1.21
5436c81 Change go version to 1.21.5
267b40e Merge pull request kubernetes-csi#244 from carlory/sig-storage
b42e5a2 nominate self (carlory) as kubernetes-csi reviewer
a17f536 Merge pull request kubernetes-csi#210 from sunnylovestiramisu/sidecar
011033d Use set -x instead of die
5deaf66 Add wrapper script for sidecar release

git-subtree-dir: release-tools
git-subtree-split: b54c1ba
andyzhangx pushed a commit to andyzhangx/external-snapshotter that referenced this issue Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants