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

Move online volume expansion to Beta #77755

Merged
merged 1 commit into from Jun 3, 2019

Conversation

@gnufied
Copy link
Member

commented May 10, 2019

xref - kubernetes/enhancements#531

cc @msau42
/sig storage

replaces - #67608

Move online volume expansion to beta
@gnufied

This comment has been minimized.

Copy link
Member Author

commented May 10, 2019

/kind feature
/priority important-soon

@msau42

This comment has been minimized.

Copy link
Member

commented May 10, 2019

We should implement the migration logic along with this too so that migration does not fall behind.

@gnufied

This comment has been minimized.

Copy link
Member Author

commented May 10, 2019

@msau42 @leakingtapan do you want me to fix migration logic in this PR for both online and offline resizing? I can keep them as separate commits but keep them in same PR.

@msau42

This comment has been minimized.

Copy link
Member

commented May 10, 2019

I would like migration logic to be merged before moving online migration to beta. Whether it's in the same or separate PR doesn't matter to me.

@gnufied gnufied changed the title Move volume expansion to Beta Move online volume expansion to Beta May 28, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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.

@gnufied

This comment has been minimized.

Copy link
Member Author

commented May 28, 2019

@gnufied gnufied force-pushed the gnufied:onlineresize-beta branch from 196c7dd to 843d8fd May 28, 2019

@jsafrane
Copy link
Member

left a comment

Looks good enough, IMO it's ready for merge even without the test fix.

framework.ExpectNoError(err, "while waiting for fs resize to finish")

pvcConditions := pvc.Status.Conditions
gomega.Expect(len(pvcConditions)).To(gomega.Equal(0), "pvc should not have conditions")

This comment has been minimized.

Copy link
@jsafrane

jsafrane May 28, 2019

Member

Can you please also do df in the pod to check the size is actually bigger?

@jsafrane

This comment has been minimized.

Copy link
Member

commented May 28, 2019

/approve
volume manager changes

@gnufied

This comment has been minimized.

Copy link
Member Author

commented May 28, 2019

@msau42 for completeness sake, PR for handling volume resizing migration is - #77994

@jsafrane I have not yet addressed - kubernetes/enhancements#737 (comment) in this PR. I think it was agreed that, we can change the implementation of resize check even after implementation merges.

@jsafrane

This comment has been minimized.

Copy link
Member

commented May 30, 2019

I checked with @gnufied reworking of DSWP and it's a bit bigger PR. I'd like to defer it to 1.16 as implementation detail, the resize feature won't change its API nor behavior.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 30, 2019

@childsb

This comment has been minimized.

Copy link
Member

commented May 30, 2019

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: childsb, gnufied, jsafrane

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

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

@msau42 @jsafrane yeah I have a WIP branch for moving "resizeRequired" error to DSW - gnufied@d9c08d5 but I need to clean it up and fix and add more tests. I will do that as a follow up.

@liggitt

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

/test pull-kubernetes-typecheck

this no longer compiles on master

@liggitt

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

/hold
to keep fejta-bot from retesting until the compile error is fixed. feel free to remove once it is fixed.

Make language of error msgs and func names consistent: ExpandVolumeInUse
change feature flag
Fix the e2e test for online and offline expansion

@gnufied gnufied force-pushed the gnufied:onlineresize-beta branch from 843d8fd to 0f62e3f Jun 3, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jun 3, 2019

@gnufied

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

Fixed compilation error.

/hold cancel

@childsb

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jun 3, 2019

@gnufied

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 99f2e78 into kubernetes:master Jun 3, 2019

21 checks passed

cla/linuxfoundation gnufied authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-slow Job succeeded.
Details
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.