Consider applications without origin setting to sanity checks (#364)

- Apps without origin setting were breaking COU execution. E.g: vault
- function _need_origin_setting_update check if the application needs to
update origin setting considering the corner cases like apps without
origin settings or with empty value.

Closes: #361

cou/apps/base.py

@@ -132,42 +132,44 @@ def __str__(self) -> str:
         return yaml.dump(summary, sort_keys=False)
 
     @property
-    def apt_source_codename(self) -> Optional[OpenStackRelease]:
+    def apt_source_codename(self) -> OpenStackRelease:
         """Identify the OpenStack release set on "openstack-origin" or "source" config.
 
-        :raises ApplicationError: If os_origin_parsed is not a valid OpenStack release or os_origin
-            is in an unexpected format (ppa, url, etc).
-        :return: OpenStackRelease object or None if the app doesn't have os_origin config.
-        :rtype: Optional[OpenStackRelease]
+        :raises ApplicationError: When origin setting is not valid.
+        :return: OpenStackRelease object.
+        :rtype: OpenStackRelease
         """
-        os_origin_parsed: Optional[str]
-        # that means that the charm doesn't have "source" or "openstack-origin" config.
-        if self.origin_setting is None:
-            return None
+        #  means that the charm doesn't have origin setting config or is using empty string.
+        if not self.os_origin:
+            return self.current_os_release
 
-        # Ex: "cloud:focal-ussuri" will result in "ussuri"
         if self.os_origin.startswith("cloud"):
-            *_, os_origin_parsed = self.os_origin.rsplit("-", maxsplit=1)
-            try:
-                return OpenStackRelease(os_origin_parsed)
-            except ValueError as exc:
-                raise ApplicationError(
-                    f"'{self.name}' has an invalid '{self.origin_setting}': {self.os_origin}"
-                ) from exc
-
-        elif self.os_origin == "distro":
+            return self._extract_from_uca_source()
+
+        if self.os_origin == "distro":
             # find the OpenStack release based on ubuntu series
-            os_origin_parsed = DISTRO_TO_OPENSTACK_MAPPING[self.series]
-            return OpenStackRelease(os_origin_parsed)
+            return OpenStackRelease(DISTRO_TO_OPENSTACK_MAPPING[self.series])
 
-        elif self.os_origin == "":
-            return None
+        # probably because user set a ppa or a url
+        raise ApplicationError(
+            f"'{self.name}' has an invalid '{self.origin_setting}': {self.os_origin}"
+        )
 
-        else:
-            # probably because user set a ppa or an url
+    def _extract_from_uca_source(self) -> OpenStackRelease:
+        """Extract the OpenStack release from Ubuntu Cloud Archive (UCA) sources.
+
+        :raises ApplicationError: When origin setting is not valid.
+        :return: OpenStackRelease object
+        :rtype: OpenStackRelease
+        """
+        # Ex: "cloud:focal-victoria" will result in "victoria"
+        try:
+            _, os_origin_parsed = self.os_origin.rsplit("-", maxsplit=1)
+            return OpenStackRelease(os_origin_parsed)
+        except ValueError as exc:
             raise ApplicationError(
                 f"'{self.name}' has an invalid '{self.origin_setting}': {self.os_origin}"
-            )
+            ) from exc
 
     @property
     def channel_codename(self) -> OpenStackRelease:
@@ -199,7 +201,6 @@ def os_origin(self) -> str:
         if origin := self.origin_setting:
             return self.config[origin].get("value", "")
 
-        logger.warning("Failed to get origin for %s, no origin config found", self.name)
         return ""
 
     @property
@@ -213,6 +214,7 @@ def origin_setting(self) -> Optional[str]:
             if origin in self.config:
                 return origin
 
+        logger.debug("%s has no origin setting config", self.name)
         return None
 
     @property

tests/unit/apps/test_auxiliary.py

@@ -504,6 +504,59 @@ def test_auxiliary_raise_halt_upgrade(model):
         app.generate_upgrade_plan(target, False)
 
 
