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: resize pvc #1904

Closed
wants to merge 16 commits into from
Closed

feat: resize pvc #1904

wants to merge 16 commits into from

Conversation

jiuker
Copy link
Contributor

@jiuker jiuker commented Dec 6, 2023

fix #1780

@jiuker jiuker requested review from harshavardhana, dvaldivia, shtripat and pjuarezd and removed request for harshavardhana December 6, 2023 11:37
guozhi.li added 2 commits December 6, 2023 19:39
helm/operator/templates/minio.min.io_tenants.yaml Outdated Show resolved Hide resolved
pkg/controller/pvc.go Outdated Show resolved Hide resolved
pkg/controller/pvc.go Outdated Show resolved Hide resolved
pkg/controller/pvc.go Outdated Show resolved Hide resolved
pkg/controller/pvc.go Outdated Show resolved Hide resolved
@jiuker jiuker requested a review from shtripat December 7, 2023 02:03
@shtripat
Copy link
Contributor

shtripat commented Dec 7, 2023

Please add few verification steps. I edited tenant and added additionalStorage: 2Gi under .spec.pools[0] but PVCs still show old storage value.

guozhi.li and others added 3 commits December 7, 2023 19:08
shtripat
shtripat previously approved these changes Dec 14, 2023
Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

LGTM. Remove the debug prints.

shtripat
shtripat previously approved these changes Dec 14, 2023
Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

lgtm

pjuarezd
pjuarezd previously approved these changes Feb 12, 2024
@pjuarezd pjuarezd requested a review from cniackz February 12, 2024 21:39
Copy link
Member

@pjuarezd pjuarezd 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 not sure this is good, what is preventing Operator from increase infinite times the Storage?

CRD's and resources are declarative ways to ask what is expected, the additionalStorage field seems a procedural one.
ie, declarative is: Give me a 10GB PV, this other seems procedural Give me 10GB PV, then add other 5GB.

I think this should be revaluated @jiuker, can the PV's be resized?, ie: I know DirectPV can Expand Volume, I think that is what we should be suggesting to users instead of workaround it in Operator.

@jiuker
Copy link
Contributor Author

jiuker commented Feb 23, 2024

I am not sure this is good, what is preventing Operator from increase infinite times the Storage?

CRD's and resources are declarative ways to ask what is expected, the additionalStorage field seems a procedural one. ie, declarative is: Give me a 10GB PV, this other seems procedural Give me 10GB PV, then add other 5GB.

I think this should be revaluated @jiuker, can the PV's be resized?, ie: I know DirectPV can Expand Volume, I think that is what we should be suggesting to users instead of workaround it in Operator.

The intention here is simply to execute a command similar to expanding a PVC without causing the STS to restart. Many STS scaling operations typically result in pod restarts. Alternatively, we could use a MinioJob to accomplish this task. If this is possible, let's close this PR. @pjuarezd

@jiuker jiuker closed this Mar 4, 2024
@jiuker jiuker reopened this Mar 4, 2024
@jiuker jiuker closed this Mar 18, 2024
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.

Support for volume expansion
5 participants