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.

Sign up for GitHub

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

Jump to bottom

Add support for expanding local volumes #102886

Merged
merged 3 commits into from Aug 12, 2021
Merged

Add support for expanding local volumes #102886

merged 3 commits into from Aug 12, 2021

Conversation

gnufied
Copy link
Member

@gnufied gnufied commented Jun 15, 2021 •
edited

Fixes #101340

This is another variant of #101415

for shared directory - we consider expansion operation as a NO-OP.

/sig storage

/assign @msau42

I0615 13:46:04.773435 4149291 local.go:415] "expansion of directory based local volumes is NO-OP" local-volume-path="/mnt/shared"
I0615 13:46:04.773463 4149291 operation_generator.go:1777] MountVolume.NodeExpandVolume succeeded for volume "example-pv" (UniqueName: "kubernetes.io/local-volume/example-pv") pod "busybox" (UID: "6bdc0604-2e9d-4224-bf8b-70c876a1c295") 
I0615 13:46:04.773555 4149291 event.go:291] "Event occurred" object="default/busybox" kind="Pod" apiVersion="v1" type="Normal" reason="FileSystemResizeSuccessful" message="MountVolume.NodeExpandVolume succeeded for volume \"example-pv\" "
I0615 13:46:04.773577 4149291 event.go:291] "Event occurred" object="default/myclaim1" kind="PersistentVolumeClaim" apiVersion="v1" type="Normal" reason="FileSystemResizeSuccessful" message="MountVolume.NodeExpandVolume succeeded for volume \"example-pv\" "

Allow node expansion of local volumes

@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. 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 Jun 15, 2021
@gnufied
Copy link
Member Author

gnufied commented Jun 15, 2021

/kind feature
/triage accepted
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 15, 2021
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 15, 2021
@gnufied
Copy link
Member Author

gnufied commented Jun 15, 2021

/retest

@gnufied gnufied mentioned this pull request Jun 16, 2021
Closed
@gnufied
Copy link
Member Author

gnufied commented Jun 23, 2021

gentle nudge on this again @msau42

msau42
msau42 reviewed Jun 25, 2021
View reviewed changes
pkg/volume/local/local.go
}

switch fileType {
case hostutil.FileTypeBlockDev:
Copy link
Member

Choose a reason for hiding this comment

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

can we add an e2e test for this? We have some tests that create a tmp device.

@dims
Copy link
Member

dims commented Aug 6, 2021

@gnufied looks like this needs a new e2e test? ( #102886 (comment) )

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 6, 2021
@gnufied
Copy link
Member Author

gnufied commented Aug 6, 2021

@msau42 added e2e , PTAL.

@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Aug 6, 2021
msau42
msau42 reviewed Aug 10, 2021
View reviewed changes
pkg/volume/local/local.go
return false, err
}
return true, nil
case hostutil.FileTypeDirectory:
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to unit test or e2e test this condition as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@msau42
Copy link
Member

msau42 commented Aug 12, 2021

/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 Aug 12, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, msau42

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 merged commit 1f8b1b8 into kubernetes:master Aug 12, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Aug 12, 2021
@gnufied gnufied mentioned this pull request Aug 30, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

@saad-ali saad-ali Awaiting requested review from saad-ali

@vishh vishh Awaiting requested review from vishh

Assignees

@msau42 msau42

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.23
Development

Successfully merging this pull request may close these issues.

Allow Node Only expansion of Local Volumes
4 participants
@gnufied @dims @msau42 @k8s-ci-robot