+def test_auxiliary_no_origin_setting_raise_halt_upgrade(model):
+    """Test auxiliary without origin setting raise halt the plan if necessary."""
+    target = OpenStackRelease("victoria")
+    charm = "vault"
+    exp_msg = (
+        f"Application '{charm}' already configured for release equal to or greater than {target}. "
+        "Ignoring."
+    )
+    machines = {
+        "0": MagicMock(spec_set=Machine),
+        "1": MagicMock(spec_set=Machine),
+        "2": MagicMock(spec_set=Machine),
+    }
+    app = AuxiliaryApplication(
+        name=charm,
+        # no channel to refresh
+        can_upgrade_to="",
+        charm=charm,
+        channel="1.7/stable",
+        # no origin setting
+        config={},
+        machines=machines,
+        model=model,
+        origin="ch",
+        series="focal",
+        subordinate_to=[],
+        units={
+            f"{charm}/0": Unit(
+                name=f"{charm}/0",
+                workload_version="1.7",
+                machine=machines["0"],
+            ),
+            f"{charm}/1": Unit(
+                name=f"{charm}/1",
+                workload_version="1.7",
+                machine=machines["1"],
+            ),
+            f"{charm}/2": Unit(
+                name=f"{charm}/2",
+                workload_version="1.7",
+                machine=machines["2"],
+            ),
+        },
+        workload_version="1.7",
+    )
+
+    # current OpenStack release is bigger than target
+    assert app.current_os_release == OpenStackRelease("yoga")
+
+    with pytest.raises(HaltUpgradePlanGeneration, match=exp_msg):
+        app._check_application_target(target)
+
+
 def test_auxiliary_no_suitable_channel(model):
     """Test auxiliary upgrade plan not suitable channel.
 
@@ -1160,7 +1213,7 @@ async def test_ceph_osd_verify_nova_compute_no_app(model):
 def test_auxiliary_upgrade_by_unit(mock_super, model):
     """Test generating plan with units doesn't create unit Upgrade steps."""
     target = OpenStackRelease("victoria")
