From 79fa35591e099a7b6981b873b930b943e5dc1827 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 26 May 2021 18:11:59 +0200 Subject: [PATCH] Autorequire the service in postgresql_psql In d69ff1ec91d02337415911fac35d8a1fa5a906fa I changed the require to be an autorequire. However, even in the PR I noted[1] it may not work. Now I realize this may not work because you can't autorequire on a class. In the same way, you can't collect classes. I started to see this on my Puppet 7 testing (edited for brevity): Notice: /Stage[main]/Postgresql::Client/Package[postgresql-client]/ensure: created Notice: /Stage[main]/Postgresql::Client/File[/usr/local/bin/validate_postgresql_connection.sh]/ensure: defined content as '{sha256}f34c53aa433754c6343e6382cd994e33f2887f9eb54503421eb0db55a55e14a6' Notice: /Stage[main]/Postgresql::Server::Install/Package[postgresql-server]/ensure: created Notice: /Stage[main]/Postgresql::Server::Initdb/File[/var/lib/pgsql/data]/ensure: created Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: The files belonging to this database system will be owned by user "postgres". Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: This user must also own the server process. Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: initdb: error: invalid locale settings; check LANG and LC_* environment variables Error: '/usr/bin/initdb --pgdata '/var/lib/pgsql/data'' returned 1 instead of one of [0] Error: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: change from 'notrun' to ['0'] failed: '/usr/bin/initdb --pgdata '/var/lib/pgsql/data'' returned 1 instead of one of [0] Info: Class[Postgresql::Server::Initdb]: Unscheduling all events on Class[Postgresql::Server::Initdb] Notice: /Stage[main]/Postgresql::Server::Config/File[/var/lib/pgsql/data/postgresql.conf]: Dependency Exec[postgresql_initdb] has failures: true Warning: /Stage[main]/Postgresql::Server::Config/File[/var/lib/pgsql/data/postgresql.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/File[systemd-conf-dir]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/File[systemd-override]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/File[old-systemd-override]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Exec[restart-systemd]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_hba.conf]/Concat_file[/var/lib/pgsql/data/pg_hba.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_hba.conf]/File[/var/lib/pgsql/data/pg_hba.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_hba.conf]/Concat_fragment[/var/lib/pgsql/data/pg_hba.conf_header]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Config_entry[port]/Postgresql_conf[port]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Config_entry[data_directory]/Postgresql_conf[data_directory]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_ident.conf]/Concat_file[/var/lib/pgsql/data/pg_ident.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_ident.conf]/File[/var/lib/pgsql/data/pg_ident.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_ident.conf]/Concat_fragment[/var/lib/pgsql/data/pg_ident.conf_header]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[local access as postgres user]/Concat::Fragment[pg_hba_rule_local access as postgres user]/Concat_fragment[pg_hba_rule_local access as postgres user]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[local access to database with same name]/Concat::Fragment[pg_hba_rule_local access to database with same name]/Concat_fragment[pg_hba_rule_local access to database with same name]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[allow localhost TCP access to postgresql user]/Concat::Fragment[pg_hba_rule_allow localhost TCP access to postgresql user]/Concat_fragment[pg_hba_rule_allow localhost TCP access to postgresql user]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[deny access to postgresql user]/Concat::Fragment[pg_hba_rule_deny access to postgresql user]/Concat_fragment[pg_hba_rule_deny access to postgresql user]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[allow access to all users]/Concat::Fragment[pg_hba_rule_allow access to all users]/Concat_fragment[pg_hba_rule_allow access to all users]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[allow access to ipv6 localhost]/Concat::Fragment[pg_hba_rule_allow access to ipv6 localhost]/Concat_fragment[pg_hba_rule_allow access to ipv6 localhost]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Service/Anchor[postgresql::server::service::begin]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Service/Service[postgresqld]: Skipping because of failed dependencies Info: Class[Postgresql::Server]: Unscheduling all events on Class[Postgresql::Server] Warning: /Stage[main]/Postgresql::Server::Service/Postgresql_conn_validator[validate_service_is_running]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Service/Anchor[postgresql::server::service::end]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Reload/Exec[postgresql_reload]: Skipping because of failed dependencies Info: Class[Foreman::Database::Postgresql]: Scheduling refresh of Postgresql::Server::Db[foreman] Info: Postgresql::Server::Db[foreman]: Scheduling refresh of Postgresql::Server::Database[foreman] Info: Postgresql::Server::Db[foreman]: Scheduling refresh of Postgresql::Server::Role[foreman] Info: Postgresql::Server::Db[foreman]: Scheduling refresh of Postgresql::Server::Database_grant[GRANT foreman - ALL - foreman] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[CREATE ROLE foreman ENCRYPTED PASSWORD ****] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOSUPERUSER] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOCREATEDB] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOCREATEROLE] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" LOGIN] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" INHERIT] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOREPLICATION] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" CONNECTION LIMIT -1] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE foreman ENCRYPTED PASSWORD ****] Info: Postgresql::Server::Database_grant[GRANT foreman - ALL - foreman]: Scheduling refresh of Postgresql::Server::Grant[database:GRANT foreman - ALL - foreman] Error: /Stage[main]/Foreman::Database::Postgresql/Postgresql::Server::Db[foreman]/Postgresql::Server::Role[foreman]/Postgresql_psql[CREATE ROLE foreman ENCRYPTED PASSWORD ****]: Could not evaluate: Error evaluating 'unless' clause, returned pid 1676 exit 2: 'psql: error: could not connect to server: No such file or directory Is the server running locally and accepting connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"? ' As can be seen, various resources are correctly skipped due to failed dependencies but Postgresql_psql isn't. This changes it to an autorequire on the service. This should work since the name of the service resource is static. There is an edge case where the service isn't managed. For that the anchor is also auto-required. This gives less guarantee, but it's a best-effort for those users. [1]: https://github.com/puppetlabs/puppetlabs-postgresql/pull/950#issuecomment-377516861 Fixes: d69ff1ec91d02337415911fac35d8a1fa5a906fa --- lib/puppet/type/postgresql_psql.rb | 3 ++- spec/unit/defines/server/database_spec.rb | 2 +- spec/unit/defines/server/grant_spec.rb | 2 +- spec/unit/defines/server/reassign_owned_by_spec.rb | 2 +- spec/unit/defines/server/role_spec.rb | 4 ++-- spec/unit/defines/server/tablespace_spec.rb | 2 +- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/puppet/type/postgresql_psql.rb b/lib/puppet/type/postgresql_psql.rb index 2ae079715a..8e04be47e8 100644 --- a/lib/puppet/type/postgresql_psql.rb +++ b/lib/puppet/type/postgresql_psql.rb @@ -131,7 +131,8 @@ def matches(value) newvalues(:true, :false) end - autorequire(:class) { ['Postgresql::Server::Service'] } + autorequire(:anchor) { ['postgresql::server::service::begin'] } + autorequire(:service) { ['postgresqld'] } def should_run_sql(refreshing = false) onlyif_param = @parameters[:onlyif] diff --git a/spec/unit/defines/server/database_spec.rb b/spec/unit/defines/server/database_spec.rb index f0984f79bf..3290dc7ce4 100644 --- a/spec/unit/defines/server/database_spec.rb +++ b/spec/unit/defines/server/database_spec.rb @@ -24,7 +24,7 @@ end it { is_expected.to contain_postgresql__server__database('test') } - it { is_expected.to contain_postgresql_psql('CREATE DATABASE "test"') } + it { is_expected.to contain_postgresql_psql('CREATE DATABASE "test"').that_requires('Service[postgresqld]') } context "with comment set to 'test comment'" do let(:params) { { comment: 'test comment' } } diff --git a/spec/unit/defines/server/grant_spec.rb b/spec/unit/defines/server/grant_spec.rb index fa61e48ebc..e2d12d5e0e 100644 --- a/spec/unit/defines/server/grant_spec.rb +++ b/spec/unit/defines/server/grant_spec.rb @@ -213,7 +213,7 @@ class {'postgresql::server':} it { is_expected.to contain_postgresql__server__role('test') } it do is_expected.to contain_postgresql_psql('grant:test') \ - .that_requires(['Class[postgresql::server::service]', 'Postgresql::Server::Role[test]']) + .that_requires(['Service[postgresqld]', 'Postgresql::Server::Role[test]']) end end diff --git a/spec/unit/defines/server/reassign_owned_by_spec.rb b/spec/unit/defines/server/reassign_owned_by_spec.rb index 62174a172c..19ea8f1ad1 100644 --- a/spec/unit/defines/server/reassign_owned_by_spec.rb +++ b/spec/unit/defines/server/reassign_owned_by_spec.rb @@ -42,6 +42,6 @@ class {'postgresql::server':} is_expected.to contain_postgresql_psql('reassign_owned_by:test:REASSIGN OWNED BY "test_old_role" TO "test_new_role"') .with_command('REASSIGN OWNED BY "test_old_role" TO "test_new_role"') .with_onlyif(%r{SELECT tablename FROM pg_catalog.pg_tables WHERE\s*schemaname NOT IN \('pg_catalog', 'information_schema'\) AND\s*tableowner = 'test_old_role'.*}m) - .that_requires('Class[Postgresql::Server::Service]') + .that_requires('Service[postgresqld]') } end diff --git a/spec/unit/defines/server/role_spec.rb b/spec/unit/defines/server/role_spec.rb index 5e3e005dfa..7180ec2bc5 100644 --- a/spec/unit/defines/server/role_spec.rb +++ b/spec/unit/defines/server/role_spec.rb @@ -69,7 +69,7 @@ .with_unless("SELECT 1 FROM pg_roles WHERE rolname = 'test'") .with_port(5432) .with_connect_settings('PGHOST' => 'postgres-db-server', 'DBVERSION' => '9.1', 'PGUSER' => 'login-user', 'PGPASSWORD' => 'login-pass') - .that_requires('Class[postgresql::server::service]') + .that_requires('Service[postgresqld]') end it 'has alter role for "test" user with password as ****' do is_expected.to contain_postgresql_psql('ALTER ROLE test ENCRYPTED PASSWORD ****') @@ -142,7 +142,7 @@ end it 'has drop role for "test" user if ensure absent' do - is_expected.to contain_postgresql_psql('DROP ROLE "test"').that_requires('Class[postgresql::server::service]') + is_expected.to contain_postgresql_psql('DROP ROLE "test"').that_requires('Service[postgresqld]') end end diff --git a/spec/unit/defines/server/tablespace_spec.rb b/spec/unit/defines/server/tablespace_spec.rb index 6d7bc49c01..336a4f773b 100644 --- a/spec/unit/defines/server/tablespace_spec.rb +++ b/spec/unit/defines/server/tablespace_spec.rb @@ -32,7 +32,7 @@ it { is_expected.to contain_file('/srv/data/foo').with_ensure('directory') } it { is_expected.to contain_postgresql__server__tablespace('test') } - it { is_expected.to contain_postgresql_psql('CREATE TABLESPACE "test"').that_requires('Class[postgresql::server::service]') } + it { is_expected.to contain_postgresql_psql('CREATE TABLESPACE "test"').that_requires('Service[postgresqld]') } context 'with different owner' do let :params do