Skip to content

Ubuntu OSMorphing Refactor #427

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

Open
Dany9966 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Ubuntu OSMorphing Refactor #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
591a944
Fix new cloud-init version service enablement.
Dany9966 May 8, 2026
280db3d
Improve upon Ubuntu OSMorphing tool check
Dany9966 May 8, 2026
fe3765c
Replace all deprecated `@abc.abstractclassmethod` declarations
Dany9966 May 11, 2026
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
19 changes: 16 additions & 3 deletions coriolis/osmorphing/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
"os_type", "distribution_name", "release_version",
"friendly_release_name"]
DEFAULT_CLOUD_USER = "cloud-user"
CLOUD_INIT_SERVICE_UNIT_NAME = "cloud-init"
CLOUD_INIT_SERVICE_UNIT_NAME_FALLBACK = "cloud-init-main"


class BaseOSMorphingTools(object, with_metaclass(abc.ABCMeta)):
Expand All @@ -48,7 +50,8 @@ def __init__(
self._osmorphing_parameters = osmorphing_parameters
self._osmorphing_operation_timeout = operation_timeout

@abc.abstractclassmethod
@classmethod
@abc.abstractmethod
def get_required_detected_os_info_fields(cls):
raise NotImplementedError("Required OS params not defined.")

Expand All @@ -74,7 +77,8 @@ def check_detected_os_info_parameters(cls, detected_os_info):
extra_os_info_fields, cls.__name__, detected_os_info))
return True

@abc.abstractclassmethod
@classmethod
@abc.abstractmethod
def check_os_supported(cls, detected_os_info):
raise NotImplementedError(
"OS compatibility check not implemented for tools class %s" % (
Expand Down Expand Up @@ -472,7 +476,16 @@ def _configure_cloud_init(self):
self._write_cloud_init_mods_config(cloud_cfg_mods)

if self._has_systemd_chroot():
self._enable_systemd_service("cloud-init")
try:
self._enable_systemd_service(CLOUD_INIT_SERVICE_UNIT_NAME)
except exception.CoriolisException:
LOG.warning(
"Failed to enable service unit with name "
f"{CLOUD_INIT_SERVICE_UNIT_NAME}. Trying new name.")
# NOTE(dvincze): New versions of cloud-init (>26) got its
# unit service name renamed.
self._enable_systemd_service(
CLOUD_INIT_SERVICE_UNIT_NAME_FALLBACK)

def _test_path_chroot(self, path):
# This method uses _exec_cmd_chroot() instead of SFTP stat()
Expand Down
3 changes: 2 additions & 1 deletion coriolis/osmorphing/osdetect/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def __init__(self, conn, os_root_dir, operation_timeout):
self._environment = {}
self._osdetect_operation_timeout = operation_timeout

@abc.abstractclassmethod
@classmethod
@abc.abstractmethod
def returned_detected_os_info_fields(cls):
raise NotImplementedError(
"No returned OS info fields by class '%s'" % cls.__name__)
Expand Down
15 changes: 8 additions & 7 deletions coriolis/osmorphing/ubuntu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ def check_os_supported(cls, detected_os_info):
UBUNTU_DISTRO_IDENTIFIER):
return False

lts_releases = [12.04, 14.04, 16.04, 18.04, 20.04]
for lts_release in lts_releases:
if cls._version_supported_util(
detected_os_info['release_version'],
minimum=lts_release, maximum=lts_release):
return True
version, subversion = detected_os_info['release_version'].split('.', 1)
if int(version) % 2 != 0 or not subversion.startswith("04"):
LOG.warning(
"Detected Ubuntu release version "
f"'{detected_os_info['release_version']}' is not an LTS one. "
"Coriolis only supports morphing Ubuntu LTS versions.")
Copy link
Copy Markdown
Member

@petrutlucian94 petrutlucian94 May 13, 2026

Choose a reason for hiding this comment

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

So we're completely dropping support for non-LTS Ubuntu releases (at the moment it accepts non-LTS versions newer than 22.04).

That's reasonable, however let's clearly state it in the PR message.

This patch makes sure that the base Ubuntu osmorphing tool checks
for the migrated machine's subversion, and makes sure that it's an
LTS subversion, instead of checking each LTS version separately.

As it's stated now, it implies that we're rewriting the version check, not that we're completely dropping support for non-LTS releases. Simply saying This patch drops os-morphing support for non-LTS Ubuntu releases. would be much clearer.

Copy link
Copy Markdown
Contributor Author

@Dany9966 Dany9966 May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We never officially supported non-LTS versions, see the old code, it literally just went through the list of LTS versions (at the time), and otherwise it refused to use any kind of other ubuntu release (if you consider the provider code as well).

Anyway, I'll clarify the PR and commit messages. Thank you!

Copy link
Copy Markdown
Member

@petrutlucian94 petrutlucian94 May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting. The current code checks if it's a LTS version. If it isn't, then it checks if it's newer than 22.04. Unless I'm mistaken, 22.10 would be reported as "supported.

This made me believe that we had the policy of supporting recent non-LTS releases.

Copy link
Copy Markdown
Contributor Author

@Dany9966 Dany9966 May 13, 2026

Choose a reason for hiding this comment

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

At least at the base level maybe it was an attempt at future-proofing, but the providers had specific supported versions, and they all had tools for LTS versions only.
We're trying to future-proof both the core and provider tools with these additions.

return False

return cls._version_supported_util(
detected_os_info['release_version'], minimum=22.04)
detected_os_info['release_version'], minimum=12.04)

