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

Remove the ability to customize the postgres_data_dir #1798

Merged

Conversation

rooftopcellist
Copy link
Member

@rooftopcellist rooftopcellist commented Mar 28, 2024

SUMMARY

Remove the ability to customize the postgres_data_dir

  • in the sclorg Postgresql 15 image, the PGDATA directory is hardcoded
  • if users were to modify this directory, they would only change the directory the pvc is mounted to, not the directory PostgreSQL uses. This would result in loss of data.
ISSUE TYPE
  • Bug, Docs Fix or other nominal change

cc @kurokobo

@rooftopcellist rooftopcellist force-pushed the hardcode-postgres-data-path branch from 6dde291 to 703a9c2 Compare March 28, 2024 21:50
* in the sclorg Postgresql 15 image, the PGDATA directory is hardcoded
* if users were to modify this directory, they would only change the
  directory the pvc is mounted to, not the directory PostgreSQL uses.
  This would result in loss of data.
* switch from /var/lib/pgsql/data/pgdata to /var/lib/pgsql/data/userdata
@rooftopcellist rooftopcellist force-pushed the hardcode-postgres-data-path branch from 703a9c2 to 88302e3 Compare March 28, 2024 22:21
@kurokobo
Copy link
Contributor

I haven't verified this yet, but when we upgrade the CRDs to this PR's while there is already an AWX CR with postgres_data_path, what happens to the AWX CR?

@rooftopcellist
Copy link
Member Author

rooftopcellist commented Mar 29, 2024

@kurokobo The k8s validator still applies the AWX CR, but omits the postgres_data_path field. As an extra measure, I renamed the internal variables to _postgres_data_path so that there is no way to override the path.

The caveat to this approach is that users won't be able to use the old postgres image from dockerhub for managed deployments because the PVC will no longer be mounted. But I think that is OK, being a managed deployment, I think it is OK be opinionated here to simplify things. Users can always create a postgres statefulset and pvc themselves and configure it as an external db if they want to use the postgres image from dockerhub.

I left in the postgres_image and postgres_image_version parameters though as they still serve the purpose of making it possible to specify a mirrored image. Users who mirror for air-gapped/disconnected environments will need to update their mirror to the new postgres image.

I added a warning about this in the release notes for 2.13.1 and 2.14.0.

@kurokobo
Copy link
Contributor

@rooftopcellist
Thanks for clarifying details for k8s validator. I also tested this PR by;

  • Deploy minimal AWX with postgres_data_path by Operator 2.12.2
    ---
    apiVersion: awx.ansible.com/v1beta1
    kind: AWX
    metadata:
      namespace: awx
      name: awx-emo
    spec:
      service_type: nodeport
      nodeport_port: 30080
      postgres_data_path: /completely/customized/psql/data/path
  • Create new users and launch jobs multiple times to make changes on DB
  • Upgrade Operator to this PR:
    gh pr checkout https://github.com/ansible/awx-operator/pull/1798
    IMG=registry.example.com/ansible/awx-operator:pr1798 BUILD_ARGS="--build-arg DEFAULT_AWX_VERSION=24.1.0" make docker-build docker-push deploy

and I can confirm this PR works as expected 👍

  • AWX CR still there, just postgres_data_path in AWX CR is removed by CRD changes
  • New reconcile is completed without any errors
  • PSQL is upgraded from 13 to 15
  • PSQL mounts new PVC on /var/lib/pgsql/data
  • AWX is live
  • All data in PSQL are migrated; users and jobs that created before upgrading are persisted after upgrading
  • Restarting PSQL 15 does not loss any data; I can still see users and jobs after restarting

@rooftopcellist
Copy link
Member Author

@kurokobo Thank you for testing this out so thoroughly!

@rooftopcellist rooftopcellist merged commit 7bf49c2 into ansible:devel Apr 1, 2024
7 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.

4 participants