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.

Sign up for GitHub

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

Jump to bottom

Delete temp pvc #1388

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Delete temp pvc #1388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a7360d7
ReplDest - spec update to cleanup temp dest pvc
tesshuflower Sep 7, 2024
388f108
restic cleanup pvcs e2e test
tesshuflower Sep 12, 2024
9027a71
delete dynamic dest pvcs docs
tesshuflower Sep 12, 2024
2d95cd2
e2e test fix for check on pvc deleted
tesshuflower Sep 13, 2024
affccc3
rename ReplDest spec field to cleanupTempPVC
tesshuflower Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions api/v1alpha1/replicationdestination_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,18 @@ type ReplicationDestinationVolumeOptions struct {
// accessModes must be specified.
//+optional
DestinationPVC *string `json:"destinationPVC,omitempty"`
// Set this to true to delete the temp destination PVC (dynamically provisioned
// by VolSync) at the end of each successful ReplicationDestination sync iteration.
// If destinationPVC is set, this will have no effect, VolSync will only
// cleanup temp PVCs that it deployed.
// Note that if this is set to true, every sync this ReplicationDestination
// makes will re-provision a new temp destination PVC and all data
// will need to be sent again during the sync.
// Dynamically provisioned destination PVCs will always be deleted if the
// owning ReplicationDestination is removed, even if this setting is false.
// The default is false.
//+optional
CleanupTempPVC bool `json:"cleanupTempPVC,omitempty"`
}

type ReplicationDestinationRsyncSpec struct {
Expand Down Expand Up @@ -225,6 +237,13 @@ type ReplicationDestinationResticSpec struct {
// accessModes can be used to set the accessModes of restic metadata cache volume
//+optional
CacheAccessModes []corev1.PersistentVolumeAccessMode `json:"cacheAccessModes,omitempty"`
// Set this to true to delete the restic cache PVC (dynamically provisioned
// by VolSync) at the end of each successful ReplicationDestination sync iteration.
// Cache PVCs will always be deleted if the owning ReplicationDestination is
// removed, even if this setting is false.
// The default is false.
//+optional
CleanupCachePVC bool `json:"cleanupCachePVC,omitempty"`
// Previous specifies the number of image to skip before selecting one to restore from
//+optional
Previous *int32 `json:"previous,omitempty"`
Expand Down
60 changes: 60 additions & 0 deletions bundle/manifests/volsync.backube_replicationdestinations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down Expand Up @@ -1394,6 +1407,27 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupCachePVC:
description: |-
Set this to true to delete the restic cache PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
Cache PVCs will always be deleted if the owning ReplicationDestination is
removed, even if this setting is false.
The default is false.
type: boolean
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down Expand Up @@ -2685,6 +2719,19 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down Expand Up @@ -2838,6 +2885,19 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down
2 changes: 1 addition & 1 deletion bundle/manifests/volsync.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ metadata:
}
]
capabilities: Basic Install
createdAt: "2024-09-05T23:36:37Z"
createdAt: "2024-09-16T17:00:41Z"
olm.skipRange: '>=0.4.0 <0.11.0'
operators.operatorframework.io/builder: operator-sdk-v1.31.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
Expand Down
60 changes: 60 additions & 0 deletions config/crd/bases/volsync.backube_replicationdestinations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down Expand Up @@ -1394,6 +1407,27 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupCachePVC:
description: |-
Set this to true to delete the restic cache PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
Cache PVCs will always be deleted if the owning ReplicationDestination is
removed, even if this setting is false.
The default is false.
type: boolean
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down Expand Up @@ -2685,6 +2719,19 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down Expand Up @@ -2838,6 +2885,19 @@ spec:
create.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
cleanupTempPVC:
description: |-
Set this to true to delete the temp destination PVC (dynamically provisioned
by VolSync) at the end of each successful ReplicationDestination sync iteration.
If destinationPVC is set, this will have no effect, VolSync will only
cleanup temp PVCs that it deployed.
Note that if this is set to true, every sync this ReplicationDestination
makes will re-provision a new temp destination PVC and all data
will need to be sent again during the sync.
Dynamically provisioned destination PVCs will always be deleted if the
owning ReplicationDestination is removed, even if this setting is false.
The default is false.
type: boolean
copyMethod:
description: |-
copyMethod describes how a point-in-time (PiT) image of the destination
Expand Down
1 change: 1 addition & 0 deletions controllers/mover/rclone/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ func (rb *Builder) FromDestination(client client.Client, logger logr.Logger,
isSource: isSource,
paused: destination.Spec.Paused,
mainPVCName: destination.Spec.Rclone.DestinationPVC,
cleanupTempPVC: destination.Spec.Rclone.CleanupTempPVC,
customCASpec: destination.Spec.Rclone.CustomCA,
privileged: privileged,
latestMoverStatus: destination.Status.LatestMoverStatus,
Expand Down
4 changes: 3 additions & 1 deletion controllers/mover/rclone/mover.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ type Mover struct {
privileged bool // true if the mover should have elevated privileges
latestMoverStatus *volsyncv1alpha1.MoverStatus
moverConfig volsyncv1alpha1.MoverConfig
// Destination-only fields
cleanupTempPVC bool
}

var _ mover.Mover = &Mover{}
Expand Down Expand Up @@ -195,7 +197,7 @@ func (m *Mover) ensureDestinationPVC(ctx context.Context) (*corev1.PersistentVol
return m.vh.UseProvidedPVC(ctx, dataPVCName)
}
// Need to allocate the incoming data volume
return m.vh.EnsureNewPVC(ctx, m.logger, dataPVCName)
return m.vh.EnsureNewPVC(ctx, m.logger, dataPVCName, m.cleanupTempPVC)
}

