-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sysctl library #99
Merged
Merged
Sysctl library #99
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
842e079
draft
zmraul 9a46eed
add snapshot creation
zmraul 9f22676
restructure lib
zmraul 619467b
remove name validation
zmraul b954fda
add unit tests and update lib
zmraul 2d3bff8
add conditional merge option
zmraul 7257317
fix lint
zmraul aa727f2
change type of Config value to str
zmraul afaffa7
fix typing and var name
zmraul fe7eb54
fix typing on tests
zmraul e0572f7
improve error naming
zmraul 82b7ddd
add PR feedback
zmraul 651eed3
remove _parse_line
zmraul befeda3
fix lint
zmraul 0cd9816
add file naming
zmraul c0fcd1c
rename to configure and add feedback
zmraul fa9fa65
remove mocks from unit tests, add integration tests
zmraul File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,24 +1,81 @@ | ||||||||||||||||||||||
"""TODO: Add a proper docstring here. | ||||||||||||||||||||||
# 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. | ||||||||||||||||||||||
|
||||||||||||||||||||||
This is a placeholder docstring for this charm library. Docstrings are | ||||||||||||||||||||||
presented on Charmhub and updated whenever you push a new version of the | ||||||||||||||||||||||
library. | ||||||||||||||||||||||
"""Handler for the sysctl config. | ||||||||||||||||||||||
Complete documentation about creating and documenting libraries can be found | ||||||||||||||||||||||
in the SDK docs at https://juju.is/docs/sdk/libraries. | ||||||||||||||||||||||
This library allows your charm to create and configure sysctl options to the machine. | ||||||||||||||||||||||
See `charmcraft publish-lib` and `charmcraft fetch-lib` for details of how to | ||||||||||||||||||||||
share and consume charm libraries. They serve to enhance collaboration | ||||||||||||||||||||||
between charmers. Use a charmer's libraries for classes that handle | ||||||||||||||||||||||
integration with their charm. | ||||||||||||||||||||||
Validation and merge capabilities are added, for situations where more than one application | ||||||||||||||||||||||
are setting values. The following files can be created: | ||||||||||||||||||||||
Bear in mind that new revisions of the different major API versions (v0, v1, | ||||||||||||||||||||||
v2 etc) are maintained independently. You can continue to update v0 and v1 | ||||||||||||||||||||||
after you have pushed v3. | ||||||||||||||||||||||
- /etc/sysctl.d/90-juju-<app-name> | ||||||||||||||||||||||
Requirements from one application requesting to configure the values. | ||||||||||||||||||||||
Markdown is supported, following the CommonMark specification. | ||||||||||||||||||||||
- /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: | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
{ | ||||||||||||||||||||||
"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: | ||||||||||||||||||||||
```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": "4096"}} | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
self.sysctl.configure(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 logging | ||||||||||||||||||||||
import re | ||||||||||||||||||||||
from pathlib import Path | ||||||||||||||||||||||
from subprocess import STDOUT, CalledProcessError, check_output | ||||||||||||||||||||||
from typing import Dict, List | ||||||||||||||||||||||
|
||||||||||||||||||||||
logger = logging.getLogger(__name__) | ||||||||||||||||||||||
|
||||||||||||||||||||||
# The unique Charmhub library identifier, never change it | ||||||||||||||||||||||
LIBID = "17a6cd4d80104d15b10f9c2420ab3266" | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
@@ -27,6 +84,205 @@ | |||||||||||||||||||||
|
||||||||||||||||||||||
# Increment this PATCH version before using `charmcraft publish-lib` or reset | ||||||||||||||||||||||
# to 0 if you are raising the major API version | ||||||||||||||||||||||
LIBPATCH = 1 | ||||||||||||||||||||||
LIBPATCH = 2 | ||||||||||||||||||||||
|
||||||||||||||||||||||
CHARM_FILENAME_PREFIX = "90-juju-" | ||||||||||||||||||||||
SYSCTL_DIRECTORY = Path("/etc/sysctl.d") | ||||||||||||||||||||||
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 | ||||||||||||||||||||||
# configurations into a single file like. | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
class Error(Exception): | ||||||||||||||||||||||
"""Base class of most errors raised by this library.""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
@property | ||||||||||||||||||||||
def message(self): | ||||||||||||||||||||||
"""Return the message passed as an argument.""" | ||||||||||||||||||||||
return self.args[0] | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
class CommandError(Error): | ||||||||||||||||||||||
"""Raised when there's an error running sysctl command.""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
class ApplyError(Error): | ||||||||||||||||||||||
"""Raised when there's an error applying values in sysctl.""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
class ValidationError(Error): | ||||||||||||||||||||||
"""Exception representing value validation 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() | ||||||||||||||||||||||
|
||||||||||||||||||||||
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) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def __iter__(self): | ||||||||||||||||||||||
"""Iterate over config.""" | ||||||||||||||||||||||
return iter(self._data) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def __getitem__(self, key: str) -> str: | ||||||||||||||||||||||
"""Get value for key form config.""" | ||||||||||||||||||||||
return self._data[key] | ||||||||||||||||||||||
|
||||||||||||||||||||||
@property | ||||||||||||||||||||||
def charm_filepath(self) -> Path: | ||||||||||||||||||||||
"""Name for resulting charm config file.""" | ||||||||||||||||||||||
return SYSCTL_DIRECTORY / f"{CHARM_FILENAME_PREFIX}{self.name}" | ||||||||||||||||||||||
|
||||||||||||||||||||||
def configure(self, config: Dict[str, str]) -> None: | ||||||||||||||||||||||
"""Configure sysctl options with a desired set of params. | ||||||||||||||||||||||
Args: | ||||||||||||||||||||||
config: dictionary with keys to configure: | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
{"vm.swappiness": "10", ...} | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
self._parse_config(config) | ||||||||||||||||||||||
|
||||||||||||||||||||||
# NOTE: case where own charm calls configure() more than once. | ||||||||||||||||||||||
if self.charm_filepath.exists(): | ||||||||||||||||||||||
self._merge(add_own_charm=False) | ||||||||||||||||||||||
|
||||||||||||||||||||||
conflict = self._validate() | ||||||||||||||||||||||
if conflict: | ||||||||||||||||||||||
raise ValidationError(f"Validation error for keys: {conflict}") | ||||||||||||||||||||||
|
||||||||||||||||||||||
snapshot = self._create_snapshot() | ||||||||||||||||||||||
logger.debug("Created snapshot for keys: %s", snapshot) | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
self._apply() | ||||||||||||||||||||||
except ApplyError: | ||||||||||||||||||||||
self._restore_snapshot(snapshot) | ||||||||||||||||||||||
raise | ||||||||||||||||||||||
|
||||||||||||||||||||||
self._create_charm_file() | ||||||||||||||||||||||
self._merge() | ||||||||||||||||||||||
|
||||||||||||||||||||||
def remove(self) -> None: | ||||||||||||||||||||||
"""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() | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _validate(self) -> List[str]: | ||||||||||||||||||||||
"""Validate the desired config params against merged ones.""" | ||||||||||||||||||||||
common_keys = set(self._data.keys()) & set(self._desired_config.keys()) | ||||||||||||||||||||||
conflict_keys = [] | ||||||||||||||||||||||
for key in common_keys: | ||||||||||||||||||||||
if self._data[key] != self._desired_config[key]: | ||||||||||||||||||||||
benhoyt marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||
logger.warning( | ||||||||||||||||||||||
"Values for key '%s' are different: %s != %s", | ||||||||||||||||||||||
key, | ||||||||||||||||||||||
self._data[key], | ||||||||||||||||||||||
self._desired_config[key], | ||||||||||||||||||||||
) | ||||||||||||||||||||||
conflict_keys.append(key) | ||||||||||||||||||||||
|
||||||||||||||||||||||
return conflict_keys | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _create_charm_file(self) -> None: | ||||||||||||||||||||||
"""Write the charm file.""" | ||||||||||||||||||||||
with open(self.charm_filepath, "w") as f: | ||||||||||||||||||||||
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. | ||||||||||||||||||||||
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] | ||||||||||||||||||||||
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: | ||||||||||||||||||||||
f.writelines(data) | ||||||||||||||||||||||
Comment on lines
+225
to
+229
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not-blocking: follow up from @benhoyt comment about creating charm file.
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
# Reload data with newly created file. | ||||||||||||||||||||||
self._data = self._load_data() | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _apply(self) -> None: | ||||||||||||||||||||||
"""Apply values to machine.""" | ||||||||||||||||||||||
cmd = [f"{key}={value}" for key, value in self._desired_config.items()] | ||||||||||||||||||||||
result = self._sysctl(cmd) | ||||||||||||||||||||||
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 ApplyError(msg) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _create_snapshot(self) -> Dict[str, str]: | ||||||||||||||||||||||
"""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.""" | ||||||||||||||||||||||
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.""" | ||||||||||||||||||||||
cmd = ["sysctl"] + cmd | ||||||||||||||||||||||
logger.debug("Executing sysctl command: %s", cmd) | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
return check_output(cmd, stderr=STDOUT, universal_newlines=True).splitlines() | ||||||||||||||||||||||
except CalledProcessError as e: | ||||||||||||||||||||||
msg = f"Error executing '{cmd}': {e.stdout}" | ||||||||||||||||||||||
logger.error(msg) | ||||||||||||||||||||||
raise CommandError(msg) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _parse_config(self, config: Dict[str, str]) -> None: | ||||||||||||||||||||||
"""Parse a config passed to the lib.""" | ||||||||||||||||||||||
self._desired_config = {k: str(v) for k, v in config.items()} | ||||||||||||||||||||||
|
||||||||||||||||||||||
def _load_data(self) -> Dict[str, str]: | ||||||||||||||||||||||
"""Get merged config.""" | ||||||||||||||||||||||
config = {} | ||||||||||||||||||||||
if not SYSCTL_FILENAME.exists(): | ||||||||||||||||||||||
return config | ||||||||||||||||||||||
|
||||||||||||||||||||||
with open(SYSCTL_FILENAME, "r") as f: | ||||||||||||||||||||||
for line in f: | ||||||||||||||||||||||
if line.startswith(("#", ";")) or not line.strip() or "=" not in line: | ||||||||||||||||||||||
continue | ||||||||||||||||||||||
|
||||||||||||||||||||||
key, _, value = line.partition("=") | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not-blocking:
Suggested change
|
||||||||||||||||||||||
config[key.strip()] = value.strip() | ||||||||||||||||||||||
|
||||||||||||||||||||||
# TODO: add your code here! Happy coding! | ||||||||||||||||||||||
return config |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
#!/usr/bin/env python3 | ||
# Copyright 2023 Canonical Ltd. | ||
# See LICENSE file for licensing details. | ||
|
||
|
||
from pathlib import Path | ||
from subprocess import check_output | ||
|
||
from charms.operator_libs_linux.v0 import sysctl | ||
|
||
EXPECTED_MERGED_RESULT = """# This config file was produced by sysctl lib v0.2 | ||
# | ||
# This file represents the output of the sysctl lib, which can combine multiple | ||
# configurations into a single file like. | ||
# test1 | ||
net.ipv4.tcp_max_syn_backlog=4096 | ||
# test2 | ||
net.ipv4.tcp_window_scaling=2 | ||
""" | ||
|
||
|
||
def test_configure(): | ||
cfg = sysctl.Config("test1") | ||
cfg.configure({"net.ipv4.tcp_max_syn_backlog": "4096"}) | ||
|
||
result = check_output(["sysctl", "net.ipv4.tcp_max_syn_backlog"]) | ||
|
||
test_file = Path("/etc/sysctl.d/90-juju-test1") | ||
merged_file = Path("/etc/sysctl.d/95-juju-sysctl.conf") | ||
assert "net.ipv4.tcp_max_syn_backlog = 4096" in result.decode() | ||
assert test_file.exists() | ||
assert merged_file.exists() | ||
|
||
|
||
def test_multiple_configure(): | ||
# file from previous test still exists, so we only need to create a new one. | ||
cfg_2 = sysctl.Config("test2") | ||
cfg_2.configure({"net.ipv4.tcp_window_scaling": "2"}) | ||
|
||
test_file_2 = Path("/etc/sysctl.d/90-juju-test2") | ||
merged_file = Path("/etc/sysctl.d/95-juju-sysctl.conf") | ||
result = check_output( | ||
["sysctl", "net.ipv4.tcp_max_syn_backlog", "net.ipv4.tcp_window_scaling"] | ||
) | ||
assert ( | ||
"net.ipv4.tcp_max_syn_backlog = 4096\nnet.ipv4.tcp_window_scaling = 2\n" in result.decode() | ||
) | ||
assert test_file_2.exists() | ||
|
||
with open(merged_file, "r") as f: | ||
assert f.read() == EXPECTED_MERGED_RESULT | ||
|
||
|
||
def test_remove(): | ||
cfg = sysctl.Config("test") | ||
cfg.remove() | ||
|
||
test_file = Path("/etc/sysctl.d/90-juju-test") | ||
assert not test_file.exists() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we providing paths to original files here? I could not found it in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this as part of file creation, see this commit: 0cd9816