-    charm = "vault"
+    charm = "my-app"
     machines = {
         "0": MagicMock(spec_set=Machine),
         "1": MagicMock(spec_set=Machine),

tests/unit/apps/test_auxiliary_subordinate.py

@@ -49,7 +49,7 @@ def test_auxiliary_subordinate(model):
     assert app.channel == "8.0/stable"
     assert app.origin == "ch"
     assert app.os_origin == ""
-    assert app.apt_source_codename is None
+    assert app.apt_source_codename == "yoga"
     assert app.channel_codename == "yoga"
     assert app.current_os_release == "yoga"
     assert app.is_subordinate is True
@@ -108,7 +108,7 @@ def test_ovn_subordinate(model):
 
     assert app.channel == "22.03/stable"
     assert app.os_origin == ""
-    assert app.apt_source_codename is None
+    assert app.apt_source_codename == "yoga"
     assert app.channel_codename == "yoga"
     assert app.current_os_release == "yoga"
     assert app.is_subordinate is True

tests/unit/apps/test_base.py

@@ -743,26 +743,41 @@ def test_get_refresh_charm_step_charmhub_migration(
 
 
 @pytest.mark.parametrize(
-    "can_upgrade_to, channel, exp_result",
+    "config, exp_result",
     [
-        ("", "ussuri/stable", False),
-        ("ch:amd64/focal/my-app-723", "ussuri/stable", True),
-        ("ch:amd64/focal/my-app-723", "victoria/stable", True),
-        # when channel_codename is bigger than target it's not necessary to refresh
-        ("ch:amd64/focal/my-app-723", "wallaby/stable", False),
-        ("", "wallaby/stable", False),
+        ({"source": {"value": "cloud:focal-victoria"}}, "victoria"),
+        ({"openstack-origin": {"value": "cloud:focal-wallaby"}}, "wallaby"),
+        ({"source": {"value": "cloud:focal-xena"}}, "xena"),
     ],
 )
-def test_need_current_channel_refresh(model, can_upgrade_to, channel, exp_result):
-    """Test when the application needs to refresh."""
-    target = OpenStackRelease("victoria")
+def test_extract_from_uca_source(model, config, exp_result):
+    """Test extraction from uca sources."""
+    app = OpenStackApplication(
+        name="app",
+        can_upgrade_to="",
+        charm="app",
+        channel=f"{exp_result}/stable",
+        config=config,
+        machines={},
+        model=model,
+        origin="ch",
+        series="focal",
+        subordinate_to=[],
+        units={},
+        workload_version="1",
+    )
+    assert app._extract_from_uca_source() == exp_result
 
+
+@pytest.mark.parametrize("wrong_uca", ["cloud:focal-foo", "cloud:focal"])
+def test_extract_from_uca_source_raise(wrong_uca, model):
+    """Test extraction from uca sources raises ApplicationError with invalid value."""
     app = OpenStackApplication(
         name="app",
-        can_upgrade_to=can_upgrade_to,
+        can_upgrade_to="",
         charm="app",
-        channel=channel,
-        config={},
+        channel="ussuri/stable",
+        config={"source": {"value": wrong_uca}},
         machines={},
         model=model,
         origin="ch",
@@ -771,4 +786,114 @@ def test_need_current_channel_refresh(model, can_upgrade_to, channel, exp_result
         units={},
         workload_version="1",
     )
-    assert app._need_current_channel_refresh(target) is exp_result
+    exp_msg = f"'app' has an invalid 'source': {wrong_uca}"
+    with pytest.raises(ApplicationError, match=exp_msg):
+        app._extract_from_uca_source()
+
+
+@pytest.mark.parametrize(
+    "config, exp_result",
+    [
+        ({"source": {"value": "distro"}}, "ussuri"),
+        ({"openstack-origin": {"value": "cloud:focal-victoria"}}, "victoria"),
+        ({"source": {"value": "cloud:focal-wallaby"}}, "wallaby"),
+    ],
+)
+def test_apt_source_codename(config, exp_result, model):
+    """Test application with empty or without origin setting."""
+    machines = {"0": MagicMock(spec_set=Machine)}
+
+    app = OpenStackApplication(
+        name="app",
+        can_upgrade_to="",
+        charm="app",
+        channel="ussuri/stable",
+        config=config,
+        machines=machines,
+        model=model,
+        origin="ch",
+        series="focal",
+        subordinate_to=[],
+        units={
+            "app/0": Unit(
+                name="app/0",
+                workload_version="1",
+                machine=machines["0"],
+            )
+        },
+        workload_version="1",
+    )
+
+    app.apt_source_codename == exp_result
+
+
+@pytest.mark.parametrize(
+    "config",
+    [
+        {"source": {"value": ""}},
+        {},
+    ],
+)
+@patch("cou.apps.base.OpenStackApplication.current_os_release", new_callable=PropertyMock)
+def test_apt_source_codename_empty_or_without_origin_setting(mock_os_release, config, model):
+    """Test application with empty or without origin setting."""
+    # apt_source_codename will have OpenStack release considering the workload version.
+    exp_result = OpenStackRelease("ussuri")
+    mock_os_release.return_value = exp_result
+    machines = {"0": MagicMock(spec_set=Machine)}
+
+    app = OpenStackApplication(
+        name="app",
+        can_upgrade_to="",
+        charm="app",
+        channel="ussuri/stable",
+        config=config,
+        machines=machines,
+        model=model,
+        origin="ch",
+        series="focal",
+        subordinate_to=[],
+        units={
+            "app/0": Unit(
+                name="app/0",
+                workload_version="1",
+                machine=machines["0"],
+            )
+        },
+        workload_version="1",
+    )
+
+    app.apt_source_codename == exp_result
+
+
+@pytest.mark.parametrize(
+    "source_value",
+    ["ppa:my-team/ppa", "http://my.archive.com/ubuntu main"],
+)
+def test_apt_source_codename_unknown_source(source_value, model):
+    """Test application with unknown origin setting."""
+    machines = {"0": MagicMock(spec_set=Machine)}
+    exp_msg = f"'app' has an invalid 'source': {source_value}"
+    app = OpenStackApplication(
+        name="app",
+        can_upgrade_to="",
+        charm="app",
+        channel="ussuri/stable",
+        config={"source": {"value": source_value}},
+        machines=machines,
+        model=model,
+        origin="ch",
+        series="focal",
+        subordinate_to=[],
+        units={
+            "app/0": Unit(
+                name="app/0",
+                workload_version="1",
+                machine=machines["0"],
+            )
+        },
+        workload_version="1",
+    )
+
+    with pytest.raises(ApplicationError, match=exp_msg):
+        app.apt_source_codename