Skip to content

Conversation

@ffapitalle
Copy link

Pull Request (PR) description

This PR adds validation check for fpm and fpm pools configuration file before applying changes.

Having introduced a misconfiguration on php-fpm that avoid the service to start normally (pm.start_servers > pm.max_spare_servers), catalog application was successfull but service failed to restart.

[13-Feb-2020 20:48:24] ALERT: [pool www] pm.start_servers(80) must not be less than pm.min_spare_servers(24) and not greater than pm.max_spare_servers(64)
[13-Feb-2020 20:48:24] ERROR: failed to post process the configuration

@ghoneycutt
Copy link
Member

Thanks for the contribution! Could you please add an acceptance test.

Hash $pools = $php::real_fpm_pools,
$log_owner = $php::log_owner,
$log_group = $php::log_group,
Boolean $disable_configtest = false,
Copy link
Member

Choose a reason for hiding this comment

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

This would need a spec test to show that validate_cmd is as expected.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, indeed. I'll add a spec test for this parameter.

@bastelfreak bastelfreak added needs-work not ready to merge just yet tests-fail enhancement New feature or request labels Apr 19, 2020
@vox-pupuli-tasks
Copy link

Dear @ffapitalle, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Federico Fapitalle added 2 commits June 19, 2020 18:36
Defines validation command for fpm service configuration files, and
sets it as a requirement.
This adds the option to enable/disable fpm config file validation.
@ffapitalle ffapitalle force-pushed the validate_fpm_config branch from 0c19042 to c7817ae Compare June 19, 2020 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request merge-conflicts needs-work not ready to merge just yet tests-fail

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants