[JUJU-3954] Fix incorrect base channel computation (#875)

This was incorrect because the compute_base_channel was outputting
'latest/stable' (as a base channel) when no series was given and the
track/risk is latest/stable. While 'latest/stable' is a correct "charm
channel", it's not a correct "base channel".

Author: cderici

juju/model.py

@@ -517,6 +517,7 @@ async def resolve(self, url, architecture,
         if series:
             base.channel = ch.normalize().compute_base_channel(series=series)
             base.name = 'ubuntu'
+
         origin = client.CharmOrigin(source=Source.CHARM_HUB.value,
                                     architecture=architecture,
                                     risk=ch.risk,
@@ -1865,7 +1866,7 @@ async def _resolve_charm(self, url, origin, force=False, series=None, model_conf
 
         result = resp.results[0]
         if result.error:
-            raise JujuError(result.error.message)
+            raise JujuError(f'resolving {url} : {result.error.message}')
 
         supported_series = result.supported_series
         resolved_origin = result.charm_origin
@@ -1876,7 +1877,7 @@ async def _resolve_charm(self, url, origin, force=False, series=None, model_conf
         result.charm_origin.base = utils.get_base_from_origin_or_channel(resolved_origin, selected_series)
         charm_url.series = selected_series
 
-        return result.url, result.charm_origin
+        return str(charm_url), resolved_origin
 
     async def _resolve_architecture(self, url):
         if url.architecture:

juju/origin.py

@@ -113,15 +113,13 @@ def __str__(self):
             path = "{}/{}".format(self.track, path)
         return path
 
-    def compute_base_channel(self, series=None):
+    def compute_base_channel(self, series):
         """Determines the channel for a client.Base
         A base channel is a track/risk/branch
 
         """
         _ch = [self.risk]
-        tr = self.track
-        if series:
-            tr = utils.get_series_version(series)
+        tr = utils.get_series_version(series)
         if tr:
             _ch = [tr] + _ch
         return "/".join(_ch)

juju/utils.py

@@ -331,12 +331,24 @@ def get_local_charm_charmcraft_yaml(path):
 
 
 def get_series_version(series_name):
+    """get_series_version outputs the version of the OS based on the given series
+    e.g. jammy -> 22.04, kubernetes -> kubernetes
+
+    :param str series_name: name of the series
+    :return str: os version
+    """
     if series_name not in ALL_SERIES_VERSIONS:
         raise errors.JujuError("Unknown series : %s", series_name)
     return ALL_SERIES_VERSIONS[series_name]
 
 
 def get_version_series(version):
+    """get_version_series is the opposite of the get_series_version. It outputs the series based
+    on given OS version
+
+    :param str version: version of the OS
+    return str: name of the series corresponding to the given version
+    """
     if version not in UBUNTU_SERIES.values():
         raise errors.JujuError("Unknown version : %s", version)
     return list(UBUNTU_SERIES.keys())[list(UBUNTU_SERIES.values()).index(version)]
@@ -418,19 +430,21 @@ def parse_base_arg(base):
 DEFAULT_SUPPORTED_LTS_BASE = client.Base(channel='22.04', name='ubuntu')
 
 
-def base_channel_from_series(track, risk, series=None):
+def base_channel_from_series(track, risk, series):
     return origin.Channel(track=track, risk=risk).normalize().compute_base_channel(series=series)
 
 
-def get_os_from_series(series=None):
-    if not series or series in UBUNTU_SERIES:
+def get_os_from_series(series):
+    if series in UBUNTU_SERIES:
         return 'ubuntu'
     raise JujuError(f'os for the series {series} needs to be added')
 
 
 def get_base_from_origin_or_channel(origin_or_channel, series=None):
-    channel = base_channel_from_series(origin_or_channel.track, origin_or_channel.risk, series)
-    os_name = get_os_from_series(series)
+    channel, os_name = None, None
+    if series:
+        channel = base_channel_from_series(origin_or_channel.track, origin_or_channel.risk, series)
+        os_name = get_os_from_series(series)
     return client.Base(channel=channel, name=os_name)
 
 

tests/unit/test_utils.py

@@ -1,7 +1,11 @@
 import unittest
 import pytest
 
-from juju.utils import series_selector, get_base_from_origin_or_channel, parse_base_arg, juju_config_dir, juju_ssh_key_paths, DEFAULT_SUPPORTED_LTS
+from juju.utils import series_selector, get_base_from_origin_or_channel, \
+    parse_base_arg, juju_config_dir, juju_ssh_key_paths, \
+    DEFAULT_SUPPORTED_LTS, get_series_version, get_version_series, \
+    base_channel_to_series, base_channel_from_series, \
+    get_os_from_series
 from juju.client import client
 from juju.errors import JujuError
 from juju.url import URL
@@ -26,13 +30,6 @@ def test_parse_base_arg(self):
         assert base.channel == '22.04'
 
 
-class TestBaseFromSeries(unittest.TestCase):
-    def test_get_base_from_series(self):
-        b = get_base_from_origin_or_channel(client.CharmOrigin(track='latest', risk='edge'), series='jammy')
-        assert b.name == 'ubuntu'
-        assert b.channel == '22.04/edge'
-
-
 class TestSeriesSelector(unittest.TestCase):
     def test_series_arg(self):
         assert series_selector('jammy', []) == 'jammy'
@@ -53,3 +50,26 @@ def test_charm_list_series(self):
 
     def test_return_lts(self):
         assert series_selector() == DEFAULT_SUPPORTED_LTS
+
+
+class TestBaseChannelOriginUtils(unittest.TestCase):
+    def test_get_series_version(self):
+        assert get_series_version(series_name='kubernetes') == 'kubernetes'
+        assert get_series_version(series_name='jammy') == '22.04'
+
+    def test_get_version_series(self):
+        assert get_version_series(version='22.04') == 'jammy'
+
+    def test_base_channel_to_series(self):
+        assert base_channel_to_series(channel='22.04/stable') == 'jammy'
+
+    def test_base_channel_from_series(self):
+        assert base_channel_from_series(track='latest', risk='stable', series='jammy') == \
+               '22.04/stable'
+
+    def test_get_os_from_series(self):
+        assert get_os_from_series('jammy') == 'ubuntu'
+
+    def test_get_base_from_series(self):
+        orgn = client.CharmOrigin(track='latest', risk='edge')
+        assert get_base_from_origin_or_channel(orgn, series='jammy') == client.Base('22.04/edge', 'ubuntu')