Skip to content

ROX-20769: Use pg_upgrade in init-entrypoint#14447

Merged
erthalion merged 4 commits intomasterfrom
feature/init-entrypoint-pg-upgrade
Apr 22, 2025
Merged

ROX-20769: Use pg_upgrade in init-entrypoint#14447
erthalion merged 4 commits intomasterfrom
feature/init-entrypoint-pg-upgrade

Conversation

@erthalion
Copy link
Copy Markdown
Contributor

Experimental trigger for major version upgrade.

User-facing documentation

  • CHANGELOG is updated OR update is not needed
  • documentation PR is created and is linked above OR is not needed

Testing and quality

  • the change is production ready: the change is GA or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

change me!

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Feb 28, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Feb 28, 2025

Images are ready for the commit at 95d5a7d.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-208-g95d5a7d214.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.98%. Comparing base (8aa699b) to head (95d5a7d).
Report is 323 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14447      +/-   ##
==========================================
- Coverage   49.26%   48.98%   -0.29%     
==========================================
  Files        2528     2550      +22     
  Lines      184979   187198    +2219     
==========================================
+ Hits        91128    91690     +562     
- Misses      86622    88262    +1640     
- Partials     7229     7246      +17     
Flag Coverage Δ
go-unit-tests 48.98% <ø> (-0.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@erthalion erthalion force-pushed the feature/init-entrypoint-pg-upgrade branch from 398431f to 6f4dd31 Compare February 28, 2025 15:54
@erthalion erthalion changed the title [PoC] Use pg_upgrade in init-entrypoint ROX-20769: Use pg_upgrade in init-entrypoint Mar 3, 2025
Copy link
Copy Markdown
Contributor

@porridge porridge left a comment

Choose a reason for hiding this comment

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

TBH I must be missing something, because this script seems to:

  • make a backup in container's root FS, which IIRC disappears as soon as the container exits
  • run the old version against another copy of the data
  • run the upgrade

@erthalion erthalion force-pushed the feature/init-entrypoint-pg-upgrade branch from 4419d70 to 9c3ee42 Compare March 11, 2025 09:43
@erthalion
Copy link
Copy Markdown
Contributor Author

erthalion commented Mar 11, 2025

@porridge thanks for checking out.

TBH I must be missing something, because this script seems to:

  • make a backup in container's root FS, which IIRC disappears as soon as the container exits

It's a temporary approach for testing, until a persistent volume is added.

  • run the old version against another copy of the data

For testing, to verify that the backup works.

@erthalion erthalion force-pushed the feature/init-entrypoint-pg-upgrade branch 2 times, most recently from e23bf12 to 286d8a1 Compare March 21, 2025 08:41
@erthalion erthalion force-pushed the feature/init-entrypoint-pg-upgrade branch from 286d8a1 to 0e43d99 Compare April 1, 2025 13:25
Turns out postgresql-upgrade wrapper doesn't allow to inspect failure
logs, since it wipes out the new data directory on the upgrade failure.
@erthalion erthalion marked this pull request as ready for review April 16, 2025 13:28
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @erthalion - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider adding a feature flag to control the execution of the upgrade logic.
  • Consider using a dedicated tool for managing file system operations, such as rsync, instead of tar.
Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟡 Security: 1 issue found
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

It will be bumped separatelly
@erthalion erthalion enabled auto-merge (squash) April 22, 2025 10:12
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 22, 2025

@erthalion: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocp-4-12-qa-e2e-tests 4d1da3b link false /test ocp-4-12-qa-e2e-tests
ci/prow/gke-operator-e2e-tests 95d5a7d link false /test gke-operator-e2e-tests
ci/prow/gke-qa-e2e-tests 95d5a7d link false /test gke-qa-e2e-tests
ci/prow/gke-upgrade-tests 95d5a7d link false /test gke-upgrade-tests
ci/prow/gke-scanner-v4-install-tests 95d5a7d link false /test gke-scanner-v4-install-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@erthalion erthalion merged commit fe8f25c into master Apr 22, 2025
83 of 93 checks passed
@erthalion erthalion deleted the feature/init-entrypoint-pg-upgrade branch April 22, 2025 11:10
janisz pushed a commit that referenced this pull request Apr 23, 2025
Introduce a major upgrade logic into CentralDB init-db script. The
implementation is based on the pg_upgrade [1] and postgresql-container [2]. We
can't use the latter upgrade scripts directly, because they require to use
provided entrypoints, which will need some more work from our side. We also
couldn't use `postgresql-upgrade` script directly, because it turns out it
doesn't allow to keep logs after upgrade if something went wrong.

The upgrade scenario looks like this:

* Check if the binaries version is newer than the data version and the upgrade
  is needed.

* Make sure the cluster was shutdown properly.

* Figure out if there is enough disk space available for the upgrade. The disk
  space needs to accomodate one mandatory backup and one temporary restored
  database to verify the backup. Hence the available size must be more than two
  times existing database size.

* Take a physical backup, and verify it via restoring a temporary copy of the
  database.

* Spin up a new PG15 database.

* Do the upgrade check first.

* If successfull, do the full upgrade, then swap old and new databases.

The script knows about a backup volume, and tries to use it for making a
backup. If it's not present, the main data volume will be used. The script is
not idempotent:

* If a backup was already taken, it will not be recreated.

* If a new PG15 database already exists, the upgrade will be aborted.

* If there is not enough disk space available, the upgrade will be aborted.

[1]: https://www.postgresql.org/docs/current/pgupgrade.html
[2]: https://github.com/sclorg/postgresql-container/
@erthalion erthalion mentioned this pull request Apr 24, 2025
9 tasks
erthalion added a commit that referenced this pull request Apr 28, 2025
Use PostgreSQL 15 for central-db. Benefit from #14447 to handle upgrade scenarios.
vikin91 pushed a commit that referenced this pull request Apr 30, 2025
Use PostgreSQL 15 for central-db. Benefit from #14447 to handle upgrade scenarios.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants