Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

rook-ceph: Add reclaim_policy #1369

Merged
merged 4 commits into from
Mar 26, 2021
Merged

rook-ceph: Add reclaim_policy #1369

merged 4 commits into from
Mar 26, 2021

Conversation

surajssd
Copy link
Member

@surajssd surajssd commented Feb 11, 2021 •
edited
Loading

  • Change the default behaviour of rook-ceph storage class to Retain
    from Delete.
  • Add a parameter reclaim_policy to storage_class block, to let user
    override the default behaviour.

Fixes: #1003

@surajssd
Copy link
Member Author

surajssd commented Feb 12, 2021 •
edited
Loading

Blocked on #1342 for tests.

@surajssd surajssd force-pushed the surajssd/change-reclaim-policy branch from f82a7bf to a22acfa Compare March 4, 2021 07:18
@surajssd surajssd marked this pull request as ready for review March 4, 2021 07:18
@surajssd surajssd changed the title add reclaim policy rook-ceph: Add reclaim_policy Mar 4, 2021
@surajssd surajssd requested review from iaguis and invidian March 4, 2021 09:50
iaguis
iaguis reviewed Mar 4, 2021
View reviewed changes
Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

We need some doc changes, right? Otherwise it looks good

invidian
invidian reviewed Mar 4, 2021
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

My questions from #1003 (comment) are still applicable. IMO this change lacks the reasoning behind.

@iaguis
Copy link
Contributor

iaguis commented Mar 4, 2021

My questions from #1003 (comment) are still applicable. IMO this change lacks the reasoning behind.

I'd say let's add same functionality and change the default for OpenEBS too and explain the reasoning, e.g. to avoid possibly surprising data loss when deleting the component.

@invidian
Copy link
Member

invidian commented Mar 4, 2021

to avoid possibly surprising data loss when deleting the component.

The component like prometheus-operator, right? Not like OpenEBS or Rook-Ceph.

@iaguis
Copy link
Contributor

iaguis commented Mar 4, 2021

The component like prometheus-operator, right? Not like OpenEBS or Rook-Ceph.

Right, components (or even user apps) using storage.

@invidian
Copy link
Member

invidian commented Mar 4, 2021 •
edited
Loading

I'd say let's add same functionality and change the default for OpenEBS too and explain the reasoning, e.g. to avoid possibly surprising data loss when deleting the component.

I'm a bit skeptical about changing the Kubernetes default, which I'd hope be more thoroughly selected. I don't mind making the reclaim policy customizable for components or even as default for storage class.

@surajssd surajssd force-pushed the surajssd/change-reclaim-policy branch from a22acfa to 97573d1 Compare March 8, 2021 09:42
@surajssd
Copy link
Member Author

surajssd commented Mar 8, 2021

Done PTAL @iaguis @invidian.

invidian
invidian previously approved these changes Mar 8, 2021
View reviewed changes
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

Looks OK

@surajssd surajssd force-pushed the surajssd/change-reclaim-policy branch from 97573d1 to 1eaf280 Compare March 24, 2021 12:22
iaguis
iaguis previously approved these changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

lgtm

@surajssd
rook-ceph: Add reclaim_policy
5a9b3fc 
- Change the default behaviour of rook-ceph storage class to `Retain`
  from `Delete`, so if a user accidentally deletes a PVC they don't lose
  all the data in it.
- Add a parameter `reclaim_policy` to `storage_class` block, to let user
  override the default behaviour.

Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd
openebs-storage-class: Add reclaim_policy
7be7e9a 
- Change the default behaviour of openebs-storage-class storage class to
  `Retain` from `Delete`, so if a user accidentally deletes a PVC they
  don't lose all the data in it.
- Add a parameter `reclaim_policy` to `storage_class` block, to let user
  override the default behaviour.

Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd
aws-ebs-csi-driver: Add reclaim_policy
2f46341 
- Change the default behaviour of aws-ebs-csi-driver storage class to
  `Retain` from `Delete`, so if a user accidentally deletes a PVC they
  don't lose all the data in it.
- Add a parameter `reclaim_policy` to `storage_class` block, to let user
  override the default behaviour.

Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd
ci: For aws-ebs driver use Delete reclaim_policy
237560a 
The default reclaim policy is set to Retain if the PVs are not deleted
explicitly by the user they might remain around costing money.

This change is introduced to reduce the proliferation of unused PVs that
might be left as a result of multiple CI runs.

Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd surajssd force-pushed the surajssd/change-reclaim-policy branch from 1eaf280 to 237560a Compare March 25, 2021 14:47
@surajssd surajssd merged commit 4e74fb6 into master Mar 26, 2021
@surajssd surajssd deleted the surajssd/change-reclaim-policy branch March 26, 2021 14:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@iaguis iaguis iaguis approved these changes

@invidian invidian invidian left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change the default reclaim policy of rook-ceph to Retain
3 participants
@surajssd @iaguis @invidian