Add support for In-place snapshot restore by ram-infrac · Pull Request #353 · libopenstorage/stork

ram-infrac · 2019-05-02T17:50:20Z

What type of PR is this?
feature

What this PR does / why we need it:
This PR add in-place restore support for volume snapshots. Snapshot now can be restored to origin volumes

Does this PR change a user-facing CRD or CLI?:
We have new CRD for In-place volume snapshot restore.
eg.

apiVersion: stork.libopenstorage.org/v1alpha1
kind: SnapshotRestore
metadata:
    creationTimestamp: null
    name: mysql-group-snap-test123
    namespace: default
spec:
  sourcetype: "local"
  sourcename: "mysql-group-snap"
  sourcenamespace: "default"
  groupsnapshot: true

sourcetype: snapshot type either local or cloud
sourcename: name of snapshot to be restore
groupsnapshot: true if snapshot is volumegroupsnapshot
Is a release note needed?:

Does this change need to be cherry-picked to a release branch?:

pkg/apis/stork/v1alpha1/snapshotrestore.go

drivers/volume/volume.go

pkg/apis/stork/v1alpha1/register.go

pkg/apis/stork/v1alpha1/snapshotrestore.go

pkg/snapshot/controllers/snapshotrestore.go

pkg/apis/stork/v1alpha1/snapshotrestore.go

pkg/snapshot/controllers/snapshotrestore.go

drivers/volume/portworx/portworx.go

codecov · 2019-06-03T09:47:29Z

Codecov Report

Merging #353 into master will increase coverage by 0.15%.
The diff coverage is 91.3%.

@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
+ Coverage   74.22%   74.37%   +0.15%     
==========================================
  Files          27       27              
  Lines        2471     2494      +23     
==========================================
+ Hits         1834     1855      +21     
- Misses        479      480       +1     
- Partials      158      159       +1

Impacted Files	Coverage Δ
pkg/log/log.go	`95.13% <100%> (+0.24%)`	⬆️
pkg/extender/extender.go	`85.83% <87.5%> (+0.12%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee02a05...8d27347. Read the comment docs.

disrani-px

Good to merge after fixing minor comments and adding UT for changes in extender.go

drivers/volume/portworx/portworx.go

pkg/apis/stork/v1alpha1/snapshotrestore.go

disrani-px · 2019-06-04T00:10:27Z

pkg/extender/extender.go

+		pvc, err := k8s.Instance().GetPersistentVolumeClaim(vol.PersistentVolumeClaim.ClaimName, pod.Namespace)
+		if err != nil {
+			msg := "Unable to find PVC %s" + vol.Name
+			storklog.PodLog(pod).Warnf(msg)


I think the Event and http.Error(...) got removed when merging the other PR

yes, thanks for pointing out. Do we need to check nil for pod.Spec here

No, PodLog() checks for nil

pkg/snapshot/controllers/snapshotrestore.go

ram-infrac · 2019-06-04T09:57:16Z

I have changed UT's for fake k8s client responses, @disrani-px please take look at extender_test.go
Will merge after your re-review for extender_test.go

disrani-px · 2019-06-05T21:03:31Z

@ram-infrac Changes look good!

disrani-px · 2019-06-05T21:00:30Z

pkg/extender/extender.go

+		pvc, err := k8s.Instance().GetPersistentVolumeClaim(vol.PersistentVolumeClaim.ClaimName, pod.Namespace)
+		if err != nil {
+			msg := "Unable to find PVC %s" + vol.Name
+			storklog.PodLog(pod).Warnf(msg)


No, PodLog() checks for nil

disrani-px · 2019-06-05T22:04:42Z

Please squash the intermediate commits before merging.

- codegen generated files - Add controller for In-place snapshot restore - Add support for local groupsnapshot in-place restore - Fix go-lint errors - Review comments - Check restore status before calling driver's snapshot restore - codegen generated files Signed-off-by: Ram <ram.suradkar@portworx.com>

- Fix go-lint errors - Review comments - Generated files for changed volumesnapshotCRD's - Restore snapshot where pvc is in use by pods - Move controller specific login from portworx driver to snapshot restore controller - Generated files by codegen - Review comments Signed-off-by: Ram <ram.suradkar@portworx.com>

- address review comments - nil check for pvcclaim Signed-off-by: Ram <ram.suradkar@portworx.com>

- adjust ut's to create pvc - address review comments Signed-off-by: Ram <ram.suradkar@portworx.com>

$@ram-infrac$ ram-infrac requested a review from disrani-px May 2, 2019 17:50

$@ram-infrac$ ram-infrac changed the title ~~Snap restore~~ Add support for In-place snapshot restore May 2, 2019

$@ram-infrac$ ram-infrac requested a review from sharma-tapas May 6, 2019 07:13

disrani-px added this to the 2.3.0 milestone May 6, 2019

disrani-px added feature The change adds a new feature release-note Information about this change needs to be added to the release note labels May 6, 2019

disrani-px reviewed May 16, 2019

View reviewed changes

$@ram-infrac$ ram-infrac mentioned this pull request May 20, 2019

In-Place Volume Snapshot restore (2) #377

Merged

$@ram-infrac$ ram-infrac force-pushed the snapRestore branch from 861bc49 to f4fdf10 Compare May 21, 2019 02:16

$@ram-infrac$ ram-infrac force-pushed the snapRestore branch from 9df78c7 to 53916c9 Compare June 3, 2019 09:47

$@ram-infrac$ ram-infrac force-pushed the snapRestore branch from 53916c9 to d001fc3 Compare June 3, 2019 10:00

disrani-px approved these changes Jun 4, 2019

View reviewed changes

$@ram-infrac$ ram-infrac force-pushed the snapRestore branch from 83a8c03 to c2c929e Compare June 4, 2019 10:45

$@ram-infrac$ ram-infrac mentioned this pull request Jun 4, 2019

Add storcktl cmd for volumesnapshotrestore crd's #399

Merged

disrani-px approved these changes Jun 5, 2019

View reviewed changes

Ram added 4 commits June 6, 2019 10:09

Add stork scheduler check for in-place restore

7dc9300

- address review comments - nil check for pvcclaim Signed-off-by: Ram <ram.suradkar@portworx.com>

Add unit test for restore check in extender_test

8d27347

- adjust ut's to create pvc - address review comments Signed-off-by: Ram <ram.suradkar@portworx.com>

$@ram-infrac$ ram-infrac force-pushed the snapRestore branch from c2c929e to 8d27347 Compare June 6, 2019 04:47

$@ram-infrac$ ram-infrac merged commit 0525e3e into master Jun 6, 2019

$@ram-infrac$ ram-infrac deleted the snapRestore branch June 10, 2019 15:56

Conversation

ram-infrac commented May 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

disrani-px left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

disrani-px Jun 4, 2019

Choose a reason for hiding this comment

Uh oh!

ram-infrac Jun 4, 2019

Choose a reason for hiding this comment

Uh oh!

disrani-px Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ram-infrac commented Jun 4, 2019

Uh oh!

disrani-px commented Jun 5, 2019

Uh oh!

disrani-px Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

disrani-px commented Jun 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

$@ram-infrac$ ram-infrac commented May 2, 2019

codecov bot commented Jun 3, 2019 •

edited

Loading

$@ram-infrac$ ram-infrac Jun 4, 2019