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

Feat: Add pcs STONITH Resource Support #475

Merged
merged 5 commits into from
Sep 22, 2019

Conversation

pdemonaco
Copy link
Contributor

Pull Request (PR) description

In the pcs based pacemaker/corosync environment found on RedHat derivatives STONITH primitives are created an managed with a slightly different pcs command than standard primitives.

This pull request enhances the pcs provider for the cs_primitive type to appropriately handle STONITH primitives using the pcs stonith subcommand.

This Pull Request (PR) fixes the following issues

n/a

In PCS environments there is a special subcommand to manage stonith
primitive types: `pcs stonith`

This change adds support for this special type to the pcs primitive
along with associated test code to support validating these changes.
Note that no testing has been done to verify whether the existing crm
provider can handle stonith resources.
In order to sucessfully execute tests for stonith primitives the
fence-agents-all package needs to be installed on the test node.
Additionally, this code has been written such that debian based nodes
should not attempt to execute the stonith tests.
Some helpful tips on running acceptance tests locally via vagrant. Not
everyone has the time to sort out docker on their local machine.
Added more detail to the local vagrant testing section of the
CONTRIBUTING.md guide. Specfically:
* Ensuring the correct version of puppet is tested by the linter
* Ensuring rubocop actually runs
* Specifying puppet agent version for beaker

Also added fixes to the cs_primitive acceptance tests as new behavior in
the latest release of pcs (for CentOS 7.6?) now rejects IPaddr2 commands
when multiple resources are declared with the same VIP.
@pdemonaco
Copy link
Contributor Author

The most recent commit is valid and passes all checks. Unfortunately the travis/docker timeout issue is still present so one of the builds failed due to this timeout.

Could a maintainer repeat the failed job?

manifests/init.pp Outdated Show resolved Hide resolved
Optional[Array] $packageopts_corosync = $corosync::params::package_install_options,
Optional[Array] $packageopts_pacemaker = $corosync::params::package_install_options,
Optional[Array] $packageopts_crmsh = $corosync::params::package_install_options,
Optional[Array] $packageopts_pcs = $corosync::params::package_install_options,
Optional[Array] $packageopts_fence_agents = $corosync::params::package_install_options,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Optional[Array] $packageopts_fence_agents = $corosync::params::package_install_options,
Array[String[1]] $packageopts_fence_agen = $corosync::params::package_install_options,

This should work as well. It allows an empty array. If data is provided, it has to be strings with at least one character.

@bastelfreak bastelfreak added the enhancement New feature or request label Sep 21, 2019
@bastelfreak
Copy link
Member

Hi @pdemonaco, thanks for the awesome PR! Can you please take a look at the inline comments I made? I can also restart the tests afterwards if they fail again.

Several of the package parameters and options specified on the parent
corosync class could be improved slightly. This change incorporates
these improvements.
@pdemonaco
Copy link
Contributor Author

Hi @bastelfreak in principal your suggestion makes sense but the way params.pp currently functions actually expects to set those packageopts parameters to undef. I've combined your suggestions with keeping those parameters optional.

@bastelfreak
Copy link
Member

It's green \o/

@bastelfreak bastelfreak merged commit 8910097 into voxpupuli:master Sep 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants