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

Supporting snapshots in volume plugins #31373

Closed
wants to merge 12 commits into from
Closed

Conversation

ciwang
Copy link
Contributor

@ciwang ciwang commented Aug 24, 2016

What this PR does / why we need it:
Support snapshots of persistent volumes by adding a new volume plugin interface and a new snapshot controller that handles snapshot requests on PVCs.
https://docs.google.com/document/d/10vDELyIAcgeRGFEgGZ1el78jrf_z2z_2QmQsH4enfhA/edit#heading=h.5vmingbzqhas

Which issue this PR fixes (optional, in fixes #<issue number>(, #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:


This change is Reviewable

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels Aug 24, 2016
DeleteFunc: sc.pvcDelete,
})

if err := sc.volumePluginMgr.InitPlugins(plugins, sc); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

@saad-ali Is that ok we InitPlugins twice, the other one is in attachdetacher?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally it should be called once per binary, but since they are separate instances it won't cause any problems. We should refactor this in the future.

@k8s-bot
Copy link

k8s-bot commented Aug 30, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@jingxu97
Copy link
Contributor

ok to test


// PostEventToPersistentVolumeClaim posts an event to the given PersistentVolumeClaim
// API object with the given message and event type.
func PostEventToPersistentVolumeClaim(
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use recorder.Eventf to post events because it will take care of the duplicate events too

@k8s-bot
Copy link

k8s-bot commented Sep 2, 2016

GCE e2e build/test passed for commit a6ec722.

@k8s-github-robot
Copy link

@ciwang PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2016
@saad-ali
Copy link
Member

saad-ali commented Sep 7, 2016

@jingxu97 will take over this PR now that @ciwang's internship has ended.

@saad-ali saad-ali closed this Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants