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

Populate supervisor FSS values for node plugin in guest cluster #2386

Merged

Conversation

akankshapanse
Copy link
Contributor

@akankshapanse akankshapanse commented May 17, 2023

What this PR does / why we need it:
In case of online expansion of volume in guest cluster, during NodeExpandVolume(), we rescan the device to get updated size of the device if online-expand-volume FSS is enabled. FSS check here looks at k8sOrchestrator.internalFSS value (populated from internal-feature-state configmap) as well as k8sOrchestrator.supervisorFSS value in isFSSEnabled() call.
k8sOrchestrator.supervisorFSS and k8sOrchestrator.internalFSS values are populated inside initFSS() where for guest cluster node plugin, only k8sOrchestrator.internalFSS is populated from internal-feature-state configmap but k8sOrchestrator.supervisorFSS is not populated at all. Due to this, isFSSEnabled() returns false when checking for online-expand-volume FSS in NodeExpandVolume(). This step skips rescan of device during online expand and operation fails.
Basically, supervisorFSS values when referred in node plugin of guest cluster, always return false value, which can fail/affect the functionality of node plugin, since it was not populated initially for node plugin in initFSS().
Hence the change made here populates the supervisor FSS for node plugin from supervisor FSS configmap, so that the values can be used correctly in node plugin during FSS check.
This fixes the regression caused by https://github.com/kubernetes-sigs/vsphere-csi-driver/pull/2192/files.
NOTE: Please refer this change before enabling "csi-sv-feature-states-replication" fss for changes to be reworked or revisited.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Testing done:
Tested online resize on GC, where the resize was consistently failing, which now succeeds with the fix.

GC setup :

root@42370ceefaae2c6607e157c12b42d036 [ ~ ]# export KUBECONFIG=gc_kubeconfig
root@42370ceefaae2c6607e157c12b42d036 [ ~ ]# kubectl get pvc
NAME                    STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS         AGE
block-pvc               Bound    pvc-5595a087-f1c0-4318-bbc5-5da795aa7e07   5Gi        RWO            gc-storage-profile   46h
block-restore           Bound    pvc-7182ce88-ff68-4725-a10b-053aeb91d2e3   10Gi       RWO            gc-storage-profile   46h
example-raw-block-pvc   Bound    pvc-16757e70-5dec-4910-85ac-ddab61cbac39   1Gi        RWO            gc-storage-profile   58m
root@42370ceefaae2c6607e157c12b42d036 [ ~ ]#

Expand 
root@42370ceefaae2c6607e157c12b42d036 [ ~ ]# kubectl edit pvc example-raw-block-pvc
persistentvolumeclaim/example-raw-block-pvc edited

root@42370ceefaae2c6607e157c12b42d036 [ ~ ]# kubectl get pvc
NAME                    STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS         AGE
block-pvc               Bound    pvc-5595a087-f1c0-4318-bbc5-5da795aa7e07   5Gi        RWO            gc-storage-profile   46h
block-restore           Bound    pvc-7182ce88-ff68-4725-a10b-053aeb91d2e3   10Gi       RWO            gc-storage-profile   46h
example-raw-block-pvc   Bound    pvc-16757e70-5dec-4910-85ac-ddab61cbac39   2Gi        RWO            gc-storage-profile   59m

Special notes for your reviewer:

Release note:

Populate supervisor FSS values for node plugin in guest cluster

@k8s-ci-robot k8s-ci-robot added 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 May 17, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @akankshapanse. 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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 17, 2023
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 18, 2023
@deepakkinni
Copy link
Contributor

/ok-to-test

@k8s-ci-robot 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 18, 2023
@divyenpatel
Copy link
Member

/ok-to-test

Copy link
Collaborator

@chethanv28 chethanv28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 18, 2023
Copy link
Member

@divyenpatel divyenpatel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/apporve
/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akankshapanse, chethanv28, divyenpatel

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 [chethanv28,divyenpatel]

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 lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2023
@k8s-ci-robot k8s-ci-robot merged commit 23d3baa into kubernetes-sigs:master May 18, 2023
4 checks passed
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 18, 2023
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 18, 2023
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 19, 2023
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 19, 2023
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 19, 2023
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 19, 2023
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 19, 2023
rpanduranga pushed a commit to rpanduranga/vsphere-csi-driver that referenced this pull request May 23, 2023
chethanv28 pushed a commit to chethanv28/vsphere-csi-driver that referenced this pull request May 31, 2023
chethanv28 added a commit that referenced this pull request May 31, 2023
* Add PVCSI 1.25 deployment yaml (#2331)

* Populate supervisor FSS values for node plugin in guest cluster (#2386)

* Add pvcsi 1.26 & 1.27 k8s yaml (#2406)

* Add latest pvcsi 1.25 yaml changes

---------

Co-authored-by: Akanksha Panse <pansea@vmware.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/vmware-vsphere-csi-driver that referenced this pull request Jul 19, 2023
…#2408)

* Add PVCSI 1.25 deployment yaml (kubernetes-sigs#2331)

* Populate supervisor FSS values for node plugin in guest cluster (kubernetes-sigs#2386)

* Add pvcsi 1.26 & 1.27 k8s yaml (kubernetes-sigs#2406)

* Add latest pvcsi 1.25 yaml changes

---------

Co-authored-by: Akanksha Panse <pansea@vmware.com>
rajguptavm pushed a commit to rajguptavm/vsphere-csi-driver that referenced this pull request Jul 20, 2023
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. 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.

None yet

5 participants