feat: DPE-5656 log rotation new options (#568)

* tune log rotation

* rendering config file

* test fixes

* typing fixes

* log rotation setup

* revert garbage in commit

* better description plus typo fix

* fix call on upgrade call

* revert typing fix as it needs to fixed at library first

* simplify log-rotation test so to avoid decompressing file to test content

* remove audit_log_filter

* test simplification

* If "ensure_all_units_continuous_writes_incrementing" runs to early it breaks the test

* typo fix

* tiny typo

Author: paulomach

config.yaml

@@ -12,7 +12,7 @@ options:
     type: string
   profile:
     description: |
-      profile representing the scope of deployment, and used to be able to enable high-level
+      Profile representing the scope of deployment, and used to be able to enable high-level
       customisation of sysconfigs, resource checks/allocation, warning levels, etc.
       Allowed values are: “production” and “testing”.
     type: string
@@ -37,7 +37,8 @@ options:
     description: The database name for the legacy 'mysql' interface (root level access)
     type: string
   plugin-audit-enabled:
-    description: Enable the audit plugin
+    description: |
+      Audit log plugin state. When the plugin is enabled (default, audit logs will be enabled).
     type: boolean
     default: true
   plugin-audit-strategy:
@@ -50,6 +51,19 @@ options:
     description: Number of days for binary logs retention
     type: int
     default: 7
+  logs_audit_policy:
+    description: |
+      Audit log policy. Allowed values are: "all", "logins" (default), "queries".
+      Ref. at https://docs.percona.com/percona-server/8.0/audit-log-plugin.html#audit_log_policy
+    type: string
+    default: logins
+  logs_retention_period:
+    description: |
+      Specifies the retention period for rotated logs, in days. Accepts an integer value of 3 or
+      greater, or the special value "auto". When set to "auto" (default), the retention period is
+      3 days, except when COS-related, where it is 1 day
+    type: string
+    default: auto
   # Experimental features
   experimental-max-connections:
     type: int

lib/charms/mysql/v0/mysql.py

@@ -133,7 +133,7 @@ def wait_until_mysql_connection(self) -> None:
 # Increment this major API version when introducing breaking changes
 LIBAPI = 0
 
-LIBPATCH = 81
+LIBPATCH = 82
 
 UNIT_TEARDOWN_LOCKNAME = "unit-teardown"
 UNIT_ADD_LOCKNAME = "unit-add"
@@ -930,6 +930,7 @@ def render_mysqld_configuration(  # noqa: C901
         profile: str,
         audit_log_enabled: bool,
         audit_log_strategy: str,
+        audit_log_policy: str,
         memory_limit: Optional[int] = None,
         experimental_max_connections: Optional[int] = None,
         binlog_retention_days: int,
@@ -1000,8 +1001,7 @@ def render_mysqld_configuration(  # noqa: C901
             "general_log_file": f"{snap_common}/var/log/mysql/general.log",
             "slow_query_log_file": f"{snap_common}/var/log/mysql/slow.log",
             "binlog_expire_logs_seconds": f"{binlog_retention_seconds}",
-            "loose-audit_log_filter": "OFF",
-            "loose-audit_log_policy": "LOGINS",
+            "loose-audit_log_policy": audit_log_policy.upper(),
             "loose-audit_log_file": f"{snap_common}/var/log/mysql/audit.log",
         }
 
@@ -2950,8 +2950,8 @@ def retrieve_backup_with_xbcloud(
         temp_restore_directory: str,
         xbcloud_location: str,
         xbstream_location: str,
-        user=None,
-        group=None,
+        user: Optional[str] = None,
+        group: Optional[str] = None,
     ) -> Tuple[str, str, str]:
         """Retrieve the specified backup from S3."""
         nproc_command = ["nproc"]
@@ -3017,8 +3017,8 @@ def prepare_backup_for_restore(
         backup_location: str,
         xtrabackup_location: str,
         xtrabackup_plugin_dir: str,
-        user=None,
-        group=None,
+        user: Optional[str] = None,
+        group: Optional[str] = None,
     ) -> Tuple[str, str]:
         """Prepare the backup in the provided dir for restore."""
         try:
@@ -3058,8 +3058,8 @@ def prepare_backup_for_restore(
     def empty_data_files(
         self,
         mysql_data_directory: str,
-        user=None,
-        group=None,
+        user: Optional[str] = None,
+        group: Optional[str] = None,
     ) -> None:
         """Empty the mysql data directory in preparation of backup restore."""
         empty_data_files_command = [
@@ -3095,8 +3095,8 @@ def restore_backup(
         defaults_config_file: str,
         mysql_data_directory: str,
         xtrabackup_plugin_directory: str,
-        user=None,
-        group=None,
+        user: Optional[str] = None,
+        group: Optional[str] = None,
     ) -> Tuple[str, str]:
         """Restore the provided prepared backup."""
         restore_backup_command = [
@@ -3129,8 +3129,8 @@ def restore_backup(
     def delete_temp_restore_directory(
         self,
         temp_restore_directory: str,
-        user=None,
-        group=None,
+        user: Optional[str] = None,
+        group: Optional[str] = None,
     ) -> None:
         """Delete the temp restore directory from the mysql data directory."""
         logger.info(f"Deleting temp restore directory in {temp_restore_directory}")

src/charm.py

@@ -7,6 +7,8 @@
 from charms.mysql.v0.architecture import WrongArchitectureWarningCharm, is_wrong_architecture
 from ops.main import main
 
+from log_rotation_setup import LogRotationSetup
+
 if is_wrong_architecture() and __name__ == "__main__":
     main(WrongArchitectureWarningCharm)
 
@@ -184,6 +186,7 @@ def __init__(self, *args):
         self.log_rotate_manager = LogRotateManager(self)
         self.log_rotate_manager.start_log_rotate_manager()
 
+        self.log_rotate_setup = LogRotationSetup(self)
         self.rotate_mysql_logs = RotateMySQLLogs(self)
         self.replication_offer = MySQLAsyncReplicationOffer(self)
         self.replication_consumer = MySQLAsyncReplicationConsumer(self)
@@ -274,11 +277,22 @@ def is_new_unit(self) -> bool:
         }
         return self.unit_peer_data.keys() == _default_unit_data_keys
 
+    @property
+    def text_logs(self) -> list:
+        """Enabled text logs."""
+        # slow logs isn't enabled by default
+        text_logs = ["error"]
+
+        if self.config.plugin_audit_enabled:
+            text_logs.append("audit")
+
+        return text_logs
+
     @property
     def unit_initialized(self) -> bool:
         """Return whether a unit is started.
 
-        Oveerride parent class method to include container accessibility check.
+        Override parent class method to include container accessibility check.
         """
         container = self.unit.get_container(CONTAINER_NAME)
         if container.can_connect():
@@ -536,34 +550,24 @@ def _on_config_changed(self, _: EventBase) -> None:  # noqa: C901
 
         previous_config_dict = self.mysql_config.custom_config(config_content)
 
-        # render the new config
-        memory_limit_bytes = (self.config.profile_limit_memory or 0) * BYTES_1MB
-        new_config_content, new_config_dict = self._mysql.render_mysqld_configuration(
-            profile=self.config.profile,
-            audit_log_enabled=self.config.plugin_audit_enabled,
-            audit_log_strategy=self.config.plugin_audit_strategy,
-            memory_limit=memory_limit_bytes,
-            experimental_max_connections=self.config.experimental_max_connections,
-            binlog_retention_days=self.config.binlog_retention_days,
-        )
+        # always setup log rotation
+        self.log_rotate_setup.setup()
 
+        logger.info("Persisting configuration changes to file")
+        new_config_dict = self._write_mysqld_configuration()
         changed_config = compare_dictionaries(previous_config_dict, new_config_dict)
 
         if self.mysql_config.keys_requires_restart(changed_config):
             # there are static configurations in changed keys
-            logger.info("Persisting configuration changes to file")
-
-            # persist config to file
-            self._mysql.write_content_to_file(path=MYSQLD_CONFIG_FILE, content=new_config_content)
 
             if self._mysql.is_mysqld_running():
                 logger.info("Configuration change requires restart")
                 if "loose-audit_log_format" in changed_config:
                     # plugins are manipulated running daemon
                     if self.config.plugin_audit_enabled:
-                        self._mysql.install_plugins(["audit_log", "audit_log_filter"])
+                        self._mysql.install_plugins(["audit_log"])
                     else:
-                        self._mysql.uninstall_plugins(["audit_log", "audit_log_filter"])
+                        self._mysql.uninstall_plugins(["audit_log"])
                 # restart the service
                 self.on[f"{self.restart.name}"].acquire_lock.emit()
                 return
@@ -572,7 +576,9 @@ def _on_config_changed(self, _: EventBase) -> None:  # noqa: C901
             # if only dynamic config changed, apply it
             logger.info("Configuration does not requires restart")
             for config in dynamic_config:
-                self._mysql.set_dynamic_variable(config, new_config_dict[config])
+                self._mysql.set_dynamic_variable(
+                    config.removeprefix("loose-"), new_config_dict[config]
+                )
 
     def _on_leader_elected(self, _) -> None:
         """Handle the leader elected event.
@@ -615,18 +621,20 @@ def _open_ports(self) -> None:
             except ops.ModelError:
                 logger.exception("failed to open port")
 
-    def _write_mysqld_configuration(self):
+    def _write_mysqld_configuration(self) -> dict:
         """Write the mysqld configuration to the file."""
         memory_limit_bytes = (self.config.profile_limit_memory or 0) * BYTES_1MB
-        new_config_content, _ = self._mysql.render_mysqld_configuration(
+        new_config_content, new_config_dict = self._mysql.render_mysqld_configuration(
             profile=self.config.profile,
             audit_log_enabled=self.config.plugin_audit_enabled,
             audit_log_strategy=self.config.plugin_audit_strategy,
+            audit_log_policy=self.config.logs_audit_policy,
             memory_limit=memory_limit_bytes,
             experimental_max_connections=self.config.experimental_max_connections,
             binlog_retention_days=self.config.binlog_retention_days,
         )
         self._mysql.write_content_to_file(path=MYSQLD_CONFIG_FILE, content=new_config_content)
+        return new_config_dict
 
     def _configure_instance(self, container) -> None:
         """Configure the instance for use in Group Replication."""
@@ -651,7 +659,7 @@ def _configure_instance(self, container) -> None:
 
             if self.config.plugin_audit_enabled:
                 # Enable the audit plugin
-                self._mysql.install_plugins(["audit_log", "audit_log_filter"])
+                self._mysql.install_plugins(["audit_log"])
 
             # Configure instance as a cluster node
             self._mysql.configure_instance()
@@ -717,8 +725,7 @@ def _on_mysql_pebble_ready(self, event) -> None:
         container = event.workload
         self._write_mysqld_configuration()
 
-        logger.info("Setting up the logrotate configurations")
-        self._mysql.setup_logrotate_config()
+        self.log_rotate_setup.setup()
 
         if self._mysql.is_data_dir_initialised():
             # Data directory is already initialised, skip configuration

src/config.py

@@ -62,6 +62,8 @@ class CharmConfig(BaseConfigModel):
     binlog_retention_days: int
     plugin_audit_enabled: bool
     plugin_audit_strategy: str
+    logs_audit_policy: str
+    logs_retention_period: str
 
     @validator("profile")
     @classmethod
@@ -152,6 +154,25 @@ def binlog_retention_days_validator(cls, value: int) -> int:
     def plugin_audit_strategy_validator(cls, value: str) -> Optional[str]:
         """Check profile config option is one of `testing` or `production`."""
         if value not in ["async", "semi-async"]:
-            raise ValueError("Value not one of 'async' or 'semi-async'")
+            raise ValueError("plugin_audit_strategy not one of 'async' or 'semi-async'")
+
+        return value
+
+    @validator("logs_audit_policy")
+    @classmethod
+    def logs_audit_policy_validator(cls, value: str) -> Optional[str]:
+        """Check values for audit log policy."""
+        valid_values = ["all", "logins", "queries"]
+        if value not in valid_values:
+            raise ValueError(f"logs_audit_policy not one of {', '.join(valid_values)}")
+
+        return value
+
+    @validator("logs_retention_period")
+    @classmethod
+    def logs_retention_period_validator(cls, value: str) -> str:
+        """Check logs retention period."""
+        if not re.match(r"auto|\d{1,3}", value) or value == "0":
+            raise ValueError("logs_retention_period must be integer greater than 0 or `auto`")
 
         return value

src/constants.py

@@ -33,10 +33,11 @@
 MYSQLD_SOCK_FILE = "/var/run/mysqld/mysqld.sock"
 MYSQLSH_SCRIPT_FILE = "/tmp/script.py"
 MYSQLD_CONFIG_FILE = "/etc/mysql/mysql.conf.d/z-custom.cnf"
+MYSQL_LOG_DIR = "/var/log/mysql"
 MYSQL_LOG_FILES = [
-    "/var/log/mysql/error.log",
-    "/var/log/mysql/audit.log",
-    "/var/log/mysql/general.log",
+    f"{MYSQL_LOG_DIR}/error.log",
+    f"{MYSQL_LOG_DIR}/audit.log",
+    f"{MYSQL_LOG_DIR}/general.log",
 ]
 MYSQL_SYSTEM_USER = "mysql"
 MYSQL_SYSTEM_GROUP = "mysql"
@@ -50,6 +51,7 @@
 GR_MAX_MEMBERS = 9
 # TODO: should be changed when adopting cos-agent
 COS_AGENT_RELATION_NAME = "metrics-endpoint"
+COS_LOGGING_RELATION_NAME = "logging"
 LOG_ROTATE_CONFIG_FILE = "/etc/logrotate.d/flush_mysql_logs"
 ROOT_SYSTEM_USER = "root"
 SECRET_KEY_FALLBACKS = {