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

feat: add support pvc deletion when need #1828

Merged
merged 12 commits into from
Nov 1, 2023

Conversation

jiuker
Copy link
Contributor

@jiuker jiuker commented Oct 23, 2023

fix: #1803
add support pvc deletion when need

 - name: pool-0
   reclaimStorage: true

When tenant has gone. The pvc will been deleted.
That's false by default. Means will not delete the pvcs by default.

@shtripat
Copy link
Contributor

shtripat commented Oct 23, 2023

I think this is disruptive action and should not be default option and that's taken care as part of labels. Also kubectl minio tenant delete should be enabled for this action with a flag e.g. --delete-pvcs in addition to the --force option I feel.
@dvaldivia @pjuarezd @cniackz thoughts?

@jiuker
Copy link
Contributor Author

jiuker commented Oct 23, 2023

I think this is disruptive action and should not be default option and that's taken care as part of labels. Also kubectl minio tenant delete should be enabled for this action with a flag e.g. --delete-pvcs in addition to the --force option I feel. @dvaldivia @pjuarezd @cniackz thoughts?

Will not delete pvcs by default. @shtripat

@shtripat
Copy link
Contributor

I think this is disruptive action and should not be default option and that's taken care as part of labels. Also kubectl minio tenant delete should be enabled for this action with a flag e.g. --delete-pvcs in addition to the --force option I feel. @dvaldivia @pjuarezd @cniackz thoughts?

Will not delete pvcs by default. @shtripat

Yes, if we set labels explicitly then only it would happen. Do we plan kubectl minio tenant changes separately?

@jiuker
Copy link
Contributor Author

jiuker commented Oct 23, 2023

I think this is disruptive action and should not be default option and that's taken care as part of labels. Also kubectl minio tenant delete should be enabled for this action with a flag e.g. --delete-pvcs in addition to the --force option I feel. @dvaldivia @pjuarezd @cniackz thoughts?

Will not delete pvcs by default. @shtripat

Yes, if we set labels explicitly then only it would happen. Do we plan kubectl minio tenant changes separately?

That happen to pipeline test. Maybe plugin are too heavy for those scenes. @shtripat

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

We must be cautious with this, we need to make sure we document it properly such that users know that this is a disastrous thing to keep it enabled.

pkg/apis/minio.min.io/v2/types.go Outdated Show resolved Hide resolved
@jiuker jiuker requested a review from harshavardhana October 24, 2023 01:31
guozhi.li added 2 commits October 24, 2023 09:33
Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

I am okay with this change with the caveat and documentation.

@cniackz cniackz self-requested a review November 1, 2023 19:10
Copy link
Contributor

@cniackz cniackz left a comment

Choose a reason for hiding this comment

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

It looks good to me

@cniackz cniackz merged commit 50b2f3b into minio:master Nov 1, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No support for declarative cleanup of PVCs
4 participants