diff --git a/src/sonic-py-common/sonic_py_common/pmon_daemon_config.py b/src/sonic-py-common/sonic_py_common/pmon_daemon_config.py new file mode 100644 index 00000000000..e26a20769a1 --- /dev/null +++ b/src/sonic-py-common/sonic_py_common/pmon_daemon_config.py @@ -0,0 +1,327 @@ +""" +Shared resolver for pmon daemon runtime tunables. + +Historically every pmon tunable was plumbed end-to-end as a command-line flag: +the platform set it in pmon_daemon_control.json, sonic-cfggen loaded that file +when rendering docker-pmon.supervisord.conf.j2, the template flattened it into +"--flag value", argparse re-parsed it, and the daemon constructor grew another +parameter. Adding one knob meant editing four places, once per daemon. + +This module removes that round trip. A daemon declares a dataclass subclass of +PmonDaemonConfig naming the section it owns and the fields it accepts; the base +locates pmon_daemon_control.json, extracts that section, and layers it over the +built-in defaults. + +Precedence, highest wins: + 1. Per-platform / per-hwsku file - the daemon's section of pmon_daemon_control.json + 2. Built-in defaults - the subclass's dataclass field defaults + +The per-platform file is read from the same device directories (and with the +same hwsku-over-platform precedence) that docker_init.j2 uses and that xcvrd's +media_settings.json / optics_si_settings.json parsers already read. + +Adding a tunable is one field plus one FIELD_SPECS entry declaring its type +coercion and valid range. Platform owners set it in the section they already +maintain; no template, argparse, or constructor change. + +Example: + + @dataclass + class XcvrdConfig(PmonDaemonConfig): + SECTION_NAME = 'xcvrd' + FIELD_SPECS = { + 'dom_update_interval': FieldSpec(caster=int, minimum=0, maximum=86400), + } + + dom_update_interval: Optional[int] = None + + config = XcvrdConfig.resolve() + +Nothing here raises on bad input: an unreadable file, a malformed section, an +uncoercible value, or an out-of-range value degrades to the built-in default +with a syslog warning, so a bad tunable can never keep a pmon daemon down. +""" + +import json +import os + +from dataclasses import dataclass, fields +from typing import Callable, ClassVar, Dict, Optional, Tuple, get_origin + +from . import device_info +from .syslogger import SysLogger + +# Per-platform / per-hwsku file. Each daemon's tunables live under its own key, +# alongside the top-level skip_ / delay_ capability keys. +PMON_DAEMON_CONTROL_FILE = "pmon_daemon_control.json" + +# One SysLogger per identifier: a daemon's config schema module and the base +# class both log under the same identifier and should share one instance. +_LOGGERS = {} + + +def get_config_logger(identifier): + """Return the SysLogger for identifier, creating it on first use.""" + logger = _LOGGERS.get(identifier) + if logger is None: + logger = SysLogger(identifier, enable_runtime_config=True) + _LOGGERS[identifier] = logger + return logger + + +def to_bool(value): + """Coerce a platform-file value to a bool. + + Boolean tunables cannot use bool() as their caster: bool("false") is True, + so a platform writing the string "false" would silently enable the feature. + Accepts real booleans, 0/1, and the usual textual spellings; anything else + raises ValueError so the caller keeps the built-in default. + """ + if isinstance(value, bool): + return value + if isinstance(value, int): + if value in (0, 1): + return bool(value) + raise ValueError("cannot interpret {!r} as a boolean".format(value)) + if isinstance(value, str): + lowered = value.strip().lower() + if lowered in ('true', 'yes', 'on', '1'): + return True + if lowered in ('false', 'no', 'off', '0'): + return False + raise ValueError("cannot interpret {!r} as a boolean".format(value)) + + +def _is_class_var(annotation): + """True if an annotation declares class-level config rather than a tunable.""" + if annotation is ClassVar or get_origin(annotation) is ClassVar: + return True + # PEP 563 string annotations never reach get_origin. + if isinstance(annotation, str): + return annotation.split('[')[0].split('.')[-1] == 'ClassVar' + return False + + +@dataclass(frozen=True) +class FieldSpec: + """Type coercion and validation policy for one tunable. + + caster - applied first, e.g. int / float / to_bool. A TypeError or + ValueError from it rejects the value. + minimum - inclusive lower bound, or None for unbounded below. + maximum - inclusive upper bound, or None for unbounded above. + choices - permitted values, or None if not an enumeration. + + Coercion alone is not validation: a negative interval coerces to a perfectly + good int but is not a valid cadence, and downstream consumers differ in + whether they notice. Declaring the range here gives every tunable one + enforced, testable contract. A field that is genuinely unbounded declares + FieldSpec(caster=...) with no bounds, so "unbounded" stays an explicit + choice rather than an omission. + """ + + caster: Optional[Callable] = None + minimum: Optional[float] = None + maximum: Optional[float] = None + choices: Optional[Tuple] = None + + def coerce(self, value): + """Apply caster. Raises TypeError/ValueError if the value is unusable.""" + if self.caster is None: + return value + return self.caster(value) + + def describe_range(self): + """Human-readable bounds, for log messages.""" + if self.choices is not None: + return "one of {}".format(sorted(self.choices, key=repr)) + low = "-inf" if self.minimum is None else self.minimum + high = "+inf" if self.maximum is None else self.maximum + return "[{}, {}]".format(low, high) + + def rejection_reason(self, value): + """Return None if value is acceptable, else why it is not.""" + if self.choices is not None and value not in self.choices: + return "expected {}".format(self.describe_range()) + if self.minimum is None and self.maximum is None: + return None + try: + below = self.minimum is not None and value < self.minimum + above = self.maximum is not None and value > self.maximum + except TypeError: + # Not comparable to the bounds at all (e.g. a dict where an int was + # declared) - treat as out of range rather than raising. + return "expected a value in {}".format(self.describe_range()) + if below or above: + return "expected a value in {}".format(self.describe_range()) + return None + + +@dataclass +class PmonDaemonConfig: + """Base class for a pmon daemon's resolved configuration. + + Subclasses are dataclasses that set SECTION_NAME, populate FIELD_SPECS, and + declare one field per tunable whose default is the built-in default. The + base carries no fields of its own, so subclass field ordering is unaffected. + """ + + # Key in pmon_daemon_control.json that holds this daemon's tunables. + SECTION_NAME: ClassVar[str] = '' + # field name -> FieldSpec. A field with no entry is stored as-is. + FIELD_SPECS: ClassVar[Dict[str, FieldSpec]] = {} + # Defaults to "
_config" when unset. + SYSLOG_IDENTIFIER: ClassVar[Optional[str]] = None + + def __init_subclass__(cls, **kwargs): + """Reject a schema whose fields and FIELD_SPECS disagree. + + FIELD_SPECS is keyed by field name, so a key matching no field is + silently ignored - and that field then gets neither coercion nor a range + check, which is precisely the failure the specs exist to prevent. A field + with no spec is likewise stored exactly as the platform wrote it. + + Both are static errors in the schema, not bad input, so they raise here + and surface the first time the module is imported. Bad values in + pmon_daemon_control.json stay non-fatal; see _merge. + + Runs before @dataclass processes the subclass, so fields are read from + annotations rather than dataclasses.fields(). + """ + super().__init_subclass__(**kwargs) + declared = cls._declared_tunables() + specced = set(cls.FIELD_SPECS) + + unknown = sorted(specced - declared) + if unknown: + raise TypeError( + "{}.FIELD_SPECS has no matching field for: {}. A spec keyed by a " + "name no field declares is never applied, leaving that tunable " + "with no type coercion and no range check.".format( + cls.__name__, ", ".join(unknown))) + + unspecced = sorted(declared - specced) + if unspecced: + raise TypeError( + "{}.FIELD_SPECS is missing an entry for: {}. Every tunable " + "declares its coercion and bounds; a field that is genuinely " + "unbounded declares FieldSpec() so that stays an explicit " + "choice.".format(cls.__name__, ", ".join(unspecced))) + + @classmethod + def _declared_tunables(cls): + """Field names across the MRO, excluding ClassVar class-level config.""" + names = set() + for klass in reversed(cls.__mro__): + # vars() rather than cls.__annotations__: the latter falls through to + # a base class's annotations when a class declares none of its own. + for name, annotation in vars(klass).get('__annotations__', {}).items(): + if _is_class_var(annotation): + names.discard(name) + else: + names.add(name) + return names + + @classmethod + def _logger(cls): + identifier = cls.SYSLOG_IDENTIFIER or '{}_config'.format( + cls.SECTION_NAME or 'pmon_daemon') + return get_config_logger(identifier) + + @classmethod + def _log_prefix(cls): + return '{} config'.format(cls.SECTION_NAME or 'pmon daemon') + + @classmethod + def resolve(cls, platform_section=None): + """Build a config by layering the platform file over the built-in defaults. + + platform_section is exposed for tests so the merge logic can be exercised + without touching the filesystem; in production it is read from disk. + """ + cfg = cls() + if platform_section is None: + platform_section = cls._read_platform_section() + cfg._merge(platform_section) + cfg._post_merge() + return cfg + + def _merge(self, overrides): + """Apply the platform section: unknown keys and unusable values are dropped.""" + logger = self._logger() + prefix = self._log_prefix() + valid = {f.name for f in fields(self)} + for key, value in overrides.items(): + if key not in valid: + logger.log_notice( + "{}: ignoring unknown key '{}' in {}".format( + prefix, key, PMON_DAEMON_CONTROL_FILE)) + continue + if value is None: + # An absent override never clobbers the default. + continue + spec = self.FIELD_SPECS.get(key) + if spec is not None: + try: + value = spec.coerce(value) + except (TypeError, ValueError): + logger.log_warning( + "{}: invalid value {!r} for '{}' in {}; keeping default".format( + prefix, value, key, PMON_DAEMON_CONTROL_FILE)) + continue + reason = spec.rejection_reason(value) + if reason is not None: + logger.log_warning( + "{}: out-of-range value {!r} for '{}' in {}; {}; keeping " + "default".format(prefix, value, key, + PMON_DAEMON_CONTROL_FILE, reason)) + continue + setattr(self, key, value) + + def _post_merge(self): + """Hook for constraints that span more than one field. + + FieldSpec validates each field in isolation; a subclass overrides this to + enforce (or clamp) relationships between fields once every override has + been applied. Default: nothing to do. + """ + return + + @classmethod + def _read_platform_section(cls): + """Return this daemon's dict from pmon_daemon_control.json, or {} if absent. + + Mirrors docker_init.j2: the hwsku file takes precedence over the platform + file, and only the first existing file is consulted (no cross-file merge). + Any failure degrades to {} so the daemon starts on its built-in defaults. + """ + logger = cls._logger() + prefix = cls._log_prefix() + try: + platform_path, hwsku_path = device_info.get_paths_to_platform_and_hwsku_dirs() + except Exception as exc: # device_info can raise if platform is undetermined + logger.log_warning( + "{}: unable to determine platform/hwsku dirs: {}".format(prefix, exc)) + return {} + + for directory in (hwsku_path, platform_path): + if not directory: + continue + path = os.path.join(directory, PMON_DAEMON_CONTROL_FILE) + if not os.path.isfile(path): + continue + try: + with open(path) as control_file: + data = json.load(control_file) + except (OSError, ValueError) as exc: + logger.log_warning( + "{}: failed to read {}: {}".format(prefix, path, exc)) + return {} + section = data.get(cls.SECTION_NAME, {}) + if not isinstance(section, dict): + logger.log_warning( + "{}: '{}' section in {} is not an object; ignoring".format( + prefix, cls.SECTION_NAME, path)) + return {} + return section + return {} diff --git a/src/sonic-py-common/tests/pmon_daemon_config_test.py b/src/sonic-py-common/tests/pmon_daemon_config_test.py new file mode 100644 index 00000000000..92d3a55600e --- /dev/null +++ b/src/sonic-py-common/tests/pmon_daemon_config_test.py @@ -0,0 +1,452 @@ +""" +Unit tests for PmonDaemonConfig: the shared resolver for pmon daemon tunables. + +Precedence under test (highest wins): + 1. the daemon's section of pmon_daemon_control.json (hwsku file over platform file) + 2. built-in dataclass defaults + +The base is exercised through small test-only subclasses so it is covered +independently of any real daemon's schema. +""" +import json +import os +import sys + +from dataclasses import dataclass +from typing import Optional + +if sys.version_info.major == 3: + from unittest import mock +else: + import mock + +import pytest + +from sonic_py_common import pmon_daemon_config +from sonic_py_common.pmon_daemon_config import ( + FieldSpec, PmonDaemonConfig, PMON_DAEMON_CONTROL_FILE, to_bool) + +PATHS_FN = "sonic_py_common.pmon_daemon_config.device_info.get_paths_to_platform_and_hwsku_dirs" + +# The quiet_logger fixture below patches get_config_logger for every test; keep a +# handle on the real one so its own caching can still be exercised. +REAL_GET_CONFIG_LOGGER = pmon_daemon_config.get_config_logger + + +@dataclass +class SampleConfig(PmonDaemonConfig): + """A stand-in daemon schema covering every FieldSpec feature.""" + + SECTION_NAME = 'sampled' + FIELD_SPECS = { + 'interval': FieldSpec(caster=int, minimum=0, maximum=86400), + 'ratio': FieldSpec(caster=float, minimum=0.0), # unbounded above + 'enabled': FieldSpec(caster=to_bool), # unbounded + 'mode': FieldSpec(choices=('fast', 'slow')), # no caster + 'freeform': FieldSpec(), # neither + } + + interval: Optional[int] = None + ratio: Optional[float] = None + enabled: bool = False + mode: Optional[str] = None + freeform: Optional[str] = None + + +@dataclass +class OtherConfig(PmonDaemonConfig): + """A second schema, to prove sections do not bleed into each other.""" + + SECTION_NAME = 'otherd' + FIELD_SPECS = {'interval': FieldSpec(caster=int, minimum=0)} + + interval: Optional[int] = None + + +@dataclass +class ClampedConfig(PmonDaemonConfig): + """Exercises the _post_merge hook for a cross-field constraint.""" + + SECTION_NAME = 'clampedd' + FIELD_SPECS = { + 'window': FieldSpec(caster=int, minimum=1), + 'heartbeat': FieldSpec(caster=int, minimum=1), + } + + window: int = 300 + heartbeat: int = 30 + + def _post_merge(self): + if self.heartbeat >= self.window: + self.heartbeat = max(1, self.window // 2) + + +@pytest.fixture(autouse=True) +def quiet_logger(): + """Keep tests off rsyslogd; the unit test environment has none.""" + with mock.patch.object(pmon_daemon_config, 'get_config_logger', + return_value=mock.MagicMock()) as logger_fn: + yield logger_fn + + +def write_control_file(directory, payload): + """Write a pmon_daemon_control.json with the given dict into directory.""" + os.makedirs(directory, exist_ok=True) + path = os.path.join(directory, PMON_DAEMON_CONTROL_FILE) + with open(path, "w") as f: + json.dump(payload, f) + return path + + +class TestToBool: + @pytest.mark.parametrize("value", [True, "true", "True", " TRUE ", "yes", "on", "1", 1]) + def test_truthy_spellings(self, value): + assert to_bool(value) is True + + @pytest.mark.parametrize("value", [False, "false", "False", " FALSE ", "no", "off", "0", 0]) + def test_falsy_spellings(self, value): + # The case bool() gets wrong: bool("false") is True. + assert to_bool(value) is False + + @pytest.mark.parametrize("value", ["maybe", "", 2, -1, 1.5, None, [], {}]) + def test_uninterpretable_raises(self, value): + with pytest.raises(ValueError): + to_bool(value) + + +class TestFieldSpec: + def test_no_caster_passes_value_through(self): + assert FieldSpec().coerce("as-is") == "as-is" + + def test_caster_failure_propagates(self): + with pytest.raises(ValueError): + FieldSpec(caster=int).coerce("not-a-number") + + def test_unbounded_spec_accepts_anything(self): + assert FieldSpec(caster=int).rejection_reason(-999999) is None + + def test_bounds_are_inclusive(self): + spec = FieldSpec(caster=int, minimum=0, maximum=10) + assert spec.rejection_reason(0) is None + assert spec.rejection_reason(10) is None + assert spec.rejection_reason(-1) is not None + assert spec.rejection_reason(11) is not None + + def test_incomparable_value_is_rejected_not_raised(self): + # A dict where an int was declared must not blow up the comparison. + assert FieldSpec(minimum=0).rejection_reason({}) is not None + + def test_choices_enforced(self): + spec = FieldSpec(choices=('fast', 'slow')) + assert spec.rejection_reason('fast') is None + assert spec.rejection_reason('turbo') is not None + + def test_describe_range_reports_open_bounds(self): + assert FieldSpec(minimum=0).describe_range() == "[0, +inf]" + assert FieldSpec(maximum=9).describe_range() == "[-inf, 9]" + + +class TestDefaults: + def test_defaults_when_no_overrides(self): + cfg = SampleConfig.resolve(platform_section={}) + assert cfg.interval is None + assert cfg.ratio is None + assert cfg.enabled is False + assert cfg.mode is None + + def test_bare_construction_matches_defaults(self): + assert SampleConfig() == SampleConfig.resolve(platform_section={}) + + +class TestMerge: + def test_section_overrides_defaults(self): + cfg = SampleConfig.resolve(platform_section={"interval": 5, "mode": "fast"}) + assert cfg.interval == 5 + assert cfg.mode == "fast" + + def test_partial_section_leaves_other_fields_at_default(self): + cfg = SampleConfig.resolve(platform_section={"interval": 5}) + assert cfg.interval == 5 + assert cfg.ratio is None + + def test_none_value_does_not_override(self): + cfg = SampleConfig.resolve(platform_section={"enabled": None}) + assert cfg.enabled is False + + def test_string_value_is_coerced(self): + cfg = SampleConfig.resolve(platform_section={"interval": "30"}) + assert cfg.interval == 30 + assert isinstance(cfg.interval, int) + + def test_uncoercible_value_keeps_default(self): + cfg = SampleConfig.resolve(platform_section={"interval": "not-a-number"}) + assert cfg.interval is None + + def test_unknown_key_is_ignored(self): + cfg = SampleConfig.resolve(platform_section={ + "interval": 5, "some_future_unknown_key": 99}) + assert cfg.interval == 5 + assert not hasattr(cfg, "some_future_unknown_key") + + def test_field_without_spec_is_stored_as_is(self): + cfg = SampleConfig.resolve(platform_section={"freeform": {"anything": 1}}) + assert cfg.freeform == {"anything": 1} + + def test_bool_string_false_disables(self): + # Without to_bool this would store the truthy string "false". + cfg = SampleConfig.resolve(platform_section={"enabled": "false"}) + assert cfg.enabled is False + + def test_bool_string_true_enables(self): + cfg = SampleConfig.resolve(platform_section={"enabled": "true"}) + assert cfg.enabled is True + + +class TestRangeValidation: + def test_below_minimum_keeps_default(self): + cfg = SampleConfig.resolve(platform_section={"interval": -1}) + assert cfg.interval is None + + def test_above_maximum_keeps_default(self): + cfg = SampleConfig.resolve(platform_section={"interval": 86401}) + assert cfg.interval is None + + def test_boundaries_are_accepted(self): + assert SampleConfig.resolve(platform_section={"interval": 0}).interval == 0 + assert SampleConfig.resolve(platform_section={"interval": 86400}).interval == 86400 + + def test_validation_runs_after_coercion(self): + # "-5" coerces to int fine, then fails the range check. + cfg = SampleConfig.resolve(platform_section={"interval": "-5"}) + assert cfg.interval is None + + def test_unbounded_above_accepts_large_value(self): + cfg = SampleConfig.resolve(platform_section={"ratio": "1e9"}) + assert cfg.ratio == 1e9 + + def test_choices_violation_keeps_default(self): + cfg = SampleConfig.resolve(platform_section={"mode": "turbo"}) + assert cfg.mode is None + + def test_rejection_is_logged_as_warning(self): + logger = mock.MagicMock() + with mock.patch.object(pmon_daemon_config, 'get_config_logger', return_value=logger): + SampleConfig.resolve(platform_section={"interval": -1}) + assert logger.log_warning.called + message = logger.log_warning.call_args[0][0] + assert "interval" in message and "keeping default" in message + + def test_one_bad_field_does_not_drop_the_others(self): + cfg = SampleConfig.resolve(platform_section={"interval": -1, "mode": "slow"}) + assert cfg.interval is None + assert cfg.mode == "slow" + + +class TestPostMergeHook: + def test_hook_is_a_noop_by_default(self): + assert SampleConfig.resolve(platform_section={"interval": 5}).interval == 5 + + def test_cross_field_constraint_applied_after_merge(self): + cfg = ClampedConfig.resolve(platform_section={"window": 20, "heartbeat": 50}) + assert cfg.heartbeat == 10 + + def test_constraint_sees_defaults_too(self): + # Lowering only window must still pull the default heartbeat under it. + cfg = ClampedConfig.resolve(platform_section={"window": 10}) + assert cfg.heartbeat == 5 + + def test_in_range_values_untouched(self): + cfg = ClampedConfig.resolve(platform_section={"window": 100, "heartbeat": 40}) + assert cfg.heartbeat == 40 + + +class TestReadPlatformSection: + def test_missing_files_yield_empty(self, tmp_path): + platform_dir = str(tmp_path / "platform") + hwsku_dir = str(tmp_path / "hwsku") + with mock.patch(PATHS_FN, return_value=(platform_dir, hwsku_dir)): + assert SampleConfig._read_platform_section() == {} + + def test_reads_platform_file_when_no_hwsku_file(self, tmp_path): + platform_dir = str(tmp_path / "platform") + hwsku_dir = str(tmp_path / "hwsku") + write_control_file(platform_dir, {"sampled": {"interval": 30}}) + with mock.patch(PATHS_FN, return_value=(platform_dir, hwsku_dir)): + assert SampleConfig._read_platform_section() == {"interval": 30} + + def test_hwsku_file_takes_precedence_over_platform_file(self, tmp_path): + platform_dir = str(tmp_path / "platform") + hwsku_dir = str(tmp_path / "hwsku") + write_control_file(platform_dir, {"sampled": {"interval": 30}}) + write_control_file(hwsku_dir, {"sampled": {"interval": 99}}) + with mock.patch(PATHS_FN, return_value=(platform_dir, hwsku_dir)): + # Mirrors docker_init: the hwsku file wins; no cross-file merge. + assert SampleConfig._read_platform_section() == {"interval": 99} + + def test_hwsku_file_without_section_does_not_fall_back(self, tmp_path): + platform_dir = str(tmp_path / "platform") + hwsku_dir = str(tmp_path / "hwsku") + write_control_file(platform_dir, {"sampled": {"interval": 30}}) + write_control_file(hwsku_dir, {"skip_sampled": False}) + with mock.patch(PATHS_FN, return_value=(platform_dir, hwsku_dir)): + assert SampleConfig._read_platform_section() == {} + + def test_each_subclass_reads_its_own_section(self, tmp_path): + platform_dir = str(tmp_path / "platform") + write_control_file(platform_dir, { + "sampled": {"interval": 30}, + "otherd": {"interval": 99}, + }) + with mock.patch(PATHS_FN, return_value=(platform_dir, "")): + assert SampleConfig.resolve().interval == 30 + assert OtherConfig.resolve().interval == 99 + + def test_no_section_yields_empty(self, tmp_path): + platform_dir = str(tmp_path / "platform") + write_control_file(platform_dir, {"skip_ledd": True}) + with mock.patch(PATHS_FN, return_value=(platform_dir, "")): + assert SampleConfig._read_platform_section() == {} + + def test_malformed_json_yields_empty(self, tmp_path): + platform_dir = str(tmp_path / "platform") + os.makedirs(platform_dir) + with open(os.path.join(platform_dir, PMON_DAEMON_CONTROL_FILE), "w") as f: + f.write("{ this is not valid json") + with mock.patch(PATHS_FN, return_value=(platform_dir, "")): + assert SampleConfig._read_platform_section() == {} + + def test_non_dict_section_yields_empty(self, tmp_path): + platform_dir = str(tmp_path / "platform") + write_control_file(platform_dir, {"sampled": "oops-not-an-object"}) + with mock.patch(PATHS_FN, return_value=(platform_dir, "")): + assert SampleConfig._read_platform_section() == {} + + def test_device_info_failure_yields_empty(self): + with mock.patch(PATHS_FN, side_effect=RuntimeError("platform undetermined")): + assert SampleConfig._read_platform_section() == {} + + def test_empty_dir_path_is_skipped(self, tmp_path): + # get_paths_to_platform_and_hwsku_dirs may return an empty hwsku path; + # that entry is skipped rather than joined into a bogus path. + platform_dir = str(tmp_path / "platform") + write_control_file(platform_dir, {"sampled": {"interval": 30}}) + with mock.patch(PATHS_FN, return_value=(platform_dir, "")): + assert SampleConfig._read_platform_section() == {"interval": 30} + + +class TestResolveEndToEnd: + def test_resolve_reads_from_disk(self, tmp_path): + platform_dir = str(tmp_path / "platform") + write_control_file(platform_dir, {"sampled": {"interval": 5, "enabled": "true"}}) + with mock.patch(PATHS_FN, return_value=(platform_dir, "")): + cfg = SampleConfig.resolve() + assert cfg.interval == 5 + assert cfg.enabled is True + + def test_resolve_defaults_when_nothing_on_disk(self, tmp_path): + with mock.patch(PATHS_FN, return_value=(str(tmp_path / "platform"), "")): + cfg = SampleConfig.resolve() + assert cfg.interval is None + assert cfg.enabled is False + + +class TestSchemaGuard: + """A spec keyed by the wrong name is inert, so the schema must not compile. + + These are developer errors in static code, caught at import; bad values in + the platform file stay non-fatal. + """ + + def test_spec_for_nonexistent_field_is_rejected(self): + with pytest.raises(TypeError) as excinfo: + @dataclass + class Typo(PmonDaemonConfig): + SECTION_NAME = 'typod' + FIELD_SPECS = {'intervl': FieldSpec(caster=int, minimum=0)} + + interval: Optional[int] = None + + assert 'intervl' in str(excinfo.value) + + def test_field_without_a_spec_is_rejected(self): + with pytest.raises(TypeError) as excinfo: + @dataclass + class Unspecced(PmonDaemonConfig): + SECTION_NAME = 'unspeccedd' + FIELD_SPECS = {'covered': FieldSpec(caster=int)} + + covered: Optional[int] = None + forgotten: Optional[int] = None + + assert 'forgotten' in str(excinfo.value) + + def test_explicitly_unbounded_field_is_accepted(self): + @dataclass + class Unbounded(PmonDaemonConfig): + SECTION_NAME = 'unboundedd' + FIELD_SPECS = {'anything': FieldSpec()} + + anything: Optional[str] = None + + assert Unbounded.resolve(platform_section={"anything": "x"}).anything == "x" + + def test_classvars_are_not_treated_as_tunables(self): + # SECTION_NAME/FIELD_SPECS/SYSLOG_IDENTIFIER are ClassVars on the base; + # a schema declaring no fields at all must still be legal. + @dataclass + class NoTunables(PmonDaemonConfig): + SECTION_NAME = 'notunablesd' + + assert NoTunables._declared_tunables() == set() + + def test_inherited_fields_need_inherited_specs(self): + # Extending a schema: the new field needs its own spec, the inherited + # ones are already covered. + with pytest.raises(TypeError) as excinfo: + @dataclass + class Extended(SampleConfig): + SECTION_NAME = 'extendedd' + + extra: Optional[int] = None + + assert 'extra' in str(excinfo.value) + + def test_extending_a_schema_with_a_spec_is_accepted(self): + @dataclass + class Extended(SampleConfig): + SECTION_NAME = 'extended2d' + FIELD_SPECS = dict(SampleConfig.FIELD_SPECS, + extra=FieldSpec(caster=int, minimum=0)) + + extra: Optional[int] = None + + cfg = Extended.resolve(platform_section={"extra": "7", "interval": 5}) + assert cfg.extra == 7 + assert cfg.interval == 5 + + +class TestLoggerIdentifier: + def test_identifier_defaults_to_section_name(self, quiet_logger): + SampleConfig._logger() + quiet_logger.assert_called_with('sampled_config') + + def test_explicit_identifier_wins(self, quiet_logger): + @dataclass + class Named(PmonDaemonConfig): + SECTION_NAME = 'named' + SYSLOG_IDENTIFIER = 'custom_identifier' + + Named._logger() + quiet_logger.assert_called_with('custom_identifier') + + def test_one_logger_instance_per_identifier(self): + # Real factory: repeated lookups must not build a new SysLogger each time. + with mock.patch.object(pmon_daemon_config, 'SysLogger') as syslogger_cls: + pmon_daemon_config._LOGGERS.clear() + try: + first = REAL_GET_CONFIG_LOGGER('dedupe_test') + second = REAL_GET_CONFIG_LOGGER('dedupe_test') + finally: + pmon_daemon_config._LOGGERS.clear() + assert first is second + assert syslogger_cls.call_count == 1