Repository: ansible/ansible
Pull request: mysql_variables: add mode parameter
##### SUMMARY
mysql_variables: add mode parameter
fixes #60119 
continues #62060

##### ISSUE TYPE
- Feature Pull Request

##### COMPONENT NAME
```lib/ansible/modules/database/mysql/mysql_variables.py```

Problem statement:
Use the ansible mysql_variables  to set mysql variables, the variables settings are invalid after restarting mysql
<!--- Verify first that your feature was not already discussed on GitHub -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
<!--- Describe the new feature/improvement briefly below -->

##### ISSUE TYPE
- Feature Idea

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below, use your best guess if unsure -->

##### ADDITIONAL INFORMATION
<!--- Describe how the feature would be used, why it is needed and what it would solve -->

<!--- Paste example playbooks or commands between quotes below -->
```yaml

```

<!--- HINT: You can also paste gist.github.com links for larger files -->

Hints from issue discussion:
The hope is that the restart database variables do not expire.
Please complete the information in the issue template so we can better understand this issue.

needs_info
!component =lib/ansible/modules/database/mysql/mysql_variables.py
https://github.com/ansible/ansible/issues/60121
cc @Alexander198961 @Andersson007 @Xyon @banyek @bmalynovytch @bmildren @kurtdavis @michaelcoburn @oneiroi @tolland
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
@yangchao0512 This issue is waiting for your response. Please respond or the issue will be closed.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: needs_info_base --->
@yangchao0512 You have not responded to information requests in this issue so we will assume it no longer affects you. If you are still interested in this, please create a new issue with the requested information.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: needs_info_base --->

Commit message: mysql_variables: add mode parameter, refactoring

