-
Notifications
You must be signed in to change notification settings - Fork 214
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
CSI ephemeral inline volume testing + fix #254
CSI ephemeral inline volume testing + fix #254
Conversation
/retest |
At least one line volume must be defined, otherwise the tests get skipped.
With storage capacity simulation enabled, trying to create ephemeral volumes with the default maximum volume size may have failed: Warning FailedMount 6s (x5 over 14s) kubelet MountVolume.SetUp failed for volume "ephemeral-volume" : rpc error: code = ResourceExhausted desc = not enough capacity: have 100Gi, need 1Ti Using a smaller size is better because then more than one volume can be created per host. Even better would be to also add a parameter for it, but that can be added later.
a85f6a2
to
93d4ef8
Compare
/lgtm |
/assign @xing-yang For approval. |
@@ -64,7 +64,9 @@ func (hp *hostPath) NodePublishVolume(ctx context.Context, req *csi.NodePublishV | |||
volID := req.GetVolumeId() | |||
volName := fmt.Sprintf("ephemeral-%s", volID) | |||
kind := req.GetVolumeContext()[storageKind] | |||
vol, err := hp.createVolume(req.GetVolumeId(), volName, maxStorageCapacity, mountAccess, ephemeralVolume, kind) | |||
// Configurable size would be nice. For now we use a small, fixed volume size of 100Mi. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific reason why 100Mi is the chosen size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's small enough that with the default fake capacity limits a large number of inline volumes can be created. But other than that, no.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane, pohly 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 |
4967685 Merge pull request kubernetes-csi#254 from bells17/add-github-actions d9bd160 Update skip list in codespell GitHub Action adb3af9 Merge pull request kubernetes-csi#252 from bells17/update-go-version f5aebfc Add GitHub Actions workflows b82ee38 Merge pull request kubernetes-csi#253 from bells17/fix-typo c317456 Fix typo 0a78505 Bump to Go 1.22.3 edd89ad Merge pull request kubernetes-csi#251 from jsafrane/add-logcheck 043fd09 Add test-logcheck target d7535ae Merge pull request kubernetes-csi#250 from jsafrane/go-1.22 b52e7ad Update go to 1.22.2 14fdb6f Merge pull request kubernetes-csi#247 from msau42/prow 9b4352e Update release playbook c7bb972 Fix release notes script to use fixed tags 463a0e9 Add script to update specific go modules git-subtree-dir: release-tools git-subtree-split: 4967685
Add GitHub Actions workflows
What type of PR is this?
/kind bug
What this PR does / why we need it:
At least one line volume must be defined, otherwise the tests
get skipped. This wasn't the case earlier, which is why support for CSI ephemeral inline volumes broke in release v1.6.0 when changing the size check.
Does this PR introduce a user-facing change?: