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 7 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
29 changes: 26 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 @@ -325,9 +326,10 @@
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 @@ -387,6 +389,18 @@
self.__collect(exclude_fields, return_empty_dbs, set(self.info))
return self.info

def is_mariadb(self):
if self.server_implementation == "mariadb":
return True
else:
return False

def is_mysql(self):
if self.server_implementation == "mysql":
return True

Check warning on line 400 in plugins/modules/mysql_info.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/mysql_info.py#L400

Added line #L400 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.

This would be more at home in module_utils/mysql.py

And it's crazy to think we have a variable db_engine in the integrations tests but we don't have an equivalent in the modules. If we implement this in Python, we could then use it in the integrations tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved it to the module_utils/mysql.py

else:
return False

Check warning on line 402 in plugins/modules/mysql_info.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/mysql_info.py#L402

Added line #L402 was not covered by tests

def __collect(self, exclude_fields, return_empty_dbs, wanted):
"""Collect all possible subsets."""
if 'version' in wanted or 'settings' in wanted:
Expand Down Expand Up @@ -497,7 +511,10 @@

def __get_slave_status(self):
"""Get slave status if the instance is a slave."""
res = self.__exec_sql('SHOW SLAVE STATUS')
if self.is_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 +755,16 @@
'Exception message: %s' % (connector_name, connector_version, config_file, to_native(e)))
module.fail_json(msg)

cursor.execute("SELECT VERSION()")
if 'mariadb' in cursor.fetchone()["VERSION()"].lower():
server_implementation = "mariadb"
else:
server_implementation = "mysql"

Copy link
Collaborator

Choose a reason for hiding this comment

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

This could use get_server_version() from module_utils/mysql.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved it to the module_utils/mysql.py and using the get_server_version() instead of running the SELECT VERSION()

###############################
# 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
44 changes: 37 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,36 @@ 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


@pytest.mark.parametrize(
'server_implementation',
[
('mysql'),
('mariadb'),
]
)
def test_is_mariadb(server_implementation):
cursor = MagicMock()

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

assert info.is_mariadb() == (server_implementation == "mariadb")


@pytest.mark.parametrize(
'server_implementation',
[
('mysql'),
('mariadb'),
]
)
def test_is_mysql(server_implementation):
cursor = MagicMock()

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

assert info.is_mysql() == (server_implementation == "mysql")