From 11e087ff7a4bc9e71375a20755ace952eaaf4823 Mon Sep 17 00:00:00 2001 From: Pilou Date: Wed, 31 Jul 2019 00:21:44 +0000 Subject: [PATCH] consul_session: improve documentation and add integration tests (#56392) (#59357) * consul_session: Python 2.6 is always required on managed node * consul_session: document all types * consul_session: add doc for 'id' parameter * consul_session: improve parameter descriptions - use formatting functions in descriptions - 'name' parameter is required when state=node * consul_session: use required_if * consul_session: add integration tests * consul_session: use 'retry' with network dependent tasks * Use ansible-ci-files bucket for consul binaries Co-Authored-By: Matt Clay (cherry picked from commit 5f8080aaa07ab1f48026bb657dc7f33ae388bdab) * add a changelog fragment --- ...improve_doc_and_add_integration_tests.yaml | 3 + .../modules/clustering/consul_session.py | 51 ++++++---- test/integration/targets/consul/aliases | 2 + test/integration/targets/consul/meta/main.yml | 3 + .../targets/consul/tasks/consul_session.yml | 81 +++++++++++++++ .../integration/targets/consul/tasks/main.yml | 99 +++++++++++++++++++ .../consul/templates/consul_config.hcl.j2 | 13 +++ test/sanity/validate-modules/ignore.txt | 1 - 8 files changed, 233 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/56392-consul_session-improve_doc_and_add_integration_tests.yaml create mode 100644 test/integration/targets/consul/aliases create mode 100644 test/integration/targets/consul/meta/main.yml create mode 100644 test/integration/targets/consul/tasks/consul_session.yml create mode 100644 test/integration/targets/consul/tasks/main.yml create mode 100644 test/integration/targets/consul/templates/consul_config.hcl.j2 diff --git a/changelogs/fragments/56392-consul_session-improve_doc_and_add_integration_tests.yaml b/changelogs/fragments/56392-consul_session-improve_doc_and_add_integration_tests.yaml new file mode 100644 index 00000000000..e1281791d7c --- /dev/null +++ b/changelogs/fragments/56392-consul_session-improve_doc_and_add_integration_tests.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - "consul_session - ``state`` parameter: use ``required_if``, document ``id`` parameter, update ``name`` parameter documentation" diff --git a/lib/ansible/modules/clustering/consul_session.py b/lib/ansible/modules/clustering/consul_session.py index 8b0c402f4dc..aa916af9c59 100644 --- a/lib/ansible/modules/clustering/consul_session.py +++ b/lib/ansible/modules/clustering/consul_session.py @@ -20,64 +20,77 @@ description: to implement distributed locks. In depth documentation for working with sessions can be found at http://www.consul.io/docs/internals/sessions.html requirements: - - python >= 2.6 - python-consul - requests version_added: "2.0" author: - Steve Gargan (@sgargan) options: + id: + description: + - ID of the session, required when I(state) is either C(info) or + C(remove). + type: str state: description: - Whether the session should be present i.e. created if it doesn't - exist, or absent, removed if present. If created, the ID for the - session is returned in the output. If absent, the name or ID is + exist, or absent, removed if present. If created, the I(id) for the + session is returned in the output. If C(absent), I(id) is required to remove the session. Info for a single session, all the sessions for a node or all available sessions can be retrieved by - specifying info, node or list for the state; for node or info, the - node name or session id is required as parameter. + specifying C(info), C(node) or C(list) for the I(state); for C(node) + or C(info), the node I(name) or session I(id) is required as parameter. choices: [ absent, info, list, node, present ] + type: str default: present name: description: - - The name that should be associated with the session. This is opaque - to Consul and not required. + - The name that should be associated with the session. Required when + I(state=node) is used. + type: str delay: description: - The optional lock delay that can be attached to the session when it is created. Locks for invalidated sessions ar blocked from being acquired until this delay has expired. Durations are in seconds. + type: int default: 15 node: description: - The name of the node that with which the session will be associated. by default this is the name of the agent. + type: str datacenter: description: - The name of the datacenter in which the session exists or should be created. + type: str checks: description: - - A list of checks that will be used to verify the session health. If + - Checks that will be used to verify the session health. If all the checks fail, the session will be invalidated and any locks associated with the session will be release and can be acquired once the associated lock delay has expired. + type: list host: description: - The host of the consul agent defaults to localhost. + type: str default: localhost port: description: - The port on which the consul agent is running. + type: int default: 8500 scheme: description: - The protocol scheme on which the consul agent is running. + type: str default: http version_added: "2.1" validate_certs: description: - - Whether to verify the tls certificate of the consul agent. + - Whether to verify the TLS certificate of the consul agent. type: bool default: True version_added: "2.1" @@ -86,6 +99,7 @@ options: - The optional behavior that can be attached to the session when it is created. This controls the behavior when a session is invalidated. choices: [ delete, release ] + type: str default: release version_added: "2.2" """ @@ -154,18 +168,12 @@ def lookup_sessions(module): sessions=sessions_list) elif state == 'node': node = module.params.get('node') - if not node: - module.fail_json( - msg="node name is required to retrieve sessions for node") sessions = consul_client.session.node(node, dc=datacenter) module.exit_json(changed=True, node=node, sessions=sessions) elif state == 'info': session_id = module.params.get('id') - if not session_id: - module.fail_json( - msg="session_id is required to retrieve indvidual session info") session_by_id = consul_client.session.info(session_id, dc=datacenter) module.exit_json(changed=True, @@ -209,9 +217,6 @@ def update_session(module): def remove_session(module): session_id = module.params.get('id') - if not session_id: - module.fail_json(msg="""A session id must be supplied in order to - remove a session.""") consul_client = get_consul_api(module) @@ -252,7 +257,15 @@ def main(): datacenter=dict(type='str'), ) - module = AnsibleModule(argument_spec, supports_check_mode=False) + module = AnsibleModule( + argument_spec=argument_spec, + required_if=[ + ('state', 'node', ['name']), + ('state', 'info', ['id']), + ('state', 'remove', ['id']), + ], + supports_check_mode=False + ) test_dependencies(module) diff --git a/test/integration/targets/consul/aliases b/test/integration/targets/consul/aliases new file mode 100644 index 00000000000..ca7c9128514 --- /dev/null +++ b/test/integration/targets/consul/aliases @@ -0,0 +1,2 @@ +shippable/posix/group2 +destructive diff --git a/test/integration/targets/consul/meta/main.yml b/test/integration/targets/consul/meta/main.yml new file mode 100644 index 00000000000..f5ea812bf87 --- /dev/null +++ b/test/integration/targets/consul/meta/main.yml @@ -0,0 +1,3 @@ +--- +dependencies: + - setup_openssl diff --git a/test/integration/targets/consul/tasks/consul_session.yml b/test/integration/targets/consul/tasks/consul_session.yml new file mode 100644 index 00000000000..6763b0ad181 --- /dev/null +++ b/test/integration/targets/consul/tasks/consul_session.yml @@ -0,0 +1,81 @@ +- name: list sessions + consul_session: + state: list + register: result + +- assert: + that: + - result is changed + - "'sessions' in result" + +- name: create a session + consul_session: + state: present + name: testsession + register: result + +- assert: + that: + - result is changed + - result['name'] == 'testsession' + - "'session_id' in result" + +- set_fact: + session_id: "{{ result['session_id'] }}" + +- name: list sessions after creation + consul_session: + state: list + register: result + +- set_fact: + session_count: "{{ result['sessions'] | length }}" + +- assert: + that: + - result is changed + # selectattr not available on Jinja 2.2 provided by CentOS 6 + # hence the two following tasks (set_fact/assert) are used + # - (result['sessions'] | selectattr('ID', 'match', '^' ~ session_id ~ '$') | first)['Name'] == 'testsession' + +- name: search created session + set_fact: + test_session_found: True + loop: "{{ result['sessions'] }}" + when: "item.get('ID') == session_id and item.get('Name') == 'testsession'" + +- name: ensure session was created + assert: + that: + - test_session_found|default(False) + +- name: fetch info about a session + consul_session: + state: info + id: '{{ session_id }}' + register: result + +- assert: + that: + - result is changed + +- name: ensure 'id' parameter is required when state=info + consul_session: + state: info + name: test + register: result + ignore_errors: True + +- assert: + that: + - result is failed + +- name: delete a session + consul_session: + state: absent + id: '{{ session_id }}' + register: result + +- assert: + that: + - result is changed diff --git a/test/integration/targets/consul/tasks/main.yml b/test/integration/targets/consul/tasks/main.yml new file mode 100644 index 00000000000..5929ee2b849 --- /dev/null +++ b/test/integration/targets/consul/tasks/main.yml @@ -0,0 +1,99 @@ +--- +- name: Install Consul and test + + vars: + consul_version: '1.5.0' + consul_uri: https://s3.amazonaws.com/ansible-ci-files/test/integration/targets/consul/consul_{{ consul_version }}_{{ ansible_system | lower }}_{{ consul_arch }}.zip + consul_cmd: '{{ output_dir }}/consul' + + block: + - name: register pyOpenSSL version + command: "{{ ansible_python_interpreter }} -c 'import OpenSSL; print(OpenSSL.__version__)'" + register: pyopenssl_version + + - name: Install requests<2.20 (CentOS/RHEL 6) + pip: + name: requests<2.20 + register: result + until: result is success + when: ansible_distribution_file_variety|default() == 'RedHat' and ansible_distribution_major_version is version('6', '<=') + + - name: Install python-consul + pip: + name: python-consul + register: result + until: result is success + + - when: pyopenssl_version.stdout is version('0.15', '>=') + block: + - name: Generate privatekey + openssl_privatekey: + path: '{{ output_dir }}/privatekey.pem' + + - name: Generate CSR + openssl_csr: + path: '{{ output_dir }}/csr.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + subject: + commonName: localhost + + - name: Generate selfsigned certificate + openssl_certificate: + path: '{{ output_dir }}/cert.pem' + csr_path: '{{ output_dir }}/csr.csr' + privatekey_path: '{{ output_dir }}/privatekey.pem' + provider: selfsigned + selfsigned_digest: sha256 + register: selfsigned_certificate + + - name: 'Install unzip' + package: + name: unzip + register: result + until: result is success + when: ansible_distribution != "MacOSX" # unzip already installed + + - assert: + # Linux: x86_64, FreeBSD: amd64 + that: ansible_architecture in ['i386', 'x86_64', 'amd64'] + - set_fact: + consul_arch: '386' + when: ansible_architecture == 'i386' + - set_fact: + consul_arch: amd64 + when: ansible_architecture in ['x86_64', 'amd64'] + + - name: 'Download consul binary' + unarchive: + src: '{{ consul_uri }}' + dest: '{{ output_dir }}' + remote_src: true + register: result + until: result is success + + - block: + # output_dir is hardcoded/created in test/runner/lib/executor.py and + # contains '~': expand remote path + - command: 'echo {{ output_dir }}' + register: echo_output_dir + + - name: 'Create configuration file' + vars: + remote_dir: '{{ echo_output_dir.stdout }}' + template: + src: consul_config.hcl.j2 + dest: '{{ output_dir }}/consul_config.hcl' + + - name: 'Start Consul (dev mode enabled)' + shell: 'nohup {{ consul_cmd }} agent -dev -config-file {{ output_dir }}/consul_config.hcl /dev/null 2>&1 &' + + - name: 'Create some data' + command: '{{ consul_cmd }} kv put data/value{{ item }} foo{{ item }}' + loop: [1, 2, 3] + + - import_tasks: consul_session.yml + + always: + - name: 'Kill consul process' + shell: "kill $(cat {{ output_dir }}/consul.pid)" + ignore_errors: true diff --git a/test/integration/targets/consul/templates/consul_config.hcl.j2 b/test/integration/targets/consul/templates/consul_config.hcl.j2 new file mode 100644 index 00000000000..9af06f02e92 --- /dev/null +++ b/test/integration/targets/consul/templates/consul_config.hcl.j2 @@ -0,0 +1,13 @@ +# {{ ansible_managed }} +server = true +pid_file = "{{ remote_dir }}/consul.pid" +ports { + http = 8500 + {% if pyopenssl_version.stdout is version('0.15', '>=') %} + https = 8501 + {% endif %} +} +{% if pyopenssl_version.stdout is version('0.15', '>=') %} +key_file = "{{ remote_dir }}/privatekey.pem" +cert_file = "{{ remote_dir }}/cert.pem" +{% endif %} diff --git a/test/sanity/validate-modules/ignore.txt b/test/sanity/validate-modules/ignore.txt index 635814fe398..d08655b0783 100644 --- a/test/sanity/validate-modules/ignore.txt +++ b/test/sanity/validate-modules/ignore.txt @@ -313,7 +313,6 @@ lib/ansible/modules/clustering/consul.py E322 lib/ansible/modules/clustering/consul.py E324 lib/ansible/modules/clustering/consul_kv.py E322 lib/ansible/modules/clustering/consul_kv.py E324 -lib/ansible/modules/clustering/consul_session.py E322 lib/ansible/modules/clustering/etcd3.py E326 lib/ansible/modules/clustering/k8s/_kubernetes.py E322 lib/ansible/modules/clustering/k8s/_kubernetes.py E323