Add pacemaker_resource module for managing cluster resources #53

matbu · 2020-03-26T09:52:00Z

Add pacemaker_resource module for managing pacemaker cluster
resources.

SUMMARY

ISSUE TYPE

Bugfix Pull Request
Docs Pull Request
Feature Pull Request
New Module Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Add pacemaker_resource module for managing pacemaker cluster resources.

gundalow

Hi,
Thank you for this PR and the other one.
Do you expect there will be other pacemaker PRs? I'm wondering if it would be better for them to be in their own pacemaker collection.

matbu · 2020-03-26T13:34:28Z

@gundalow Hi,
nop for now, I don't think we will need more pacemaker modules.
Those two allow us to mainly control the cluster and its resources.

gundalow

I don't know anything about pacemaker, though here are some general comments on the module format.

gundalow · 2020-03-26T13:56:45Z

plugins/modules/clustering/pacemaker_resource.py

+  gather_facts: no
+  tasks:
+    - name: enable haproxy
+      pacemaker_resource: state=enable resource=haproxy


Please use key: value

For new modules they need to use the FQCN (Fully Qualified Collection Name):

Suggested change

pacemaker_resource: state=enable resource=haproxy

community.general.pacemaker_resource:

state: enable

resource: haproxy

gundalow · 2020-03-26T13:57:01Z

plugins/modules/clustering/pacemaker_resource.py

+module: pacemaker_resource
+short_description: Manage a pacemaker resource
+extends_documentation_fragment: openstack
+version_added: "2.9"


Suggested change

version_added: "2.9"

gundalow · 2020-03-26T13:57:14Z

plugins/modules/clustering/pacemaker_resource.py

+    resource:
+      description:
+        - Specify which resource you want to handle
+      required: false


Suggested change

required: false

gundalow · 2020-03-26T13:57:23Z

plugins/modules/clustering/pacemaker_resource.py

+    timeout:
+      description:
+        - Timeout when the module should considered that the action has failed
+      required: false


Suggested change

required: false

gundalow · 2020-03-26T13:57:32Z

plugins/modules/clustering/pacemaker_resource.py

+    check_mode:
+        description:
+          - Check only the status of the resource
+        required: false


Suggested change

required: false

gundalow · 2020-03-26T13:57:46Z

plugins/modules/clustering/pacemaker_resource.py

+        description:
+          - Wait for resource to get the required state, will failed if the
+            timeout is reach
+        required: false


Suggested change

required: false

gundalow · 2020-03-26T14:01:48Z

plugins/modules/clustering/pacemaker_resource.py

+    wait_for_resource:
+        description:
+          - Wait for resource to get the required state, will failed if the
+            timeout is reach


I(...) is how we use Italics to refer to another paramater

Suggested change

timeout is reach

I(timeout) seconds is reached.

gundalow · 2020-03-26T14:04:32Z

plugins/modules/clustering/pacemaker_resource.py

+      description:
+        - Timeout when the module should considered that the action has failed
+      required: false
+      default: 300


Suggested change

default: 300

default: 300

type: int

Akasurde · 2020-03-27T11:23:40Z

plugins/modules/clustering/pacemaker_resource.py

+---
+module: pacemaker_resource
+short_description: Manage a pacemaker resource
+extends_documentation_fragment: openstack


You want to change this as well to match openstack documentation fragment.

Akasurde · 2020-03-27T11:23:47Z

plugins/modules/clustering/pacemaker_resource.py

+version_added: "2.9"
+author: "Mathieu Bultel (matbu)"
+description:
+   - Manage a pacemaker resource from Ansible


Suggested change

- Manage a pacemaker resource from Ansible

- Manage a pacemaker resource from Ansible.

Akasurde · 2020-03-27T11:23:54Z

plugins/modules/clustering/pacemaker_resource.py

+options:
+    state:
+      description:
+        - Indicate desired state of the cluster


Suggested change

- Indicate desired state of the cluster

- Indicate the desired state of the cluster.

Akasurde · 2020-03-27T11:24:30Z

plugins/modules/clustering/pacemaker_resource.py

+      required: true
+    resource:
+      description:
+        - Specify which resource you want to handle


Suggested change

- Specify which resource you want to handle

- Specify which resource user wants to handle.

Akasurde · 2020-03-27T11:25:02Z

plugins/modules/clustering/pacemaker_resource.py

+      default: None
+    timeout:
+      description:
+        - Timeout when the module should considered that the action has failed


Suggested change

- Timeout when the module should considered that the action has failed

- Timeout when the module should be considered that the action has failed.

Akasurde · 2020-03-27T11:25:55Z

plugins/modules/clustering/pacemaker_resource.py

+        required: false
+        default: false
+requirements:
+    - "python >= 2.6"


Suggested change

- "python >= 2.6"

- "python >= 2.7"

Akasurde · 2020-03-27T11:26:57Z

plugins/modules/clustering/pacemaker_resource.py

+'''
+
+RETURN = '''
+


Please add a RETURN block. This will help users to understand the return of the module without running the module actually. Gives a good UI/UX to the module.

Akasurde · 2020-03-27T11:27:48Z

plugins/modules/clustering/pacemaker_resource.py

+
+def check_resource_state(module, resource, state):
+    # get resources
+    cmd = "bash -c 'pcs status --full | grep -w \"%s[ \t]\"'" % resource


Please parse output in Python. This will lead to security vulnerability since resource is governed by user input.

Also, don't assume bash will be available on the given machine.

Akasurde · 2020-03-27T11:29:50Z

