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

Make raw block pvc creation from volume snapshot idempotent #220

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

RaunakShah
Copy link
Contributor

@RaunakShah RaunakShah commented Nov 11, 2020

What type of PR is this?
/kind bug

What this PR does / why we need it:
Previously, the dd command run at https://github.com/kubernetes-csi/csi-driver-host-path/blob/master/pkg/hostpath/hostpath.go#L334 for raw block volumes was taking longer than the timeout for CreateVolume. As a result, the first call to CreateVolume fails with DeadlineExceeded.

Subsequent calls to CreateVolume were failing at https://github.com/kubernetes-csi/csi-driver-host-path/blob/master/pkg/hostpath/controllerserver.go#L138 because this volume was added to the local cache in the first call.

This PR makes creation of raw block PVC's from volume snapshots idempotent, so that retries for the same volume succeed.

Testing:

  1. Create raw block PVC
  2. Create volume snapshot
  3. Create new PVC with volume snapshot as source

Both PVC's are in Bound state:

$ kubectl get  pvc
NAME              STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS      AGE
pvc-raw           Bound    pvc-e8b2e03e-e2be-462e-aea4-4c6eab52ff95   1Gi        RWO            csi-hostpath-sc   3m59s
raw-pvc-restore   Bound    pvc-9fd5af15-ceb6-4fd9-b6d9-6e131b54b55b   1Gi        RWO            csi-hostpath-sc   2m56s

$ kubectl get volumesnapshot
NAME               READYTOUSE   SOURCEPVC   SOURCESNAPSHOTCONTENT   RESTORESIZE   SNAPSHOTCLASS            SNAPSHOTCONTENT                                    CREATIONTIME   AGE
raw-pvc-snapshot   true         pvc-raw                             1Gi           csi-hostpath-snapclass   snapcontent-d2bac861-3ba2-4839-a85e-b06674041cd4   3m27s          4m11s

Logs showing CreateVolume is called twice:

I1111 17:57:25.570725       1 server.go:117] GRPC call: /csi.v1.Controller/CreateVolume
I1111 17:57:25.570775       1 server.go:118] GRPC request: {"accessibility_requirements":{"preferred":[{"segments":{"topology.hostpath.csi/node":"csi-prow-worker"}}],"requisite":[{"segments":{"topology.hostpath.csi/node":"csi-prow-worker"}}]},"capacity_range":{"required_bytes":1073741824},"name":"pvc-9fd5af15-ceb6-4fd9-b6d9-6e131b54b55b","volume_capabilities":[{"AccessType":{"Block":{}},"access_mode":{"mode":1}}],"volume_content_source":{"Type":{"Snapshot":{"snapshot_id":"53ff081f-2447-11eb-bed8-1ec29f23ce4b"}}}}
I1111 17:57:25.650608       1 volume_path_handler_linux.go:41] Creating device for path: /csi-data-dir/5b2b7374-2447-11eb-bed8-1ec29f23ce4b
I1111 17:57:25.668637       1 controllerserver.go:165] created volume 5b2b7374-2447-11eb-bed8-1ec29f23ce4b at path /csi-data-dir/5b2b7374-2447-11eb-bed8-1ec29f23ce4b
I1111 17:57:26.591090       1 server.go:117] GRPC call: /csi.v1.Controller/CreateVolume
I1111 17:57:26.591117       1 server.go:118] GRPC request: {"accessibility_requirements":{"preferred":[{"segments":{"topology.hostpath.csi/node":"csi-prow-worker"}}],"requisite":[{"segments":{"topology.hostpath.csi/node":"csi-prow-worker"}}]},"capacity_range":{"required_bytes":1073741824},"name":"pvc-9fd5af15-ceb6-4fd9-b6d9-6e131b54b55b","volume_capabilities":[{"AccessType":{"Block":{}},"access_mode":{"mode":1}}],"volume_content_source":{"Type":{"Snapshot":{"snapshot_id":"53ff081f-2447-11eb-bed8-1ec29f23ce4b"}}}}
I1111 17:57:26.593464       1 server.go:123] GRPC response: {"volume":{"capacity_bytes":1073741824,"content_source":{"Type":{"Snapshot":{"snapshot_id":"53ff081f-2447-11eb-bed8-1ec29f23ce4b"}}},"volume_id":"5b2b7374-2447-11eb-bed8-1ec29f23ce4b"}}

