Merge pull request #184 from Aiven-Open/0xlianhu-BF-2436-simplify-env…

…-file-update

Restrict env update command to specific variable and values

myhoard.json

@@ -62,7 +62,7 @@
     },
     "systemctl_command": ["sudo", "/usr/bin/systemctl"],
     "systemd_env_update_command": [
-        "sudo", "/usr/bin/myhoard_mysql_env_update", "--", "/etc/systemd/system/mysqld.environment", "MYSQLD_OPTS"
+        "sudo", "/usr/bin/myhoard_mysql_env_update", "-f", "/etc/systemd/system/mysqld.environment"
     ],
     "systemd_service": "mysql-server",
     "temporary_directory": "/var/tmp/sample_temp_dir",

myhoard/myhoard.py

@@ -113,19 +113,18 @@ def _notify_systemd(self):
         self.systemd_notified = True
 
     def _restart_mysqld(self, *, with_binlog, with_gtids):
-        mysqld_options = []
-        if not with_binlog:
-            mysqld_options.append("--disable-log-bin")
-            # If config says slave-preserve-commit-order=ON MySQL would refuse to start if binlog is
-            # disabled. To prevent that from happening ensure preserve commit order is disabled
-            mysqld_options.append("--skip-slave-preserve-commit-order")
-        if not with_gtids:
-            mysqld_options.append("--gtid-mode=OFF")
-
         systemd_service = self.config.get("systemd_service")
         if systemd_service:
-            self._restart_systemd(mysqld_options=" ".join(mysqld_options), service=systemd_service)
+            self._restart_systemd(with_binlog=with_binlog, with_gtids=with_gtids, service=systemd_service)
         else:
+            mysqld_options = []
+            if not with_binlog:
+                mysqld_options.append("--disable-log-bin")
+                # If config says slave-preserve-commit-order=ON MySQL would refuse to start if binlog is
+                # disabled. To prevent that from happening ensure preserve commit order is disabled
+                mysqld_options.append("--skip-slave-preserve-commit-order")
+            if not with_gtids:
+                mysqld_options.append("--gtid-mode=OFF")
             self._restart_process(mysqld_options=mysqld_options)
 
         # Ensure the server is accepting connections
@@ -152,10 +151,12 @@ def _restart_process(self, *, mysqld_options):
         self.mysqld_pid = proc.pid
         self.log.info("Process %r started, pid %s", full_command, proc.pid)
 
-    def _restart_systemd(self, *, mysqld_options, service):
+    def _restart_systemd(self, with_binlog, with_gtids, service):
         self.log.info("Restarting service %r", service)
 
-        command = self.config["systemd_env_update_command"] + [mysqld_options or ""]
+        command = self.config["systemd_env_update_command"].copy()
+        command.extend(["-b", "true"] if with_binlog else ["-b", "false"])
+        command.extend(["-g", "true"] if with_gtids else ["-g", "false"])
         try:
             subprocess.run(command, stderr=subprocess.PIPE, stdout=subprocess.PIPE, check=True)
         except subprocess.CalledProcessError as e:

myhoard/update_mysql_environment.py

@@ -1,4 +1,6 @@
 # Copyright (c) 2019 Aiven, Helsinki, Finland. https://aiven.io/
+from myhoard.util import atomic_create_file
+
 import argparse
 import logging
 import sys
@@ -13,26 +15,34 @@ def __init__(self, args):
         self.log = logging.getLogger(self.__class__.__name__)
 
     def update(self):
+        # make sure we only update the parameter we need to update i.e., MYSQLD_OPTS
+        key = "MYSQLD_OPTS"  # we only update this environment variable
+        options = []
+        if self.args.with_bin_log != "true":
+            options.append("--disable-log-bin")
+            # If config says slave-preserve-commit-order=ON MySQL would refuse to start if binlog is
+            # disabled. To prevent that from happening ensure preserve commit order is disabled
+            options.append("--skip-slave-preserve-commit-order")
+        if self.args.gtid_mode != "true":
+            options.append("--gtid-mode=OFF")
         try:
-            with open(self.args.environment_file, "r") as f:
-                contents = [
-                    line.rstrip("\n") for line in f.readlines() if line.strip() and not line.startswith(self.args.key)
-                ]
+            with open(self.args.env_file, "r") as f:
+                contents = [line.rstrip("\n") for line in f.readlines() if line.strip() and not line.startswith(key)]
         except FileNotFoundError:
             contents = []
-
-        if self.args.value:
-            contents.append(f"{self.args.key}={self.args.value}")
-        with open(self.args.environment_file, "w+") as f:
+        value = " ".join(options)
+        if value:
+            contents.append(f"{key}={value}")
+        with atomic_create_file(self.args.env_file) as f:
             f.write("\n".join(contents) + "\n")
 
 
 def main():
     logging.basicConfig(level=logging.INFO)
     parser = argparse.ArgumentParser(description="Aiven MySQL environment updater")
-    parser.add_argument("environment_file", metavar="FILE", help="Environment file")
-    parser.add_argument("key", help="Environment variable name")
-    parser.add_argument("value", help="Environment variable value")
+    parser.add_argument("-f", dest="env_file", metavar="FILE", help="The Environment file to be updated")
+    parser.add_argument("-b", dest="with_bin_log", choices=["true", "false"], help="Flag to enable bin log or not")
+    parser.add_argument("-g", dest="gtid_mode", choices=["true", "false"], help="Flag to turn GTID mode on or off")
     args = parser.parse_args()
     EnvironmentUpdater(args).update()
     return 0

pyproject.toml

@@ -33,6 +33,8 @@ dependencies = [
     "paramiko",
     "PyMySQL >= 0.9.2",
     "PySocks",
+    # rohmu is incompatible with latest version snappy 0.7.1
+    "python-snappy == 0.6.1",
     "rohmu == 1.0.7",
     "sentry-sdk==1.14.0",
 ]

test/conftest.py

@@ -376,9 +376,8 @@ def fixture_myhoard_config(default_backup_site, mysql_master, session_tmpdir):
         "systemd_env_update_command": [
             "sudo",
             "/usr/bin/myhoard_mysql_env_update",
-            "--",
+            "-f",
             "/etc/systemd/system/mysqld.environment",
-            "MYSQLD_OPTS",
         ],
         "systemd_service": None,
         "temporary_directory": temp_dir,