Create a cloned OpenEBS (jiva) Volume from a snapshot #283
Conversation
This PR will add snapshot API for clone feature. Signed-off-by: prateekpandey14 <prateekpandey14@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #283 +/- ##
=======================================
Coverage 27.72% 27.72%
=======================================
Files 58 58
Lines 4476 4476
=======================================
Hits 1241 1241
Misses 3089 3089
Partials 146 146
Continue to review full report at Codecov.
|
| @@ -25,6 +25,8 @@ func (s *HTTPServer) snapshotSpecificRequest(resp http.ResponseWriter, req *http | |||
| return s.snapshotCreate(resp, req) | |||
| case strings.Contains(path, "/revert/"): | |||
| return s.snapshotRevert(resp, req) | |||
| case strings.Contains(path, "/clone/"): | |||
There was a problem hiding this comment.
Do you think it will be good if we explain somewhere in the comments or commit or PR on:
- Snapshot
- Clone
- Promote
This helps contributors as well as docs.
There was a problem hiding this comment.
Workflow is changed now, API no longer exists.
| @@ -167,6 +169,17 @@ func (s *HTTPServer) snapshotList(resp http.ResponseWriter, req *http.Request, v | |||
|
|
|||
| } | |||
|
|
|||
| // SnapshotClone is http handler for restore a snapshot to a persistent volume | |||
There was a problem hiding this comment.
Some more explanation might be nice. Some questions that arise here are:
- Are the PV already available?
- Are the snapshots already available?
- Is this possible if all are in the same namespace, different namespaces, different clusters?
You need not put these info here. But this should be answered somewhere.
Was there a design doc that tries to talk about these things?
There was a problem hiding this comment.
Workflow is changed now, API no longer exists.
| // SnapshotClone is http handler for restore a snapshot to a persistent volume | ||
| func (s *HTTPServer) snapshotClone(resp http.ResponseWriter, req *http.Request) (interface{}, error) { | ||
|
|
||
| clone, err := s.volumeAdd(resp, req) |
There was a problem hiding this comment.
It is adding a volume. Why are we calling it as a clone.
There was a problem hiding this comment.
Let us not reuse the methods here & inject if else conditionals throughout the code.
We will be better off if we go for well defined/dedicated structures that handle clone & snapshot respectively.
| // | ||
| // NOTE: | ||
| // This is replaced at runtime | ||
| JivaCloneIPHolder JivaAnnotations = "__CLONE_IP__" |
There was a problem hiding this comment.
Why are we calling these as annotations? I guess this is an old type that we are re-using. But lets make it a point to avoid once we feel that this design/code is not good.
There was a problem hiding this comment.
No, these are coming from the Volume struct, just using this to pass as replica args .
types/v1/util.go
Outdated
|
|
||
| repArgs := make([]string, len(JivaReplicaArgs)) | ||
| if cloneIP == "" { |
There was a problem hiding this comment.
I believe its best to avoid this if else by structuring the code better.
Probably a new struct that deals with clone would have been great.
When the complexity arises with new business logic/features, a well defined struct will be able to handle this better.
This PR will extend the maya-apiserver volume create API's to create volume clone feature. Signed-off-by: prateekpandey14 <prateekpandey14@gmail.com>
prateekpandey14
changed the title
|
@prateekpandey14 The changes look fine. Couple of high level comments.
|
kmova
changed the title
What this PR does / why we need it:
This PR will extends the volume create API to create a cloned volume by taking in source volume and source snapshot details. This feature will be used by the Snapshot provisioner to promote a snapshot into a PV.
Special Note:
This PR is part of a feature that spans across multiple repos.
Related PRs:
Note on the Changes:
The Volume Create Spec is extended to include:
The Jiva Replica command line takes 3 new ags:
cloneIP: This is frontendIP of running source controller, which will be used to make a clone request, will be passed as a argument while starting clone replica.type=clone: Type of replica, now we can pass type ascloneto trigger the clone API of jiva, which says the new replica is getting added as clone not as usual replica.snapName: Name of the snapshot which will be synced/cloned to a new persistent volume.The replica when it comes up as clone, will register itself with it controller in write-only mode. Then issue a sync request to the source controller. Restore to the snapshot provided and convert itself into read-write mode.
Current implementation only support launching of clone volume in read-only mode. To support read-write clones, further cases need to be handled.
The changes were tested by using custom images of jiva and provisioner:
demo-vol1-claim( 5GB volume ), which has been formatted/mounted, and write some data inside that.fastfuriousfor the ``demo-vol1-claim` pvc.fastfuriousto restore as a new PVCsnapshot-pv-provisioning**Detailed Output of snapshot and new PVC **:
fastfuriousof a pvc `demo-vol1-claimfastfuriousas a new PVCsnapshot-pv-provisioningSigned-off-by: prateekpandey14 prateekpandey14@gmail.com