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

Fixes #28697: Add Postgres upgrade to Foreman scenario #444

Merged
merged 1 commit into from
Jan 27, 2020

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jan 8, 2020

No description provided.

@ehelms
Copy link
Member Author

ehelms commented Jan 8, 2020

This is designed to run Postgres upgrade in all situations where Foreman is present and its RHEL 7 regardless of upgrade parameter.

hooks/pre/30-upgrade-postgresql.rb Outdated Show resolved Hide resolved
hooks/pre_validations/30-upgrade-postgresql.rb Outdated Show resolved Hide resolved
hooks/pre/30-upgrade-postgresql.rb Outdated Show resolved Hide resolved
hooks/pre_validations/30-upgrade-postgresql.rb Outdated Show resolved Hide resolved
def postgresql_10_upgrade
execute('foreman-maintain service start --only=postgresql')
(name, owner, enconding, collate, ctype, privileges) = %x{runuser postgres -c 'psql -lt | grep -E "^\s+postgres"'}.chomp.split('|').map(&:strip)
execute('foreman-maintain service stop')
Copy link
Member Author

Choose a reason for hiding this comment

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

Checking, @ekohl are you OK with this being here with a requirement on RPM foreman-installer packaging to require foreman-maintain? This code is gated on el7? so Debian is unaffected.

Copy link
Member

Choose a reason for hiding this comment

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

While I'm not Ewoud, I'd be OK with that. But I think I'd prefer the file to be called 30-el7_upgrade_postgresql to denote it's only relevant on el7.

@ehelms
Copy link
Member Author

ehelms commented Jan 14, 2020

Files renamed

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

katello/hooks/pre_validations/32-upgrade.rb can now be removed, right? That's now transformed into hooks/pre_validations/30-el7_upgrade_postgresql.rb.

Edit: also a merge conflict now.

@ehelms ehelms force-pushed the fixes-28697 branch 3 times, most recently from e801135 to 918b674 Compare January 16, 2020 17:35
@ehelms
Copy link
Member Author

ehelms commented Jan 17, 2020

Welp, this happened http://mirror.centos.org/centos/7/sclo/x86_64/rh/rh-postgresql12/ so I suppose I'll go and test that in case I can just change this to that.

@evgeni
Copy link
Member

evgeni commented Jan 17, 2020

Yeah, it'd be cool if we can go straight to 12 with 2.0

@@ -40,6 +40,10 @@ def local_redis?
(foreman_server? && !param_value('foreman', 'jobs_sidekiq_redis_url')) || param_value('foreman_proxy::plugin::pulp', 'pulpcore_enabled')
end

def needs_postgresql_scl_upgrade?
!File.exist?('/var/opt/rh/rh-postgresql12/lib/pgsql/data') && File.exist?('/var/lib/pgsql/data')
Copy link
Member Author

Choose a reason for hiding this comment

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

@evgeni I had to add a condition here for this to work on fresh installations. Before adding the && part, it attempted an upgrade on a fresh install.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense.

This will still try to upgrade if the user installed postgres before running the installer, but I think that's fine as being labeled "unsupported"?

@ehelms
Copy link
Member Author

ehelms commented Jan 18, 2020

This has been updated instead to PostgreSQL 12. @tbrisker for your awareness that we'd be switching to PG 12 instead of 10 now that it's published (http://mirror.centos.org/centos/7/sclo/x86_64/rh/rh-postgresql12/). Bats tests passed for me.

This scratch build can be used to test: http://koji.katello.org/koji/taskinfo?taskID=272814

@ehelms
Copy link
Member Author

ehelms commented Jan 21, 2020

My testing of this with Pulp 3 yielded no errors.

@evgeni
Copy link
Member

evgeni commented Jan 24, 2020

I am not merging this in case you first want to fix Katello nightly pipelines.

@ehelms
Copy link
Member Author

ehelms commented Jan 25, 2020

Niightly is greeeen. This will, however, need theforeman/foreman-packaging#4577 resolved as well to not fail on nightly tests.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Could you make a post to discourse with some notes for users who have already upgraded to SCL PG 10?

@ehelms ehelms merged commit 07b2164 into theforeman:develop Jan 27, 2020
@ehelms ehelms deleted the fixes-28697 branch January 27, 2020 08:38
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