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

Disable X_CSI_SPEC_DISABLE_LEN_CHECK for CSI controller #801

Merged
merged 1 commit into from Apr 22, 2021
Merged

Disable X_CSI_SPEC_DISABLE_LEN_CHECK for CSI controller #801

merged 1 commit into from Apr 22, 2021

Conversation

chethanv28
Copy link
Collaborator

What this PR does / why we need it:

When customers create VCP volumes with long datastore names & with spbm policy, it is observed that volume names canend up with more than 128 characters. Now when customers move to CSI using the CSI migration feature, pod creation fails with the below error:
For ex: [Very-long-vsanDatastore-name] 4bb55260-de8a-c071-913b-020049bb8da4/kubernetes-dynamic-pvc-f8464f80-e90f-4356-ba4f-480fd47e7ea2.vmdk

rpc error: code = InvalidArgument desc = exceeds size limit: VolumeId: max=128, size=132

See https://github.com/kubernetes/kubernetes/issues/101153 for more details.
In vSphere CSI driver we can address this issue by disabling X_CSI_SPEC_DISABLE_LEN_CHECK ENV in the deployment yaml file.

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

Special notes for your reviewer:
Before:

  Warning  FailedAttachVolume  3s (x5 over 10s)  attachdetach-controller  AttachVolume.Attach failed for volume "pvc-f8464f80-e90f-4356-ba4f-480fd47e7ea2" : rpc error: code = InvalidArgument desc = exceeds size limit: VolumeId: max=128, size=147

After:

  Normal  SuccessfulAttachVolume  0s         attachdetach-controller  AttachVolume.Attach succeeded for volume "pvc-f8464f80-e90f-4356-ba4f-480fd47e7ea2"

Release note:

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 16, 2021
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 16, 2021
@chethanv28
Copy link
Collaborator Author

jtest block-vanilla

@svcbot-qecnsdp
Copy link

Started Vanilla block pipeline...

@svcbot-qecnsdp
Copy link

Block vanilla build status: FAILURE 
Stage before exit: testbed-deploy 

@chethanv28
Copy link
Collaborator Author

jtest block-vanilla

@svcbot-qecnsdp
Copy link

Started Vanilla block pipeline...

@svcbot-qecnsdp
Copy link

Block vanilla build status: FAILURE 
Stage before exit: testbed-deploy 

@chethanv28
Copy link
Collaborator Author

jtest block-vanilla

@svcbot-qecnsdp
Copy link

Started Vanilla block pipeline...

@svcbot-qecnsdp
Copy link

Block vanilla build status: SUCCESS 
Stage before exit: finally 
Jenkins E2E Test Results: 

Ran 40 of 167 Specs in 8174.823 seconds
SUCCESS! -- 40 Passed | 0 Failed | 0 Pending | 127 Skipped
PASS

Ginkgo ran 1 suite in 2h18m25.482624268s
Test Suite Passed
make: Leaving directory `/home/worker/workspace/github-csi-block-vanilla/Results/475/vsphere-csi-driver`

@chethanv28
Copy link
Collaborator Author

chethanv28 commented Apr 20, 2021

I see we are not updating - https://github.com/kubernetes-sigs/vsphere-csi-driver/tree/master/manifests/dev/vsphere-7.0u2/vanilla

We will be removing the following directories, so if you can just update this in the 7.0u2 folder, it is fine.

https://github.com/kubernetes-sigs/vsphere-csi-driver/tree/master/manifests/dev/vsphere-67u3
https://github.com/kubernetes-sigs/vsphere-csi-driver/tree/master/manifests/dev/vsphere-7.0/vanilla and
https://github.com/kubernetes-sigs/vsphere-csi-driver/tree/master/manifests/dev/vsphere-7.0u1/vanilla

I have updated yaml in 70u2 folder. Also I am keeping the changes in 6.7u3, 7.0 and 7.0u1 since the PR to consolidate yamls will eventually remove the folder.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2021
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 22, 2021
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.

approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

@divyenpatel
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 22, 2021
@k8s-ci-robot k8s-ci-robot merged commit e2b218d into kubernetes-sigs:master Apr 22, 2021
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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants