feat: add EL10 support#28
Conversation
373218a to
0dd504a
Compare
|
Issue: puppetlabs/postgresql defines default version in a manfest, and it does not have EL10. Tried to force it in Hiera but did not succeed... @corporate-gadfly can I ask for help again in hiera? :D |
|
@d1nuc0m : What you are trying to do in this PR won't work, IMO. APL only works in your own module, not in some other module. Your best bet is to add support for newer OS in https://github.com/puppetlabs/puppetlabs-postgresql/blob/main/metadata.json (I know it is not an openvox module). Get that released, and then bump the supported OS in this module. HTH. |
|
@corporate-gadfly thank you, I though it was possible... there already is PR 1650, guess I'll have to wait |
66e1559 to
5c17a4c
Compare
|
@d1nuc0m : I had a chance to look at the remaining failures. I believe the failures are happening because of the rigid puppet-openvoxdb/spec/unit/classes/init_spec.rb Lines 22 to 25 in d823b9a What about the following diff which swaps in a different manifest (temporarily) for EL10 support (until the other module itself starts supporting EL10)? diff --git a/spec/unit/classes/init_spec.rb b/spec/unit/classes/init_spec.rb
index 3c07f7f..51b0d42 100644
--- a/spec/unit/classes/init_spec.rb
+++ b/spec/unit/classes/init_spec.rb
@@ -17,12 +17,23 @@ describe 'openvoxdb', type: :class do
it { is_expected.to contain_postgresql__server__db('puppetdb') }
end
+ el10 = facts.dig(:os, 'family') == 'RedHat' && facts.dig(:os, 'release', 'major') == '10'
+
describe 'without managed postgresql' do
let :pre_condition do
- <<-HEREDOC
- class { 'postgresql::server':
- }
- HEREDOC
+ if el10
+ <<-HEREDOC
+ class { 'postgresql::globals':
+ version => '16',
+ }
+ -> class { 'postgresql::server': }
+ HEREDOC
+ else
+ <<-HEREDOC
+ class { 'postgresql::server':
+ }
+ HEREDOC
+ end
end
let :params doAnd please ask some experienced folks about this unholy hack. |
Thanks, I wasn't continuing with the PR precisely because "unholy" is not enough :D Do we prefer EL10 support now (and fixing this later) or do we want to wait for postgresql support? @bastelfreak @binford2k |
|
@d1nuc0m : can you try pushing with the above diff and we'll see if the failing checks get fixed? |
|
Apologies, as I committed directly to I see in the logs: It seems like we are making progress, but then again who knows? So, the above error is likely related to: puppet-openvoxdb/spec/support/acceptance/shared/puppetserver.pp Lines 15 to 26 in d823b9a |
|
And then later on: |
|
I feel like I have made more progress 🤷 . |
|
Make more progress. Looks like I will try to see how to make it not fail initially and then make it idempotent. |
| if facts.dig(:os, 'family') == 'RedHat' && facts.dig(:os, 'release', 'major') == '10' | ||
| <<-HEREDOC | ||
| class { 'postgresql::globals': | ||
| version => '16', |
There was a problem hiding this comment.
This is our only chance to sneak in the version number into the downstream postgresql::server module.
For EL10, we replace a single-line with two classes.
|
@d1nuc0m I apologize again for committing directly to your branch (the GitHub UI allowed me to edit files in-place and was the easiest thing for me to try). In any case, all the unit and acceptance tests are passing now. I learned a lot about acceptance tests, for sure. |
|
@d1nuc0m : Could you also explain the motivation behind the following deviations from EL8/9?
I would suggest rebasing the multiple commits into single commit, force pushing and then try out any extra modifications that you may deem necessary. |
|
@corporate-gadfly thank you for all the contributions, I had to pause this for other committments. I used "non standard" paths + not managing the repo as I was trying to make it work with the PostgreSQL version/packages in default EL10 repositories |
327db5b to
1bdc18f
Compare
|
Tried to set variables but now it's broken on EL9 due to dnf... |
86721cd to
4b8c659
Compare
Add an "unholy hack" (quote) to support EL10 while it's added to postgresql Co-authored-by: Corporate Gadfly <corporate-gadfly@users.noreply.github.com>
Pull Request (PR) description
Add EL10 support and tests
This Pull Request (PR) fixes the following issues
n/a