-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
Add data-plane pre upgrade sanity checks #222
Merged
rgildein
merged 11 commits into
canonical:dev/data-plane
from
gabrielcocenza:pre_plan_sane_check
Jan 24, 2024
Merged
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
f66362a
Custom COU Namespace
gabrielcocenza b8f0e58
Add data-plane pre upgrade sane checks
gabrielcocenza fdb598e
Merge branch 'main' into cou_namespace
gabrielcocenza 5d04a32
- change from Namespace to CLIargs
gabrielcocenza 672496b
Merge branch 'cou_namespace' into pre_plan_sane_check
gabrielcocenza 4d5e94c
- added is_data_plane_command property on CLIargs
gabrielcocenza a0cac77
small re-word
gabrielcocenza d861643
small changes to make sanity checks more readble
gabrielcocenza a364440
Merge branch 'dev/data-plane' into pre_plan_sane_check
gabrielcocenza b98d1a4
- small re-word
gabrielcocenza a9ee8ed
- changed from is_* to verify_* because it seems that will return
gabrielcocenza File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ | |
"""Upgrade planning utilities.""" | ||
|
||
import logging | ||
from typing import Callable, Optional | ||
from typing import Callable | ||
|
||
# NOTE we need to import the modules to register the charms with the register_application | ||
# decorator | ||
|
@@ -39,6 +39,7 @@ | |
) | ||
from cou.commands import CLIargs | ||
from cou.exceptions import ( | ||
DataPlaneCannotUpgrade, | ||
HaltUpgradePlanGeneration, | ||
HighestReleaseAchieved, | ||
NoTargetError, | ||
|
@@ -53,6 +54,150 @@ | |
logger = logging.getLogger(__name__) | ||
|
||
|
||
def pre_plan_sanity_checks(args: CLIargs, analysis_result: Analysis) -> None: | ||
"""Pre checks to generate the upgrade plan. | ||
|
||
:param args: CLI arguments | ||
:type args: Namespace | ||
agileshaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
:param analysis_result: Analysis result. | ||
:type analysis_result: Analysis | ||
""" | ||
is_valid_openstack_cloud(analysis_result) | ||
is_supported_series(analysis_result) | ||
is_highest_release_achieved(analysis_result) | ||
|
||
if args.is_data_plane_command: | ||
is_data_plane_ready_to_upgrade(analysis_result) | ||
|
||
|
||
def is_valid_openstack_cloud(analysis_result: Analysis) -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naming suggests this should return a boolean ( |
||
"""Check if the model passed is a valid OpenStack cloud. | ||
|
||
:param analysis_result: Analysis result | ||
:type analysis_result: Analysis | ||
:raises NoTargetError: When cannot determine the current OS release | ||
or Ubuntu series. | ||
""" | ||
if not analysis_result.current_cloud_os_release: | ||
raise NoTargetError( | ||
"Cannot determine the current OS release in the cloud. " | ||
"Is this a valid OpenStack cloud?" | ||
) | ||
|
||
if not analysis_result.current_cloud_series: | ||
raise NoTargetError( | ||
"Cannot determine the current Ubuntu series in the cloud. " | ||
"Is this a valid OpenStack cloud?" | ||
) | ||
|
||
|
||
def is_supported_series(analysis_result: Analysis) -> None: | ||
"""Check the Ubuntu series of the cloud to see if it is supported. | ||
|
||
:param analysis_result: Analysis result. | ||
:type analysis_result: Analysis | ||
:raises OutOfSupportRange: When series is not supported. | ||
""" | ||
supporting_lts_series = ", ".join(LTS_TO_OS_RELEASE) | ||
current_series = analysis_result.current_cloud_series | ||
if current_series not in LTS_TO_OS_RELEASE: | ||
raise OutOfSupportRange( | ||
f"Cloud series '{current_series}' is not a Ubuntu LTS series supported by COU. " | ||
f"The supporting series are: {supporting_lts_series}" | ||
) | ||
|
||
|
||
def is_highest_release_achieved(analysis_result: Analysis) -> None: | ||
"""Check if the highest OpenStack release is reached by the ubuntu series. | ||
agileshaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
:param analysis_result: Analysis result. | ||
:type analysis_result: Analysis | ||
:raises HighestReleaseAchieved: When the OpenStack release is the last supported by the series. | ||
""" | ||
current_os_release = analysis_result.current_cloud_os_release | ||
current_series = analysis_result.current_cloud_series | ||
if ( | ||
current_series | ||
and current_series | ||
and str(current_os_release) == LTS_TO_OS_RELEASE.get(current_series, [])[-1] | ||
): | ||
raise HighestReleaseAchieved( | ||
f"No upgrades available for OpenStack {str(current_os_release).capitalize()} on " | ||
f"Ubuntu {current_series.capitalize()}.\nNewer OpenStack releases " | ||
"may be available after upgrading to a later Ubuntu series." | ||
) | ||
|
||
|
||
def is_data_plane_ready_to_upgrade(analysis_result: Analysis) -> None: | ||
"""Check if data plane is ready to upgrade. | ||
|
||
To be able to upgrade data-plane, first all control plane apps should be upgraded. | ||
|
||
:param analysis_result: Analysis result | ||
:type analysis_result: Analysis | ||
:raises DataPlaneCannotUpgrade: When data-plane is not ready to upgrade. | ||
""" | ||
if not analysis_result.min_os_version_data_plane: | ||
raise DataPlaneCannotUpgrade( | ||
"Cannot find data-plane apps. Is this a valid OpenStack cloud?" | ||
) | ||
if not is_control_plane_upgraded(analysis_result): | ||
raise DataPlaneCannotUpgrade("Please, upgrade control-plane before data-plane") | ||
|
||
|
||
def is_control_plane_upgraded(analysis_result: Analysis) -> bool: | ||
"""Check if control plane is already upgraded. | ||
agileshaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Control-plane will be considered as upgraded when the OpenStack version of it | ||
is bigger than the data-plane. | ||
agileshaw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
:param analysis_result: Analysis result | ||
:type analysis_result: Analysis | ||
:return: Whether the control plane is already upgraded or not. | ||
:rtype: bool | ||
""" | ||
control_plane = analysis_result.min_os_version_control_plane | ||
data_plane = analysis_result.min_os_version_data_plane | ||
|
||
return bool(control_plane and data_plane and control_plane > data_plane) | ||
|
||
|
||
def determine_upgrade_target(analysis_result: Analysis) -> OpenStackRelease: | ||
"""Determine the target release to upgrade to. | ||
|
||
:param analysis_result: Analysis result. | ||
:type analysis_result: Analysis | ||
:raises NoTargetError: When cannot find target to upgrade | ||
:raises OutOfSupportRange: When the upgrade scope is not supported | ||
by the current series. | ||
:return: The target OS release to upgrade the cloud to. | ||
:rtype: OpenStackRelease | ||
""" | ||
if ( | ||
(current_os_release := analysis_result.current_cloud_os_release) | ||
and (current_series := analysis_result.current_cloud_series) | ||
and not (target := current_os_release.next_release) | ||
): | ||
rgildein marked this conversation as resolved.
Show resolved
Hide resolved
|
||
raise NoTargetError( | ||
"Cannot find target to upgrade. Current minimum OS release is " | ||
f"'{str(current_os_release)}'. Current Ubuntu series is '{current_series}'." | ||
) | ||
|
||
if ( | ||
current_series | ||
and (supporting_os_release := ", ".join(LTS_TO_OS_RELEASE[current_series])) | ||
and str(current_os_release) not in supporting_os_release | ||
or str(target) not in supporting_os_release | ||
): | ||
raise OutOfSupportRange( | ||
f"Unable to upgrade cloud from Ubuntu series `{current_series}` to '{target}'. " | ||
"Both the from and to releases need to be supported by the current " | ||
f"Ubuntu series '{current_series}': {supporting_os_release}." | ||
) | ||
|
||
return target # type: ignore | ||
|
||
|
||
async def generate_plan(analysis_result: Analysis, args: CLIargs) -> UpgradePlan: | ||
"""Generate plan for upgrade. | ||
|
||
|
@@ -63,9 +208,8 @@ async def generate_plan(analysis_result: Analysis, args: CLIargs) -> UpgradePlan | |
:return: Plan with all upgrade steps necessary based on the Analysis. | ||
:rtype: UpgradePlan | ||
""" | ||
target = determine_upgrade_target( | ||
analysis_result.current_cloud_os_release, analysis_result.current_cloud_series | ||
) | ||
pre_plan_sanity_checks(args, analysis_result) | ||
target = determine_upgrade_target(analysis_result) | ||
|
||
plan = UpgradePlan( | ||
f"Upgrade cloud from '{analysis_result.current_cloud_os_release}' to '{target}'" | ||
|
@@ -149,75 +293,6 @@ async def create_upgrade_group( | |
return group_upgrade_plan | ||
|
||
|
||
def determine_upgrade_target( | ||
current_os_release: Optional[OpenStackRelease], current_series: Optional[str] | ||
) -> OpenStackRelease: | ||
"""Determine the target release to upgrade to. | ||
|
||
Inform user if the cloud is already at the highest supporting release of the current series. | ||
:param current_os_release: The current minimum OS release in cloud. | ||
:type current_os_release: Optional[OpenStackRelease] | ||
:param current_series: The current minimum Ubuntu series in cloud. | ||
:type current_series: Optional[str] | ||
:raises NoTargetError: When cannot find target to upgrade. | ||
:raises HighestReleaseAchieved: When the highest possible OpenStack release is | ||
already achieved. | ||
:raises OutOfSupportRange: When the OpenStack release or Ubuntu series is out of the current | ||
supporting range. | ||
:return: The target OS release to upgrade the cloud to. | ||
:rtype: OpenStackRelease | ||
""" | ||
if not current_os_release: | ||
raise NoTargetError( | ||
"Cannot determine the current OS release in the cloud. " | ||
"Is this a valid OpenStack cloud?" | ||
) | ||
|
||
if not current_series: | ||
raise NoTargetError( | ||
"Cannot determine the current Ubuntu series in the cloud. " | ||
"Is this a valid OpenStack cloud?" | ||
) | ||
|
||
# raise exception if the series is not supported | ||
supporting_lts_series = ", ".join(LTS_TO_OS_RELEASE) | ||
if current_series not in supporting_lts_series: | ||
raise OutOfSupportRange( | ||
f"Cloud series '{current_series}' is not a Ubuntu LTS series supported by COU. " | ||
f"The supporting series are: {supporting_lts_series}" | ||
) | ||
|
||
# Check if the release is the "last" supported by the series | ||
if str(current_os_release) == LTS_TO_OS_RELEASE[current_series][-1]: | ||
raise HighestReleaseAchieved( | ||
f"No upgrades available for OpenStack {str(current_os_release).capitalize()} on " | ||
f"Ubuntu {current_series.capitalize()}.\nNewer OpenStack releases may be available " | ||
"after upgrading to a later Ubuntu series." | ||
) | ||
|
||
# get the next release as the target from the current cloud os release | ||
target = current_os_release.next_release | ||
if not target: | ||
raise NoTargetError( | ||
"Cannot find target to upgrade. Current minimum OS release is " | ||
f"'{str(current_os_release)}'. Current Ubuntu series is '{current_series}'." | ||
) | ||
|
||
supporting_os_release = ", ".join(LTS_TO_OS_RELEASE[current_series]) | ||
# raise exception if the upgrade scope is not supported by the current series | ||
if ( | ||
str(current_os_release) not in supporting_os_release | ||
or str(target) not in supporting_os_release | ||
): | ||
raise OutOfSupportRange( | ||
f"Unable to upgrade cloud from `{current_series}` to '{target}'. Both the from and " | ||
f"to releases need to be supported by the current Ubuntu series '{current_series}': " | ||
f"{supporting_os_release}." | ||
) | ||
|
||
return target | ||
|
||
|
||
def manually_upgrade_data_plane(analysis_result: Analysis) -> None: | ||
"""Warning message to upgrade data plane charms if necessary. | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be documented that this works by raising a particular set of exceptions if a check fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see little benefit on doing this. In
Java
for example, the checked exception forced the signature of every method would list all of the exceptions that it could pass to its caller and has a high price. If we start getting deep into this we could have a situation where we would need to go three or more levels checking all the exceptions that can happen.On theory a change at a low level of the software would imply on updating on the higher level and I don't think it worth.
If you have the Clean Code book, chapter 7 has a session talking about this that worth the reading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelengineer that's good point, I lost myself with warlock operators in this PR.
@gabrielcocenza I see your point, however all those
verify_*
function are simple helper functions, and we expected this function to raise those exceptions. If I look at the function namepre_plan_sanity_checks
, and then the description, I could not tell what this function actually do? It's not returning anything and not mention raising anything.Please stop referring to that book (I'm going to buy and read it, because there could be good point). I believe that you're misinterpreting things from it, or it has written not for Python. Thanks