From 74546a2302c9d538cdce48a1a237964c3db7de8d Mon Sep 17 00:00:00 2001 From: Matt Verran Date: Thu, 17 Sep 2026 17:52:03 +0100 Subject: [PATCH] feat: add FeatureGroup capability This is required for Cloudkitty to correctly register its need for observability to be enabled. Unlike telemetry for example, observability has two subfeatures, embedded and external that are both equally valid, but one MUST be provided hence optional is not a valid way to handle in the current setup. Authored-By: Hemanth Nakkina Signed-off-by: Matt Verran --- .../sunbeam/features/interface/v1/base.py | 65 ++++++++- .../tests/unit/sunbeam/features/test_base.py | 124 +++++++++++++++++- 2 files changed, 186 insertions(+), 3 deletions(-) diff --git a/sunbeam-python/sunbeam/features/interface/v1/base.py b/sunbeam-python/sunbeam/features/interface/v1/base.py index 6ab568661..07d975ccb 100644 --- a/sunbeam-python/sunbeam/features/interface/v1/base.py +++ b/sunbeam-python/sunbeam/features/interface/v1/base.py @@ -566,6 +566,32 @@ def klass(self) -> Type["EnableDisableFeature"]: ) return klass + @property + def group(self) -> typing.Optional[Type["BaseFeatureGroup"]]: + """Return the feature group class if the requirement names a group.""" + return groups().get(self.name) + + @property + def feature_klasses(self) -> list[Type["EnableDisableFeature"]]: + """Return the feature classes satisfying this requirement. + + A requirement naming a group is satisfied by any one of the group's + features, which are mutually exclusive providers of the same + functionality. + """ + if group := self.group: + klasses = [ + klass + for klass in features().values() + if klass.group is group and issubclass(klass, EnableDisableFeature) + ] + if not klasses: + raise InvalidRequirementError( + f"Feature group {self.name} has no enable/disable features" + ) + return klasses + return [self.klass] + @typing.runtime_checkable class NamedEnabledDisableFeatureProtocol(typing.Protocol): @@ -714,11 +740,19 @@ def check_enablement_requirements( if not feature.is_enabled(deployment.get_client()): continue for requirement in feature.get_requirements(deployment): - if requirement.name != self.name: + """ + For future: + This check looks for one of the features being enabled + It does not consider the case of both features being enabled. + e.g observability.embedded or observability.external. + """ + if requirement.name != self.name and not ( + requirement.group is not None and self.group is requirement.group + ): continue if state == "disable": raise HasRequirersFeaturesError( - f"{feature.name} is enabled and requires {self.name}" + f"{feature.name} is enabled and requires {requirement.name}" ) message = ( f"Feature {feature.name} is enabled and " @@ -733,6 +767,33 @@ def check_enablement_requirements( def enable_requirements(self, deployment: Deployment, show_hints: bool): """Iterate through requirements, enable features if possible.""" for requirement in self.get_requirements(deployment): + if group := requirement.group: + # A group requirement is satisfied by any enabled member and + # is never auto-enabled because the user chooses the provider. + klasses = requirement.feature_klasses + enabled_klass = next( + ( + klass + for klass in klasses + if klass().is_enabled(deployment.get_client()) + ), + None, + ) + if enabled_klass is not None: + self.check_enabled_requirement_is_compatible( + deployment, + FeatureRequirement( + f"{enabled_klass().name}{requirement.specifier}" + ), + ) + continue + if requirement.optional: + continue + members = ", ".join(f"'{klass().name}'" for klass in klasses) + raise FeatureError( + f"Feature {self.name} requires the {group.name} feature" + f" to be enabled. Enable one of {members} and retry." + ) if not issubclass(requirement.klass, EnableDisableFeature): LOG.debug( "Skipping %s as it is not of type EnableDisableFeature", diff --git a/sunbeam-python/tests/unit/sunbeam/features/test_base.py b/sunbeam-python/tests/unit/sunbeam/features/test_base.py index 8aa8f70c5..8202de72c 100644 --- a/sunbeam-python/tests/unit/sunbeam/features/test_base.py +++ b/sunbeam-python/tests/unit/sunbeam/features/test_base.py @@ -13,9 +13,11 @@ from sunbeam.feature_manager import FeatureManager from sunbeam.features.interface.v1.base import ( BaseFeature, + BaseFeatureGroup, EnableDisableFeature, FeatureError, FeatureRequirement, + HasRequirersFeaturesError, IncompatibleVersionError, MissingFeatureError, MissingVersionInfoError, @@ -310,6 +312,34 @@ def run_disable_plans(self, deployment) -> None: return CompatibleFeature +class TestFeatureGroup(BaseFeatureGroup): + name = "test_requirement_group" + + +def group_member_klass(name_: str, enabled: bool) -> type[EnableDisableFeature]: + class GroupMember(EnableDisableFeature): + name = name_ + group = TestFeatureGroup + + def is_enabled(self, client) -> bool: + return enabled + + return GroupMember + + +def test_requirement_feature_group(): + member_a = group_member_klass("test_requirement_group.a", enabled=False) + member_b = group_member_klass("test_requirement_group.b", enabled=False) + + requirement = FeatureRequirement("test_requirement_group") + + assert requirement.group is TestFeatureGroup + assert {klass.name for klass in requirement.feature_klasses} == { + member_a.name, + member_b.name, + } + + class TestEnableDisableFeature: def test_check_enabled_feature_is_compatible_with_compatible_requirement( self, deployment, mocker @@ -446,7 +476,8 @@ def test_core_features_requirements(self, deployment, klass): feature = klass() for requirement in feature.requires: - feature.check_feature_class_is_compatible(requirement.klass(), requirement) + for required_klass in requirement.feature_klasses: + feature.check_feature_class_is_compatible(required_klass(), requirement) def test_check_enablement_requirements_with_enabled_compatible_requirement( self, deployment, mocker @@ -591,6 +622,97 @@ def test_pre_disable_runs_juju_login_preflight_check(self, deployment, mocker): assert len(checks) == 1 assert checks[0].step.juju_account == juju_account + def test_enable_requirements_with_enabled_group_requirement(self, deployment): + group_member_klass("test_requirement_group.a", enabled=True) + group_member_klass("test_requirement_group.b", enabled=False) + + feature = DummyFeature() + feature.requires = {FeatureRequirement("test_requirement_group")} + feature.enable_requirements(deployment, show_hints=False) + + def test_enable_requirements_with_disabled_group_requirement(self, deployment): + group_member_klass("test_requirement_group.a", enabled=False) + group_member_klass("test_requirement_group.b", enabled=False) + + feature = DummyFeature() + feature.requires = {FeatureRequirement("test_requirement_group")} + with pytest.raises(FeatureError, match="test_requirement_group"): + feature.enable_requirements(deployment, show_hints=False) + + def test_enable_requirements_with_optional_group_requirement(self, deployment): + group_member_klass("test_requirement_group.a", enabled=False) + group_member_klass("test_requirement_group.b", enabled=False) + + feature = DummyFeature() + feature.requires = {FeatureRequirement("test_requirement_group", optional=True)} + feature.enable_requirements(deployment, show_hints=False) + + def test_enable_requirements_with_group_requirement_version_compatible( + self, deployment, mocker + ): + group_member_klass("test_requirement_group.a", enabled=True) + group_member_klass("test_requirement_group.b", enabled=False) + + feature = DummyFeature() + feature.requires = {FeatureRequirement("test_requirement_group>=1.0.0")} + mocker.patch.object( + feature, "fetch_feature_version", return_value=Version("1.0.1") + ) + feature.enable_requirements(deployment, show_hints=False) + + def test_enable_requirements_with_group_requirement_version_incompatible( + self, deployment, mocker + ): + group_member_klass("test_requirement_group.a", enabled=True) + group_member_klass("test_requirement_group.b", enabled=False) + + feature = DummyFeature() + feature.requires = {FeatureRequirement("test_requirement_group>=2.0.0")} + mocker.patch.object( + feature, "fetch_feature_version", return_value=Version("1.0.1") + ) + with pytest.raises(IncompatibleVersionError): + feature.enable_requirements(deployment, show_hints=False) + + def test_check_enablement_requirements_with_enabled_dependant_group( + self, deployment, mocker + ): + member_klass = group_member_klass("test_requirement_group.a", enabled=False) + + class Requirer(EnableDisableFeature): + name = "test_group_requirer" + requires = {FeatureRequirement("test_requirement_group")} + + def is_enabled(self, client) -> bool: + return True + + mocker.patch( + "sunbeam.features.interface.v1.base.features", + Mock(return_value={Requirer.name: Requirer}), + ) + with pytest.raises(HasRequirersFeaturesError): + member_klass().check_enablement_requirements(deployment, "disable") + + def test_check_enablement_requirements_with_group_requirement( + self, deployment, mocker + ): + member_klass = group_member_klass("test_requirement_group.a", enabled=False) + + class Requirer(EnableDisableFeature): + name = "test_group_requirer" + requires = {FeatureRequirement("test_requirement_group>=1.0.0")} + + def is_enabled(self, client) -> bool: + return True + + mocker.patch( + "sunbeam.features.interface.v1.base.features", + Mock(return_value={Requirer.name: Requirer}), + ) + member = member_klass() + member.version = Version("1.0.1") + member.check_enablement_requirements(deployment) + class TestFeatureManager: """Test FeatureManager methods."""