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.

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

Introducing a ReclaimPolicy for ephemerally created Volume resource #1153

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ushabelgur
Copy link
Contributor

Proposed Changes

  • Add ReclaimPolicy for ephemerally created Volume resource with 2 supported modes:
    • Retain: the resource is not deleted after the managing resource has been deleted
    • Delete: the current behavior, the resource is garbage-collected when the managing resource has been deleted
  • By default ReclaimPolicy would be assumed to be Delete to not to not break current behavior.
  • OwnerReference is added to ephemerally created volume only in case of ReclaimPolicy is set to Delete
  • Add wrapper type for VolumeTemplateSpec to support ReclaimPolicy type along with existing VolumeSpec
  • Add Test cases
    [Note: volume_release_controller is already taking care of releasing volumes whose claimer doesn't existing, by setting .spec.claimRef to nil when claimer Machine object is deleted. So not adding any extra logic for this]

Fixes #1114

@ushabelgur ushabelgur self-assigned this Nov 14, 2024
@ushabelgur ushabelgur requested a review from balpert89 November 14, 2024 09:46
@ushabelgur ushabelgur marked this pull request as ready for review November 18, 2024 05:08
@ushabelgur ushabelgur requested a review from a team as a code owner November 18, 2024 05:08
@afritzler afritzler added the enhancement New feature or request label Nov 18, 2024
@@ -36,6 +36,10 @@ func IsDefaultEphemeralControlledBy(o metav1.Object, owner metav1.Object) bool {
return metav1.IsControlledBy(o, owner) && IsEphemeralManagedBy(o, commonv1alpha1.DefaultEphemeralManager)
}

func IsDefaultEphemeralOrControlledBy(o metav1.Object, owner metav1.Object) bool {
return metav1.IsControlledBy(o, owner) || IsEphemeralManagedBy(o, commonv1alpha1.DefaultEphemeralManager)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I added new method with OR condition here, as above original method IsDefaultEphemeralControlledBy() is used by all other ephemeral controllers and it will break behavior of those controllers.

@lukas016 lukas016 force-pushed the osc/enh-volume-reclaim branch from 987deb1 to c7fbc7e Compare November 21, 2024 05:21
@lukas016 lukas016 force-pushed the osc/enh-volume-reclaim branch from c7fbc7e to 3eb6dca Compare November 25, 2024 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce a reclaim policy for ephemerally created Volume resources
3 participants