From 2fbac8948d4ce2f5be86759687b5fa313d5fc051 Mon Sep 17 00:00:00 2001 From: Andrey Klychkov Date: Thu, 4 Apr 2019 15:31:14 +0300 Subject: [PATCH] postgresql_idx: added CI tests for check_mode, rewrite code related with check_mode, misc fixes (#54848) * postgresql_idx: added CI tests, misc fixes * postgresql_idx: fix sanity --- .../database/postgresql/postgresql_idx.py | 70 +++++++---- .../postgresql/tasks/postgresql_idx.yml | 111 ++++++++++++++++++ 2 files changed, 156 insertions(+), 25 deletions(-) diff --git a/lib/ansible/modules/database/postgresql/postgresql_idx.py b/lib/ansible/modules/database/postgresql/postgresql_idx.py index ebc48dc84db..55c830e779e 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_idx.py +++ b/lib/ansible/modules/database/postgresql/postgresql_idx.py @@ -18,9 +18,10 @@ DOCUMENTATION = r''' module: postgresql_idx short_description: Create or drop indexes from a PostgreSQL database description: -- Creates or drops indexes from a remote PostgreSQL database +- Create or drop indexes from a PostgreSQL database U(https://www.postgresql.org/docs/current/sql-createindex.html). -version_added: "2.8" +version_added: '2.8' + options: idxname: description: @@ -103,6 +104,8 @@ options: - List of index columns. - Mutually exclusive with I(state=absent). type: list + aliases: + - column cond: description: - Index conditions. @@ -118,7 +121,7 @@ options: concurrent: description: - Enable or disable concurrent mode (CREATE / DROP INDEX CONCURRENTLY). - - Mutually exclusive with check mode and I(cascade=yes). + - Mutually exclusive with I(cascade=yes). type: bool default: yes tablespace: @@ -140,31 +143,31 @@ options: - Mutually exclusive with I(concurrent=yes) type: bool default: no + notes: - The default authentication assumes that you are either logging in as or sudo'ing to the postgres account on the host. -- I(cuncurrent=yes) cannot be used in check mode because - "CREATE INDEX CONCURRENTLY" cannot run inside a transaction block. - This module uses psycopg2, a Python PostgreSQL database adapter. You must ensure that psycopg2 is installed on the host before using this module. - If the remote host is the PostgreSQL server (which is the default case), then PostgreSQL must also be installed on the remote host. - For Ubuntu-based systems, install the postgresql, libpq-dev, and python-psycopg2 packages on the remote host before using this module. -requirements: [ psycopg2 ] + +requirements: +- psycopg2 + author: - Andrew Klychkov (@Andersson007) ''' EXAMPLES = r''' -# For create / drop index in check mode use concurrent=no and --check - - name: Create btree index if not exists test_idx concurrently covering columns id and name of table products postgresql_idx: db: acme table: products columns: id,name - idxname: test_idx + name: test_idx - name: Create btree index test_idx concurrently with tablespace called ssd and storage parameter postgresql_idx: @@ -258,14 +261,15 @@ valid: import traceback +PSYCOPG2_IMP_ERR = None try: import psycopg2 HAS_PSYCOPG2 = True except ImportError: HAS_PSYCOPG2 = False + PSYCOPG2_IMP_ERR = traceback.format_exc() -import ansible.module_utils.postgres as pgutils -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, missing_required_lib from ansible.module_utils.database import SQLParseError from ansible.module_utils.postgres import postgres_common_argument_spec from ansible.module_utils._text import to_native @@ -441,7 +445,7 @@ def main(): concurrent=dict(type='bool', default=True), table=dict(type='str'), idxtype=dict(type='str', aliases=['type']), - columns=dict(type='list'), + columns=dict(type='list', aliases=['column']), cond=dict(type='str'), session_role=dict(type='str'), tablespace=dict(type='str'), @@ -454,6 +458,9 @@ def main(): supports_check_mode=True, ) + if not HAS_PSYCOPG2: + module.fail_json(msg=missing_required_lib('psycopg2'), exception=PSYCOPG2_IMP_ERR) + idxname = module.params["idxname"] state = module.params["state"] concurrent = module.params["concurrent"] @@ -468,8 +475,8 @@ def main(): cascade = module.params["cascade"] schema = module.params["schema"] - if concurrent and (module.check_mode or cascade): - module.fail_json(msg="Cuncurrent mode and check mode/cascade are mutually exclusive") + if concurrent and cascade: + module.fail_json(msg="Cuncurrent mode and cascade parameters are mutually exclusive") if state == 'present': if not table: @@ -485,9 +492,6 @@ def main(): if cascade and state != 'absent': module.fail_json(msg="cascade parameter used only with state=absent") - if not HAS_PSYCOPG2: - module.fail_json(msg="the python psycopg2 module is required") - # To use defaults values, keyword arguments must be absent, so # check which values are empty and don't include in the **kw # dictionary @@ -511,11 +515,6 @@ def main(): if psycopg2.__version__ < '2.4.3' and sslrootcert is not None: module.fail_json(msg='psycopg2 must be at least 2.4.3 in order to user the ca_cert parameter') - if module.check_mode and concurrent: - module.fail_json(msg="Cannot concurrently create or drop index %s " - "inside the transaction block. The check is possible " - "in not concurrent mode only" % idxname) - try: db_connection = psycopg2.connect(**kw) if concurrent: @@ -529,9 +528,9 @@ def main(): if 'sslrootcert' in e.args[0]: module.fail_json(msg='Postgresql server must be at least version 8.4 to support sslrootcert') - module.fail_json(msg="unable to connect to database: %s" % to_native(e)) + module.fail_json(msg="Unable to connect to database: %s" % to_native(e)) except Exception as e: - module.fail_json(msg="unable to connect to database: %s" % to_native(e)) + module.fail_json(msg="Unable to connect to database: %s" % to_native(e)) if session_role: try: @@ -547,6 +546,27 @@ def main(): kw = index.get_info() kw['query'] = '' + # + # check_mode start + if module.check_mode: + if state == 'present' and index.exists: + kw['changed'] = False + module.exit_json(**kw) + + elif state == 'present' and not index.exists: + kw['changed'] = True + module.exit_json(**kw) + + elif state == 'absent' and not index.exists: + kw['changed'] = False + module.exit_json(**kw) + + elif state == 'absent' and index.exists: + kw['changed'] = True + module.exit_json(**kw) + # check_mode end + # + if state == "present": if idxtype and idxtype.upper() not in VALID_IDX_TYPES: module.fail_json(msg="Index type '%s' of %s is not in valid types" % (idxtype, idxname)) @@ -570,7 +590,7 @@ def main(): kw['state'] = 'absent' kw['query'] = index.executed_query - if not module.check_mode and not kw['valid'] and concurrent: + if not kw['valid']: db_connection.rollback() module.warn("Index %s is invalid! ROLLBACK" % idxname) diff --git a/test/integration/targets/postgresql/tasks/postgresql_idx.yml b/test/integration/targets/postgresql/tasks/postgresql_idx.yml index 26d490ac117..0c4c6f3f066 100644 --- a/test/integration/targets/postgresql/tasks/postgresql_idx.yml +++ b/test/integration/targets/postgresql/tasks/postgresql_idx.yml @@ -59,6 +59,46 @@ # Do main tests # +# Create index in check_mode +- name: postgresql_idx - create btree index in check_mode + become_user: "{{ pg_user }}" + become: yes + postgresql_idx: + db: postgres + login_user: "{{ pg_user }}" + table: test_table + columns: id, story + idxname: test0_idx + check_mode: yes + register: result + ignore_errors: yes + +- assert: + that: + - result.changed == true + - result.tblname == '' + - result.name == 'test0_idx' + - result.state == 'absent' + - result.valid != '' + - result.tblspace == '' + - result.storage_params == [] + - result.schema == '' + - result.query == '' + +# Check that actually nothing changed, rowcount must be 0 +- name: postgresql_idx - check nothing changed after the previous step + become_user: "{{ pg_user }}" + become: yes + postgresql_query: + db: postgres + login_user: "{{ pg_user }}" + query: "SELECT 1 FROM pg_indexes WHERE indexname = 'test0_idx'" + register: result + +- assert: + that: + - result.rowcount == 0 + # Create btree index if not exists test_idx concurrently covering id and story columns - name: postgresql_idx - create btree index concurrently become_user: "{{ pg_user }}" @@ -84,6 +124,20 @@ - result.schema == 'public' - result.query == 'CREATE INDEX CONCURRENTLY test0_idx ON public.test_table USING BTREE (id, story)' +# Check that the index exists after the previous step, rowcount must be 1 +- name: postgresql_idx - check the index exists after the previous step + become_user: "{{ pg_user }}" + become: yes + postgresql_query: + db: postgres + login_user: "{{ pg_user }}" + query: "SELECT 1 FROM pg_indexes WHERE indexname = 'test0_idx'" + register: result + +- assert: + that: + - result.rowcount == 1 + # Check that if index exists that changes nothing - name: postgresql_idx - try to create existing index again become_user: "{{ pg_user }}" @@ -198,6 +252,47 @@ - result.schema == 'public' - result.query == 'CREATE INDEX CONCURRENTLY test1_idx ON public.test_table USING BTREE (id) WHERE id > 1 AND id != 10' +# Drop index from spacific schema with cascade in check_mode +- name: postgresql_idx - drop index from specific schema cascade in check_mode + become_user: "{{ pg_user }}" + become: yes + postgresql_idx: + db: postgres + login_user: "{{ pg_user }}" + schema: foo + name: foo_test_idx + cascade: yes + state: absent + concurrent: no + check_mode: yes + register: result + ignore_errors: yes + +- assert: + that: + - result.changed == true + - result.name == 'foo_test_idx' + - result.state == 'present' + - result.schema == 'foo' + - result.query == '' + when: tablespace.rc == 0 + +# Check that the index exists after the previous step, rowcount must be 1 +- name: postgresql_idx - check the index exists after the previous step + become_user: "{{ pg_user }}" + become: yes + postgresql_query: + db: postgres + login_user: "{{ pg_user }}" + query: "SELECT 1 FROM pg_indexes WHERE indexname = 'foo_test_idx'" + register: result + when: tablespace.rc == 0 + +- assert: + that: + - result.rowcount == 1 + when: tablespace.rc == 0 + # Drop index from spacific schema with cascade - name: postgresql_idx - drop index from specific schema cascade become_user: "{{ pg_user }}" @@ -222,6 +317,22 @@ - result.query == 'DROP INDEX foo.foo_test_idx CASCADE' when: tablespace.rc == 0 +# Check that the index doesn't exist after the previous step, rowcount must be 0 +- name: postgresql_idx - check the index doesn't exist after the previous step + become_user: "{{ pg_user }}" + become: yes + postgresql_query: + db: postgres + login_user: "{{ pg_user }}" + query: "SELECT 1 FROM pg_indexes WHERE indexname = 'foo_test_idx'" + register: result + when: tablespace.rc == 0 + +- assert: + that: + - result.rowcount == 0 + when: tablespace.rc == 0 + # Try to drop not existing index - name: postgresql_idx - try to drop not existing index become_user: "{{ pg_user }}"