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

Restoring from a snapshot to a larger size #485

Closed
4 tasks
msau42 opened this issue Sep 28, 2020 · 34 comments
Closed
4 tasks

Restoring from a snapshot to a larger size #485

msau42 opened this issue Sep 28, 2020 · 34 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@msau42
Copy link
Collaborator

msau42 commented Sep 28, 2020

Today, what happens with many block-based plugins is that the disk will be created at the larger size, but the filesystem (from the snapshot) is at the smaller size. Filesystem expansion won't be triggered because PV is created at the requested PVC size.

We discussed this at the csi standup last week and agreed that the behavior should be:

  • If a plugin is not going to resize the filesystem during the restore from snapshot operation, then they should return the original snapshot size (the original block size), so that the resize operation can be done.
  • If a plugin is going to resize everything as part of restore, including the filesystem, then they should return the new size.
  • If the plugin returns nothing (because size is optional today), then we continue the same behavior of setting the pv to the new size regardless.

This bug can track the following aspects:

  • Do we need to clarify anything in the CSI spec?
  • Make sure external-provisioner is doing the right thing, and we have unit tests
  • Update mock driver and add e2e tests for the different scenarios
  • Add generic e2e test that can be used by all plugins

Updating individual csi drivers to return the proper behavior can be tracked separately.

@msau42
Copy link
Collaborator Author

msau42 commented Sep 28, 2020

/kind bug

cc @jsafrane @gnufied @xing-yang @bswartz

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 28, 2020
@msau42
Copy link
Collaborator Author

msau42 commented Sep 28, 2020

Another open issue: should we apply the same logic to clone?

@msau42
Copy link
Collaborator Author

msau42 commented Sep 28, 2020

cc @j-griffith

@msau42
Copy link
Collaborator Author

msau42 commented Sep 28, 2020

@Jiawei0227 will help take a look at implementing this

@gnufied
Copy link
Contributor

gnufied commented Sep 28, 2020

issue @jsafrane created in k/k repo has summary of discussion so far - kubernetes/kubernetes#94929

@bswartz
Copy link
Contributor

bswartz commented Sep 29, 2020

The text from the CreateVolumeRequest message in the spec says:

// This field is OPTIONAL. This allows the CO to specify the capacity
// requirement of the volume to be provisioned. If not specified, the
// Plugin MAY choose an implementation-defined capacity range. If
// specified it MUST always be honored, even when creating volumes
// from a source; which MAY force some backends to internally extend
// the volume after creating it.
CapacityRange capacity_range = 2;

The current implication is that any expansions are magically taken care of inside the CreateVolume call. We definitely need spec language here clarifying that while the new size must be honored insofar as the controller's notion of the volume size, the CO may need to do the NodeExpand portion of the work to bring the node's notion of the volume size into sync.

@Jiawei0227
Copy link
Contributor

There is currently a discussion here: container-storage-interface/spec#452
I will start to implement this as soon as we get the final decision on how the CO and SP should behave.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 7, 2021
@Jiawei0227
Copy link
Contributor

/remove-lifecycle stale

I am grooming my backlog and found this one. It seems from the last CSI meeting, we decide that to let each CSI driver to handle it. They should call the expand. So I guess there is no more work involved in the csi-provisioner for now?

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 14, 2021
@xing-yang
Copy link
Contributor

I am grooming my backlog and found this one. It seems from the last CSI meeting, we decide that to let each CSI driver to handle it. They should call the expand. So I guess there is no more work involved in the csi-provisioner for now?

Yes. CSI driver needs to handle this. @jsafrane has updated the CSI spec PR: container-storage-interface/spec#452

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 28, 2021
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

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.

@k8s-ci-robot k8s-ci-robot reopened this Nov 1, 2021
@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Nov 1, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 30, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 1, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

@jsafrane
Copy link
Contributor

jsafrane commented Apr 5, 2022

/reopen

@k8s-ci-robot
Copy link
Contributor

@jsafrane: Reopened this issue.

In response to this:

/reopen

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.

@k8s-ci-robot k8s-ci-robot reopened this Apr 5, 2022
@jsafrane
Copy link
Contributor

jsafrane commented Apr 5, 2022

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Apr 5, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 4, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 3, 2022
@xing-yang
Copy link
Contributor

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 3, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 1, 2022
@humblec
Copy link
Contributor

humblec commented Nov 10, 2022

@msau42 @xing-yang @jsafrane afaict, this is handled in provisioner code already and I am closing this issue. Please reopen if required.
/close

@humblec humblec closed this as completed Nov 10, 2022
@amacaskill
Copy link
Contributor

@humblec Could you please point to where in the provisioner code this is handled?

@msau42
Copy link
Collaborator Author

msau42 commented Nov 30, 2022

@amacaskill the conclusion of this bug is that each CSI driver needs to make changes to their NodeStage/NodePublish implementations to resize the filesystem if possible. See https://github.com/container-storage-interface/spec/pull/452/files for the spec clarification and https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/595/files for an example. Resize utilities have been exported in https://github.com/kubernetes/kubernetes/pull/99088/files so that CSI drivers can share similar logic.

For pd csi driver it looks like it was added in kubernetes-sigs/gcp-compute-persistent-disk-csi-driver#972

@humblec
Copy link
Contributor

humblec commented Nov 30, 2022

@amacaskill addition to what @msau42 mentioned above, this make sure the controller allows to have a capacity >= snapshot ->restore size. https://github.com/kubernetes-csi/external-provisioner/blob/master/pkg/controller/controller.go#L1134

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests