Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utilities: Convert juju base into Ubuntu series #622

Closed
wants to merge 1 commit into from
+200 −5
Closed

utilities: Convert juju base into Ubuntu series #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 72 additions & 2 deletions unit_tests/utilities/test_zaza_utilities_juju.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,16 +370,86 @@ def test_get_machine_series(self):
new_callable=mock.MagicMock(),
name='_get_machine_status'
)
self._get_machine_status.return_value = 'xenial'
self._get_machine_status.return_value = {'series': 'xenial'}
expected = 'xenial'
actual = juju_utils.get_machine_series('6')
self._get_machine_status.assert_called_with(
machine='6',
key='series',
model_name=None
)
self.assertEqual(expected, actual)

def test_get_machine_series_juju3x_exceptions(self):
self.patch(
'zaza.utilities.juju.get_machine_status',
new_callable=mock.MagicMock(),
name='_get_machine_status'
)
self.patch(
'zaza.utilities.juju.launchpad.get_ubuntu_series_by_version',
new_callable=mock.MagicMock(),
name='_get_ubuntu_series_by_version'
)
self._get_ubuntu_series_by_version.return_value = {
'22.04': {'name': 'jammy'}}

status = mock.MagicMock()
status.__getitem__.side_effect = KeyError

base = mock.MagicMock()
base.name = 'ubuntu'
base.channel = '22.04/stable'
status.get.return_value = base
self._get_machine_status.return_value = status

try:
juju_utils.get_machine_series('6')
except KeyError:
self.fail('Did not expect `get_machine_series` '
'to raise a KeyError')
self._get_machine_status.reset_mock()

self._get_machine_status.return_value = {}
self.assertRaises(
ValueError,
juju_utils.get_machine_series,
'6')

base = mock.MagicMock()
base.name = 'someOtherDistro'
base.channel = '22.04/stable'
self._get_machine_status.return_value = {'base': base}
self.assertRaises(
NotImplementedError,
juju_utils.get_machine_series,
'6')

def test_get_machine_series_juju3x(self):
self.patch(
'zaza.utilities.juju.get_machine_status',
new_callable=mock.MagicMock(),
name='_get_machine_status'
)
self.patch(
'zaza.utilities.juju.launchpad.get_ubuntu_series_by_version',
new_callable=mock.MagicMock(),
name='_get_ubuntu_series_by_version'
)
base = mock.MagicMock()
base.name = 'ubuntu'
base.channel = '22.04/stable'
self._get_machine_status.return_value = {'base': base}
self._get_ubuntu_series_by_version.return_value = {
'22.04': {'name': 'jammy'}}
expected = 'jammy'
actual = juju_utils.get_machine_series('6')
self._get_machine_status.assert_called_with(
machine='6',
model_name=None
)
self._get_ubuntu_series_by_version.assert_called_once_with()
self.assertEqual(expected, actual)

def test_get_subordinate_units(self):
juju_status = mock.MagicMock()
juju_status.applications = {
Expand Down
55 changes: 55 additions & 0 deletions unit_tests/utilities/test_zaza_utilities_launchpad.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Copyright 2023 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import json
import unittest

import unit_tests.utils as ut_utils
import zaza.utilities.launchpad as launchpad


class TestUtilitiesLaunchpad(ut_utils.BaseTestCase):

def test_get_ubuntu_series(self):
self.patch_object(launchpad.requests, 'get')
expect = {'entries': {}}
r = unittest.mock.MagicMock()
r.text = json.dumps(expect)
self.get.return_value = r
self.assertEquals(
launchpad.get_ubuntu_series(),
expect,
)
self.get.assert_called_once_with(
'https://api.launchpad.net/devel/ubuntu/series')

def test_get_ubuntu_series_by_version(self):
self.patch_object(launchpad, 'get_ubuntu_series')

self.get_ubuntu_series.return_value = {
'entries': [{'version': 'fakeVersion'}]}

self.assertEquals(
launchpad.get_ubuntu_series_by_version(),
{'fakeVersion': {'version': 'fakeVersion'}})

def test_get_ubuntu_series_by_name(self):
self.patch_object(launchpad, 'get_ubuntu_series')

self.get_ubuntu_series.return_value = {
'entries': [{'name': 'fakeName'}]}

self.assertEquals(
launchpad.get_ubuntu_series_by_name(),
{'fakeName': {'name': 'fakeName'}})
24 changes: 21 additions & 3 deletions zaza/utilities/juju.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@
model,
controller,
)
from zaza.utilities import generic as generic_utils
from zaza.utilities import exceptions as zaza_exceptions
from zaza.utilities import generic as generic_utils
from zaza.utilities import launchpad

KUBERNETES_PROVIDER_NAME = 'kubernetes'

Expand Down Expand Up @@ -279,11 +280,28 @@
:returns: Juju series
:rtype: string
"""
return get_machine_status(
status = get_machine_status(
machine=machine,
key='series',
model_name=model_name
)
try:
if 'series' in status:
return status.get('series')
except KeyError:

Check warning on line 290 in zaza/utilities/juju.py

View check run for this annotation

Codecov / codecov/patch

zaza/utilities/juju.py#L290 

Added line #L290 was not covered by tests
lmlg marked this conversation as resolved.
Show resolved Hide resolved
# libjuju will raise make the above check return KeyError when not
# present...
pass

Check warning on line 293 in zaza/utilities/juju.py

View check run for this annotation

Codecov / codecov/patch

zaza/utilities/juju.py#L293 

Added line #L293 was not covered by tests

base = status.get('base')
if not base:
raise ValueError("Unable to determine distro from status: '{}'"
.format(status))
if base.name != 'ubuntu':
raise NotImplementedError("Series resolution not implemented for "
"distro: '{}'".format(base.name))

version, risk = base.channel.split('/')
return launchpad.get_ubuntu_series_by_version()[version]['name']


def get_machine_uuids_for_application(application, model_name=None):
Expand Down
52 changes: 52 additions & 0 deletions zaza/utilities/launchpad.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Copyright 2023 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Module for interacting with Launchpad API."""

import json
import requests
import typing


def get_ubuntu_series(
) -> typing.Dict[str, typing.List[typing.Dict[str, any]]]:
"""Contact Launchpad API and retrieve a list of all Ubuntu releases.
Launchpad documentation for the returned data structure can be found here:
https://launchpad.net/+apidoc/devel.html#distribution
https://launchpad.net/+apidoc/devel.html#distro_series
"""
r = requests.get('https://api.launchpad.net/devel/ubuntu/series')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm frankly not really fond of using HTTP requests to get the Ubuntu version mapping, simple as it is, since it restraints somewhat the environments in which this can be run, as well as placing a dependency on an external asset. OTOH, alternatives like including charmhelpers aren't particularly endearing either. At the very least, I would include an escape valve, like an environment variable that allows one to bypass the need for internet access.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed this part is tricky, and I understand your reservations for an external network dependency. I guess that's why the previous attempts at this has ended up hard coding with the perpetual per-release maintenance burden that entails.

I strongly dislike anything that invites manual work, and IMHO it is worth spending quite a bit of extra time on development to avoid it.

What about a fallback to a on-disk file with a documented format and perhaps a helpful error message in case this request fails?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd be onboard with that and it's essentially what I was hinting at when I mentioned the environment variable thingy (I'm opened to any other solution as well, like an optional parameter to the base function).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reading through both your comments (@fnordahl and @lmlg) and just wanted to add some thoughts for your consideration:

I really like @lmlg reaction to a code change that adds a new requirement; in this case requiring network access to launchpad.

I also really like @fnordahl constant hunt to reduce manual work and automate where possible, particular around things that would need maintaining year-to-year as new releases come into being. This is the kind of busywork we all want to avoid.

So, in this case, how would/should we proceed?

So my thoughts revolve around keeping it simple (less code = less maintenance/testing requirements), which is also an antidote to gold-plating, but making sure it is still appropriate for it's environment. In this case, this is zaza code, which is exclusively a test framework, and does run in a variety of places, but all (I think?) are in non-air-gapped locations. Some might require proxies, but I don't think we've hit that for launchpad.

i.e. I would probably favour @fnordahl 's solution of simplicity over adding more complexity in ENV variable that would have to be tested, add documentation on how to use it, for a use-case that hasn't yet come up. Maybe add a TODO comment that basically says "add a ENV variable here if an air-gapped solution is required".

Maintaining an additional text file of releases feels very counter-productive as it's entirely what @fnordahl is seeking to avoid!

Anyway, I hope this helps! Just an 3rd party's thoughts after reading your comments. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: an alternative implementation is simply to calculate the base from the distro-info output. The distro-info is updated across releases.

The following does the trick and is future-proof as long as:

  1. The LTS structure doesn't change
  2. All releases continue to ship twice per year
  3. The distro-info continues to be updated
import re
import subprocess
from typing import List


BASE_REGEX = r'^(?:[\w+]*@)?(?P<year>[\d]{1,2})\.(?P<month>[\d]{2})$'


def _get_known_series() -> List[str]:
    """Returns the list of the known Ubuntu series from the distro-info.

    Returns a list of all known ubuntu series from the distro-info command.
    """
    known_series = subprocess.check_output(['distro-info', '--all']).decode('utf-8').strip().split('\n')
    return known_series


def get_base_from_series(series: str) -> str:
    """Returns the Ubuntu base format from the series name.

    Converts the series name to an Ubuntu base, e.g. focal => 20.04,
    jammy => 22.04, etc.

    :param series: the Ubuntu series name.
    :raises: ValueError if the series is not a known Ubuntu release name
    """
    known_series = _get_known_series()
    series = series.strip().lower()

    index = known_series.index(series)
    if index < 0:
        raise ValueError(f"{series} is not a known Ubuntu series name")

    # Only Ubuntu release to ever ship late (first LTS had its challenges)
    if series == 'dapper':
        return "[email protected]"

    year = (index >> 1) + (index % 2) + 4
    month = '04' if (index % 2) == 1 else '10'
    return f"ubuntu@{year}.{month}"


def get_series_from_base(base: str) -> str:
    """Returns the Ubuntu series name from the base format.

    Converts the base name to an Ubuntu series name, e.g. 20.04 => focal,
    22.04 => jammy, etc.

    :param base: the base name that includes the distro release and the channel.
                 May optionally include a distro@ prefix.
    """
    known_series = _get_known_series()

    match = re.match(BASE_REGEX, base)
    if not match:
        raise ValueError(f"{base} is not of the correct format. Expected to be of form [distro@]YY.MM")

    year = int(match.group("year"))
    month = int(match.group("month"))

    offset = -1 if month <= 6 else 0
    # 2 releases per year, starting in 2004. First release was in the fall
    # so we account for that with the offset.
    index = (year - 4) * 2 + offset
    return known_series[index]


def is_lts(base_or_series: str) -> bool:
    """Returns True iff the base or series specified is an LTS release.

    :param base_or_series: either a base or series.
    """
    match = re.match(BASE_REGEX, base_or_series)
    if not match:
        # Looks like we have a series. Convert it to base.
        match = re.match(BASE_REGEX, get_base_from_series(base_or_series))

    year = int(match.group("year"))
    month = int(match.group("month"))
    # LTS releases are only in even numbered years.
    if int(year) % 2 > 0:
        return False

    # LTS release is only in the first half of the year.
    return int(month) <= 6

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice alternative indeed, and parsing the output ourselves is actually a plus as the distro-info Python package is not particularly compatible with virtual environments (not really pip installable).

A potential downside could be that whether it works or not depends on the test executor being up to date, as this information comes from a system deb package. From previous experience those have a tendency to end up running pretty old versions from time to time.

I guess we could run the check on one of the deployed nodes to ensure up to date data.

return json.loads(r.text)


def get_ubuntu_series_by_version() -> typing.Dict[str, typing.Dict[str, any]]:
"""Get a Dict of distro series information indexed by version number.
Please refer to the `get_ubuntu_series()` function docstring for docs.
"""
return {
entry['version']: entry
for entry in get_ubuntu_series().get('entries', {})
}


def get_ubuntu_series_by_name() -> typing.Dict[str, typing.Dict[str, any]]:
"""Get a Dict of distro series information indexed by version name.
Please refer to the `get_ubuntu_series()` function docstring for docs.
"""
return {
entry['name']: entry
for entry in get_ubuntu_series().get('entries', {})
}
Loading