fix(snapshot): garbage collect creating snapshots with no transactions

Signed-off-by: Abhinandan Purkait <[email protected]>
openebs · Jul 3, 2023 · dbe8741 · dbe8741
1 parent 2124d48
commit dbe8741
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 11 deletions.
diff --git a/control-plane/agents/src/bin/core/controller/reconciler/snapshot/garbage_collector.rs b/control-plane/agents/src/bin/core/controller/reconciler/snapshot/garbage_collector.rs
@@ -2,7 +2,7 @@ use crate::{
     controller::{
         reconciler::{poller::ReconcilerWorker, GarbageCollect},
         resources::{
-            operations::{ResourceLifecycleWithLifetime, ResourcePruning},
+            operations::{ResourceLifecycleWithLifetime, ResourcePruning, ResourceSnapshotting},
             operations_helper::{OperationSequenceGuard, SpecOperationsHelper},
             OperationGuardArc,
         },
@@ -59,6 +59,7 @@ impl GarbageCollect for OperationGuardArc<VolumeSnapshot> {
             self.destroy_deleting(context).await,
             creating_orphaned_volume_snapshot_reconciler(self, context).await,
             prune_volume_snapshot_reconciler(self, context).await,
+            delete_no_transaction_volume_snapshot_reconciler(self, context).await,
         ])
     }
 
@@ -90,15 +91,12 @@ async fn deleting_volume_snapshot_reconciler(
         return Ok(PollerState::Idle);
     }
 
-    let Some(snap_rsc) = context.specs().volume_snapshot_rsc(snapshot.uuid()) else {
-        return Ok(PollerState::Idle);
-    };
+    let snap_rsc = snapshot.resource().clone();
 
-    let snap_uuid = snapshot.uuid().clone();
     match snapshot
         .destroy(
             context.registry(),
-            &DestroyVolumeSnapshotRequest::new(snap_rsc, None, snap_uuid),
+            &DestroyVolumeSnapshotRequest::new(snap_rsc, None, snapshot.uuid().clone()),
         )
         .await
     {
@@ -133,15 +131,12 @@ async fn creating_orphaned_volume_snapshot_reconciler(
         return Ok(PollerState::Idle);
     }
 
-    let Some(snap_rsc) = context.specs().volume_snapshot_rsc(snapshot.uuid()) else {
-        return Ok(PollerState::Idle);
-    };
+    let snap_rsc = snapshot.resource().clone();
 
-    let snap_uuid = snapshot.uuid().clone();
     match snapshot
         .destroy(
             context.registry(),
-            &DestroyVolumeSnapshotRequest::new(snap_rsc, None, snap_uuid),
+            &DestroyVolumeSnapshotRequest::new(snap_rsc, None, snapshot.uuid().clone()),
         )
         .await
     {
@@ -181,3 +176,48 @@ async fn prune_volume_snapshot_reconciler(
 
     Ok(PollerState::Idle)
 }
+
+#[tracing::instrument(skip(snapshot, context), level = "trace", fields(snapshot.id = %snapshot.uuid(), request.reconcile = true))]
+async fn delete_no_transaction_volume_snapshot_reconciler(
+    snapshot: &mut OperationGuardArc<VolumeSnapshot>,
+    context: &PollContext,
+) -> PollResult {
+    // Only proceed if the spec is in creating and has no transactions.
+    if !snapshot.as_ref().status().creating()
+        && !snapshot.as_ref().metadata().transactions().is_empty()
+    {
+        return Ok(PollerState::Idle);
+    }
+
+    let snap_rsc = snapshot.resource().clone();
+
+    match context
+        .specs()
+        .volume(snapshot.as_ref().spec().source_id())
+        .await
+    {
+        Ok(mut vol_op_guard) => {
+            vol_op_guard
+                .destroy_snap(
+                    context.registry(),
+                    &DestroyVolumeSnapshotRequest::new(
+                        snap_rsc,
+                        Some(vol_op_guard.uuid().clone()),
+                        snapshot.uuid().clone(),
+                    ),
+                )
+                .await
+        }
+        Err(_) => {
+            snapshot
+                .destroy(
+                    context.registry(),
+                    &DestroyVolumeSnapshotRequest::new(snap_rsc, None, snapshot.uuid().clone()),
+                )
+                .await
+        }
+    }
+    .ok();
+
+    Ok(PollerState::Idle)
+}
diff --git a/control-plane/agents/src/bin/core/controller/resources/mod.rs b/control-plane/agents/src/bin/core/controller/resources/mod.rs
@@ -198,6 +198,13 @@ impl<T: OperationSequencer + Sized, R> OperationGuard<T, R> {
     }
 }
 
+impl<T: Clone + Debug + AsOperationSequencer, R> OperationGuard<ResourceMutex<T>, R> {
+    /// Get the ResourceMutex for the concerned resource.
+    pub fn resource(&self) -> &ResourceMutex<T> {
+        &self.inner
+    }
+}
+
 /// Tracing simple string messages with resource specific information
 /// eg, volume.uuid for volumes and replica.uuid for replicas
 pub(crate) trait TraceStrLog {