Add pre-upgrade step to verify nova-compute #280
Conversation
Add pre-upgrade step for CephOsd, which will check nova-compute upgrade status. raleted: canonical#275
cou/apps/auxiliary.py
Outdated
| @AppFactory.register_application(["ceph-osd"]) | ||
| class CephOSD(AuxiliaryApplication): | ||
| """Application for ceph-osd.""" | ||
|
|
||
| def _are_collocated(self, units: list[COUUnit], charm: str) -> bool: |
There was a problem hiding this comment.
| def _are_collocated(self, units: list[COUUnit], charm: str) -> bool: | |
| def _are_colocated(self, units: list[COUUnit], charm: str) -> bool: |
and elsewhere
There was a problem hiding this comment.
I'm ok with the current approach and the suggestions are not critical.
I think it would be nice to add a custom upgrade_plan_sanity_checks that checks if units were passed and in that case raise an Exception saying that ceph-osd doesn't accept being upgraded unit-by-unit
cou/apps/auxiliary.py
Outdated
| """ | ||
| steps = [ | ||
| PreUpgradeStep( | ||
| description="Check if colocated nova-compute units had been upgraded.", |
There was a problem hiding this comment.
| description="Check if colocated nova-compute units had been upgraded.", | |
| description="Check if all nova-compute units had been upgraded.", |
Since we add logic of units to all OpenStackApplication was introduced to all OpenStack applications, and we spoke that it's ok, I do not see why we need to raise an exception here. If you want to upgrade a single unit, it still means that all nova-computes needs to be upgraded. |
There was a problem hiding this comment.
Since we add logic of units to all OpenStackApplication was introduced to all OpenStack applications, and we spoke that it's ok, I do not see why we need to raise an exception here. If you want to upgrade a single unit, it still means that all nova-computes needs to be upgraded
OpenStack Applications have actions to upgrade unit by unit. Ceph can't do that because it doesn't have the actions necessary to do it.
Actually, I think all AuxiliaryApplication can't upgrade unit by unit, so this should be target in a different PR
|
Unit tests are failing due #286. |
| target = OpenStackRelease("victoria") | ||
| nova_compute = MagicMock(spec_set=NovaCompute)() | ||
| nova_compute.charm = "nova-compute" | ||
| nova_compute.units = {"nova-compute/0": COUUnit("nova-compute/0", None, "22.0.0")} |
There was a problem hiding this comment.
nit: I know that mocking find_compatible_versions the workload_version is irrelevant, but changing to the ussuri workload_version is preferable.
| nova_compute.units = {"nova-compute/0": COUUnit("nova-compute/0", None, "22.0.0")} | |
| nova_compute.units = {"nova-compute/0": COUUnit("nova-compute/0", None, "21.0.0")} |
There was a problem hiding this comment.
Yes, I accidentally change it base on this comment, if there will be another change-request I will change it. Thanks
Add pre-upgrade step for CephOsd, which will check nova-compute upgrade status.
raleted: #275