-
Notifications
You must be signed in to change notification settings - Fork 639
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
Make PostgreSQL PVC storage requirements immutable #1638
Conversation
2d76bf0
to
74c485d
Compare
@@ -583,8 +583,8 @@ spec: | |||
x-descriptors: | |||
- urn:alm:descriptor:com.tectonic.ui:advanced | |||
- urn:alm:descriptor:com.tectonic.ui:hidden | |||
- displayName: Postgres Extra Volumes | |||
description: Specify extra volumes to add to the postgres pod | |||
- description: Specify extra volumes to add to the postgres pod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added in this so that make bundle
doesn't create diff for it every time.
74c485d
to
2204b12
Compare
I ran into this same error that you mentioned and documented it in #1642. That said, I think we should do the reverse and still allow it in the CRD, but then in the ansible code that the operator runs, have it better handle that change to the PVC. There is already error catching in that step, the issue is that the error catching doesn't quite work as expected. The issue I see with making this immutable is that there is no process to increase the size without doing a lot of manual work. Alternatively, if the field is mutable and the error handling in the playbook works as expected, then you still need to manually resize the PVC (due to Kubernetes not automatically resizing PVC from statefulstatesets) but then you have a resized PVC. If we do decide to go the route of making the fields immutable, we need to also include much better documentation on how to migrate to a larger PVC. |
Marking this as a draft PR for now because I think we need to workshop how best to handle this and weigh up our options more. |
@tylergmuir You bring up a good point, if a user has a storage class that does support dynamic expansion, this would make things more difficult for them to utilize it. It would be great to take the time to figure out the steps needed for expanding the db PVC and document them in a markdown file here in the repo. I think we would want to cover two scenarios: A) How to migrate if your storageclass supports dynamic expansion (currently modify the AWX CR spec B) How to use the migration.md logic to migrate the awx instance to a fresh instance and provision a larger PVC in the process. I will close this PR and open an issue for adding documentation for this. It would be nice to find a way to prevent users without SC's that support dynamic expansion from falling into this reconcilation fail loop... Maybe the option described here could help with that. |
I created an issue for documenting the steps currently needed to expand a PVC. |
SUMMARY
k8s does not allow a user to modify a PVC's size after it has been created, so we should prevent users from trying to change this on the AWX CR after the initial deployment. Currently, if a user does change the requests or limits on the db PVC, the operator will crashloop with the following error:
ISSUE TYPE