func (m *Mover) getDestinationPVCName() (bool, string) {
Expand Down
28 changes: 27 additions & 1 deletion controllers/mover/rclone/rclone_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1734,7 +1734,7 @@ var _ = Describe("Rclone as a destination", func() {
destVolCap = resource.MustParse("6Gi")
rd.Spec.Rclone.Capacity = &destVolCap
})
It("creates a temporary PVC", func() {
It("creates a dynamic PVC", func() {
pvc, e := mover.ensureDestinationPVC(ctx)
Expect(e).NotTo(HaveOccurred())
Expect(pvc).NotTo(BeNil())
Expand All @@ -1743,6 +1743,18 @@ var _ = Describe("Rclone as a destination", func() {
// It should NOT be marked for cleaned up
Expect(pvc.Labels).ToNot(HaveKey("volsync.backube/cleanup"))
})
When("cleanupTempPVC is set to true", func() {
BeforeEach(func() {
rd.Spec.Rclone.CleanupTempPVC = true
})
It("The dynamic PVC should be marked for deletion", func() {
pvc, e := mover.ensureDestinationPVC(ctx)
Expect(e).NotTo(HaveOccurred())
Expect(pvc).NotTo(BeNil())
// Cleanup label should be set on this PVC
Expect(pvc.Labels).To(HaveKey("volsync.backube/cleanup"))
})
})
})
When("a destination volume is supplied", func() {
var dPVC *corev1.PersistentVolumeClaim
Expand Down Expand Up @@ -1781,7 +1793,21 @@ var _ = Describe("Rclone as a destination", func() {
Expect(pvc.OwnerReferences).To(BeEmpty())
// It won't be cleaned up at the end of the transfer
Expect(pvc.Labels).NotTo(HaveKey("volsync.backube/cleanup"))
})

// We will NOT cleanup a users destination PVC, only ones we create dynamically
// So we should ignore the cleanupTempPVC setting if destinationPVC is set
When("cleanupTempPVC is set to true", func() {
BeforeEach(func() {
rd.Spec.Rclone.CleanupTempPVC = true
})
It("The user supplied PVC should NOT be marked for deletion", func() {
pvc, e := mover.ensureDestinationPVC(ctx)
Expect(e).NotTo(HaveOccurred())
Expect(pvc).NotTo(BeNil())
// Cleanup label should NOT be set on this PVC
Expect(pvc.Labels).NotTo(HaveKey("volsync.backube/cleanup"))
})
})
})
})
Expand Down
2 changes: 2 additions & 0 deletions controllers/mover/restic/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,12 @@ func (rb *Builder) FromDestination(client client.Client, logger logr.Logger,
cacheAccessModes: destination.Spec.Restic.CacheAccessModes,
cacheCapacity: destination.Spec.Restic.CacheCapacity,
cacheStorageClassName: destination.Spec.Restic.CacheStorageClassName,
cleanupCachePVC: destination.Spec.Restic.CleanupCachePVC,
repositoryName: destination.Spec.Restic.Repository,
isSource: isSource,
paused: destination.Spec.Paused,
mainPVCName: destination.Spec.Restic.DestinationPVC,
cleanupTempPVC: destination.Spec.Restic.CleanupTempPVC,
customCASpec: volsyncv1alpha1.CustomCASpec(destination.Spec.Restic.CustomCA),
privileged: privileged,
restoreAsOf: destination.Spec.Restic.RestoreAsOf,
Expand Down
13 changes: 8 additions & 5 deletions controllers/mover/restic/mover.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@
previous *int32
restoreAsOf *string
enableFileDeletionOnRestore bool
cleanupTempPVC bool
cleanupCachePVC bool
}

var _ mover.Mover = &Mover{}
Expand Down Expand Up @@ -113,7 +115,8 @@
}

