From d3b61b8d3e94dff1d3a335ad44d5c8e3d54771f9 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Tue, 18 Jun 2024 13:23:14 -0600 Subject: [PATCH 1/3] feat: Add deprecation boundary to logger (#5411) This will all stable distros to define whether a key is deprecated based on the version of cloud-init which deprecated the key. --- cloudinit/cmd/main.py | 21 ++++++++------------- cloudinit/features.py | 29 +++++++++++++++++++++++++++++ cloudinit/util.py | 35 +++++++++++++++++++++++++---------- 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index c16d2703dab..4a1c8b2e28c 100644 --- a/cloudinit/cmd/main.py +++ b/cloudinit/cmd/main.py @@ -236,17 +236,12 @@ def attempt_cmdline_url(path, network=True, cmdline=None) -> Tuple[int, str]: is_cloud_cfg = False if is_cloud_cfg: if cmdline_name == "url": - return ( - log.DEPRECATED, - str( - util.deprecate( - deprecated="The kernel command line key `url`", - deprecated_version="22.3", - extra_message=" Please use `cloud-config-url` " - "kernel command line parameter instead", - return_log=True, - ), - ), + return util.deprecate( + deprecated="The kernel command line key `url`", + deprecated_version="22.3", + extra_message=" Please use `cloud-config-url` " + "kernel command line parameter instead", + skip_log=True, ) else: if cmdline_name == "cloud-config-url": @@ -972,8 +967,8 @@ def main(sysv_args=None): deprecated="`init`", deprecated_version="24.1", extra_message="Use `cloud-init init` instead.", - return_log=True, - ) + skip_log=True, + ).message parser_mod.add_argument( "--mode", "-m", diff --git a/cloudinit/features.py b/cloudinit/features.py index d661b940b29..c3fdae18658 100644 --- a/cloudinit/features.py +++ b/cloudinit/features.py @@ -87,6 +87,35 @@ to write /etc/apt/sources.list directly. """ +DEPRECATION_INFO_BOUNDARY = "devel" +""" +DEPRECATION_INFO_BOUNDARY is used by distros to configure at which upstream +version to start logging deprecations at a level higher than INFO. + +The default value "devel" tells cloud-init to log all deprecations higher +than INFO. This value may be overriden by downstreams in order to maintain +stable behavior across releases. + +Jsonschema key deprecations and inline logger deprecations include a +deprecated_version key. When the variable below is set to a version, +cloud-init will use that version as a demarcation point. Deprecations which +are added after this version will be logged as at an INFO level. Deprecations +which predate this version will be logged at the higher DEPRECATED level. +Downstreams that want stable log behavior may set the variable below to the +first version released in their stable distro. By doing this, they can expect +that newly added deprecations will be logged at INFO level. The implication of +the different log levels is that logs at DEPRECATED level result in a return +code of 2 from `cloud-init status`. + +format: + + :: = | + ::= "devel" + ::= "." ["." ] + +where , , and are positive integers +""" + def get_features() -> Dict[str, bool]: """Return a dict of applicable features/overrides and their values.""" diff --git a/cloudinit/util.py b/cloudinit/util.py index 947da4c6337..f42e641440b 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -49,6 +49,7 @@ Generator, List, Mapping, + NamedTuple, Optional, Sequence, TypeVar, @@ -61,6 +62,7 @@ from cloudinit import ( features, importer, + log, mergers, net, settings, @@ -89,6 +91,11 @@ FALSE_STRINGS = ("off", "0", "no", "false") +class DeprecationLog(NamedTuple): + log_level: int + message: str + + def kernel_version(): return tuple(map(int, os.uname().release.split(".")[:2])) @@ -3209,8 +3216,8 @@ def deprecate( deprecated_version: str, extra_message: Optional[str] = None, schedule: int = 5, - return_log: bool = False, -): + skip_log: bool = False, +) -> DeprecationLog: """Mark a "thing" as deprecated. Deduplicated deprecations are logged. @@ -3226,8 +3233,10 @@ def deprecate( @param schedule: Manually set the deprecation schedule. Defaults to 5 years. Leave a comment explaining your reason for deviation if setting this value. - @param return_log: Return log text rather than logging it. Useful for + @param skip_log: Return log text rather than logging it. Useful for running prior to logging setup. + @return: NamedTuple containing log level and log message + DeprecationLog(level: int, message: str) Note: uses keyword-only arguments to improve legibility """ @@ -3242,14 +3251,20 @@ def deprecate( f"{deprecated_version} and scheduled to be removed in " f"{version_removed}. {message}" ).rstrip() - if return_log: - return deprecate_msg - if dedup not in deprecate._log: # type: ignore + if ( + "devel" != features.DEPRECATION_INFO_BOUNDARY + and Version.from_str(features.DEPRECATION_INFO_BOUNDARY) < version + ): + LOG.info(deprecate_msg) + level = logging.INFO + elif hasattr(LOG, "deprecated"): + level = log.DEPRECATED + else: + level = logging.WARN + if not skip_log and dedup not in deprecate._log: # type: ignore deprecate._log.add(dedup) # type: ignore - if hasattr(LOG, "deprecated"): - LOG.deprecated(deprecate_msg) # type: ignore - else: - LOG.warning(deprecate_msg) + LOG.log(level, deprecate_msg) + return DeprecationLog(level, deprecate_msg) def deprecate_call( From a396816a41682db2daa9a957b1e42d38e2f20068 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Wed, 19 Jun 2024 15:55:26 -0600 Subject: [PATCH 2/3] feat: Add deprecation boundary support to schema validator (#5411) This will all stable distros to define whether a key is deprecated based on the version of cloud-init which deprecated the key. --- cloudinit/config/schema.py | 57 +++++++++++++++++++-------- tests/unittests/config/test_schema.py | 20 +++++----- tests/unittests/test_data.py | 2 + tests/unittests/test_features.py | 2 + 4 files changed, 54 insertions(+), 27 deletions(-) diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index c641362f0cb..e552483e543 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -31,13 +31,14 @@ import yaml -from cloudinit import importer, safeyaml +from cloudinit import features, importer, safeyaml from cloudinit.cmd.devel import read_cfg_paths from cloudinit.handlers import INCLUSION_TYPES_MAP, type_from_starts_with from cloudinit.helpers import Paths from cloudinit.sources import DataSourceNotFoundException from cloudinit.temp_utils import mkdtemp from cloudinit.util import ( + Version, error, get_modules_from_dir, load_text_file, @@ -137,7 +138,14 @@ class MetaSchema(TypedDict): class SchemaDeprecationError(ValidationError): - pass + def __init__( + self, + message: str, + version: str, + **kwargs, + ): + super().__init__(message, **kwargs) + self.version: str = version class SchemaProblem(NamedTuple): @@ -363,7 +371,7 @@ def _validator( msg = _add_deprecated_changed_or_new_msg( schema, annotate=True, filter_key=[filter_key] ) - yield error_type(msg) + yield error_type(msg, schema.get("deprecated_version", "devel")) _validator_deprecated = partial(_validator, filter_key="deprecated") @@ -770,6 +778,7 @@ def validate_cloudconfig_schema( errors: SchemaProblems = [] deprecations: SchemaProblems = [] + info_deprecations: SchemaProblems = [] for schema_error in sorted( validator.iter_errors(config), key=lambda e: e.path ): @@ -785,25 +794,39 @@ def validate_cloudconfig_schema( ) if prop_match: path = prop_match["name"] - problem = (SchemaProblem(path, schema_error.message),) if isinstance( schema_error, SchemaDeprecationError ): # pylint: disable=W1116 - deprecations += problem + if ( + "devel" != features.DEPRECATION_INFO_BOUNDARY + and Version.from_str(schema_error.version) + > Version.from_str(features.DEPRECATION_INFO_BOUNDARY) + ): + info_deprecations.append( + SchemaProblem(path, schema_error.message) + ) + else: + deprecations.append(SchemaProblem(path, schema_error.message)) else: - errors += problem + errors.append(SchemaProblem(path, schema_error.message)) - if log_deprecations and deprecations: - message = _format_schema_problems( - deprecations, - prefix="Deprecated cloud-config provided:\n", - separator="\n", - ) - # This warning doesn't fit the standardized util.deprecated() utility - # format, but it is a deprecation log, so log it directly. - LOG.deprecated(message) # type: ignore - if strict and (errors or deprecations): - raise SchemaValidationError(errors, deprecations) + if log_deprecations: + if info_deprecations: + message = _format_schema_problems( + info_deprecations, + prefix="Deprecated cloud-config provided: ", + ) + LOG.info(message) + if deprecations: + message = _format_schema_problems( + deprecations, + prefix="Deprecated cloud-config provided: ", + ) + # This warning doesn't fit the standardized util.deprecated() + # utility format, but it is a deprecation log, so log it directly. + LOG.deprecated(message) # type: ignore + if strict and (errors or deprecations or info_deprecations): + raise SchemaValidationError(errors, deprecations + info_deprecations) if errors: if log_details: details = _format_schema_problems( diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index cd78faa9f14..76d89c3652e 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -629,7 +629,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"a-b": "asdf"}, - "Deprecated cloud-config provided:\na-b: " + "Deprecated cloud-config provided: a-b: " "Deprecated in version 22.1.", ), ( @@ -650,7 +650,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"x": "+5"}, - "Deprecated cloud-config provided:\nx: " + "Deprecated cloud-config provided: x: " "Deprecated in version 22.1.", ), ( @@ -671,7 +671,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"x": "5"}, - "Deprecated cloud-config provided:\nx: " + "Deprecated cloud-config provided: x: " "Deprecated in version 22.1. ", ), ( @@ -692,7 +692,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"x": "5"}, - "Deprecated cloud-config provided:\nx: " + "Deprecated cloud-config provided: x: " "Deprecated in version 22.1.", ), ( @@ -708,7 +708,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"x": "+5"}, - "Deprecated cloud-config provided:\nx: " + "Deprecated cloud-config provided: x: " "Deprecated in version 22.1.", ), ( @@ -745,7 +745,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"x": "+5"}, - "Deprecated cloud-config provided:\nx: " + "Deprecated cloud-config provided: x: " "Deprecated in version 32.3.", ), ( @@ -770,7 +770,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"x": "+5"}, - "Deprecated cloud-config provided:\nx: Deprecated in " + "Deprecated cloud-config provided: x: Deprecated in " "version 27.2.", ), ( @@ -786,7 +786,7 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"a-b": "asdf"}, - "Deprecated cloud-config provided:\na-b: " + "Deprecated cloud-config provided: a-b: " "Deprecated in version 27.2.", ), pytest.param( @@ -804,8 +804,8 @@ def test_validateconfig_strict_metaschema_do_not_raise_exception( }, }, {"a-b": "asdf"}, - "Deprecated cloud-config provided:\na-b: Deprecated " - "in version 27.2.\na-b: Changed in version 22.2. " + "Deprecated cloud-config provided: a-b: Deprecated " + "in version 27.2., a-b: Changed in version 22.2. " "Drop ballast.", id="deprecated_pattern_property_without_description", ), diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py index 1ee3dfa9007..14be6fa48e3 100644 --- a/tests/unittests/test_data.py +++ b/tests/unittests/test_data.py @@ -482,6 +482,7 @@ def test_mime_text_plain(self, init_tmp, caplog): ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES=True, EXPIRE_APPLIES_TO_HASHED_USERS=False, NETPLAN_CONFIG_ROOT_READ_ONLY=True, + DEPRECATION_INFO_BOUNDARY="devel", NOCLOUD_SEED_URL_APPEND_FORWARD_SLASH=False, APT_DEB822_SOURCE_LIST_FILE=True, ) @@ -513,6 +514,7 @@ def test_shellscript(self, init_tmp, tmpdir, caplog): "ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES": True, "EXPIRE_APPLIES_TO_HASHED_USERS": False, "NETPLAN_CONFIG_ROOT_READ_ONLY": True, + "DEPRECATION_INFO_BOUNDARY": "devel", "NOCLOUD_SEED_URL_APPEND_FORWARD_SLASH": False, "APT_DEB822_SOURCE_LIST_FILE": True, }, diff --git a/tests/unittests/test_features.py b/tests/unittests/test_features.py index c9eff407064..e5e81fbffa8 100644 --- a/tests/unittests/test_features.py +++ b/tests/unittests/test_features.py @@ -19,6 +19,7 @@ def test_feature_without_override(self): ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES=True, EXPIRE_APPLIES_TO_HASHED_USERS=False, NETPLAN_CONFIG_ROOT_READ_ONLY=True, + DEPRECATION_INFO_BOUNDARY="devel", NOCLOUD_SEED_URL_APPEND_FORWARD_SLASH=False, APT_DEB822_SOURCE_LIST_FILE=True, ): @@ -29,4 +30,5 @@ def test_feature_without_override(self): "NETPLAN_CONFIG_ROOT_READ_ONLY": True, "NOCLOUD_SEED_URL_APPEND_FORWARD_SLASH": False, "APT_DEB822_SOURCE_LIST_FILE": True, + "DEPRECATION_INFO_BOUNDARY": "devel", } == features.get_features() From 7bb1a72be47b614ccb2dd90f5677adc32775077f Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 27 Jun 2024 12:11:37 -0600 Subject: [PATCH 3/3] test: Add unit tests for features.DEPRECATION_INFO_BOUNDARY (#5411) --- tests/unittests/config/test_schema.py | 38 +++++++++++++++++++++-- tests/unittests/test_log.py | 44 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 76d89c3652e..f8f0dcdc563 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -20,6 +20,7 @@ import pytest import yaml +from cloudinit import features from cloudinit.config.schema import ( VERSIONED_USERDATA_SCHEMA_FILE, MetaSchema, @@ -2757,10 +2758,11 @@ def test_handle_schema_unable_to_read_cfg_paths( assert expected_log in caplog.text @pytest.mark.parametrize( - "annotate, expected_output", + "annotate, deprecation_info_boundary, expected_output", [ - ( + pytest.param( True, + "devel", dedent( """\ #cloud-config @@ -2778,9 +2780,33 @@ def test_handle_schema_unable_to_read_cfg_paths( Valid schema {cfg_file} """ # noqa: E501 ), + id="test_annotated_deprecation_info_boundary_devel_shows", ), - ( + pytest.param( + True, + "22.1", + dedent( + """\ + #cloud-config + packages: + - htop + apt_update: true # D1 + apt_upgrade: true # D2 + apt_reboot_if_required: true # D3 + + # Deprecations: ------------- + # D1: Default: ``false``. Deprecated in version 22.2. Use ``package_update`` instead. + # D2: Default: ``false``. Deprecated in version 22.2. Use ``package_upgrade`` instead. + # D3: Default: ``false``. Deprecated in version 22.2. Use ``package_reboot_if_required`` instead. + + Valid schema {cfg_file} + """ # noqa: E501 + ), + id="test_annotated_deprecation_info_boundary_below_unredacted", + ), + pytest.param( False, + "18.2", dedent( """\ Cloud config schema deprecations: \ @@ -2792,6 +2818,7 @@ def test_handle_schema_unable_to_read_cfg_paths( Valid schema {cfg_file} """ # noqa: E501 ), + id="test_deprecation_info_boundary_does_unannotated_unredacted", ), ], ) @@ -2800,11 +2827,13 @@ def test_handle_schema_args_annotate_deprecated_config( self, read_cfg_paths, annotate, + deprecation_info_boundary, expected_output, paths, caplog, capsys, tmpdir, + mocker, ): paths.get_ipath = paths.get_ipath_cur read_cfg_paths.return_value = paths @@ -2822,6 +2851,9 @@ def test_handle_schema_args_annotate_deprecated_config( """ ) ) + mocker.patch.object( + features, "DEPRECATION_INFO_BOUNDARY", deprecation_info_boundary + ) args = self.Args( config_file=str(user_data_fn), schema_type="cloud-config", diff --git a/tests/unittests/test_log.py b/tests/unittests/test_log.py index 3a8d9683ee2..e68dcc48029 100644 --- a/tests/unittests/test_log.py +++ b/tests/unittests/test_log.py @@ -7,6 +7,8 @@ import logging import time +import pytest + from cloudinit import log, util from cloudinit.analyze.dump import CLOUD_INIT_ASCTIME_FMT from tests.unittests.helpers import CiTestCase @@ -66,6 +68,48 @@ def test_deprecated_log_level(self, caplog): assert "DEPRECATED" == caplog.records[0].levelname assert "deprecated message" in caplog.text + @pytest.mark.parametrize( + "expected_log_level, deprecation_info_boundary", + ( + pytest.param( + "DEPRECATED", + "19.2", + id="test_same_deprecation_info_boundary_is_deprecated_level", + ), + pytest.param( + "INFO", + "19.1", + id="test_lower_deprecation_info_boundary_is_info_level", + ), + ), + ) + def test_deprecate_log_level_based_on_features( + self, expected_log_level, deprecation_info_boundary, caplog, mocker + ): + """Deprecation log level depends on key deprecation_version + + When DEPRECATION_INFO_BOUNDARY is set to a version number, and a key + has a deprecated_version with a version greater than the boundary + the log level is INFO instead of DEPRECATED. If + DEPRECATION_INFO_BOUNDARY is set to the default, "devel", all + deprecated keys are logged at level DEPRECATED. + """ + mocker.patch.object( + util.features, + "DEPRECATION_INFO_BOUNDARY", + deprecation_info_boundary, + ) + util.deprecate( + deprecated="some key", + deprecated_version="19.2", + extra_message="dont use it", + ) + assert expected_log_level == caplog.records[0].levelname + assert ( + "some key is deprecated in 19.2 and scheduled to be removed in" + " 24.2" in caplog.text + ) + def test_log_deduplication(self, caplog): log.define_deprecation_logger() util.deprecate(