fix: incorrect snapshot size with default 100Gi #2040

umagnus · 2024-08-06T07:50:28Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

fix: fix snapshot size using default 100Gi

Which issue(s) this PR fixes:

Fixes #2039

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Special notes for your reviewer:

$ k get pvc
NAME                    STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS             VOLUMEATTRIBUTESCLASS   AGE
pvc-azurefile           Bound    pvc-be4afe83-4f7e-473d-8e9f-6ce525816e5b   1Gi        RWX            azurefile-csi            <unset>                 5s
$ k get vs
NAME                        READYTOUSE   SOURCEPVC       SOURCESNAPSHOTCONTENT   RESTORESIZE   SNAPSHOTCLASS       SNAPSHOTCONTENT                                    CREATIONTIME   AGE
azurefile-volume-snapshot   true         pvc-azurefile                           1Gi           csi-azurefile-vsc   snapcontent-9d59b667-1e22-4f67-ab3a-23b498209f39   5s             6s

Release note:

none

k8s-ci-robot · 2024-08-06T07:50:37Z

Hi @umagnus. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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-sigs/prow repository.

andyzhangx · 2024-08-06T07:53:08Z

pkg/azurefile/controllerserver.go

+		if err != nil {
+			return nil, status.Errorf(codes.Internal, "failed to get file share(%s) quota: %v", fileShareName, err)
+		}
+		itemSnapshotQuota = *fileshare.ShareQuota


use ptr.Deref(fileshare.ShareQuota, 0) to avoid nil pointer panic

I think here should be itemSnapshotQuota = pointer.Int32Deref(fileshare.ShareQuota, defaultAzureFileQuota) since 0 will also return an error

andyzhangx

/ok-to-test

umagnus · 2024-08-06T08:39:15Z

/retest

k8s-ci-robot · 2024-08-06T10:19:13Z

@umagnus: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-azurefile-csi-driver-e2e-migration-windows	`03dee0e`	link	true	`/test pull-azurefile-csi-driver-e2e-migration-windows`
pull-azurefile-csi-driver-e2e-capz-windows-2022-hostprocess	`03dee0e`	link	true	`/test pull-azurefile-csi-driver-e2e-capz-windows-2022-hostprocess`
pull-azurefile-csi-driver-e2e-capz-windows-2019	`03dee0e`	link	true	`/test pull-azurefile-csi-driver-e2e-capz-windows-2019`
pull-azurefile-csi-driver-e2e-capz-windows-2019-hostprocess	`03dee0e`	link	true	`/test pull-azurefile-csi-driver-e2e-capz-windows-2019-hostprocess`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-sigs/prow repository. I understand the commands that are listed here.

RomanBednar · 2024-08-06T10:55:03Z

I've quickly checked the patch in OpenShift and it looks good.

Driver log with snapshot creation - 0GiB quota is logged before getting the real value:

I0806 10:02:27.304528       1 utils.go:101] GRPC call: /csi.v1.Controller/CreateSnapshot
I0806 10:02:27.304546       1 utils.go:102] GRPC request: {"name":"snapshot-e5b45995-6e63-436e-bb7c-e7806aa5a05e","source_volume_id":"ci-ln-1s480k2-1d09d-spnqd-rg#clusterjqpxb#pvc-45efa23a-3af6-4608-b78a-9c47eb33eb48###default"}
I0806 10:02:28.068302       1 controllerserver.go:915] created share snapshot: 2024-08-06T10:02:27.0000000Z, time: 2024-08-06 10:02:27 +0000 UTC, quota: 0GiB
I0806 10:02:28.262893       1 utils.go:108] GRPC response: {"snapshot":{"creation_time":{"seconds":1722938547},"ready_to_use":true,"size_bytes":1073741824,"snapshot_id":"ci-ln-1s480k2-1d09d-spnqd-rg#clusterjqpxb#pvc-45efa23a-3af6-4608-b78a-9c47eb33eb48###default#2024-08-06T10:02:27.0000000Z","source_volume_id":"ci-ln-1s480k2-1d09d-spnqd-rg#clusterjqpxb#pvc-45efa23a-3af6-4608-b78a-9c47eb33eb48###default"}}

Snapshot created with correct restore size:

$ oc get pvc
NAME                     STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS    VOLUMEATTRIBUTESCLASS   AGE
pvc-1                    Bound    pvc-45efa23a-3af6-4608-b78a-9c47eb33eb48   1Gi        RWO            azurefile-csi   <unset>                 8m47s
pvc-1-snapshot-restore   Bound    pvc-4770552b-fa64-413b-995c-1eddcada5cd8   5Gi        RWO            azurefile-csi   <unset>                 14s

$ oc get volumesnapshot
NAME             READYTOUSE   SOURCEPVC   SOURCESNAPSHOTCONTENT   RESTORESIZE   SNAPSHOTCLASS       SNAPSHOTCONTENT                                    CREATIONTIME   AGE
pvc-1-snapshot   true         pvc-1                               1Gi           csi-azurefile-vsc   snapcontent-e5b45995-6e63-436e-bb7c-e7806aa5a05e   33s            33s

