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

Using show all slaves status when using MariaDB to be consistent with MySQL #602

Merged
merged 17 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/602-show-all-slaves-status.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- mysql_info - the ``slave_status`` filter was returning an empty list on MariaDB with multiple replication channels. It now returns all channels by running ``SHOW ALL SLAVES STATUS`` for MariaDB servers (https://github.com/ansible-collections/community.mysql/issues/603).
21 changes: 21 additions & 0 deletions plugins/module_utils/mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,27 @@
return version_str


def get_server_implementation(cursor):
if 'mariadb' in get_server_version(cursor).lower():
return "mariadb"
else:
return "mysql"


def is_mariadb(implementation):
if implementation == "mariadb":
return True
else:
return False


def is_mysql(implementation):
if implementation == "mysql":
return True

Check warning on line 226 in plugins/module_utils/mysql.py

View check run for this annotation

Codecov / codecov/patch

plugins/module_utils/mysql.py#L226

Added line #L226 was not covered by tests
else:
return False

Check warning on line 228 in plugins/module_utils/mysql.py

View check run for this annotation

Codecov / codecov/patch

plugins/module_utils/mysql.py#L228

Added line #L228 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def is_mariadb(implementation):
if implementation == "mariadb":
return True
else:
return False
def is_mysql(implementation):
if implementation == "mysql":
return True
else:
return False



def set_session_vars(module, cursor, session_vars):
"""Set session vars."""
for var, value in session_vars.items():
Expand Down
15 changes: 12 additions & 3 deletions plugins/modules/mysql_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)

from __future__ import absolute_import, division, print_function

__metaclass__ = type

DOCUMENTATION = r'''
Expand Down Expand Up @@ -292,6 +293,8 @@
mysql_driver_fail_msg,
get_connector_name,
get_connector_version,
get_server_implementation,
is_mariadb,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
is_mariadb,

)

from ansible_collections.community.mysql.plugins.module_utils.user import (
Expand Down Expand Up @@ -325,9 +328,10 @@ class MySQL_Info(object):
5. add info about the new subset with an example to RETURN block
"""

def __init__(self, module, cursor):
def __init__(self, module, cursor, server_implementation):
self.module = module
self.cursor = cursor
self.server_implementation = server_implementation
self.info = {
'version': {},
'databases': {},
Expand Down Expand Up @@ -497,7 +501,10 @@ def __get_master_status(self):

def __get_slave_status(self):
"""Get slave status if the instance is a slave."""
res = self.__exec_sql('SHOW SLAVE STATUS')
if is_mariadb(self.server_implementation):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if is_mariadb(self.server_implementation):
if self.server_implementation == "mariadb":

res = self.__exec_sql('SHOW ALL SLAVES STATUS')
else:
res = self.__exec_sql('SHOW SLAVE STATUS')
if res:
for line in res:
host = line['Master_Host']
Expand Down Expand Up @@ -738,10 +745,12 @@ def main():
'Exception message: %s' % (connector_name, connector_version, config_file, to_native(e)))
module.fail_json(msg)

server_implementation = get_server_implementation(cursor)

###############################
# Create object and do main job

mysql = MySQL_Info(module, cursor)
mysql = MySQL_Info(module, cursor, server_implementation)

module.exit_json(changed=False,
connector_name=connector_name,
Expand Down
37 changes: 36 additions & 1 deletion tests/unit/plugins/module_utils/test_mysql.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from __future__ import (absolute_import, division, print_function)

__metaclass__ = type

import pytest

from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version
from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version, get_server_implementation, is_mariadb, is_mysql
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version, get_server_implementation, is_mariadb, is_mysql
from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version, get_server_implementation

from ..utils import dummy_cursor_class


Expand All @@ -22,3 +23,37 @@ def test_get_server_version(cursor_return_version, cursor_return_type):
"""
cursor = dummy_cursor_class(cursor_return_version, cursor_return_type)
assert get_server_version(cursor) == cursor_return_version


@pytest.mark.parametrize(
'cursor_return_version,cursor_return_type,server_implementation',
[
('5.7.0-mysql', 'dict', 'mysql'),
('8.0.0-mysql', 'list', 'mysql'),
('10.5.0-mariadb', 'dict', 'mariadb'),
('10.5.1-mariadb', 'list', 'mariadb'),
]
)
def test_get_server_implamentation(cursor_return_version, cursor_return_type, server_implementation):
"""
Test that server implementation are handled properly by get_server_implementation() whether the server version returned as a list or dict.
"""
cursor = dummy_cursor_class(cursor_return_version, cursor_return_type)

assert get_server_implementation(cursor) == server_implementation


def test_is_mysql():
"""
Test that server is_mysql return expect results
"""
assert is_mysql("mysql") is True
assert is_mysql("mariadb") is False


def test_is_mariadb():
"""
Test that server is_mariadb return expect results
"""
assert is_mariadb("mariadb") is True
assert is_mariadb("mysql") is False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def test_is_mysql():
"""
Test that server is_mysql return expect results
"""
assert is_mysql("mysql") is True
assert is_mysql("mariadb") is False
def test_is_mariadb():
"""
Test that server is_mariadb return expect results
"""
assert is_mariadb("mariadb") is True
assert is_mariadb("mysql") is False

14 changes: 7 additions & 7 deletions tests/unit/plugins/modules/test_mysql_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@


@pytest.mark.parametrize(
'suffix,cursor_output',
'suffix,cursor_output,server_implementation',
[
('mysql', '5.5.1-mysql'),
('log', '5.7.31-log'),
('mariadb', '10.5.0-mariadb'),
('', '8.0.22'),
('mysql', '5.5.1-mysql', 'mysql'),
('log', '5.7.31-log', 'mysql'),
('mariadb', '10.5.0-mariadb', 'mariadb'),
('', '8.0.22', 'mysql'),
]
)
def test_get_info_suffix(suffix, cursor_output):
def test_get_info_suffix(suffix, cursor_output, server_implementation):
def __cursor_return_value(input_parameter):
if input_parameter == "SHOW GLOBAL VARIABLES":
cursor.fetchall.return_value = [{"Variable_name": "version", "Value": cursor_output}]
Expand All @@ -32,6 +32,6 @@ def __cursor_return_value(input_parameter):
cursor = MagicMock()
cursor.execute.side_effect = __cursor_return_value

info = MySQL_Info(MagicMock(), cursor)
info = MySQL_Info(MagicMock(), cursor, server_implementation)

assert info.get_info([], [], False)['version']['suffix'] == suffix