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.

Sign up for GitHub

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

Refs #37797 - ensure we can start PostgreSQL by forcing the old version #974

Merged
merged 1 commit into from
Sep 12, 2024
Merged

Refs #37797 - ensure we can start PostgreSQL by forcing the old version #974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion hooks/boot/06-postgresql-upgrade-extensions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,23 @@

module PostgresqlUpgradeHookContextExtension
def needs_postgresql_upgrade?(new_version)
File.read('/var/lib/pgsql/data/PG_VERSION').chomp.to_i < new_version.to_i
current_version.to_i < new_version.to_i
rescue Errno::ENOENT
false
end

def current_version
File.read('/var/lib/pgsql/data/PG_VERSION').chomp
end

def os_needs_postgresql_upgrade?
el8? && needs_postgresql_upgrade?(13)
end

def postgresql_upgrade(new_version)
logger.notice("Performing upgrade of PostgreSQL to #{new_version}")

execute!("dnf module switch-to postgresql:#{current_version} -y", false, true)
Copy link
Member Author

@evgeni evgeni Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes this means we execute:

  • (maintain/user) switch-to 13
  • (installer) switch-to 12
  • (installer) switch-to 13

But I didn't find another way to get the locale of the existing (12) cluster w/o it being up, and it can only be up if it's PostgreSQL 12 that's installed on the system.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to move the PG13 upgrade into f-maintain and have it in two spots based on entry point?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because the user (w/o maintain) also needs to perform switch-to 13 to get the new installer.

We need to get rid of this monstrosity called modularity

Copy link
Member

Choose a reason for hiding this comment

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

I guess you have figured it all out by now so might as well take this in. My second thought when reading this is -- should we add a check to catch this before the upgrade is started and have the user take a corrective action first?

Copy link
Member

Choose a reason for hiding this comment

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

Also, is this user / cluster locale mismatch a known issue within PG land?

Copy link
Member

Choose a reason for hiding this comment

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

Can we use systemd-run postgresql --property to run it temporarily with a different ExecStart?

Copy link
Member

Choose a reason for hiding this comment

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

Looking better at it, systemd-run doesn't load an existing unit file. You can only specify the fields

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, is this user / cluster locale mismatch a known issue within PG land?

Yes. The Debian tooling does this (extracting the locale of the old cluster and using it for the new) automatically, RHEL's does not.

Copy link
Member Author

Choose a reason for hiding this comment

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

And I've opened https://issues.redhat.com/browse/RHEL-58410 to track this problem as a whole

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, turned out starting a cluster in single-user mode is not that hard, and solves stupid problems with dnf…

#982

start_services(['postgresql'])
postgres_list = `runuser -l postgres -c 'psql --list --tuples-only --csv'`
postgres_databases = CSV.parse(postgres_list)
Expand Down
1 change: 1 addition & 0 deletions spec/postgresql_upgrade_hook_context_extension_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
subject { context.postgresql_upgrade(13) }

before do
allow(File).to receive(:read).with('/var/lib/pgsql/data/PG_VERSION').and_return('12')
allow(context).to receive(:logger).and_return(logger)
allow(context).to receive(:'execute!')
allow(context).to receive(:ensure_packages)
Expand Down