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

Jump to bottom

pdksync - (GH-cat-12) Add Support for Redhat 9 #2239

Merged
merged 3 commits into from
Jul 18, 2022
Merged

pdksync - (GH-cat-12) Add Support for Redhat 9 #2239

merged 3 commits into from
Jul 18, 2022

Conversation

david22swan
Copy link
Member

(GH-cat-12) Add Support for Redhat 9
pdk version: 2.4.0

@david22swan david22swan requested a review from a team as a code owner May 31, 2022 15:26
@david22swan david22swan force-pushed the pdksync_GH-cat-12/main/add_redhat_9_support branch 15 times, most recently from 86086e1 to 5f52942 Compare June 9, 2022 09:29
ekohl
ekohl reviewed Jun 9, 2022
View reviewed changes
spec/acceptance/apache_parameters_spec.rb Outdated Show resolved Hide resolved
spec/spec_helper_acceptance_local.rb Outdated Show resolved Hide resolved
@david22swan david22swan force-pushed the pdksync_GH-cat-12/main/add_redhat_9_support branch 3 times, most recently from 5c25865 to 16bec81 Compare June 9, 2022 11:59
ekohl
ekohl reviewed Jun 9, 2022
View reviewed changes
Copy link
Collaborator

@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.

I haven't dug through the rest of the code to see if something's missing, but the changes that are here look good to me. The (acceptance) tests should also be helpful. IMHO both commits should be squashed on merge, but there's a good chance this PR is WIP and more changes will be needed anyway. 👍 for the progress.

@ekohl
Copy link
Collaborator

ekohl commented Jun 9, 2022

SLES-15 failure looks unrelated:

 WARNING: Unsuported provisioner 'provision::provision_service', try abs/docker/docker_exp/provision_service/vagrant/vmpooler
Provisioning sles-15 using provision::provision_service provisioner.
rake aborted!
provisioning of sles-15 failed.

@david22swan david22swan force-pushed the pdksync_GH-cat-12/main/add_redhat_9_support branch from bec6e9c to 434fea3 Compare June 10, 2022 08:25
@ekohl
Copy link
Collaborator

ekohl commented Jun 10, 2022 •
edited
Loading 

       Jun 10 08:41:10 litmus-247093d500b27662 httpd[71482]: AH00526: Syntax error on line 19 of /etc/httpd/conf.d/25-defaults.example.com.conf:
       Jun 10 08:41:10 litmus-247093d500b27662 httpd[71482]: Invalid command 'AuthBasicFake', perhaps misspelled or defined by a module not included in the server configuration

@david22swan
Copy link
Member Author 

       Jun 10 08:41:10 litmus-247093d500b27662 httpd[71482]: AH00526: Syntax error on line 19 of /etc/httpd/conf.d/25-defaults.example.com.conf:
       Jun 10 08:41:10 litmus-247093d500b27662 httpd[71482]: Invalid command 'AuthBasicFake', perhaps misspelled or defined by a module not included in the server configuration

Tracked those down to a pair of tests in default_mods_spec.rb and commented them out.
The php tests still failed out in the same way, just without the above output.

@david22swan david22swan force-pushed the pdksync_GH-cat-12/main/add_redhat_9_support branch 2 times, most recently from a17270d to 96b948d Compare June 16, 2022 12:27
ekohl
ekohl reviewed Jun 22, 2022
View reviewed changes
Copy link
Collaborator

@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.

In #2250 I have a mechanism to automatically include the right modules, which would probably fix the failure you see here if I extend it a bit with the right auth bits. I have those changes locally but I'm looking at the CI failures in the current PR before I push again.

spec/classes/mod/php_spec.rb Outdated
@@ -103,7 +103,8 @@
it { is_expected.to contain_apache__mod('php5') } if facts[:os]['release']['major'].to_i < 8
it { is_expected.to contain_package('php') } if facts[:os]['release']['major'].to_i > 5
it { is_expected.to contain_file('php5.load').with(content: "LoadModule php5_module modules/libphp5.so\n") } if facts[:os]['release']['major'].to_i < 8
it { is_expected.to contain_file('php7.load').with(content: "LoadModule php7_module modules/libphp7.so\n") } if facts[:os]['release']['major'].to_i >= 8
it { is_expected.to contain_file('php7.load').with(content: "LoadModule php7_module modules/libphp7.so\n") } if facts[:os]['release']['major'].to_i == 8
it { is_expected.to contain_file('php.load').with(content: "LoadModule php_module modules/libphp.so\n") } if facts[:os]['release']['major'].to_i >= 9
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given it's doing multiple comparisons on the release major, perhaps this should be if/elsif instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Swapped it over to if/elsif while rebasing onto the current main.

@david22swan david22swan force-pushed the pdksync_GH-cat-12/main/add_redhat_9_support branch 2 times, most recently from de39a56 to dc9b4c2 Compare July 4, 2022 11:07
@david22swan david22swan force-pushed the pdksync_GH-cat-12/main/add_redhat_9_support branch from dc9b4c2 to edbdb65 Compare July 4, 2022 11:09
@david22swan
Copy link
Member Author

Investigating on a saved box, look's like libphp.so isn't getting dropped into the modules folder when php is installed.

@david22swan
(GH-2261) Temporary test exclusion
12a3a36 
Test's have been temporarily excluded on Redhat 9 pending completion of: #2261
@david22swan
Copy link
Member Author

A temporary test exclusion has been put in for PHP so that this can be moved along.
Associated issue can be found here: #2261

@david22swan
Copy link
Member Author

spec test failures replicated in master

@chelnak chelnak merged commit 83d077b into main Jul 18, 2022
@chelnak chelnak deleted the pdksync_GH-cat-12/main/add_redhat_9_support branch July 18, 2022 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl left review comments

@chelnak chelnak chelnak approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@david22swan @ekohl @chelnak