Patch to review:
diff --git a/changelogs/fragments/63547-mysql_variables_add_mode_param.yml b/changelogs/fragments/63547-mysql_variables_add_mode_param.yml
new file mode 100644
index 00000000000000..3eeb05bda2a85d
--- /dev/null
+++ b/changelogs/fragments/63547-mysql_variables_add_mode_param.yml
@@ -0,0 +1,2 @@
+minor_changes:
+- mysql_variables - add ``mode`` parameter (https://github.com/ansible/ansible/issues/60119).
diff --git a/lib/ansible/modules/database/mysql/mysql_variables.py b/lib/ansible/modules/database/mysql/mysql_variables.py
index 1ce7ef2c44cc0a..b750514c268001 100644
--- a/lib/ansible/modules/database/mysql/mysql_variables.py
+++ b/lib/ansible/modules/database/mysql/mysql_variables.py
@@ -19,7 +19,7 @@
 short_description: Manage MySQL global variables
 description:
 - Query / Set MySQL variables.
-version_added: 1.3
+version_added: '1.3'
 author:
 - Balazs Pocze (@banyek)
 options:
@@ -32,6 +32,22 @@
     description:
     - If set, then sets variable value to this
     type: str
+  mode:
+    description:
+    - C(global) assigns C(value) to a global system variable which will be changed at runtime
+      but won't persist across server restarts.
+    - C(persist) assigns C(value) to a global system variable and persists it to
+      the mysqld-auto.cnf option file in the data directory
+      (the variable will survive service restarts).
+    - C(persist_only) persists C(value) to the mysqld-auto.cnf option file in the data directory
+      but without setting the global variable runtime value
+      (the value will be changed after the next service restart).
+    - Supported by MySQL 8.0 or later.
+    - For more information see U(https://dev.mysql.com/doc/refman/8.0/en/set-variable.html).
+    type: str
+    choices: ['global', 'persist', 'persist_only']
+    default: global
+    version_added: '2.10'
 
 seealso:
 - module: mysql_info
@@ -48,10 +64,11 @@
   mysql_variables:
     variable: sync_binlog
 
-- name: Set read_only variable to 1
+- name: Set read_only variable to 1 persistently
   mysql_variables:
     variable: read_only
     value: 1
+    mode: persist
 '''
 
 RETURN = r'''
@@ -75,6 +92,24 @@
 executed_queries = []
 
 
+def check_mysqld_auto(module, cursor, mysqlvar):
+    """Check variable's value in mysqld-auto.cnf."""
+    query = ("SELECT VARIABLE_VALUE "
+             "FROM performance_schema.persisted_variables "
+             "WHERE VARIABLE_NAME = %s")
+    try:
+        cursor.execute(query, (mysqlvar,))
+        res = cursor.fetchone()
+    except Exception as e:
+        if "Table 'performance_schema.persisted_variables' doesn't exist" in str(e):
+            module.fail_json(msg='Server version must be 8.0 or greater.')
+
+    if res:
+        return res[0]
+    else:
+        return None
+
+
 def typedvalue(value):
     """
     Convert value to number whenever possible, return same value
@@ -110,7 +145,7 @@ def getvariable(cursor, mysqlvar):
         return None
 
 
-def setvariable(cursor, mysqlvar, value):
+def setvariable(cursor, mysqlvar, value, mode='global'):
     """ Set a global mysql variable to a given value
 
     The DB driver will handle quoting of the given value based on its
@@ -118,7 +153,13 @@ def setvariable(cursor, mysqlvar, value):
     should be passed as numeric literals.
 
     """
-    query = "SET GLOBAL %s = " % mysql_quote_identifier(mysqlvar, 'vars')
+    if mode == 'persist':
+        query = "SET PERSIST %s = " % mysql_quote_identifier(mysqlvar, 'vars')
+    elif mode == 'global':
+        query = "SET GLOBAL %s = " % mysql_quote_identifier(mysqlvar, 'vars')
+    elif mode == 'persist_only':
+        query = "SET PERSIST_ONLY %s = " % mysql_quote_identifier(mysqlvar, 'vars')
+
     try:
         cursor.execute(query + "%s", (value,))
         executed_queries.append(query + "%s" % value)
@@ -126,6 +167,7 @@ def setvariable(cursor, mysqlvar, value):
         result = True
     except Exception as e:
         result = to_native(e)
+
     return result
 
 
@@ -144,6 +186,7 @@ def main():
             ca_cert=dict(type='path', aliases=['ssl_ca']),
             connect_timeout=dict(type='int', default=30),
             config_file=dict(type='path', default='~/.my.cnf'),
+            mode=dict(type='str', choices=['global', 'persist', 'persist_only'], default='global'),
         ),
     )
     user = module.params["login_user"]
@@ -157,6 +200,8 @@ def main():
 
     mysqlvar = module.params["variable"]
     value = module.params["value"]
+    mode = module.params["mode"]
+
     if mysqlvar is None:
         module.fail_json(msg="Cannot run without variable to operate with")
     if match('^[0-9a-z_]+$', mysqlvar) is None:
@@ -177,27 +222,52 @@ def main():
         else:
             module.fail_json(msg="unable to find %s. Exception message: %s" % (config_file, to_native(e)))
 
+    mysqlvar_val = None
+    var_in_mysqld_auto_cnf = None
+
     mysqlvar_val = getvariable(cursor, mysqlvar)
     if mysqlvar_val is None:
         module.fail_json(msg="Variable not available \"%s\"" % mysqlvar, changed=False)
+
     if value is None:
         module.exit_json(msg=mysqlvar_val)
-    else:
-        # Type values before using them
-        value_wanted = typedvalue(value)
-        value_actual = typedvalue(mysqlvar_val)
-        if value_wanted == value_actual:
-            module.exit_json(msg="Variable already set to requested value", changed=False)
-        try:
-            result = setvariable(cursor, mysqlvar, value_wanted)
-        except SQLParseError as e:
-            result = to_native(e)
-
-        if result is True:
-            module.exit_json(msg="Variable change succeeded prev_value=%s" % value_actual,
-                             changed=True, queries=executed_queries)
+
+    if mode in ('persist', 'persist_only'):
+        var_in_mysqld_auto_cnf = check_mysqld_auto(module, cursor, mysqlvar)
+        if var_in_mysqld_auto_cnf is None:
+            mysqlvar_val = False
         else:
-            module.fail_json(msg=result, changed=False)
+            mysqlvar_val = var_in_mysqld_auto_cnf
+
+    # Type values before using them
+    value_wanted = typedvalue(value)
+    value_actual = typedvalue(mysqlvar_val)
+    value_in_auto_cnf = None
+    if var_in_mysqld_auto_cnf is not None:
+        value_in_auto_cnf = typedvalue(var_in_mysqld_auto_cnf)
+
+    if value_wanted == value_actual and mode in ('global', 'persist'):
+        if mode == 'persist' and value_wanted == value_in_auto_cnf:
+            module.exit_json(msg="Variable is already set to requested value globally"
+                                 "and stored into mysqld-auto.cnf file.", changed=False)
+
+        elif mode == 'global':
+            module.exit_json(msg="Variable is already set to requested value.", changed=False)
+
+    if mode == 'persist_only' and value_wanted == value_in_auto_cnf:
+        module.exit_json(msg="Variable is already stored into mysqld-auto.cnf "
+                             "with requested value.", changed=False)
+
+    try:
+        result = setvariable(cursor, mysqlvar, value_wanted, mode)
+    except SQLParseError as e:
+        result = to_native(e)
+
+    if result is True:
+        module.exit_json(msg="Variable change succeeded prev_value=%s" % value_actual,
+                         changed=True, queries=executed_queries)
+    else:
+        module.fail_json(msg=result, changed=False)
 
 
 if __name__ == '__main__':
diff --git a/test/integration/targets/mysql_variables/tasks/mysql_variables.yml b/test/integration/targets/mysql_variables/tasks/mysql_variables.yml
index 5ebc4921225feb..c7f5cb57768d3d 100644
--- a/test/integration/targets/mysql_variables/tasks/mysql_variables.yml
+++ b/test/integration/targets/mysql_variables/tasks/mysql_variables.yml
@@ -308,3 +308,89 @@
     login_unix_socket: '{{ mysql_socket }}'
     login_user: root
     login_password: '{{ root_pass }}'
+
+#=========================================
+# Check mode 'persist' and 'persist_only':
+#
+- name: update mysql variable value (expect changed=true) in persist mode
+  mysql_variables:
+    variable: '{{ set_name }}'
+    value: '{{ set_value }}'
+    login_unix_socket: '{{ mysql_socket }}'
+    login_user: root
+    login_password: '{{ root_pass }}'
+    mode: persist
+  register: result
+
+- assert:
+    that:
+    - result.queries == ["SET PERSIST `{{ set_name }}` = {{ set_value }}"]
+
+- include: assert_var.yml changed=true output={{result}} var_name={{set_name}} var_value='{{set_value}}'
+
+- name: try to update mysql variable value (expect changed=false) in persist mode again
+  mysql_variables:
+    variable: '{{ set_name }}'
+    value: '{{ set_value }}'
+    login_unix_socket: '{{ mysql_socket }}'
+    login_user: root
+    login_password: '{{ root_pass }}'
+    mode: persist
+  register: result
+
+- include: assert_var.yml changed=false output={{result}} var_name={{set_name}} var_value='{{set_value}}'
+
+- name: set mysql variable to a temp value
+  mysql_variables:
+    variable: '{{ set_name }}'
+    value: '200'
+    login_unix_socket: '{{ mysql_socket }}'
+    login_user: root
+    login_password: '{{ root_pass }}'
+    mode: persist
+
+- name: update mysql variable value (expect changed=true) in persist_only mode
+  mysql_variables:
+    variable: '{{ set_name }}'
+    value: '{{ set_value }}'
+    login_unix_socket: '{{ mysql_socket }}'
+    login_user: root
+    login_password: '{{ root_pass }}'
+    mode: persist_only
+  register: result
+
+- assert:
+    that:
+    - result is changed
+    - result.queries == ["SET PERSIST_ONLY `{{ set_name }}` = {{ set_value }}"]
+
+- name: try to update mysql variable value (expect changed=false) in persist_only mode again
+  mysql_variables:
+    variable: '{{ set_name }}'
+    value: '{{ set_value }}'
+    login_unix_socket: '{{ mysql_socket }}'
+    login_user: root
+    login_password: '{{ root_pass }}'
+    mode: persist_only
+  register: result
+
+- assert:
+    that:
+    - result is not changed
+
+- set_fact:
+    set_name: max_connections
+    set_value: 105
+    def_val: 151
+
+- name: update mysql variable value (expect changed=true) in persist_only mode
+  mysql_variables:
+    variable: '{{ set_name }}'
+    value: '{{ set_value }}'
+    login_unix_socket: '{{ mysql_socket }}'
+    login_user: root
+    login_password: '{{ root_pass }}'
+    mode: persist_only
+  register: result
+
+- include: assert_var.yml changed=true output={{result}} var_name={{set_name}} var_value='{{def_val}}'


Code review comments:
diff_hunk: @@ -177,27 +222,52 @@ def main():
         else:
             module.fail_json(msg="unable to find %s. Exception message: %s" % (config_file, to_native(e)))
 
+    mysqlvar_val = None
+    var_in_mysqld_auto_cnf = None
+
     mysqlvar_val = getvariable(cursor, mysqlvar)
     if mysqlvar_val is None:
         module.fail_json(msg="Variable not available \"%s\"" % mysqlvar, changed=False)
+
     if value is None:
         module.exit_json(msg=mysqlvar_val)
-    else:
-        # Type values before using them
-        value_wanted = typedvalue(value)
-        value_actual = typedvalue(mysqlvar_val)
-        if value_wanted == value_actual:
-            module.exit_json(msg="Variable already set to requested value", changed=False)
-        try:
-            result = setvariable(cursor, mysqlvar, value_wanted)
-        except SQLParseError as e:
-            result = to_native(e)
-
-        if result is True:
-            module.exit_json(msg="Variable change succeeded prev_value=%s" % value_actual,
-                             changed=True, queries=executed_queries)
+
+    if mode in ('persist', 'persist_only'):
+        var_in_mysqld_auto_cnf = check_mysqld_auto(module, cursor, mysqlvar)
+        if var_in_mysqld_auto_cnf is None:
path: lib/ansible/modules/database/mysql/mysql_variables.py
text: @user1:
I think you should only do this whole `if` if `mode == 'persist_only'`.

@author:
Probably i understood incorrect, why?
We need to get value from mysqld-auto.cnf ```var_in_mysqld_auto_cnf = check_mysqld_auto(module, cursor, mysqlvar)``` and check it for ```persist``` mode too

@user1:
If `mode == 'persist'`, you want that *both* the global value currently set, and the value stored in the config file, are both equal to `value`. If you read both values, but overwrite one with the other, you cannot compare both to `value`.

@author:
it seems work, PTAL :)