Get volumeHandle of PV's:

$ kubectl describe pv  | grep -i handle
    VolumeHandle:      5b2b7374-2447-11eb-bed8-1ec29f23ce4b
    VolumeHandle:      35df4fe4-2447-11eb-bed8-1ec29f23ce4b

Exec into the hostpath driver and verify that only 3 volumes exist (2 with PV volume handles):

/ # ls /csi-data-dir/
35df4fe4-2447-11eb-bed8-1ec29f23ce4b       53ff081f-2447-11eb-bed8-1ec29f23ce4b.snap  5b2b7374-2447-11eb-bed8-1ec29f23ce4b

Verify that only two volumes are attached:

/ # losetup
NAME       SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE                                          DIO LOG-SEC
/dev/loop1         0      0         0  0 /csi-data-dir/5b2b7374-2447-11eb-bed8-1ec29f23ce4b   0     512
/dev/loop0         0      0         0  0 /csi-data-dir/35df4fe4-2447-11eb-bed8-1ec29f23ce4b   0     512

Which issue(s) this PR fixes:

Fixes #219

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Make raw block PVC creation from VolumeSnapshot idempotent.

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. 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 Nov 11, 2020
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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 Nov 11, 2020
@xing-yang
Copy link
Contributor

/assign

@xing-yang
Copy link
Contributor

CC @prafull01

@xing-yang
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 11, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2020
@k8s-ci-robot k8s-ci-robot merged commit 52149ac into kubernetes-csi:master Nov 11, 2020
sunnylovestiramisu added a commit to sunnylovestiramisu/csi-driver-host-path that referenced this pull request Apr 12, 2023
6613c39 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update
0e7ae99 Update k8s image repo url
77e47cc Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version
155854b Fix dep version mismatch
8f83905 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update
1d3f94d Update go version to 1.20 to match k/k v1.27
e322ce5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error
b74a512 test: fix golint error
aa61bfd Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver
7563d19 Update CSI_PROW_DRIVER_VERSION to v1.11.0
a2171be Merge pull request kubernetes-csi#216 from msau42/process
cb98782 Merge pull request kubernetes-csi#217 from msau42/owners
a11216e add new reviewers and remove inactive reviewers
dd98675 Add step for checking builds

git-subtree-dir: release-tools
git-subtree-split: 6613c39
sunnylovestiramisu added a commit to sunnylovestiramisu/csi-driver-host-path that referenced this pull request Apr 13, 2023
6613c39 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update
0e7ae99 Update k8s image repo url
77e47cc Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version
155854b Fix dep version mismatch
8f83905 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update
1d3f94d Update go version to 1.20 to match k/k v1.27
e322ce5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error
b74a512 test: fix golint error
aa61bfd Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver
7563d19 Update CSI_PROW_DRIVER_VERSION to v1.11.0
a2171be Merge pull request kubernetes-csi#216 from msau42/process
cb98782 Merge pull request kubernetes-csi#217 from msau42/owners
a11216e add new reviewers and remove inactive reviewers
dd98675 Add step for checking builds

git-subtree-dir: release-tools
git-subtree-split: 6613c39
sunnylovestiramisu added a commit to sunnylovestiramisu/csi-driver-host-path that referenced this pull request Apr 13, 2023
6613c39 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update
0e7ae99 Update k8s image repo url
77e47cc Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version
155854b Fix dep version mismatch
8f83905 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update
1d3f94d Update go version to 1.20 to match k/k v1.27
e322ce5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error
b74a512 test: fix golint error
aa61bfd Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver
7563d19 Update CSI_PROW_DRIVER_VERSION to v1.11.0
a2171be Merge pull request kubernetes-csi#216 from msau42/process
cb98782 Merge pull request kubernetes-csi#217 from msau42/owners
a11216e add new reviewers and remove inactive reviewers
dd98675 Add step for checking builds

git-subtree-dir: release-tools
git-subtree-split: 6613c39
TerryHowe pushed a commit to TerryHowe/csi-driver-host-path that referenced this pull request Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cant create raw block pvc from volume snapshot
3 participants