From ff9b4d02567fbbac539e8399ad60c28ccf1846e7 Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Fri, 7 Jul 2023 15:13:29 +0200 Subject: [PATCH 01/11] integrate sysctl lib into kafka charm --- lib/charms/grafana_agent/v0/cos_agent.py | 114 +++++++--- lib/charms/operator_libs_linux/v0/sysctl.py | 237 ++++++++++++++++++++ src/charm.py | 17 ++ src/literals.py | 4 + src/templates/sysctl.yaml | 11 + 5 files changed, 347 insertions(+), 36 deletions(-) create mode 100644 lib/charms/operator_libs_linux/v0/sysctl.py create mode 100644 src/templates/sysctl.yaml diff --git a/lib/charms/grafana_agent/v0/cos_agent.py b/lib/charms/grafana_agent/v0/cos_agent.py index f8ddac88..37f539ef 100644 --- a/lib/charms/grafana_agent/v0/cos_agent.py +++ b/lib/charms/grafana_agent/v0/cos_agent.py @@ -185,13 +185,13 @@ class _MetricsEndpointDict(TypedDict): port: int except ModuleNotFoundError: - _MetricsEndpointDict = dict + _MetricsEndpointDict = Dict # pyright: ignore LIBID = "dc15fa84cef84ce58155fb84f6c6213a" LIBAPI = 0 -LIBPATCH = 3 +LIBPATCH = 5 -PYDEPS = ["cosl", "pydantic"] +PYDEPS = ["cosl", "pydantic<2"] DEFAULT_RELATION_NAME = "cos-agent" DEFAULT_PEER_RELATION_NAME = "peers" @@ -217,8 +217,12 @@ def _serialize(raw_json: Union[str, bytes]) -> "GrafanaDashboard": return GrafanaDashboard(encoded) def _deserialize(self) -> Dict: - raw = lzma.decompress(base64.b64decode(self.encode("utf-8"))).decode() - return json.loads(raw) + try: + raw = lzma.decompress(base64.b64decode(self.encode("utf-8"))).decode() + return json.loads(raw) + except json.decoder.JSONDecodeError as e: + logger.error("Invalid Dashboard format: %s", e) + return {} def __repr__(self): """Return string representation of self.""" @@ -247,7 +251,7 @@ class CosAgentProviderUnitData(pydantic.BaseModel): class CosAgentPeersUnitData(pydantic.BaseModel): - """Unit databag model for `cluster` cos-agent machine charm peer relation.""" + """Unit databag model for `peers` cos-agent machine charm peer relation.""" # We need the principal unit name and relation metadata to be able to render identifiers # (e.g. topology) on the leader side, after all the data moves into peer data (the grafana @@ -307,12 +311,11 @@ def __init__( refresh_events: List of events on which to refresh relation data. """ super().__init__(charm, relation_name) - metrics_endpoints = metrics_endpoints or [DEFAULT_METRICS_ENDPOINT] dashboard_dirs = dashboard_dirs or ["./src/grafana_dashboards"] self._charm = charm self._relation_name = relation_name - self._metrics_endpoints = metrics_endpoints + self._metrics_endpoints = metrics_endpoints or [DEFAULT_METRICS_ENDPOINT] self._metrics_rules = metrics_rules_dir self._logs_rules = logs_rules_dir self._recursive = recurse_rules_dirs @@ -339,14 +342,20 @@ def _on_refresh(self, event): # Add a guard to make sure it doesn't happen. if relation.data and self._charm.unit in relation.data: # Subordinate relations can communicate only over unit data. - data = CosAgentProviderUnitData( - metrics_alert_rules=self._metrics_alert_rules, - log_alert_rules=self._log_alert_rules, - dashboards=self._dashboards, - metrics_scrape_jobs=self._scrape_jobs, - log_slots=self._log_slots, - ) - relation.data[self._charm.unit][data.KEY] = data.json() + try: + data = CosAgentProviderUnitData( + metrics_alert_rules=self._metrics_alert_rules, + log_alert_rules=self._log_alert_rules, + dashboards=self._dashboards, + metrics_scrape_jobs=self._scrape_jobs, + log_slots=self._log_slots, + ) + relation.data[self._charm.unit][data.KEY] = data.json() + except ( + pydantic.ValidationError, + json.decoder.JSONDecodeError, + ) as e: + logger.error("Invalid relation data provided: %s", e) @property def _scrape_jobs(self) -> List[Dict]: @@ -387,16 +396,33 @@ class COSAgentDataChanged(EventBase): """Event emitted by `COSAgentRequirer` when relation data changes.""" +class COSAgentValidationError(EventBase): + """Event emitted by `COSAgentRequirer` when there is an error in the relation data.""" + + def __init__(self, handle, message: str = ""): + super().__init__(handle) + self.message = message + + def snapshot(self) -> Dict: + """Save COSAgentValidationError source information.""" + return {"message": self.message} + + def restore(self, snapshot): + """Restore COSAgentValidationError source information.""" + self.message = snapshot["message"] + + class COSAgentRequirerEvents(ObjectEvents): """`COSAgentRequirer` events.""" data_changed = EventSource(COSAgentDataChanged) + validation_error = EventSource(COSAgentValidationError) class COSAgentRequirer(Object): """Integration endpoint wrapper for the Requirer side of the cos_agent interface.""" - on = COSAgentRequirerEvents() + on = COSAgentRequirerEvents() # pyright: ignore def __init__( self, @@ -426,7 +452,7 @@ def __init__( ) # TODO: do we need this? self.framework.observe(events.relation_changed, self._on_relation_data_changed) for event in self._refresh_events: - self.framework.observe(event, self.trigger_refresh) + self.framework.observe(event, self.trigger_refresh) # pyright: ignore # Peer relation events # A peer relation is needed as it is the only mechanism for exchanging data across @@ -450,7 +476,7 @@ def _on_peer_relation_changed(self, _): # Peer data is used for forwarding data from principal units to the grafana agent # subordinate leader, for updating the app data of the outgoing o11y relations. if self._charm.unit.is_leader(): - self.on.data_changed.emit() + self.on.data_changed.emit() # pyright: ignore def _on_relation_data_changed(self, event: RelationChangedEvent): # Peer data is the only means of communication between subordinate units. @@ -474,7 +500,9 @@ def _on_relation_data_changed(self, event: RelationChangedEvent): if not (raw := cos_agent_relation.data[principal_unit].get(CosAgentProviderUnitData.KEY)): return - provider_data = CosAgentProviderUnitData(**json.loads(raw)) + + if not (provider_data := self._validated_provider_data(raw)): + return # Copy data from the principal relation to the peer relation, so the leader could # follow up. @@ -492,12 +520,19 @@ def _on_relation_data_changed(self, event: RelationChangedEvent): # We can't easily tell if the data that was changed is limited to only the data # that goes into peer relation (in which case, if this is not a leader unit, we wouldn't # need to emit `on.data_changed`), so we're emitting `on.data_changed` either way. - self.on.data_changed.emit() + self.on.data_changed.emit() # pyright: ignore + + def _validated_provider_data(self, raw) -> Optional[CosAgentProviderUnitData]: + try: + return CosAgentProviderUnitData(**json.loads(raw)) + except (pydantic.ValidationError, json.decoder.JSONDecodeError) as e: + self.on.validation_error.emit(message=str(e)) # pyright: ignore + return None def trigger_refresh(self, _): """Trigger a refresh of relation data.""" # FIXME: Figure out what we should do here - self.on.data_changed.emit() + self.on.data_changed.emit() # pyright: ignore @property def _principal_unit(self) -> Optional[Unit]: @@ -529,17 +564,24 @@ def _principal_unit_data(self) -> Optional[CosAgentProviderUnitData]: Relies on the fact that, for subordinate relations, the only remote unit visible to *this unit* is the principal unit that this unit is attached to. """ - if relations := self._principal_relations: - # Technically it's a list, but for subordinates there can only be one relation - principal_relation = next(iter(relations)) - if units := principal_relation.units: - # Technically it's a list, but for subordinates there can only be one - unit = next(iter(units)) - raw = principal_relation.data[unit].get(CosAgentProviderUnitData.KEY) - if raw: - return CosAgentProviderUnitData(**json.loads(raw)) + if not (relations := self._principal_relations): + return None - return None + # Technically it's a list, but for subordinates there can only be one relation + principal_relation = next(iter(relations)) + + if not (units := principal_relation.units): + return None + + # Technically it's a list, but for subordinates there can only be one + unit = next(iter(units)) + if not (raw := principal_relation.data[unit].get(CosAgentProviderUnitData.KEY)): + return None + + if not (provider_data := self._validated_provider_data(raw)): + return None + + return provider_data def _gather_peer_data(self) -> List[CosAgentPeersUnitData]: """Collect data from the peers. @@ -578,7 +620,7 @@ def metrics_alerts(self) -> Dict[str, Any]: alert_rules = {} seen_apps: List[str] = [] - for data in self._gather_peer_data(): # type: CosAgentPeersUnitData + for data in self._gather_peer_data(): if rules := data.metrics_alert_rules: app_name = data.app_name if app_name in seen_apps: @@ -649,7 +691,7 @@ def logs_alerts(self) -> Dict[str, Any]: alert_rules = {} seen_apps: List[str] = [] - for data in self._gather_peer_data(): # type: CosAgentPeersUnitData + for data in self._gather_peer_data(): if rules := data.log_alert_rules: # This is only used for naming the file, so be as specific as we can be app_name = data.app_name @@ -678,10 +720,10 @@ def dashboards(self) -> List[Dict[str, str]]: Dashboards are assumed not to vary across units of the same primary. """ - dashboards: List[Dict[str, str]] = [] + dashboards: List[Dict[str, Any]] = [] seen_apps: List[str] = [] - for data in self._gather_peer_data(): # type: CosAgentPeersUnitData + for data in self._gather_peer_data(): app_name = data.app_name if app_name in seen_apps: continue # dedup! diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py new file mode 100644 index 00000000..1df43438 --- /dev/null +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -0,0 +1,237 @@ +# Copyright 2023 Canonical Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Handler for the sysctl config +""" + +import logging +import os +import re +import glob +from dataclasses import dataclass +from subprocess import STDOUT, CalledProcessError, check_output + +logger = logging.getLogger(__name__) + +# The unique Charmhub library identifier, never change it +LIBID = "17a6cd4d80104d15b10f9c2420ab3266" + +# Increment this major API version when introducing breaking changes +LIBAPI = 0 + +# Increment this PATCH version before using `charmcraft publish-lib` or reset +# to 0 if you are raising the major API version +LIBPATCH = 2 + + +SYSCTL_DIRECTORY = "/etc/sysctl.d" +SYSCTL_FILENAME = f"{SYSCTL_DIRECTORY}/95-juju-sysctl.conf" +SYSCTL_HEADER = f"""# This config file was produced by sysctl lib v{LIBAPI}.{LIBPATCH} +# +# This file represents the output of the sysctl lib, which can combine multiple +# configurations into a single file like this with the help of validation rules. Such +# rules are used to automatically resolve simple conflicts between two or more charms +# on one host and is defined as a comment after the configuration option, see example +# below. +# +# Description of validation rules to check if values are valid. +# - "range(a,b)" - all numbers between 'a' (included) and 'b' (excluded) +# - "a|b|c" - choices 'a', 'b' and 'c' +# - "*" - any value +# - "" or no comment - only current value, same as "" +# - "disable" - This value will be ignored in any validation and should only be used by +# the charm operator manually or via a charm to override any system +# configuration. +# Examples: +# # any value in [10, 50] is valid value +# vm.swappiness=10 # range(10,51) +# +# # 60 and 80 are valid values +# vm.dirty_ratio = 80 # 60|80 +# +# # any value is valid +# vm.dirty_background_ratio = 3 # * +# +# # only value 1 is valid value +# net.ipv6.conf.all.use_tempaddr = 1 +# +# # validation is disabled +# net.ipv6.conf.default.use_tempaddr = 0 # disable +""" + + +class Error(Exception): + """Base class of most errors raised by this library.""" + + def __repr__(self): + """Represent the Error.""" + return "<{}.{} {}>".format(type(self).__module__, type(self).__name__, self.args) + + @property + def name(self): + """Return a string representation of the model plus class.""" + return "<{}.{}>".format(type(self).__module__, type(self).__name__) + + @property + def message(self): + """Return the message passed as an argument.""" + return self.args[0] + +class SysctlError(Error): + """Raised when there's an error running sysctl command.""" + + +class SysctlPermissionError(Error): + """Raised when there's an error applying values in sysctl.""" + + +class ValidationError(Error): + """Raised when there's an error in the validation process.""" + + +@dataclass() +class ConfigOption: + """Definition of a config option. + + NOTE: this class can be extended to handle the rule tied to each option. + """ + name: str + value: int + + @property + def string_format(self) -> str: + return f"{self.name}={self.value}" + + +class SysctlConfig: + """Represents the state of the config that a charm wants to enforce.""" + + def __init__(self, config_params: dict[str, dict], app_name: str) -> None: + self.config_params = config_params + self.app_name = app_name + + @property + def config_params(self) -> list[ConfigOption]: + """Config options passed to the lib.""" + return self._config_params + + @config_params.setter + def config_params(self, value: dict): + result = [] + for k, v in value.items(): + result += [ConfigOption(k, v["value"])] + self._config_params = result + + @property + def charm_filepath(self) -> str: + """Name for resulting charm config file.""" + return f"{SYSCTL_DIRECTORY}/90-juju-{self.app_name}" + + @property + def charm_config_exists(self) -> bool: + """Return whether the charm config file exists.""" + return os.path.exists(self.charm_filepath) + + @property + def merged_config_exists(self) -> bool: + """Return whether a merged config file exists.""" + return os.path.exists(SYSCTL_FILENAME) + + @property + def merged_config(self) -> list[ConfigOption] | None: + """Return applied internal config.""" + if not self.merged_config_exists: + return None + + with open(SYSCTL_FILENAME, "r") as f: + return [ConfigOption(param.strip(), int(value.strip())) + for line in f.read().splitlines() + if line and not line.startswith('#') + for param, value in [line.split('=')]] + + def update(self) -> None: + """Update sysctl config options.""" + if self.charm_config_exists: + return + self._create_charm_file() + + if not self.validate(): + raise ValidationError() + + snapshot = self._create_snapshot() + logger.debug(f"Created snapshot for keys: {snapshot}") + try: + self._apply() + except SysctlPermissionError: + self._restore_snapshot(snapshot) + raise + except ValidationError: + raise + self._merge() + + def validate(self) -> bool: + """Validate the desired config params against merged ones.""" + return True + + def _create_charm_file(self) -> None: + """Write the charm file.""" + charm_params = [f"{param.string_format}\n" for param in self.config_params] + with open(self.charm_filepath, "w") as f: + f.writelines(charm_params) + + def _merge(self) -> None: + """Create the merged sysctl file.""" + # get all files that start by 90-juju- + charm_files = [f for f in glob.glob(f"{SYSCTL_DIRECTORY}/90-juju-*")] + data = [SYSCTL_HEADER] + for path in charm_files: + with open(path, "r") as f: + data += f.readlines() + with open(SYSCTL_FILENAME, "w") as f: + f.writelines(data) + + def _apply(self) -> list[str] | None: + """Apply values to machine. + + Returns: + none or the list of keys that failed to apply. + """ + cmd = ["-p", self.charm_filepath] + result = self._sysctl(cmd) + expr = re.compile(r'^sysctl: permission denied on key \"([a-z_\.]+)\", ignoring$') + failed_values = [expr.match(line) for line in result if expr.match(line)] + + if failed_values: + msg = f"Unable to set params: {[f.group(1) for f in failed_values]}" + logger.error(msg) + raise SysctlPermissionError(msg) + + def _create_snapshot(self) -> list[ConfigOption]: + """Create a snaphot of config options that are going to be set.""" + keys = [param.name for param in self.config_params] + return [ConfigOption(key, int(self._sysctl([key, "-n"])[0])) for key in keys] + + def _restore_snapshot(self, snapshot: list[ConfigOption]) -> None: + """Restore a snapshot to the machine.""" + for param in snapshot: + self._sysctl([param.string_format]) + + def _sysctl(self, cmd: list[str]) -> list[str]: + """Execute a sysctl command.""" + cmd = ["sysctl"] + cmd + logger.debug(f"Executing sysctl command: {cmd}") + try: + return check_output(cmd, stderr=STDOUT, universal_newlines=True).splitlines() + except CalledProcessError as e: + raise SysctlError(f"Error executing '{cmd}': {e.output}") diff --git a/src/charm.py b/src/charm.py index 2dd75568..b6f16c55 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,9 +7,16 @@ import logging import subprocess from typing import MutableMapping, Optional +from pathlib import Path +import yaml from charms.data_platform_libs.v0.data_models import TypedCharmBase from charms.grafana_agent.v0.cos_agent import COSAgentProvider +from charms.operator_libs_linux.v0.sysctl import ( + SysctlConfig, + SysctlError, + SysctlPermissionError, +) from charms.operator_libs_linux.v1.snap import SnapError from charms.rolling_ops.v0.rollingops import RollingOpsManager, RunWithLock from ops.charm import ( @@ -238,6 +245,16 @@ def _on_storage_detaching(self, event: StorageDetachingEvent) -> None: def _on_install(self, _) -> None: """Handler for `install` event.""" + sysctl_yaml = yaml.safe_load(Path("./src/templates/sysctl.yaml").read_text()) + sysctl = SysctlConfig(config_params=sysctl_yaml["testing"], app_name=CHARM_KEY) + try: + sysctl.update() + except SysctlPermissionError as e: + logger.error(f"Error setting values on sysctl: {e.message}") + self._set_status(Status.SYSCONF_NOT_POSSIBLE) + except SysctlError: + logger.error("Error on sysctl") + if self.snap.install(): self.kafka_config.set_environment() self._set_status(Status.ZK_NOT_RELATED) diff --git a/src/literals.py b/src/literals.py index 66d7dbf0..89c3d961 100644 --- a/src/literals.py +++ b/src/literals.py @@ -88,3 +88,7 @@ class Status(Enum): ActiveStatus("machine system settings are not optimal - see logs for info"), "WARNING", ) + SYSCONF_NOT_POSSIBLE = StatusLevel( + BlockedStatus("sysctl params cannot be set. Is the machine running on a container?"), + "WARNING", + ) diff --git a/src/templates/sysctl.yaml b/src/templates/sysctl.yaml new file mode 100644 index 00000000..4a9a20cb --- /dev/null +++ b/src/templates/sysctl.yaml @@ -0,0 +1,11 @@ +testing: + vm.swappiness: + value: 1 + vm.max_map_count: + value: 262144 + vm.dirty_ratio: + value: 80 + vm.dirty_background_ratio: + value: 5 + net.ipv4.tcp_max_syn_backlog: + value: 4096 From 0225cdac442437ccbf54a5412773496e1cf53ea1 Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Mon, 10 Jul 2023 13:16:02 +0200 Subject: [PATCH 02/11] restructure lib --- lib/charms/operator_libs_linux/v0/sysctl.py | 156 ++++++++++---------- src/charm.py | 9 +- 2 files changed, 79 insertions(+), 86 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index 1df43438..53d87ea5 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -19,8 +19,8 @@ import os import re import glob -from dataclasses import dataclass from subprocess import STDOUT, CalledProcessError, check_output +from typing import Mapping, Dict logger = logging.getLogger(__name__) @@ -40,34 +40,7 @@ SYSCTL_HEADER = f"""# This config file was produced by sysctl lib v{LIBAPI}.{LIBPATCH} # # This file represents the output of the sysctl lib, which can combine multiple -# configurations into a single file like this with the help of validation rules. Such -# rules are used to automatically resolve simple conflicts between two or more charms -# on one host and is defined as a comment after the configuration option, see example -# below. -# -# Description of validation rules to check if values are valid. -# - "range(a,b)" - all numbers between 'a' (included) and 'b' (excluded) -# - "a|b|c" - choices 'a', 'b' and 'c' -# - "*" - any value -# - "" or no comment - only current value, same as "" -# - "disable" - This value will be ignored in any validation and should only be used by -# the charm operator manually or via a charm to override any system -# configuration. -# Examples: -# # any value in [10, 50] is valid value -# vm.swappiness=10 # range(10,51) -# -# # 60 and 80 are valid values -# vm.dirty_ratio = 80 # 60|80 -# -# # any value is valid -# vm.dirty_background_ratio = 3 # * -# -# # only value 1 is valid value -# net.ipv6.conf.all.use_tempaddr = 1 -# -# # validation is disabled -# net.ipv6.conf.default.use_tempaddr = 0 # disable +# configurations into a single file like. """ @@ -88,6 +61,7 @@ def message(self): """Return the message passed as an argument.""" return self.args[0] + class SysctlError(Error): """Raised when there's an error running sysctl command.""" @@ -97,46 +71,59 @@ class SysctlPermissionError(Error): class ValidationError(Error): - """Raised when there's an error in the validation process.""" + """Exception representing value validation error.""" + +class SysctlConfig(Mapping[str, int]): + """Represents the state of the config that a charm wants to enforce.""" -@dataclass() -class ConfigOption: - """Definition of a config option. - - NOTE: this class can be extended to handle the rule tied to each option. - """ - name: str - value: int + def __init__(self, name: str = "") -> None: + self.name = name - @property - def string_format(self) -> str: - return f"{self.name}={self.value}" + def __contains__(self, key: str) -> bool: + """Check if key is in config.""" + return key in self._data + def __len__(self): + """Get size of config.""" + return len(self._data) -class SysctlConfig: - """Represents the state of the config that a charm wants to enforce.""" + def __iter__(self): + """Iterate over config.""" + return iter(self._data) + + def __getitem__(self, key: str) -> int: + """Get value for key form config.""" + return self._data[key] + + @property + def _data(self) -> Dict[str, int]: + """Return applied internal config.""" + if not self.merged_config_exists: + return {} - def __init__(self, config_params: dict[str, dict], app_name: str) -> None: - self.config_params = config_params - self.app_name = app_name + with open(SYSCTL_FILENAME, "r") as f: + return {param.strip(): int(value.strip()) + for line in f.read().splitlines() + if line and not line.startswith('#') + for param, value in [line.split('=')]} @property - def config_params(self) -> list[ConfigOption]: - """Config options passed to the lib.""" - return self._config_params + def name(self) -> str: + """Name used to create the lib file.""" + return self._name - @config_params.setter - def config_params(self, value: dict): - result = [] - for k, v in value.items(): - result += [ConfigOption(k, v["value"])] - self._config_params = result + @name.setter + def name(self, value: str): + if value == "": + self._name, *_ = os.getenv("JUJU_UNIT_NAME", "unknown").split("/") + else: + self._name = value @property def charm_filepath(self) -> str: """Name for resulting charm config file.""" - return f"{SYSCTL_DIRECTORY}/90-juju-{self.app_name}" + return f"{SYSCTL_DIRECTORY}/90-juju-{self.name}" @property def charm_config_exists(self) -> bool: @@ -148,26 +135,17 @@ def merged_config_exists(self) -> bool: """Return whether a merged config file exists.""" return os.path.exists(SYSCTL_FILENAME) - @property - def merged_config(self) -> list[ConfigOption] | None: - """Return applied internal config.""" - if not self.merged_config_exists: - return None - - with open(SYSCTL_FILENAME, "r") as f: - return [ConfigOption(param.strip(), int(value.strip())) - for line in f.read().splitlines() - if line and not line.startswith('#') - for param, value in [line.split('=')]] - - def update(self) -> None: - """Update sysctl config options.""" + def update(self, config: Dict[str, dict]) -> None: + """Update sysctl config options with a desired set of config params.""" + self._parse_config(config) if self.charm_config_exists: return self._create_charm_file() - if not self.validate(): - raise ValidationError() + conflict = self.validate() + if conflict: + msg = f"Validation error for keys: {conflict}" + raise ValidationError(msg) snapshot = self._create_snapshot() logger.debug(f"Created snapshot for keys: {snapshot}") @@ -176,17 +154,25 @@ def update(self) -> None: except SysctlPermissionError: self._restore_snapshot(snapshot) raise - except ValidationError: + except SysctlError: raise self._merge() - def validate(self) -> bool: + def validate(self) -> list[str]: """Validate the desired config params against merged ones.""" - return True + common_keys = set(self._data.keys()) & set(self._desired_config.keys()) + confict_keys = [] + for key in common_keys: + if self._data[key] != self._desired_config[key]: + msg = f"Values for key '{key}' are different: {self._data[key]} != {self._desired_config[key]}" + logger.warning(msg) + confict_keys += key + + return confict_keys def _create_charm_file(self) -> None: """Write the charm file.""" - charm_params = [f"{param.string_format}\n" for param in self.config_params] + charm_params = [f"{key}={value}" for key, value in self._desired_config.items()] with open(self.charm_filepath, "w") as f: f.writelines(charm_params) @@ -217,15 +203,14 @@ def _apply(self) -> list[str] | None: logger.error(msg) raise SysctlPermissionError(msg) - def _create_snapshot(self) -> list[ConfigOption]: + def _create_snapshot(self) -> Dict[str, int]: """Create a snaphot of config options that are going to be set.""" - keys = [param.name for param in self.config_params] - return [ConfigOption(key, int(self._sysctl([key, "-n"])[0])) for key in keys] + return {key: int(self._sysctl([key, "-n"])[0]) for key in self._desired_config.keys()} - def _restore_snapshot(self, snapshot: list[ConfigOption]) -> None: + def _restore_snapshot(self, snapshot: Dict[str, int]) -> None: """Restore a snapshot to the machine.""" - for param in snapshot: - self._sysctl([param.string_format]) + for key, value in snapshot.items(): + self._sysctl([f"{key}={value}"]) def _sysctl(self, cmd: list[str]) -> list[str]: """Execute a sysctl command.""" @@ -235,3 +220,10 @@ def _sysctl(self, cmd: list[str]) -> list[str]: return check_output(cmd, stderr=STDOUT, universal_newlines=True).splitlines() except CalledProcessError as e: raise SysctlError(f"Error executing '{cmd}': {e.output}") + + def _parse_config(self, config: Dict[str, dict]) -> None: + """Parse a config passed to the lib.""" + result = {} + for k, v in config.items(): + result[k]=v["value"] + self._desired_config: Dict[str, int] = result diff --git a/src/charm.py b/src/charm.py index b6f16c55..289673d7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -6,8 +6,8 @@ import logging import subprocess -from typing import MutableMapping, Optional from pathlib import Path +from typing import MutableMapping, Optional import yaml from charms.data_platform_libs.v0.data_models import TypedCharmBase @@ -16,6 +16,7 @@ SysctlConfig, SysctlError, SysctlPermissionError, + ValidationError, ) from charms.operator_libs_linux.v1.snap import SnapError from charms.rolling_ops.v0.rollingops import RollingOpsManager, RunWithLock @@ -73,6 +74,7 @@ def __init__(self, *args): self.name = CHARM_KEY self.snap = KafkaSnap() self.kafka_config = KafkaConfig(self) + self.sysctl_config = SysctlConfig(name=CHARM_KEY) self.tls = KafkaTLS(self) self.provider = KafkaProvider(self) self.health = KafkaHealth(self) @@ -246,10 +248,9 @@ def _on_storage_detaching(self, event: StorageDetachingEvent) -> None: def _on_install(self, _) -> None: """Handler for `install` event.""" sysctl_yaml = yaml.safe_load(Path("./src/templates/sysctl.yaml").read_text()) - sysctl = SysctlConfig(config_params=sysctl_yaml["testing"], app_name=CHARM_KEY) try: - sysctl.update() - except SysctlPermissionError as e: + self.sysctl_config.update(config=sysctl_yaml["testing"]) + except (SysctlPermissionError, ValidationError) as e: logger.error(f"Error setting values on sysctl: {e.message}") self._set_status(Status.SYSCONF_NOT_POSSIBLE) except SysctlError: From 4cac1f5440db8cac96eaaaa9e2709bf7d1f5e05b Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Tue, 11 Jul 2023 12:40:07 +0200 Subject: [PATCH 03/11] address some PR feedback --- lib/charms/operator_libs_linux/v0/sysctl.py | 44 ++++++++++----------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index 53d87ea5..5404fa20 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -18,9 +18,9 @@ import logging import os import re -import glob +from pathlib import Path from subprocess import STDOUT, CalledProcessError, check_output -from typing import Mapping, Dict +from typing import Mapping, Dict, Optional logger = logging.getLogger(__name__) @@ -35,8 +35,8 @@ LIBPATCH = 2 -SYSCTL_DIRECTORY = "/etc/sysctl.d" -SYSCTL_FILENAME = f"{SYSCTL_DIRECTORY}/95-juju-sysctl.conf" +SYSCTL_DIRECTORY = Path("/etc/sysctl.d") +SYSCTL_FILENAME = Path(f"{SYSCTL_DIRECTORY}/95-juju-sysctl.conf") SYSCTL_HEADER = f"""# This config file was produced by sysctl lib v{LIBAPI}.{LIBPATCH} # # This file represents the output of the sysctl lib, which can combine multiple @@ -77,7 +77,7 @@ class ValidationError(Error): class SysctlConfig(Mapping[str, int]): """Represents the state of the config that a charm wants to enforce.""" - def __init__(self, name: str = "") -> None: + def __init__(self, name: Optional[str] = None) -> None: self.name = name def __contains__(self, key: str) -> bool: @@ -115,15 +115,15 @@ def name(self) -> str: @name.setter def name(self, value: str): - if value == "": + if value is None: self._name, *_ = os.getenv("JUJU_UNIT_NAME", "unknown").split("/") else: self._name = value @property - def charm_filepath(self) -> str: + def charm_filepath(self) -> Path: """Name for resulting charm config file.""" - return f"{SYSCTL_DIRECTORY}/90-juju-{self.name}" + return Path(f"{SYSCTL_DIRECTORY}/90-juju-{self.name}") @property def charm_config_exists(self) -> bool: @@ -140,7 +140,6 @@ def update(self, config: Dict[str, dict]) -> None: self._parse_config(config) if self.charm_config_exists: return - self._create_charm_file() conflict = self.validate() if conflict: @@ -156,6 +155,8 @@ def update(self, config: Dict[str, dict]) -> None: raise except SysctlError: raise + + self._create_charm_file() self._merge() def validate(self) -> list[str]: @@ -172,31 +173,28 @@ def validate(self) -> list[str]: def _create_charm_file(self) -> None: """Write the charm file.""" - charm_params = [f"{key}={value}" for key, value in self._desired_config.items()] + charm_params = [f"{key}={value}\n" for key, value in self._desired_config.items()] with open(self.charm_filepath, "w") as f: f.writelines(charm_params) def _merge(self) -> None: """Create the merged sysctl file.""" - # get all files that start by 90-juju- - charm_files = [f for f in glob.glob(f"{SYSCTL_DIRECTORY}/90-juju-*")] - data = [SYSCTL_HEADER] - for path in charm_files: - with open(path, "r") as f: - data += f.readlines() - with open(SYSCTL_FILENAME, "w") as f: - f.writelines(data) + data = ( + [f"# {self.name}\n"] + [f"{key}={value}\n" for key, value in self._desired_config.items()] + ) + if not self.merged_config_exists: + data.insert(0, SYSCTL_HEADER) - def _apply(self) -> list[str] | None: - """Apply values to machine. + with open(SYSCTL_FILENAME, "a+") as f: + f.writelines(data) - Returns: - none or the list of keys that failed to apply. - """ + def _apply(self) -> None: + """Apply values to machine.""" cmd = ["-p", self.charm_filepath] result = self._sysctl(cmd) expr = re.compile(r'^sysctl: permission denied on key \"([a-z_\.]+)\", ignoring$') failed_values = [expr.match(line) for line in result if expr.match(line)] + logger.debug(f"Failed values: {failed_values}") if failed_values: msg = f"Unable to set params: {[f.group(1) for f in failed_values]}" From 67edc05b83771001569d0a43878d664d47ea5926 Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Wed, 12 Jul 2023 10:18:36 +0200 Subject: [PATCH 04/11] add remove and fix bugs --- lib/charms/operator_libs_linux/v0/sysctl.py | 74 ++++++++++++--------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index 5404fa20..ba1988af 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -12,15 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Handler for the sysctl config -""" +"""Handler for the sysctl config.""" +import glob import logging import os import re from pathlib import Path from subprocess import STDOUT, CalledProcessError, check_output -from typing import Mapping, Dict, Optional +from typing import Dict, Mapping, Optional logger = logging.getLogger(__name__) @@ -79,6 +79,7 @@ class SysctlConfig(Mapping[str, int]): def __init__(self, name: Optional[str] = None) -> None: self.name = name + self._data = self._load_data() def __contains__(self, key: str) -> bool: """Check if key is in config.""" @@ -96,18 +97,6 @@ def __getitem__(self, key: str) -> int: """Get value for key form config.""" return self._data[key] - @property - def _data(self) -> Dict[str, int]: - """Return applied internal config.""" - if not self.merged_config_exists: - return {} - - with open(SYSCTL_FILENAME, "r") as f: - return {param.strip(): int(value.strip()) - for line in f.read().splitlines() - if line and not line.startswith('#') - for param, value in [line.split('=')]} - @property def name(self) -> str: """Name used to create the lib file.""" @@ -138,10 +127,13 @@ def merged_config_exists(self) -> bool: def update(self, config: Dict[str, dict]) -> None: """Update sysctl config options with a desired set of config params.""" self._parse_config(config) + + # NOTE: case where own charm calls update() more than once. Remove first so + # we don't get validation errors. if self.charm_config_exists: - return + self.remove() - conflict = self.validate() + conflict = self._validate() if conflict: msg = f"Validation error for keys: {conflict}" raise ValidationError(msg) @@ -159,7 +151,13 @@ def update(self, config: Dict[str, dict]) -> None: self._create_charm_file() self._merge() - def validate(self) -> list[str]: + def remove(self) -> None: + """Remove config for charm.""" + self.charm_filepath.unlink(missing_ok=True) + logger.info("charm config file %s was removed", self.charm_filepath) + self._merge() + + def _validate(self) -> list[str]: """Validate the desired config params against merged ones.""" common_keys = set(self._data.keys()) & set(self._desired_config.keys()) confict_keys = [] @@ -167,7 +165,7 @@ def validate(self) -> list[str]: if self._data[key] != self._desired_config[key]: msg = f"Values for key '{key}' are different: {self._data[key]} != {self._desired_config[key]}" logger.warning(msg) - confict_keys += key + confict_keys.append(key) return confict_keys @@ -179,18 +177,21 @@ def _create_charm_file(self) -> None: def _merge(self) -> None: """Create the merged sysctl file.""" - data = ( - [f"# {self.name}\n"] + [f"{key}={value}\n" for key, value in self._desired_config.items()] - ) - if not self.merged_config_exists: - data.insert(0, SYSCTL_HEADER) - - with open(SYSCTL_FILENAME, "a+") as f: + # get all files that start by 90-juju- + charm_files = list(glob.glob(f"{SYSCTL_DIRECTORY}/90-juju-*")) + data = [SYSCTL_HEADER] + for path in charm_files: + with open(path, "r") as f: + data += f.readlines() + with open(SYSCTL_FILENAME, "w") as f: f.writelines(data) + # Reload data with newly created file. + self._data = self._load_data() + def _apply(self) -> None: """Apply values to machine.""" - cmd = ["-p", self.charm_filepath] + cmd = [f"{key}={value}" for key, value in self._desired_config.items()] result = self._sysctl(cmd) expr = re.compile(r'^sysctl: permission denied on key \"([a-z_\.]+)\", ignoring$') failed_values = [expr.match(line) for line in result if expr.match(line)] @@ -207,8 +208,8 @@ def _create_snapshot(self) -> Dict[str, int]: def _restore_snapshot(self, snapshot: Dict[str, int]) -> None: """Restore a snapshot to the machine.""" - for key, value in snapshot.items(): - self._sysctl([f"{key}={value}"]) + values = [f"{key}={value}" for key, value in snapshot.items()] + self._sysctl(values) def _sysctl(self, cmd: list[str]) -> list[str]: """Execute a sysctl command.""" @@ -217,7 +218,9 @@ def _sysctl(self, cmd: list[str]) -> list[str]: try: return check_output(cmd, stderr=STDOUT, universal_newlines=True).splitlines() except CalledProcessError as e: - raise SysctlError(f"Error executing '{cmd}': {e.output}") + msg = f"Error executing '{cmd}': {e.stdout}" + logger.error(msg) + raise SysctlError(msg) def _parse_config(self, config: Dict[str, dict]) -> None: """Parse a config passed to the lib.""" @@ -225,3 +228,14 @@ def _parse_config(self, config: Dict[str, dict]) -> None: for k, v in config.items(): result[k]=v["value"] self._desired_config: Dict[str, int] = result + + def _load_data(self) -> Dict[str, int]: + """Gets merged config.""" + if not self.merged_config_exists: + return {} + + with open(SYSCTL_FILENAME, "r") as f: + return {param.strip(): int(value.strip()) + for line in f.read().splitlines() + if line and not line.startswith('#') + for param, value in [line.split('=')]} From 16e963d1b9bfa71270943b13b927dde428ce1b0d Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Mon, 17 Jul 2023 18:00:18 +0200 Subject: [PATCH 05/11] simplify class --- lib/charms/operator_libs_linux/v0/sysctl.py | 148 ++++++++++++-------- src/charm.py | 28 ++-- src/templates/sysctl.yaml | 3 + 3 files changed, 114 insertions(+), 65 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index ba1988af..95438136 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -12,15 +12,57 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Handler for the sysctl config.""" +"""Handler for the sysctl config. + +A charm using the sysctl lib will need a data structure like the following: +```yaml +vm.swappiness: + value: 1 +vm.max_map_count: + value: 262144 +vm.dirty_ratio: + value: 80 +vm.dirty_background_ratio: + value: 5 +net.ipv4.tcp_max_syn_backlog: + value: 4096 +``` + +Now, it can use that template within the charm, or just declare the values directly: + +```python +from charms.operator_libs_linux.v0 import sysctl + +class MyCharm(CharmBase): + + def __init__(self, *args): + ... + self.sysctl = sysctl.Config(self.meta.name) + + self.framework.observe(self.on.install, self._on_install) + self.framework.observe(self.on.remove, self._on_remove) + + def _on_install(self, _): + # Altenatively, read the values from a template + sysctl_data = {"net.ipv4.tcp_max_syn_backlog": {"value": 4096}} + + try: + self.sysctl.update(config=sysctl_data) + except (sysctl.SysctlPermissionError, sysctl.ValidationError) as e: + logger.error(f"Error setting values on sysctl: {e.message}") + self.unit.status = BlockedStatus("Sysctl config not possible") + except sysctl.SysctlError: + logger.error("Error on sysctl") + + def _on_remove(self, _): + self.sysctl.remove() +""" -import glob import logging -import os import re from pathlib import Path from subprocess import STDOUT, CalledProcessError, check_output -from typing import Dict, Mapping, Optional +from typing import Dict, List logger = logging.getLogger(__name__) @@ -34,7 +76,7 @@ # to 0 if you are raising the major API version LIBPATCH = 2 - +CHARM_FILENAME_PREFIX = "90-juju-" SYSCTL_DIRECTORY = Path("/etc/sysctl.d") SYSCTL_FILENAME = Path(f"{SYSCTL_DIRECTORY}/95-juju-sysctl.conf") SYSCTL_HEADER = f"""# This config file was produced by sysctl lib v{LIBAPI}.{LIBPATCH} @@ -74,10 +116,10 @@ class ValidationError(Error): """Exception representing value validation error.""" -class SysctlConfig(Mapping[str, int]): +class Config(Dict): """Represents the state of the config that a charm wants to enforce.""" - def __init__(self, name: Optional[str] = None) -> None: + def __init__(self, name: str) -> None: self.name = name self._data = self._load_data() @@ -97,56 +139,38 @@ def __getitem__(self, key: str) -> int: """Get value for key form config.""" return self._data[key] - @property - def name(self) -> str: - """Name used to create the lib file.""" - return self._name - - @name.setter - def name(self, value: str): - if value is None: - self._name, *_ = os.getenv("JUJU_UNIT_NAME", "unknown").split("/") - else: - self._name = value - @property def charm_filepath(self) -> Path: """Name for resulting charm config file.""" - return Path(f"{SYSCTL_DIRECTORY}/90-juju-{self.name}") - - @property - def charm_config_exists(self) -> bool: - """Return whether the charm config file exists.""" - return os.path.exists(self.charm_filepath) - - @property - def merged_config_exists(self) -> bool: - """Return whether a merged config file exists.""" - return os.path.exists(SYSCTL_FILENAME) + return SYSCTL_DIRECTORY / f"{CHARM_FILENAME_PREFIX}{self.name}" def update(self, config: Dict[str, dict]) -> None: - """Update sysctl config options with a desired set of config params.""" + """Update sysctl config options with a desired set of config params. + + Args: + config: dictionary with keys to update: + ``` + {"vm.swappiness": {"value": 10}, ...} + ``` + """ self._parse_config(config) # NOTE: case where own charm calls update() more than once. Remove first so # we don't get validation errors. - if self.charm_config_exists: + if self.charm_filepath.exists(): self.remove() conflict = self._validate() if conflict: - msg = f"Validation error for keys: {conflict}" - raise ValidationError(msg) + raise ValidationError(f"Validation error for keys: {conflict}") snapshot = self._create_snapshot() - logger.debug(f"Created snapshot for keys: {snapshot}") + logger.debug("Created snapshot for keys: %s", snapshot) try: self._apply() except SysctlPermissionError: self._restore_snapshot(snapshot) raise - except SysctlError: - raise self._create_charm_file() self._merge() @@ -154,17 +178,21 @@ def update(self, config: Dict[str, dict]) -> None: def remove(self) -> None: """Remove config for charm.""" self.charm_filepath.unlink(missing_ok=True) - logger.info("charm config file %s was removed", self.charm_filepath) + logger.info("Charm config file %s was removed", self.charm_filepath) self._merge() - def _validate(self) -> list[str]: + def _validate(self) -> List[str]: """Validate the desired config params against merged ones.""" common_keys = set(self._data.keys()) & set(self._desired_config.keys()) confict_keys = [] for key in common_keys: if self._data[key] != self._desired_config[key]: - msg = f"Values for key '{key}' are different: {self._data[key]} != {self._desired_config[key]}" - logger.warning(msg) + logger.warning( + "Values for key '%s' are different: %s != %s", + key, + self._data[key], + self._desired_config[key], + ) confict_keys.append(key) return confict_keys @@ -178,9 +206,8 @@ def _create_charm_file(self) -> None: def _merge(self) -> None: """Create the merged sysctl file.""" # get all files that start by 90-juju- - charm_files = list(glob.glob(f"{SYSCTL_DIRECTORY}/90-juju-*")) data = [SYSCTL_HEADER] - for path in charm_files: + for path in SYSCTL_DIRECTORY.glob(f"{CHARM_FILENAME_PREFIX}*"): with open(path, "r") as f: data += f.readlines() with open(SYSCTL_FILENAME, "w") as f: @@ -193,9 +220,9 @@ def _apply(self) -> None: """Apply values to machine.""" cmd = [f"{key}={value}" for key, value in self._desired_config.items()] result = self._sysctl(cmd) - expr = re.compile(r'^sysctl: permission denied on key \"([a-z_\.]+)\", ignoring$') + expr = re.compile(r"^sysctl: permission denied on key \"([a-z_\.]+)\", ignoring$") failed_values = [expr.match(line) for line in result if expr.match(line)] - logger.debug(f"Failed values: {failed_values}") + logger.debug("Failed values: %s", failed_values) if failed_values: msg = f"Unable to set params: {[f.group(1) for f in failed_values]}" @@ -211,10 +238,10 @@ def _restore_snapshot(self, snapshot: Dict[str, int]) -> None: values = [f"{key}={value}" for key, value in snapshot.items()] self._sysctl(values) - def _sysctl(self, cmd: list[str]) -> list[str]: + def _sysctl(self, cmd: List[str]) -> List[str]: """Execute a sysctl command.""" cmd = ["sysctl"] + cmd - logger.debug(f"Executing sysctl command: {cmd}") + logger.debug("Executing sysctl command: %s", cmd) try: return check_output(cmd, stderr=STDOUT, universal_newlines=True).splitlines() except CalledProcessError as e: @@ -225,17 +252,26 @@ def _sysctl(self, cmd: list[str]) -> list[str]: def _parse_config(self, config: Dict[str, dict]) -> None: """Parse a config passed to the lib.""" result = {} - for k, v in config.items(): - result[k]=v["value"] + for key, value in config.items(): + result[key] = int(value["value"]) self._desired_config: Dict[str, int] = result def _load_data(self) -> Dict[str, int]: - """Gets merged config.""" - if not self.merged_config_exists: - return {} + """Get merged config.""" + config = {} + if not SYSCTL_FILENAME.exists(): + return config with open(SYSCTL_FILENAME, "r") as f: - return {param.strip(): int(value.strip()) - for line in f.read().splitlines() - if line and not line.startswith('#') - for param, value in [line.split('=')]} + for line in f: + config.update(self._parse_line(line)) + + return config + + def _parse_line(self, line: str) -> Dict[str, int]: + """Parse a line from juju-sysctl.conf file.""" + if line.startswith("#") or line == "\n": + return {} + + param, value = line.split("=") + return {param.strip(): int(value.strip())} diff --git a/src/charm.py b/src/charm.py index 289673d7..dc112185 100755 --- a/src/charm.py +++ b/src/charm.py @@ -12,12 +12,7 @@ import yaml from charms.data_platform_libs.v0.data_models import TypedCharmBase from charms.grafana_agent.v0.cos_agent import COSAgentProvider -from charms.operator_libs_linux.v0.sysctl import ( - SysctlConfig, - SysctlError, - SysctlPermissionError, - ValidationError, -) +from charms.operator_libs_linux.v0 import sysctl from charms.operator_libs_linux.v1.snap import SnapError from charms.rolling_ops.v0.rollingops import RollingOpsManager, RunWithLock from ops.charm import ( @@ -74,7 +69,7 @@ def __init__(self, *args): self.name = CHARM_KEY self.snap = KafkaSnap() self.kafka_config = KafkaConfig(self) - self.sysctl_config = SysctlConfig(name=CHARM_KEY) + self.sysctl_config = sysctl.Config(name=CHARM_KEY) self.tls = KafkaTLS(self) self.provider = KafkaProvider(self) self.health = KafkaHealth(self) @@ -84,6 +79,7 @@ def __init__(self, *args): self.framework.observe(getattr(self.on, "install"), self._on_install) self.framework.observe(getattr(self.on, "config_changed"), self._on_config_changed) self.framework.observe(getattr(self.on, "update_status"), self._on_update_status) + self.framework.observe(getattr(self.on, "stop"), self._on_stop) self.framework.observe(self.on[PEER].relation_changed, self._on_config_changed) @@ -192,6 +188,10 @@ def healthy(self) -> bool: return True + def _on_stop(self, _) -> None: + """Handler for stop.""" + self.sysctl_config.remove() + def _on_update_status(self, event: EventBase) -> None: """Handler for `update-status` events.""" if not self.healthy: @@ -250,10 +250,10 @@ def _on_install(self, _) -> None: sysctl_yaml = yaml.safe_load(Path("./src/templates/sysctl.yaml").read_text()) try: self.sysctl_config.update(config=sysctl_yaml["testing"]) - except (SysctlPermissionError, ValidationError) as e: + except (sysctl.SysctlPermissionError, sysctl.ValidationError) as e: logger.error(f"Error setting values on sysctl: {e.message}") self._set_status(Status.SYSCONF_NOT_POSSIBLE) - except SysctlError: + except sysctl.SysctlError: logger.error("Error on sysctl") if self.snap.install(): @@ -341,6 +341,16 @@ def _on_config_changed(self, event: EventBase) -> None: event.defer() return + # SYSCTL EXAMPLE + sysctl_yaml = yaml.safe_load(Path("./src/templates/sysctl.yaml").read_text()) + try: + self.sysctl_config.update(config=sysctl_yaml[self.config.profile]) + except (SysctlPermissionError, ValidationError) as e: + logger.error(f"Error setting values on sysctl: {e.message}") + self._set_status(Status.SYSCONF_NOT_POSSIBLE) + except SysctlError: + logger.error("Error on sysctl") + # Load current properties set in the charm workload properties = safe_get_file(self.kafka_config.server_properties_filepath) if not properties: diff --git a/src/templates/sysctl.yaml b/src/templates/sysctl.yaml index 4a9a20cb..433a8444 100644 --- a/src/templates/sysctl.yaml +++ b/src/templates/sysctl.yaml @@ -9,3 +9,6 @@ testing: value: 5 net.ipv4.tcp_max_syn_backlog: value: 4096 +production: + net.ipv4.tcp_max_syn_backlog: + value: 4096 From b4ea9a05584d1e5a18b2b7e3b8aca0b89c214df9 Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Wed, 19 Jul 2023 13:28:53 +0200 Subject: [PATCH 06/11] add conditional merge option --- lib/charms/operator_libs_linux/v0/sysctl.py | 16 ++++++++++++---- src/charm.py | 8 ++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index 95438136..16429f6f 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -158,7 +158,7 @@ def update(self, config: Dict[str, dict]) -> None: # NOTE: case where own charm calls update() more than once. Remove first so # we don't get validation errors. if self.charm_filepath.exists(): - self.remove() + self._merge(add_own_charm=False) conflict = self._validate() if conflict: @@ -203,11 +203,19 @@ def _create_charm_file(self) -> None: with open(self.charm_filepath, "w") as f: f.writelines(charm_params) - def _merge(self) -> None: - """Create the merged sysctl file.""" + def _merge(self, add_own_charm=True) -> None: + """Create the merged sysctl file. + + Args: + add_own_charm : bool, if false it will skip the charm file from the merge. + """ # get all files that start by 90-juju- data = [SYSCTL_HEADER] - for path in SYSCTL_DIRECTORY.glob(f"{CHARM_FILENAME_PREFIX}*"): + paths = set(SYSCTL_DIRECTORY.glob(f"{CHARM_FILENAME_PREFIX}*")) + if not add_own_charm: + paths.discard(self.charm_filepath.as_posix()) + + for path in paths: with open(path, "r") as f: data += f.readlines() with open(SYSCTL_FILENAME, "w") as f: diff --git a/src/charm.py b/src/charm.py index dc112185..1ba0b328 100755 --- a/src/charm.py +++ b/src/charm.py @@ -79,7 +79,7 @@ def __init__(self, *args): self.framework.observe(getattr(self.on, "install"), self._on_install) self.framework.observe(getattr(self.on, "config_changed"), self._on_config_changed) self.framework.observe(getattr(self.on, "update_status"), self._on_update_status) - self.framework.observe(getattr(self.on, "stop"), self._on_stop) + self.framework.observe(getattr(self.on, "remove"), self._on_remove) self.framework.observe(self.on[PEER].relation_changed, self._on_config_changed) @@ -188,7 +188,7 @@ def healthy(self) -> bool: return True - def _on_stop(self, _) -> None: + def _on_remove(self, _) -> None: """Handler for stop.""" self.sysctl_config.remove() @@ -345,10 +345,10 @@ def _on_config_changed(self, event: EventBase) -> None: sysctl_yaml = yaml.safe_load(Path("./src/templates/sysctl.yaml").read_text()) try: self.sysctl_config.update(config=sysctl_yaml[self.config.profile]) - except (SysctlPermissionError, ValidationError) as e: + except (sysctl.SysctlPermissionError, sysctl.ValidationError) as e: logger.error(f"Error setting values on sysctl: {e.message}") self._set_status(Status.SYSCONF_NOT_POSSIBLE) - except SysctlError: + except sysctl.SysctlError: logger.error("Error on sysctl") # Load current properties set in the charm workload From c8b7aa78115c9e71765fb3ddf8c11f4eb6c98f4e Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Wed, 19 Jul 2023 16:53:43 +0200 Subject: [PATCH 07/11] add PR feedback --- lib/charms/operator_libs_linux/v0/sysctl.py | 40 +++++++++++++-------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index 16429f6f..26a6b5b7 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -14,6 +14,18 @@ """Handler for the sysctl config. +This library allows your charm to create and update sysctl config options to the machine. + +Validation and merge capabilities are added, for situations where more than one application +are setting values. The following files can be created: + +- /etc/sysctl.d/90-juju- + Requirements from one application requesting to update the values. + +- /etc/sysctl.d/95-juju-sysctl.conf + Merged file resulting from all other `90-juju-*` application files. + + A charm using the sysctl lib will need a data structure like the following: ```yaml vm.swappiness: @@ -56,6 +68,7 @@ def _on_install(self, _): def _on_remove(self, _): self.sysctl.remove() +``` """ import logging @@ -135,7 +148,7 @@ def __iter__(self): """Iterate over config.""" return iter(self._data) - def __getitem__(self, key: str) -> int: + def __getitem__(self, key: str) -> str: """Get value for key form config.""" return self._data[key] @@ -155,8 +168,7 @@ def update(self, config: Dict[str, dict]) -> None: """ self._parse_config(config) - # NOTE: case where own charm calls update() more than once. Remove first so - # we don't get validation errors. + # NOTE: case where own charm calls update() more than once. if self.charm_filepath.exists(): self._merge(add_own_charm=False) @@ -184,7 +196,7 @@ def remove(self) -> None: def _validate(self) -> List[str]: """Validate the desired config params against merged ones.""" common_keys = set(self._data.keys()) & set(self._desired_config.keys()) - confict_keys = [] + conflict_keys = [] for key in common_keys: if self._data[key] != self._desired_config[key]: logger.warning( @@ -193,9 +205,9 @@ def _validate(self) -> List[str]: self._data[key], self._desired_config[key], ) - confict_keys.append(key) + conflict_keys.append(key) - return confict_keys + return conflict_keys def _create_charm_file(self) -> None: """Write the charm file.""" @@ -237,11 +249,11 @@ def _apply(self) -> None: logger.error(msg) raise SysctlPermissionError(msg) - def _create_snapshot(self) -> Dict[str, int]: + def _create_snapshot(self) -> Dict[str, str]: """Create a snaphot of config options that are going to be set.""" - return {key: int(self._sysctl([key, "-n"])[0]) for key in self._desired_config.keys()} + return {key: self._sysctl([key, "-n"])[0] for key in self._desired_config.keys()} - def _restore_snapshot(self, snapshot: Dict[str, int]) -> None: + def _restore_snapshot(self, snapshot: Dict[str, str]) -> None: """Restore a snapshot to the machine.""" values = [f"{key}={value}" for key, value in snapshot.items()] self._sysctl(values) @@ -261,10 +273,10 @@ def _parse_config(self, config: Dict[str, dict]) -> None: """Parse a config passed to the lib.""" result = {} for key, value in config.items(): - result[key] = int(value["value"]) - self._desired_config: Dict[str, int] = result + result[key] = value["value"] + self._desired_config: Dict[str, str] = result - def _load_data(self) -> Dict[str, int]: + def _load_data(self) -> Dict[str, str]: """Get merged config.""" config = {} if not SYSCTL_FILENAME.exists(): @@ -276,10 +288,10 @@ def _load_data(self) -> Dict[str, int]: return config - def _parse_line(self, line: str) -> Dict[str, int]: + def _parse_line(self, line: str) -> Dict[str, str]: """Parse a line from juju-sysctl.conf file.""" if line.startswith("#") or line == "\n": return {} param, value = line.split("=") - return {param.strip(): int(value.strip())} + return {param.strip(): value.strip()} From 15a42f072b3961f8e1215594d984b659c62e5eda Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Wed, 19 Jul 2023 17:09:56 +0200 Subject: [PATCH 08/11] cast to str --- lib/charms/operator_libs_linux/v0/sysctl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index 26a6b5b7..e73dc286 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -273,7 +273,7 @@ def _parse_config(self, config: Dict[str, dict]) -> None: """Parse a config passed to the lib.""" result = {} for key, value in config.items(): - result[key] = value["value"] + result[key] = str(value["value"]) self._desired_config: Dict[str, str] = result def _load_data(self) -> Dict[str, str]: From 277c23d5aaf7a38137f34d3ad2304e80af90e0ca Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Thu, 27 Jul 2023 10:06:39 +0200 Subject: [PATCH 09/11] update sysctl integration --- lib/charms/operator_libs_linux/v0/sysctl.py | 109 +++++++++----------- src/charm.py | 33 +++--- src/literals.py | 6 ++ tests/unit/test_charm.py | 29 +++++- 4 files changed, 96 insertions(+), 81 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/sysctl.py b/lib/charms/operator_libs_linux/v0/sysctl.py index e73dc286..16b0159f 100644 --- a/lib/charms/operator_libs_linux/v0/sysctl.py +++ b/lib/charms/operator_libs_linux/v0/sysctl.py @@ -14,30 +14,27 @@ """Handler for the sysctl config. -This library allows your charm to create and update sysctl config options to the machine. +This library allows your charm to create and configure sysctl options to the machine. Validation and merge capabilities are added, for situations where more than one application are setting values. The following files can be created: - /etc/sysctl.d/90-juju- - Requirements from one application requesting to update the values. + Requirements from one application requesting to configure the values. - /etc/sysctl.d/95-juju-sysctl.conf Merged file resulting from all other `90-juju-*` application files. A charm using the sysctl lib will need a data structure like the following: -```yaml -vm.swappiness: - value: 1 -vm.max_map_count: - value: 262144 -vm.dirty_ratio: - value: 80 -vm.dirty_background_ratio: - value: 5 -net.ipv4.tcp_max_syn_backlog: - value: 4096 +``` +{ +"vm.swappiness": "1", +"vm.max_map_count": "262144", +"vm.dirty_ratio": "80", +"vm.dirty_background_ratio": "5", +"net.ipv4.tcp_max_syn_backlog": "4096", +} ``` Now, it can use that template within the charm, or just declare the values directly: @@ -56,14 +53,14 @@ def __init__(self, *args): def _on_install(self, _): # Altenatively, read the values from a template - sysctl_data = {"net.ipv4.tcp_max_syn_backlog": {"value": 4096}} + sysctl_data = {"net.ipv4.tcp_max_syn_backlog": "4096"}} try: - self.sysctl.update(config=sysctl_data) - except (sysctl.SysctlPermissionError, sysctl.ValidationError) as e: + self.sysctl.configure(config=sysctl_data) + except (sysctl.ApplyError, sysctl.ValidationError) as e: logger.error(f"Error setting values on sysctl: {e.message}") self.unit.status = BlockedStatus("Sysctl config not possible") - except sysctl.SysctlError: + except sysctl.CommandError: logger.error("Error on sysctl") def _on_remove(self, _): @@ -87,11 +84,11 @@ def _on_remove(self, _): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 2 +LIBPATCH = 3 CHARM_FILENAME_PREFIX = "90-juju-" SYSCTL_DIRECTORY = Path("/etc/sysctl.d") -SYSCTL_FILENAME = Path(f"{SYSCTL_DIRECTORY}/95-juju-sysctl.conf") +SYSCTL_FILENAME = SYSCTL_DIRECTORY / "95-juju-sysctl.conf" SYSCTL_HEADER = f"""# This config file was produced by sysctl lib v{LIBAPI}.{LIBPATCH} # # This file represents the output of the sysctl lib, which can combine multiple @@ -102,26 +99,17 @@ def _on_remove(self, _): class Error(Exception): """Base class of most errors raised by this library.""" - def __repr__(self): - """Represent the Error.""" - return "<{}.{} {}>".format(type(self).__module__, type(self).__name__, self.args) - - @property - def name(self): - """Return a string representation of the model plus class.""" - return "<{}.{}>".format(type(self).__module__, type(self).__name__) - @property def message(self): """Return the message passed as an argument.""" return self.args[0] -class SysctlError(Error): +class CommandError(Error): """Raised when there's an error running sysctl command.""" -class SysctlPermissionError(Error): +class ApplyError(Error): """Raised when there's an error applying values in sysctl.""" @@ -132,6 +120,8 @@ class ValidationError(Error): class Config(Dict): """Represents the state of the config that a charm wants to enforce.""" + _apply_re = re.compile(r"sysctl: permission denied on key \"([a-z_\.]+)\", ignoring$") + def __init__(self, name: str) -> None: self.name = name self._data = self._load_data() @@ -157,18 +147,18 @@ def charm_filepath(self) -> Path: """Name for resulting charm config file.""" return SYSCTL_DIRECTORY / f"{CHARM_FILENAME_PREFIX}{self.name}" - def update(self, config: Dict[str, dict]) -> None: - """Update sysctl config options with a desired set of config params. + def configure(self, config: Dict[str, str]) -> None: + """Configure sysctl options with a desired set of params. Args: - config: dictionary with keys to update: + config: dictionary with keys to configure: ``` - {"vm.swappiness": {"value": 10}, ...} + {"vm.swappiness": "10", ...} ``` """ self._parse_config(config) - # NOTE: case where own charm calls update() more than once. + # NOTE: case where own charm calls configure() more than once. if self.charm_filepath.exists(): self._merge(add_own_charm=False) @@ -180,7 +170,7 @@ def update(self, config: Dict[str, dict]) -> None: logger.debug("Created snapshot for keys: %s", snapshot) try: self._apply() - except SysctlPermissionError: + except ApplyError: self._restore_snapshot(snapshot) raise @@ -188,7 +178,11 @@ def update(self, config: Dict[str, dict]) -> None: self._merge() def remove(self) -> None: - """Remove config for charm.""" + """Remove config for charm. + + The removal process won't apply any sysctl configuration. It will only merge files from + remaining charms. + """ self.charm_filepath.unlink(missing_ok=True) logger.info("Charm config file %s was removed", self.charm_filepath) self._merge() @@ -211,9 +205,10 @@ def _validate(self) -> List[str]: def _create_charm_file(self) -> None: """Write the charm file.""" - charm_params = [f"{key}={value}\n" for key, value in self._desired_config.items()] with open(self.charm_filepath, "w") as f: - f.writelines(charm_params) + f.write(f"# {self.name}\n") + for key, value in self._desired_config.items(): + f.write(f"{key}={value}\n") def _merge(self, add_own_charm=True) -> None: """Create the merged sysctl file. @@ -240,18 +235,21 @@ def _apply(self) -> None: """Apply values to machine.""" cmd = [f"{key}={value}" for key, value in self._desired_config.items()] result = self._sysctl(cmd) - expr = re.compile(r"^sysctl: permission denied on key \"([a-z_\.]+)\", ignoring$") - failed_values = [expr.match(line) for line in result if expr.match(line)] + failed_values = [ + self._apply_re.match(line) for line in result if self._apply_re.match(line) + ] logger.debug("Failed values: %s", failed_values) if failed_values: msg = f"Unable to set params: {[f.group(1) for f in failed_values]}" logger.error(msg) - raise SysctlPermissionError(msg) + raise ApplyError(msg) def _create_snapshot(self) -> Dict[str, str]: - """Create a snaphot of config options that are going to be set.""" - return {key: self._sysctl([key, "-n"])[0] for key in self._desired_config.keys()} + """Create a snapshot of config options that are going to be set.""" + cmd = ["-n"] + list(self._desired_config.keys()) + values = self._sysctl(cmd) + return dict(zip(list(self._desired_config.keys()), values)) def _restore_snapshot(self, snapshot: Dict[str, str]) -> None: """Restore a snapshot to the machine.""" @@ -267,14 +265,11 @@ def _sysctl(self, cmd: List[str]) -> List[str]: except CalledProcessError as e: msg = f"Error executing '{cmd}': {e.stdout}" logger.error(msg) - raise SysctlError(msg) + raise CommandError(msg) - def _parse_config(self, config: Dict[str, dict]) -> None: + def _parse_config(self, config: Dict[str, str]) -> None: """Parse a config passed to the lib.""" - result = {} - for key, value in config.items(): - result[key] = str(value["value"]) - self._desired_config: Dict[str, str] = result + self._desired_config = {k: str(v) for k, v in config.items()} def _load_data(self) -> Dict[str, str]: """Get merged config.""" @@ -284,14 +279,10 @@ def _load_data(self) -> Dict[str, str]: with open(SYSCTL_FILENAME, "r") as f: for line in f: - config.update(self._parse_line(line)) + if line.startswith(("#", ";")) or not line.strip() or "=" not in line: + continue - return config + key, _, value = line.partition("=") + config[key.strip()] = value.strip() - def _parse_line(self, line: str) -> Dict[str, str]: - """Parse a line from juju-sysctl.conf file.""" - if line.startswith("#") or line == "\n": - return {} - - param, value = line.split("=") - return {param.strip(): value.strip()} + return config diff --git a/src/charm.py b/src/charm.py index 1ba0b328..1090ca16 100755 --- a/src/charm.py +++ b/src/charm.py @@ -6,10 +6,8 @@ import logging import subprocess -from pathlib import Path from typing import MutableMapping, Optional -import yaml from charms.data_platform_libs.v0.data_models import TypedCharmBase from charms.grafana_agent.v0.cos_agent import COSAgentProvider from charms.operator_libs_linux.v0 import sysctl @@ -38,6 +36,7 @@ JMX_EXPORTER_PORT, LOGS_RULES_DIR, METRICS_RULES_DIR, + OS_REQUIREMENTS, PEER, REL_NAME, ZK, @@ -247,16 +246,8 @@ def _on_storage_detaching(self, event: StorageDetachingEvent) -> None: def _on_install(self, _) -> None: """Handler for `install` event.""" - sysctl_yaml = yaml.safe_load(Path("./src/templates/sysctl.yaml").read_text()) - try: - self.sysctl_config.update(config=sysctl_yaml["testing"]) - except (sysctl.SysctlPermissionError, sysctl.ValidationError) as e: - logger.error(f"Error setting values on sysctl: {e.message}") - self._set_status(Status.SYSCONF_NOT_POSSIBLE) - except sysctl.SysctlError: - logger.error("Error on sysctl") - if self.snap.install(): + self._set_os_config() self.kafka_config.set_environment() self._set_status(Status.ZK_NOT_RELATED) else: @@ -341,16 +332,6 @@ def _on_config_changed(self, event: EventBase) -> None: event.defer() return - # SYSCTL EXAMPLE - sysctl_yaml = yaml.safe_load(Path("./src/templates/sysctl.yaml").read_text()) - try: - self.sysctl_config.update(config=sysctl_yaml[self.config.profile]) - except (sysctl.SysctlPermissionError, sysctl.ValidationError) as e: - logger.error(f"Error setting values on sysctl: {e.message}") - self._set_status(Status.SYSCONF_NOT_POSSIBLE) - except sysctl.SysctlError: - logger.error("Error on sysctl") - # Load current properties set in the charm workload properties = safe_get_file(self.kafka_config.server_properties_filepath) if not properties: @@ -507,6 +488,16 @@ def _create_internal_credentials(self) -> list[tuple[str, str]]: return credentials + def _set_os_config(self) -> None: + """Sets sysctl config.""" + try: + self.sysctl_config.configure(OS_REQUIREMENTS) + except (sysctl.ApplyError, sysctl.ValidationError) as e: + logger.error(f"Error setting values on sysctl: {e.message}") + self._set_status(Status.SYSCONF_NOT_POSSIBLE) + except sysctl.CommandError: + logger.error("Error on sysctl") + def get_secret(self, scope: str, key: str) -> Optional[str]: """Get TLS secret from the secret storage. diff --git a/src/literals.py b/src/literals.py index 89c3d961..7a966edc 100644 --- a/src/literals.py +++ b/src/literals.py @@ -34,6 +34,12 @@ JVM_MEM_MIN_GB = 1 JVM_MEM_MAX_GB = 6 +OS_REQUIREMENTS = { + "vm.max_map_count": "262144", + "vm.swappiness": "1", + "vm.dirty_ratio": "80", + "vm.dirty_background_ratio": "5", +} @dataclass diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index fda5e373..1b79b04b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -8,12 +8,13 @@ import pytest import yaml +from charms.operator_libs_linux.v0.sysctl import ApplyError from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from ops.testing import Harness from tenacity.wait import wait_none from charm import KafkaCharm -from literals import CHARM_KEY, INTERNAL_USERS, PEER, REL_NAME, ZK +from literals import CHARM_KEY, INTERNAL_USERS, OS_REQUIREMENTS, PEER, REL_NAME, ZK logger = logging.getLogger(__name__) @@ -306,6 +307,7 @@ def test_install_sets_env_vars(harness): with ( patch("snap.KafkaSnap.install"), patch("config.KafkaConfig.set_environment") as patched_kafka_opts, + patch("charm.sysctl.Config.configure"), ): harness.charm.on.install.emit() @@ -317,6 +319,7 @@ def test_install_waits_until_zookeeper_relation(harness): with ( patch("snap.KafkaSnap.install"), patch("config.KafkaConfig.set_environment"), + patch("charm.sysctl.Config.configure"), ): harness.charm.on.install.emit() assert isinstance(harness.charm.unit.status, BlockedStatus) @@ -327,11 +330,35 @@ def test_install_blocks_snap_install_failure(harness): with ( patch("snap.KafkaSnap.install", return_value=False), patch("config.KafkaConfig.set_environment"), + patch("charm.sysctl.Config.configure"), ): harness.charm.on.install.emit() assert isinstance(harness.charm.unit.status, BlockedStatus) +def test_install_configures_os(harness): + with ( + patch("snap.KafkaSnap.install"), + patch("config.KafkaConfig.set_environment"), + patch("charm.sysctl.Config.configure") as patched_os_config, + ): + harness.charm.on.install.emit() + + patched_os_config.assert_called_once_with(OS_REQUIREMENTS) + + +def test_install_sets_status_if_os_config_fails(harness): + with ( + patch("snap.KafkaSnap.install"), + patch("config.KafkaConfig.set_environment"), + patch("charm.sysctl.Config.configure") as patched_os_config, + ): + patched_os_config.side_effect = ApplyError("Error setting values") + harness.charm.on.install.emit() + + assert isinstance(harness.charm.unit.status, BlockedStatus) + + def test_zookeeper_changed_sets_passwords_and_creates_users_with_zk(harness, zk_data): """Checks inter-broker passwords are created on zookeeper-changed hook using zk auth.""" with harness.hooks_disabled(): From c902a7a941a3c6085f596a8ab8b4d2195a8b76c5 Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Thu, 27 Jul 2023 10:09:56 +0200 Subject: [PATCH 10/11] remove template --- src/templates/sysctl.yaml | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 src/templates/sysctl.yaml diff --git a/src/templates/sysctl.yaml b/src/templates/sysctl.yaml deleted file mode 100644 index 433a8444..00000000 --- a/src/templates/sysctl.yaml +++ /dev/null @@ -1,14 +0,0 @@ -testing: - vm.swappiness: - value: 1 - vm.max_map_count: - value: 262144 - vm.dirty_ratio: - value: 80 - vm.dirty_background_ratio: - value: 5 - net.ipv4.tcp_max_syn_backlog: - value: 4096 -production: - net.ipv4.tcp_max_syn_backlog: - value: 4096 From 64d31fd2c34984adce356b756f173118988e182b Mon Sep 17 00:00:00 2001 From: Raul Zamora Date: Tue, 8 Aug 2023 09:41:24 +0200 Subject: [PATCH 11/11] merge error handling --- src/charm.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 1090ca16..290e2c77 100755 --- a/src/charm.py +++ b/src/charm.py @@ -492,11 +492,9 @@ def _set_os_config(self) -> None: """Sets sysctl config.""" try: self.sysctl_config.configure(OS_REQUIREMENTS) - except (sysctl.ApplyError, sysctl.ValidationError) as e: + except (sysctl.ApplyError, sysctl.ValidationError, sysctl.CommandError) as e: logger.error(f"Error setting values on sysctl: {e.message}") self._set_status(Status.SYSCONF_NOT_POSSIBLE) - except sysctl.CommandError: - logger.error("Error on sysctl") def get_secret(self, scope: str, key: str) -> Optional[str]: """Get TLS secret from the secret storage.