def _set_netplan_ethernet_configs(
self, nics_info, dhcp=False, iface_name_prefix=None):
Expand Down
12 changes: 8 additions & 4 deletions coriolis/tasks/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,17 @@ def get_shared_libs_for_providers(

return required_libs

@abc.abstractclassmethod
@classmethod
@abc.abstractmethod
def get_required_task_info_properties(cls):
""" Returns a list of the string fields which are required
to be present during the tasks' run method. """
raise NotImplementedError(
"No required task info properties specified for task class of "
"type '%s'." % cls)

@abc.abstractclassmethod
@classmethod
@abc.abstractmethod
def get_returned_task_info_properties(cls):
""" Returns a list of the string fields which are returned by the
tasks' run method to be added to the task info.
Expand All @@ -65,7 +67,8 @@ def get_returned_task_info_properties(cls):
"No returned task info properties specified for task class of "
"type '%s'." % cls)

@abc.abstractclassmethod
@classmethod
@abc.abstractmethod
def get_required_provider_types(cls):
""" Returns a dict with 'source/destination' as keys containing a list
of all the provider types (constants.PROVIDER_TYPE_*) required for the
Expand All @@ -75,7 +78,8 @@ def get_required_provider_types(cls):
"No required provider types specified for task class of "
"type '%s'." % cls)

@abc.abstractclassmethod
@classmethod
@abc.abstractmethod
def get_required_platform(cls):
""" Returns whether the task operates on the source platform, the
destination, or both. (constants.TASK_PLATFORM_*)
Expand Down
23 changes: 14 additions & 9 deletions coriolis/tests/osmorphing/test_ubuntu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
import logging
from unittest import mock

import ddt

from coriolis.osmorphing import base
from coriolis.osmorphing import ubuntu
from coriolis.tests import test_base


@ddt.ddt
class BaseUbuntuMorphingToolsTestCase(test_base.CoriolisBaseTestCase):
"""Test suite for the BaseUbuntuMophingTools class."""

Expand Down Expand Up @@ -37,19 +40,21 @@ def test_check_os_supported_not_supported(self):

self.assertFalse(result)

def test_check_os_supported_lts_release(self):
self.detected_os_info['release_version'] = '20.04'

result = ubuntu.BaseUbuntuMorphingTools.check_os_supported(
self.detected_os_info)

self.assertTrue(result)
@ddt.data(
("20.04", True),
("20.04.2 LTS", True),
("20.10", False),
("20.054", False),
("19.04", False),
)
@ddt.unpack
def test_check_os_supported_lts_release(self, release_version, expected):
self.detected_os_info['release_version'] = release_version

def test_check_os_supported_non_lts_release(self):
result = ubuntu.BaseUbuntuMorphingTools.check_os_supported(
self.detected_os_info)

self.assertTrue(result)
self.assertEqual(expected, result)

@mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd')
@mock.patch.object(base.BaseLinuxOSMorphingTools, '_write_file_sudo')
Expand Down
Loading