plugins/modules/clustering/pacemaker_resource.py

+
+
+def get_resource(module, resource):
+    cmd = "pcs resource show %s" % resource


Is there any check to sanitize resource?

Akasurde · 2020-03-27T11:30:27Z

plugins/modules/clustering/pacemaker_resource.py

@@ -0,0 +1,153 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-


Could you please add a unit test for this module?

ansibullbot · 2020-04-17T17:46:38Z

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module community.general.pacemaker_resource" returned exit status 0.
>>> Standard Error
[WARNING]: module community.general.pacemaker_resource not found in:
/dev/null:/root/ansible/lib/ansible/modules

click here for bot help

gundalow · 2020-04-18T08:33:06Z

plugins/modules/clustering/pacemaker_resource.py

@@ -0,0 +1,153 @@
+#!/usr/bin/python


Please add a symlink to this file in plugins/modules

gundalow · 2020-06-17T14:17:20Z

@matbu Hi, anything we can do to help you with this, please feel free to ask here.

Or if you are used to IRC feel free to join in #ansible-devel on Freenode

Andersson007 · 2020-10-16T11:32:12Z

@matbu any updates on this?
needs_info

* Update README and add CHANGELOG * Prepare changelog for version 0.2.0

gundalow · 2020-12-01T15:15:07Z

@matbu Hi, is there anything we can do to help you with this PR for adding a new pacemaker_resource Ansible module for managing cluster resources

ansibullbot · 2020-12-01T15:16:31Z

@matbu This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

felixfontein · 2021-03-28T19:54:53Z

Closing since there was no more reaction. @matbu if you're interested again in working on this, please ping me. If someone else is interested in continuing this PR, please also write it here so we can coordinate.

matbu added 4 commits March 26, 2020 10:47

Add pacemaker_resource module for managing cluster resources

064ad17

Add pacemaker_resource module for managing pacemaker cluster resources.

fix syntax error and use changed var instead of hardcoded bool

d5299b2

fix pep8 lint missing whitespace between operator

4fe2782

update module name in the documentation

52390de

gundalow reviewed Mar 26, 2020

View reviewed changes

Akasurde requested changes Mar 27, 2020

View reviewed changes

felixfontein added the new plugin label Apr 7, 2020

ansibullbot added backport bug This issue/PR relates to a bug module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_module New module new_plugin New plugin stale_ci CI is older than 7 days, rerun before merging labels Apr 9, 2020

gundalow removed backport labels Apr 10, 2020

ansibullbot added affects_2.10 labels Apr 17, 2020

gundalow reviewed Apr 18, 2020

View reviewed changes

gundalow added the ci_verified Push fixes to PR branch to re-run CI label Apr 18, 2020

settings bot removed affects_2.10 labels Jun 16, 2020

gundalow added the pr_day Has been reviewed during a PR review Day. https://github.com/ansible/community/issues/407 label Jun 17, 2020

felixfontein changed the base branch from master to main July 6, 2020 06:54

ansibullbot added the affects_1 label Oct 16, 2020

ansibullbot added needs_info This issue requires further information. Please answer any outstanding questions new_contributor Help guide this first time contributor labels Oct 16, 2020

amenzhinsky pushed a commit to amenzhinsky/community.general that referenced this pull request Nov 13, 2020

Update README (ansible-collections#53)

9479446

* Update README and add CHANGELOG * Prepare changelog for version 0.2.0

felixfontein removed the bug This issue/PR relates to a bug label Dec 20, 2020

felixfontein closed this Mar 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pacemaker_resource module for managing cluster resources #53

Add pacemaker_resource module for managing cluster resources #53

matbu commented Mar 26, 2020

gundalow left a comment

matbu commented Mar 26, 2020

gundalow left a comment

gundalow Mar 26, 2020

gundalow Mar 26, 2020

gundalow Mar 26, 2020

gundalow Mar 26, 2020

gundalow Mar 26, 2020

gundalow Mar 26, 2020

gundalow Mar 26, 2020

gundalow Mar 26, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

Akasurde Mar 27, 2020

ansibullbot commented Apr 17, 2020

gundalow Apr 18, 2020

gundalow commented Jun 17, 2020

Andersson007 commented Oct 16, 2020

gundalow commented Dec 1, 2020

ansibullbot commented Dec 1, 2020

felixfontein commented Mar 28, 2021

-      pacemaker_resource: state=enable resource=haproxy
+      community.general.pacemaker_resource:
+        state: enable
+        resource: haproxy

	- Manage a pacemaker resource from Ansible
	- Manage a pacemaker resource from Ansible.

	- Indicate desired state of the cluster
	- Indicate the desired state of the cluster.

	- Specify which resource you want to handle
	- Specify which resource user wants to handle.

	- Timeout when the module should considered that the action has failed
	- Timeout when the module should be considered that the action has failed.



		def get_resource(module, resource):
		cmd = "pcs resource show %s" % resource

Add pacemaker_resource module for managing cluster resources #53

Add pacemaker_resource module for managing cluster resources #53

Conversation

matbu commented Mar 26, 2020

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

gundalow left a comment

Choose a reason for hiding this comment

matbu commented Mar 26, 2020

gundalow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansibullbot commented Apr 17, 2020

Choose a reason for hiding this comment

gundalow commented Jun 17, 2020

Andersson007 commented Oct 16, 2020

gundalow commented Dec 1, 2020

ansibullbot commented Dec 1, 2020

felixfontein commented Mar 28, 2021