andyzhangx

/lgtm

k8s-ci-robot · 2024-08-06T12:52:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, umagnus

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~~ [andyzhangx]

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

andyzhangx · 2024-08-06T12:52:11Z

/cherrypick release-1.30

andyzhangx · 2024-08-06T12:52:16Z

/cherrypick release-1.29

andyzhangx · 2024-08-06T12:52:20Z

/cherrypick release-1.28

k8s-infra-cherrypick-robot · 2024-08-06T12:53:01Z

@andyzhangx: new pull request created: #2041

In response to this:

/cherrypick release-1.30

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-sigs/prow repository.

k8s-infra-cherrypick-robot · 2024-08-06T12:53:02Z

@andyzhangx: #2040 failed to apply on top of branch "release-1.28":

Applying: fix snapshot size
Using index info to reconstruct a base tree...
M	pkg/azurefile/controllerserver.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/azurefile/controllerserver.go
CONFLICT (content): Merge conflict in pkg/azurefile/controllerserver.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix snapshot size
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-1.28

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-sigs/prow repository.

k8s-infra-cherrypick-robot · 2024-08-06T12:53:03Z

@andyzhangx: new pull request created: #2042

In response to this:

/cherrypick release-1.29

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-sigs/prow repository.

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 6, 2024

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 6, 2024

k8s-ci-robot requested review from andyzhangx and gnufied August 6, 2024 07:50

andyzhangx reviewed Aug 6, 2024

View reviewed changes

fix snapshot size

03dee0e

umagnus force-pushed the fix_snapshot_size branch from 1ff9478 to 03dee0e Compare August 6, 2024 07:59

andyzhangx reviewed Aug 6, 2024

View reviewed changes

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 6, 2024

RomanBednar mentioned this pull request Aug 6, 2024

STOR-2002: add Azure File snapshot support openshift/csi-operator#253

Merged

andyzhangx changed the title ~~fix: fix snapshot size using default 100Gi~~ fix: incorrect snapshot size with default 100Gi Aug 6, 2024

andyzhangx approved these changes Aug 6, 2024

View reviewed changes

k8s-ci-robot assigned andyzhangx Aug 6, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2024

andyzhangx merged commit 1ec0deb into kubernetes-sigs:master Aug 6, 2024
19 of 24 checks passed

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 6, 2024

k8s-infra-cherrypick-robot mentioned this pull request Aug 6, 2024

[release-1.30] fix: incorrect snapshot size with default 100Gi #2041

Merged

k8s-infra-cherrypick-robot mentioned this pull request Aug 6, 2024

[release-1.29] fix: incorrect snapshot size with default 100Gi #2042

Merged

umagnus mentioned this pull request Aug 7, 2024

[release-1.28] fix: incorrect snapshot size with default 100Gi #2044

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: incorrect snapshot size with default 100Gi #2040

fix: incorrect snapshot size with default 100Gi #2040

umagnus commented Aug 6, 2024 •

edited

Loading

k8s-ci-robot commented Aug 6, 2024

andyzhangx Aug 6, 2024

umagnus Aug 6, 2024

andyzhangx left a comment

umagnus commented Aug 6, 2024

k8s-ci-robot commented Aug 6, 2024

RomanBednar commented Aug 6, 2024

andyzhangx left a comment

k8s-ci-robot commented Aug 6, 2024

andyzhangx commented Aug 6, 2024

andyzhangx commented Aug 6, 2024

andyzhangx commented Aug 6, 2024

k8s-infra-cherrypick-robot commented Aug 6, 2024

k8s-infra-cherrypick-robot commented Aug 6, 2024

k8s-infra-cherrypick-robot commented Aug 6, 2024

fix: incorrect snapshot size with default 100Gi #2040

fix: incorrect snapshot size with default 100Gi #2040

Conversation

umagnus commented Aug 6, 2024 • edited Loading

k8s-ci-robot commented Aug 6, 2024

andyzhangx Aug 6, 2024

Choose a reason for hiding this comment

umagnus Aug 6, 2024

Choose a reason for hiding this comment

andyzhangx left a comment

Choose a reason for hiding this comment

umagnus commented Aug 6, 2024

k8s-ci-robot commented Aug 6, 2024

RomanBednar commented Aug 6, 2024

andyzhangx left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 6, 2024

andyzhangx commented Aug 6, 2024

andyzhangx commented Aug 6, 2024

andyzhangx commented Aug 6, 2024

k8s-infra-cherrypick-robot commented Aug 6, 2024

k8s-infra-cherrypick-robot commented Aug 6, 2024

k8s-infra-cherrypick-robot commented Aug 6, 2024

umagnus commented Aug 6, 2024 •

edited

Loading