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

Implement kubelet side online file system resize for volume #62460

Merged
merged 1 commit into from May 31, 2018
Merged

Implement kubelet side online file system resize for volume #62460

merged 1 commit into from May 31, 2018

Conversation

mlmhl
Copy link
Contributor

@mlmhl mlmhl commented Apr 12, 2018

What this PR does / why we need it:

Implement kubelet side online file system resize.

xref - kubernetes/feature#531
proposal - kubernetes/community#1535

Release note:

Implement kubelet side online file system resizing

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 12, 2018
@mlmhl
Copy link
Contributor Author

mlmhl commented Apr 12, 2018

/sig storage
/kind feature

@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. kind/feature Categorizes issue or PR as related to a new feature. labels Apr 12, 2018
@mlmhl mlmhl force-pushed the volume-online-resize branch 2 times, most recently from 51a4fbf to 5f75db7 Compare April 12, 2018 12:28
@gnufied
Copy link
Member

gnufied commented Apr 12, 2018

Very quick feedback before I dive in - please feature gate everything that is pertaining to resizing. None of the changes that are needed for resizing should leak into general code without feature gate.

@sjenning
Copy link
Contributor

fyi @derekwaynecarr

@childsb
Copy link
Contributor

childsb commented Apr 12, 2018

This should be feature gated separate of the 1.10 resize function and treated as alpha function in 1.11

@mlmhl
Copy link
Contributor Author

mlmhl commented Apr 13, 2018

@gnufied @childsb
Added a separate feature gate ExpandOnlinePersistentVolumes to gate all online resizing related codes.

@gnufied
Copy link
Member

gnufied commented Apr 13, 2018

@mlmhl can you also update the proposal to reflect this.

@mlmhl
Copy link
Contributor Author

mlmhl commented Apr 14, 2018

@gnufied Thanks for reminding, the proposal is updated.

@mlmhl mlmhl mentioned this pull request Apr 17, 2018
Closed
@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 18, 2018
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 20, 2018
@mlmhl
Copy link
Contributor Author

mlmhl commented Apr 20, 2018

/test pull-kubernetes-verify

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 28, 2018
gnufied
gnufied reviewed May 22, 2018
View reviewed changes
pkg/kubelet/volumemanager/cache/actual_state_of_world.go Outdated
volumeName v1.UniqueVolumeName) error {
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandOnlinePersistentVolumes) {
return fmt.Errorf("online resizing is not enabled for this volume %s", volumeName)
}
Copy link
Member

Choose a reason for hiding this comment

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

do we need this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this check isn't necessary as the MarkVolumeAsResized method won't be invoked if the feature gate disabled. I've removed this check.

pkg/kubelet/volumemanager/cache/actual_state_of_world.go Outdated
podName volumetypes.UniquePodName) {
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandOnlinePersistentVolumes) {
// Do not perform online resizing if feature gate is disabled.
return
Copy link
Member

Choose a reason for hiding this comment

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

do we need this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 30, 2018
@gnufied
Copy link
Member

gnufied commented May 30, 2018

@mlmhl can you please address the pending comments before it gets merged?

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 31, 2018
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 31, 2018
@mlmhl
Copy link
Contributor Author

mlmhl commented May 31, 2018

@gnufied Thanks for reminding, the pending comments are addressed.

@mlmhl
Copy link
Contributor Author

mlmhl commented May 31, 2018

/test pull-kubernetes-kubemark-e2e-gce-big

@gnufied
Copy link
Member

gnufied commented May 31, 2018

@mlmhl how did you address them? I do not see feature gate renamed as I was suggesting and I do not see a response to my comment either.

@saad-ali saad-ali added this to the v1.11 milestone May 31, 2018
@saad-ali saad-ali added kind/enhancement priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 31, 2018
@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@gnufied @mlmhl

Pull Request Labels
  • sig/storage: Pull Request will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/feature: New functionality.
Help

@gnufied
Copy link
Member

gnufied commented May 31, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 31, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, mlmhl, saad-ali

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

@gnufied
Copy link
Member

gnufied commented May 31, 2018

/test pull-kubernetes-kubemark-e2e-gce-big

1 similar comment
@gnufied
Copy link
Member

gnufied commented May 31, 2018

/test pull-kubernetes-kubemark-e2e-gce-big

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 62460, 64480, 63774, 64540, 64337). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 7303776 into kubernetes:master May 31, 2018
@gnufied gnufied mentioned this pull request May 31, 2018
Merged
@k8s-ci-robot
Copy link
Contributor

@mlmhl: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-kubemark-e2e-gce-big ca12c73 link /test pull-kubernetes-kubemark-e2e-gce-big

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

k8s-github-robot pushed a commit that referenced this pull request Jun 1, 2018
Merge pull request #64577 from gnufied/rename-online-resize-feature-gate
d922a4e 
Automatic merge from submit-queue (batch tested with PRs 59938, 63777, 64577, 63999, 64431). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Rename online resizine feature gate

This was left as a unaddressed comment from #62460

cc @saad-ali 
/sig storage

```release-note
None
```
@mlmhl
Copy link
Contributor Author

mlmhl commented Jun 1, 2018

@gnufied I'm sorry I missed your comment, I saw you have fixed this, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnufied gnufied gnufied left review comments

@jsafrane jsafrane jsafrane left review comments

@saad-ali saad-ali saad-ali approved these changes

@jingxu97 jingxu97 Awaiting requested review from jingxu97

@Random-Liu Random-Liu Awaiting requested review from Random-Liu

@verult verult Awaiting requested review from verult

Assignees

@gnufied gnufied

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. 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-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. 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. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.11
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@mlmhl @gnufied @sjenning @childsb @saad-ali @k8s-github-robot @k8s-ci-robot @jsafrane