mysql_replication: fix crashes caused by deprecated terminology (#71)

* mysql_replication: fix crashes caused by deprecated terminology

* Fix unrelated sanity errors

* Tests: mysql 8.0.21 -> 8.022

* Adjust integration tests

* Add version check to the tests

* Add debug statement

* Adjust mysql version

* Fix tests

* Add unit tests

* Add changelog fragment

* Improve code and coverage

* Get rid of extra blank line

* Improve coverage

* Change suggested

.github/workflows/ansible-test-plugins.yml

@@ -55,7 +55,7 @@ jobs:
       matrix:
         mysql:
           - 5.7.31
-          - 8.0.21
+          - 8.0.22
         ansible:
           - stable-2.9
           - stable-2.10
@@ -67,7 +67,7 @@ jobs:
           - pymysql==0.9.3
           - mysqlclient==2.0.1
         exclude:
-          - mysql: 8.0.21
+          - mysql: 8.0.22
             connector: pymysql==0.7.10
     steps:
 
@@ -101,3 +101,45 @@ jobs:
       - uses: codecov/codecov-action@v1
         with:
           fail_ci_if_error: false
+
+  units:
+    runs-on: ubuntu-latest
+    name: Units (Ⓐ${{ matrix.ansible }})
+    strategy:
+      # As soon as the first unit test fails,
+      # cancel the others to free up the CI queue
+      fail-fast: true
+      matrix:
+        ansible:
+          - stable-2.9
+          - stable-2.10
+          - devel
+
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v2
+        with:
+          path: ./ansible_collections/community/mysql
+
+      - name: Set up Python
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.8
+
+      - name: Install ansible-base (${{matrix.ansible}})
+        run: pip install https://github.com/ansible/ansible/archive/${{ matrix.ansible }}.tar.gz --disable-pip-version-check
+
+      # Run the unit tests
+      - name: Run unit test
+        run: ansible-test units -v --color --docker --coverage
+        working-directory: ./ansible_collections/community/mysql
+
+      # ansible-test support producing code coverage date
+      - name: Generate coverage report
+        run: ansible-test coverage xml -v --requirements --group-by command --group-by version
+        working-directory: ./ansible_collections/community/mysql
+
+      # See the reports at https://codecov.io/gh/GITHUBORG/REPONAME
+      - uses: codecov/codecov-action@v1
+        with:
+          fail_ci_if_error: false

changelogs/fragments/71-mysql_replication_add_replica_keyword_support.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- mysql_replication - fix crashes of mariadb >= 10.5.1 and mysql >= 8.0.22 caused by using deprecated terminology (https://github.com/ansible-collections/community.mysql/issues/70).

plugins/modules/mysql_replication.py

@@ -242,21 +242,40 @@ import warnings
 from ansible.module_utils.basic import AnsibleModule
 from ansible_collections.community.mysql.plugins.module_utils.mysql import mysql_connect, mysql_driver, mysql_driver_fail_msg, mysql_common_argument_spec
 from ansible.module_utils._text import to_native
+from distutils.version import LooseVersion
 
 executed_queries = []
 
 
+def uses_replica_terminology(cursor):
+    """Checks if REPLICA must be used instead of SLAVE"""
+    cursor.execute("SELECT VERSION() AS version")
+    result = cursor.fetchone()
+
+    if isinstance(result, dict):
+        version_str = result['version']
+    else:
+        version_str = result[0]
+
+    version = LooseVersion(version_str)
+
+    if 'mariadb' in version_str.lower():
+        return version >= LooseVersion('10.5.1')
+    else:
+        return version >= LooseVersion('8.0.22')
+
+
 def get_master_status(cursor):
     cursor.execute("SHOW MASTER STATUS")
     masterstatus = cursor.fetchone()
     return masterstatus
 
 
-def get_slave_status(cursor, connection_name='', channel=''):
+def get_slave_status(cursor, connection_name='', channel='', term='REPLICA'):
     if connection_name:
-        query = "SHOW SLAVE '%s' STATUS" % connection_name
+        query = "SHOW %s '%s' STATUS" % (term, connection_name)
     else:
-        query = "SHOW SLAVE STATUS"
+        query = "SHOW %s STATUS" % term
 
     if channel:
         query += " FOR CHANNEL '%s'" % channel
@@ -266,11 +285,11 @@ def get_slave_status(cursor, connection_name='', channel=''):
     return slavestatus
 
 
-def stop_slave(module, cursor, connection_name='', channel='', fail_on_error=False):
+def stop_slave(module, cursor, connection_name='', channel='', fail_on_error=False, term='REPLICA'):
     if connection_name:
-        query = "STOP SLAVE '%s'" % connection_name
+        query = "STOP %s '%s'" % (term, connection_name)
     else:
-        query = 'STOP SLAVE'
+        query = 'STOP %s' % term
 
     if channel:
         query += " FOR CHANNEL '%s'" % channel
@@ -288,11 +307,11 @@ def stop_slave(module, cursor, connection_name='', channel='', fail_on_error=Fal
     return stopped
 
 
-def reset_slave(module, cursor, connection_name='', channel='', fail_on_error=False):
+def reset_slave(module, cursor, connection_name='', channel='', fail_on_error=False, term='REPLICA'):
     if connection_name:
-        query = "RESET SLAVE '%s'" % connection_name
+        query = "RESET %s '%s'" % (term, connection_name)
     else:
-        query = 'RESET SLAVE'
+        query = 'RESET %s' % term
 
     if channel:
         query += " FOR CHANNEL '%s'" % channel
@@ -310,11 +329,11 @@ def reset_slave(module, cursor, connection_name='', channel='', fail_on_error=Fa
     return reset
 
 
-def reset_slave_all(module, cursor, connection_name='', channel='', fail_on_error=False):
+def reset_slave_all(module, cursor, connection_name='', channel='', fail_on_error=False, term='REPLICA'):
     if connection_name:
-        query = "RESET SLAVE '%s' ALL" % connection_name
+        query = "RESET %s '%s' ALL" % (term, connection_name)
     else:
-        query = 'RESET SLAVE ALL'
+        query = 'RESET %s ALL' % term
 
     if channel:
         query += " FOR CHANNEL '%s'" % channel
@@ -347,11 +366,11 @@ def reset_master(module, cursor, fail_on_error=False):
     return reset
 
 
-def start_slave(module, cursor, connection_name='', channel='', fail_on_error=False):
+def start_slave(module, cursor, connection_name='', channel='', fail_on_error=False, term='REPLICA'):
     if connection_name:
-        query = "START SLAVE '%s'" % connection_name
+        query = "START %s '%s'" % (term, connection_name)
     else:
-        query = 'START SLAVE'
+        query = 'START %s' % term
 
     if channel:
         query += " FOR CHANNEL '%s'" % channel
@@ -467,6 +486,13 @@ def main():
         else:
             module.fail_json(msg="unable to find %s. Exception message: %s" % (config_file, to_native(e)))
 
+    # Since MySQL 8.0.22 and MariaDB 10.5.1,
+    # "REPLICA" must be used instead of "SLAVE"
+    if uses_replica_terminology(cursor):
+        replica_term = 'REPLICA'
+    else:
+        replica_term = 'SLAVE'
+
     if mode in "getmaster":
         status = get_master_status(cursor)
         if not isinstance(status, dict):
@@ -476,7 +502,7 @@ def main():
         module.exit_json(queries=executed_queries, **status)
 
     elif mode in "getslave":
-        status = get_slave_status(cursor, connection_name, channel)
+        status = get_slave_status(cursor, connection_name, channel, replica_term)
         if not isinstance(status, dict):
             status = dict(Is_Slave=False, msg="Server is not configured as mysql slave")
         else:
@@ -531,13 +557,13 @@ def main():
         result['changed'] = True
         module.exit_json(queries=executed_queries, **result)
     elif mode in "startslave":
-        started = start_slave(module, cursor, connection_name, channel, fail_on_error)
+        started = start_slave(module, cursor, connection_name, channel, fail_on_error, replica_term)
         if started is True:
             module.exit_json(msg="Slave started ", changed=True, queries=executed_queries)
         else:
             module.exit_json(msg="Slave already started (Or cannot be started)", changed=False, queries=executed_queries)
     elif mode in "stopslave":
-        stopped = stop_slave(module, cursor, connection_name, channel, fail_on_error)
+        stopped = stop_slave(module, cursor, connection_name, channel, fail_on_error, replica_term)
         if stopped is True:
             module.exit_json(msg="Slave stopped", changed=True, queries=executed_queries)
         else:
@@ -549,13 +575,13 @@ def main():
         else:
             module.exit_json(msg="Master already reset", changed=False, queries=executed_queries)
     elif mode in "resetslave":
-        reset = reset_slave(module, cursor, connection_name, channel, fail_on_error)
+        reset = reset_slave(module, cursor, connection_name, channel, fail_on_error, replica_term)
         if reset is True:
             module.exit_json(msg="Slave reset", changed=True, queries=executed_queries)
         else:
             module.exit_json(msg="Slave already reset", changed=False, queries=executed_queries)
     elif mode in "resetslaveall":
-        reset = reset_slave_all(module, cursor, connection_name, channel, fail_on_error)
+        reset = reset_slave_all(module, cursor, connection_name, channel, fail_on_error, replica_term)
         if reset is True:
             module.exit_json(msg="Slave reset", changed=True, queries=executed_queries)
         else:

tests/integration/targets/setup_mysql/defaults/main.yml

@@ -7,7 +7,7 @@ percona_client_version: 5.7
 
 mariadb_install: false
 
-mysql_version: 8.0.21
+mysql_version: 8.0.22
 mariadb_version: 10.5.4
 
 mysql_base_port: 3306

tests/integration/targets/test_mysql_replication/tasks/mysql_replication_channel.yml

@@ -48,7 +48,7 @@
     - assert:
         that:
         - result is changed
-        - result.queries == ["START SLAVE FOR CHANNEL '{{ test_channel }}'"]
+        - result.queries == ["START SLAVE FOR CHANNEL '{{ test_channel }}'"] or result.queries == ["START REPLICA FOR CHANNEL '{{ test_channel }}'"]
 
     # Test getslave mode:
     - name: Get standby status with channel
@@ -69,6 +69,20 @@
         - slave_status.Last_IO_Error == ''
         - slave_status.Channel_Name == '{{ test_channel }}'
         - slave_status is not changed
+      when: mysql8022_and_higher == false
+
+    - assert:
+        that:
+        - slave_status.Is_Slave == true
+        - slave_status.Source_Host == '{{ mysql_host }}'
+        - slave_status.Exec_Source_Log_Pos == mysql_primary_status.Position
+        - slave_status.Source_Port == {{ mysql_primary_port }}
+        - slave_status.Last_IO_Errno == 0
+        - slave_status.Last_IO_Error == ''
+        - slave_status.Channel_Name == '{{ test_channel }}'
+        - slave_status is not changed
+      when: mysql8022_and_higher == true
+
 
     # Test stopslave mode:
     - name: Stop slave with channel
@@ -82,7 +96,7 @@
     - assert:
         that:
         - result is changed
-        - result.queries == ["STOP SLAVE FOR CHANNEL '{{ test_channel }}'"]
+        - result.queries == ["STOP SLAVE FOR CHANNEL '{{ test_channel }}'"] or result.queries == ["STOP REPLICA FOR CHANNEL '{{ test_channel }}'"]
 
     # Test reset
     - name: Reset slave with channel
@@ -96,7 +110,7 @@
     - assert:
         that:
         - result is changed
-        - result.queries == ["RESET SLAVE FOR CHANNEL '{{ test_channel }}'"]
+        - result.queries == ["RESET SLAVE FOR CHANNEL '{{ test_channel }}'"] or result.queries == ["RESET REPLICA FOR CHANNEL '{{ test_channel }}'"]
 
     # Test reset all
     - name: Reset slave all with channel
@@ -110,4 +124,4 @@
     - assert:
         that:
         - result is changed
-        - result.queries == ["RESET SLAVE ALL FOR CHANNEL '{{ test_channel }}'"]
+        - result.queries == ["RESET SLAVE ALL FOR CHANNEL '{{ test_channel }}'"] or result.queries == ["RESET REPLICA ALL FOR CHANNEL '{{ test_channel }}'"]

tests/integration/targets/test_mysql_replication/tasks/mysql_replication_initial.yml

@@ -8,6 +8,22 @@
       login_host: 127.0.0.1
 
   block:
+    - name: find out the database version
+      mysql_info:
+        <<: *mysql_params
+        login_port: '{{ mysql_primary_port }}'
+        filter: version
+      register: db
+
+    - name: Set mysql8022_and_higher
+      set_fact:
+        mysql8022_and_higher: false
+
+    - name: Set mysql8022_and_higher
+      set_fact:
+        mysql8022_and_higher: true
+      when:
+      - db.version.major > 8 or (db.version.major == 8 and db.version.minor > 0) or (db.version.major == 8 and db.version.minor == 0 and db.version.release >= 22)
 
     - name: alias mysql command to include default options
       set_fact:
@@ -120,7 +136,7 @@
     - assert:
         that:
         - result is changed
-        - result.queries == ["START SLAVE"]
+        - result.queries == ["START SLAVE"] or result.queries == ["START REPLICA"]
 
     # Test getslave mode:
     - name: Get standby status
@@ -139,6 +155,18 @@
         - slave_status.Last_IO_Errno == 0
         - slave_status.Last_IO_Error == ''
         - slave_status is not changed
+      when: mysql8022_and_higher == false
+
+    - assert:
+        that:
+        - slave_status.Is_Slave == true
+        - slave_status.Source_Host == '{{ mysql_host }}'
+        - slave_status.Exec_Source_Log_Pos == mysql_primary_status.Position
+        - slave_status.Source_Port == {{ mysql_primary_port }}
+        - slave_status.Last_IO_Errno == 0
+        - slave_status.Last_IO_Error == ''
+        - slave_status is not changed
+      when: mysql8022_and_higher == true
 
     # Create test table and add data to it:
     - name: Create test table
@@ -164,6 +192,12 @@
     - assert:
         that:
         - slave_status.Exec_Master_Log_Pos != mysql_primary_status.Position
+      when: mysql8022_and_higher == false
+
+    - assert:
+        that:
+        - slave_status.Exec_Source_Log_Pos != mysql_primary_status.Position
+      when: mysql8022_and_higher == true
 
     - shell: pip show pymysql | awk '/Version/ {print $2}'
       register: pymysql_version
@@ -192,7 +226,7 @@
     - assert:
         that:
         - result is changed
-        - result.queries == ["STOP SLAVE"]
+        - result.queries == ["STOP SLAVE"] or result.queries == ["STOP REPLICA"]
 
     # Test stopslave mode:
     - name: Stop slave that is no longer running

tests/unit/plugins/modules/test_mysql_replication.py

@@ -0,0 +1,47 @@
+# -*- coding: utf-8 -*-
+# Copyright: (c) 2020, Andrew Klychkov (@Andersson007) <aaklychkov@mail.ru>
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import pytest
+
+from ansible_collections.community.mysql.plugins.modules.mysql_replication import uses_replica_terminology
+
+
+class dummy_cursor_class():
+    def __init__(self, output, ret_val_type='dict'):
+        self.output = output
+        self.ret_val_type = ret_val_type
+
+    def execute(self, query):
+        pass
+
+    def fetchone(self):
+        if self.ret_val_type == 'dict':
+            return {'version': self.output}
+
+        elif self.ret_val_type == 'list':
+            return [self.output]
+
+
+@pytest.mark.parametrize(
+    'f_output,c_output,c_ret_type',
+    [
+        (False, '5.5.1-mysql', 'list'),
+        (False, '5.7.0-mysql', 'dict'),
+        (False, '8.0.0-mysql', 'list'),
+        (False, '8.0.11-mysql', 'dict'),
+        (False, '8.0.21-mysql', 'list'),
+        (False, '10.5.0-mariadb', 'dict'),
+        (True, '8.0.22-mysql', 'list'),
+        (True, '8.1.2-mysql', 'dict'),
+        (True, '9.0.0-mysql', 'list'),
+        (True, '10.5.1-mariadb', 'dict'),
+        (True, '10.6.0-mariadb', 'dict'),
+        (True, '11.5.1-mariadb', 'dict'),
+    ]
+)
+def test_uses_replica_terminology(f_output, c_output, c_ret_type):
+    cursor = dummy_cursor_class(c_output, c_ret_type)
+    assert uses_replica_terminology(cursor) == f_output