// Allocate cache volume
cachePVC, err := m.ensureCache(ctx, dataPVC)
// cleanupCachePVC will always be false for replicationsources - it's only set in the builder FromDestination()
cachePVC, err := m.ensureCache(ctx, dataPVC, m.cleanupCachePVC)

Check warning on line 119 in controllers/mover/restic/mover.go

View check run for this annotation

Codecov / codecov/patch

controllers/mover/restic/mover.go#L119 

Added line #L119 was not covered by tests
if cachePVC == nil || err != nil {
return mover.InProgress(), err
}
Expand Down Expand Up @@ -177,7 +180,7 @@
}

func (m *Mover) ensureCache(ctx context.Context,
dataPVC *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) {
dataPVC *corev1.PersistentVolumeClaim, isTemporary bool) (*corev1.PersistentVolumeClaim, error) {
// Create a separate vh for the Restic cache volume that's based on the main
// vh, but override options where necessary.
cacheConfig := []volumehandler.VHOption{
Expand Down Expand Up @@ -213,8 +216,8 @@

// Allocate cache volume
cacheName := mover.VolSyncPrefix + m.owner.GetName() + "-cache"
m.logger.Info("allocating cache volume", "PVC", cacheName)
return cacheVh.EnsureNewPVC(ctx, m.logger, cacheName)
m.logger.Info("allocating cache volume", "PVC", cacheName, "isTemporary", isTemporary)
return cacheVh.EnsureNewPVC(ctx, m.logger, cacheName, isTemporary)
}

func (m *Mover) ensureSourcePVC(ctx context.Context) (*corev1.PersistentVolumeClaim, error) {
Expand Down Expand Up @@ -249,7 +252,7 @@
return m.vh.UseProvidedPVC(ctx, dataPVCName)
}
// Need to allocate the incoming data volume
return m.vh.EnsureNewPVC(ctx, m.logger, dataPVCName)
return m.vh.EnsureNewPVC(ctx, m.logger, dataPVCName, m.cleanupTempPVC)
}

func (m *Mover) getDestinationPVCName() (bool, string) {
Expand Down
Loading