-
Notifications
You must be signed in to change notification settings - Fork 6
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
Integrate PVC creation feature using snapshot to replication controller for single cluster #153
Draft
YichenYa0
wants to merge
29
commits into
main
Choose a base branch
from
feature/snapshot-failover
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+638
−25
Draft
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
5bdb4cf
migrated volume snapshot pipeline from repctl to rg controller
shainabanduri3 055a231
removed condition to abort if vsc not specified
shainabanduri3 f46ad16
add volume handle label
YichenYa0 ac4fd16
add default storage class
YichenYa0 7863a7e
fix regex
YichenYa0 6554319
fix snclass variable
YichenYa0 a3e9cf0
fix snapshot class creation
YichenYa0 5274ee6
add create pvcs unit test
hynguyenw4dell f97e713
add fernando code
hynguyenw4dell ee9b7f6
add fernando code
hynguyenw4dell f70ac9d
fix default class creation logic
YichenYa0 f3e3d7b
remove unnecessary err msg
YichenYa0 40fd807
handle namespace creation
YichenYa0 ff369e6
add snapshot test
hynguyenw4dell 262cf34
adjust snapshot test
hynguyenw4dell 44ce58a
add snapshot tests, change latest snapshotcontent retrieval
YichenYa0 976fc79
finish writing assertions for snapshot test
YichenYa0 5974523
add test scenario of default snapshotclass
YichenYa0 e2c8360
linting
YichenYa0 a738169
fixed snapshotclass detection
YichenYa0 6a31b7b
fix linting
YichenYa0 d82c267
add documentation, make consistent function naming
YichenYa0 cad09a4
update comment
YichenYa0 5cad19e
Reorganize pvc creation logic
falfaroc cb90033
Restructure pvc creation from snapshot logic
falfaroc c825a90
Improve and add unit tests for snapshots
falfaroc 901dbaf
Address PR check issues
falfaroc a724cd6
Merge branch 'main' into feature/snapshot-failover
donatwork e310fb5
Merge branch 'main' into feature/snapshot-failover
donatwork File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ import ( | |
v1 "k8s.io/api/core/v1" | ||
"k8s.io/apimachinery/pkg/api/errors" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/types" | ||
"k8s.io/utils/pointer" | ||
reconcile "sigs.k8s.io/controller-runtime/pkg/controller" | ||
"sigs.k8s.io/controller-runtime/pkg/ratelimiter" | ||
|
||
|
@@ -317,6 +319,8 @@ func (r *ReplicationGroupReconciler) Reconcile(ctx context.Context, req ctrl.Req | |
} | ||
|
||
func (r *ReplicationGroupReconciler) processLastActionResult(ctx context.Context, group *repv1.DellCSIReplicationGroup, remoteClient connection.RemoteClusterClient, log logr.Logger) error { | ||
var returnErr error | ||
|
||
if len(group.Status.Conditions) == 0 || group.Status.LastAction.Time == nil { | ||
log.V(common.InfoLevel).Info("No action to process") | ||
return nil | ||
|
@@ -332,21 +336,25 @@ func (r *ReplicationGroupReconciler) processLastActionResult(ctx context.Context | |
return nil | ||
} | ||
|
||
if val == group.Status.LastAction.Time.GoString() { | ||
if val == group.Status.LastAction.Time.String() { | ||
log.V(common.InfoLevel).Info("Last action has already been processed") | ||
return nil | ||
} | ||
|
||
if strings.Contains(group.Status.LastAction.Condition, "CREATE_SNAPSHOT") { | ||
if err := r.processSnapshotEvent(ctx, group, remoteClient, log); err != nil { | ||
return err | ||
returnErr = err | ||
} | ||
} | ||
|
||
// Informing the RG that the last action has been processed. | ||
controller.AddAnnotation(group, controller.ActionProcessedTime, group.Status.LastAction.Time.GoString()) | ||
// Informing the RG that the last action has been processed. Do not retry on error. | ||
controller.AddAnnotation(group, controller.ActionProcessedTime, group.Status.LastAction.Time.String()) | ||
err := r.Update(ctx, group) | ||
if err != nil { | ||
return fmt.Errorf("failed to update rg") | ||
} | ||
|
||
return r.Update(ctx, group) | ||
return returnErr | ||
} | ||
|
||
func (r *ReplicationGroupReconciler) processSnapshotEvent(ctx context.Context, group *repv1.DellCSIReplicationGroup, remoteClient connection.RemoteClusterClient, log logr.Logger) error { | ||
|
@@ -365,29 +373,74 @@ func (r *ReplicationGroupReconciler) processSnapshotEvent(ctx context.Context, g | |
return err | ||
} | ||
|
||
if _, err := remoteClient.GetSnapshotClass(ctx, actionAnnotation.SnapshotClass); err != nil { | ||
log.Error(err, "Snapshot class does not exist on remote cluster. Not creating the remote snapshots.") | ||
return err | ||
} | ||
namespace := actionAnnotation.SnapshotNamespace | ||
|
||
if _, err := remoteClient.GetNamespace(ctx, actionAnnotation.SnapshotNamespace); err != nil { | ||
log.V(common.InfoLevel).Info("Namespace - " + actionAnnotation.SnapshotNamespace + " not found, creating it.") | ||
nsRef := makeNamespaceReference(actionAnnotation.SnapshotNamespace) | ||
|
||
err = remoteClient.CreateNamespace(ctx, nsRef) | ||
if _, err := remoteClient.GetNamespace(ctx, namespace); err != nil { | ||
log.V(common.InfoLevel).Info("Namespace - " + namespace + " not found, creating it.") | ||
err = createNamespace(ctx, namespace, remoteClient) | ||
if err != nil { | ||
msg := "unable to create the desired namespace" + actionAnnotation.SnapshotNamespace | ||
log.V(common.ErrorLevel).Error(err, msg) | ||
return err | ||
} | ||
} | ||
|
||
// create default snapshot class if it does not exist | ||
// example driver class: csi-vxflexos.dellemc.com | ||
// example default snapshot class: default-csi-vxflexos | ||
snClass := group.Annotations[controller.SnapshotClass] | ||
driverClass := group.Labels[controller.DriverName] | ||
if snClass == "" { | ||
part := strings.Split(driverClass, ".")[0] | ||
snClass = "default-" + strings.TrimPrefix(part, "csi-") + "-snapshotclass" | ||
} else { | ||
if _, err := remoteClient.GetSnapshotClass(ctx, snClass); err != nil { | ||
log.V(common.ErrorLevel).Error(err, "user defined snapshot class does not exist") | ||
return err | ||
} | ||
} | ||
|
||
shouldCreatePvc := false | ||
storageClass := group.Annotations[r.Domain+"/snapshotStorageClass"] | ||
createPVC := group.Annotations[r.Domain+"/snapshotCreatePVC"] | ||
Comment on lines
+402
to
+403
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think, in the other PR, there were more concise consts for these array indexes. |
||
|
||
if createPVC == "true" && storageClass != "" { | ||
shouldCreatePvc = true | ||
} | ||
|
||
sc, err := remoteClient.GetSnapshotClass(ctx, snClass) | ||
if err != nil { | ||
if !errors.IsNotFound(err) { | ||
return fmt.Errorf("error getting snapshot class: %s", err.Error()) | ||
} | ||
|
||
log.V(common.InfoLevel).Info("Snapshotclass %s not found, creating a default class", snClass) | ||
sc = makeSnapshotClassRef(driverClass, snClass) | ||
if err = remoteClient.CreateSnapshotClass(ctx, sc); err != nil { | ||
return fmt.Errorf("unable to create default snapshot class: %s", err.Error()) | ||
} | ||
} | ||
|
||
for volumeHandle, snapshotHandle := range lastAction.ActionAttributes { | ||
msg := "ActionAttributes - volumeHandle: " + volumeHandle + ", snapshotHandle: " + snapshotHandle | ||
log.V(common.InfoLevel).Info(msg) | ||
|
||
snapRef := makeSnapReference(snapshotHandle, actionAnnotation.SnapshotNamespace) | ||
sc := makeStorageClassContent(group.Labels[controller.DriverName], actionAnnotation.SnapshotClass) | ||
var pvc *v1.PersistentVolumeClaim | ||
if shouldCreatePvc { | ||
pvc, err = r.getPVCInformation(ctx, group, volumeHandle) | ||
if err != nil { | ||
log.V(common.ErrorLevel).Error(err, "unable to get PVC information") | ||
} | ||
|
||
if pvc != nil && pvc.Namespace == namespace { | ||
log.V(common.InfoLevel).Info("Namespace - " + namespace + " not found, creating clone.") | ||
namespace = "cloned-" + namespace | ||
err = createNamespace(ctx, namespace, remoteClient) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
} | ||
|
||
snapRef := makeSnapReference(snapshotHandle, namespace) | ||
snapContent := makeVolSnapContent(snapshotHandle, volumeHandle, *snapRef, sc) | ||
|
||
err = remoteClient.CreateSnapshotContent(ctx, snapContent) | ||
|
@@ -396,17 +449,62 @@ func (r *ReplicationGroupReconciler) processSnapshotEvent(ctx context.Context, g | |
return err | ||
} | ||
|
||
snapshot := makeSnapshotObject(snapRef.Name, snapContent.Name, sc.ObjectMeta.Name, actionAnnotation.SnapshotNamespace) | ||
snapshot := makeSnapshotObject(snapRef.Name, snapContent.Name, sc.ObjectMeta.Name, namespace) | ||
err = remoteClient.CreateSnapshotObject(ctx, snapshot) | ||
if err != nil { | ||
log.Error(err, "unable to create snapshot object") | ||
return err | ||
} | ||
|
||
if shouldCreatePvc && pvc != nil { | ||
// Check to see if the storage class has replication enabled. Continue making snapshots but not PVCs. | ||
if sc, err := remoteClient.GetStorageClass(ctx, storageClass); err == nil { | ||
if val, ok := sc.Parameters[controller.StorageClassReplicationParam]; ok && val == "true" { | ||
log.V(common.ErrorLevel).Error(err, fmt.Sprintf("storage class %s has replication enabled, PVC %s not created", storageClass, pvc.Name)) | ||
continue | ||
} | ||
} | ||
|
||
newPVC := makePersistentVolumeClaimFromSnapshot(pvc.Name, namespace, snapContent.Spec.VolumeSnapshotRef.Name, storageClass, pvc.Spec) | ||
err = remoteClient.CreatePersistentVolumeClaim(ctx, newPVC) | ||
if err != nil { | ||
log.Error(err, "unable to create PVC") | ||
return err | ||
} | ||
|
||
log.V(common.InfoLevel).Info("Created PVC " + newPVC.Name + " in namespace " + namespace + " from snapshot") | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (r *ReplicationGroupReconciler) getPVCInformation(ctx context.Context, group *repv1.DellCSIReplicationGroup, volumeHandle string) (*v1.PersistentVolumeClaim, error) { | ||
// Retrieve the list of pvcs in the source cluster. | ||
var pvcList v1.PersistentVolumeClaimList | ||
err := r.List(ctx, &pvcList, client.MatchingLabels{controller.ReplicationGroup: group.Name}) | ||
if err != nil { | ||
return nil, fmt.Errorf("unable to get pvcs: %s", err.Error()) | ||
} | ||
|
||
for _, pvc := range pvcList.Items { | ||
pvName := pvc.Spec.VolumeName | ||
|
||
var pv v1.PersistentVolume | ||
err = r.Get(ctx, types.NamespacedName{Name: pvName}, &pv) | ||
if err != nil { | ||
return nil, fmt.Errorf("error getting pv %s: %s", pvName, err.Error()) | ||
} | ||
|
||
if pv.Spec.PersistentVolumeSource.CSI.VolumeHandle == volumeHandle { | ||
fmt.Printf("Found PVC %s with PV %s\n", pvc.Name, pvName) | ||
return &pvc, nil | ||
} | ||
} | ||
|
||
return nil, nil | ||
} | ||
|
||
func makeNamespaceReference(namespace string) *v1.Namespace { | ||
return &v1.Namespace{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
|
@@ -440,10 +538,10 @@ func makeSnapshotObject(snapName, contentName, className, namespace string) *s1. | |
return volsnap | ||
} | ||
|
||
func makeStorageClassContent(driver, snapClass string) *s1.VolumeSnapshotClass { | ||
func makeSnapshotClassRef(driver, snapClass string) *s1.VolumeSnapshotClass { | ||
return &s1.VolumeSnapshotClass{ | ||
Driver: driver, | ||
DeletionPolicy: "Retain", | ||
DeletionPolicy: "Delete", | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: snapClass, | ||
}, | ||
|
@@ -468,6 +566,35 @@ func makeVolSnapContent(snapName, volumeName string, snapRef v1.ObjectReference, | |
return volsnapcontent | ||
} | ||
|
||
func makePersistentVolumeClaimFromSnapshot(name, namespace, snName, storageClass string, pvcSpec v1.PersistentVolumeClaimSpec) *v1.PersistentVolumeClaim { | ||
return &v1.PersistentVolumeClaim{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: name, | ||
Namespace: namespace, | ||
}, | ||
Spec: v1.PersistentVolumeClaimSpec{ | ||
StorageClassName: &storageClass, | ||
AccessModes: pvcSpec.AccessModes, | ||
Resources: pvcSpec.Resources, | ||
DataSource: &v1.TypedLocalObjectReference{ | ||
APIGroup: pointer.String("snapshot.storage.k8s.io"), | ||
Kind: "VolumeSnapshot", | ||
Name: snName, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func createNamespace(ctx context.Context, namespace string, remoteClient connection.RemoteClusterClient) error { | ||
nsRef := makeNamespaceReference(namespace) | ||
err := remoteClient.CreateNamespace(ctx, nsRef) | ||
if err != nil { | ||
return fmt.Errorf("unable to create the desired namespace %s: %s", namespace, err.Error()) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// SetupWithManager start using reconciler by creating new controller managed by provided manager | ||
func (r *ReplicationGroupReconciler) SetupWithManager(mgr ctrl.Manager, limiter ratelimiter.RateLimiter, maxReconcilers int) error { | ||
return ctrl.NewControllerManagedBy(mgr). | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example and the implementation do not match.
Not a big deal, but if you're making any other changes, maybe update here as well.