Finish the vm_manager_cluster coverage and fix the two CLI flag bugs - #98
Merged
Merged
Conversation
disable_vm, start, stop, status and is_enabled are thin Pacemaker wrappers that no test reached. They now run against the fake Pacemaker, which grows show(), delete(), start() and stop(), plus a resource state and an undeletable-resource set the tests drive. list_resources() moves from the fake to the stub the cluster fixture builds, so it answers on the class as well as on an instance. The real Pacemaker declares it static, and is_enabled() calls it that way. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
_create_vm_group owns the force path that removes an existing VM and the check that the group Ceph reported creating is really there. The fake gets create_group() and a set of group names it must accept without creating, to reach that check. list_vms picks its source from the enabled flag: the Ceph groups, or the Pacemaker resources without touching Ceph at all. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
list_snapshots, list_metadata, get_metadata, set_metadata, add_colocation, add_pacemaker_remote and remove_pacemaker_remote are the last uncovered functions of the module. The fakes grow list_image_metadata() on the Ceph side and add_colocation(), add_meta() and remove_meta() on the Pacemaker side. The tests pin down what the wrappers actually decide: the name validation set_metadata, add_colocation and add_pacemaker_remote run before touching anything, the optional port and timeout that stay out of both stores when they are not given, and the KeyError remove_pacemaker_remote swallows on a VM that carries no remote configuration. With them vm_manager_cluster.py reaches 100% statement and branch coverage. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
Both assignments sat behind "enable" in args and "live_migration" in args. Neither is an argparse dest, so both guards were always false and the two flags did nothing. Assign unconditionally, the way add-to-cluster already does. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
The clone branch never set args.enable, and _configure_vm() reads a missing key as True, so --disable enabled the clone anyway. Signed-off-by: Florent Carli <florent.carli@rte-france.com>
insatomcat
changed the title
|
eroussy
approved these changes
Sep 7, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Follow-up to #97, which left fourteen functions of
vm_manager_cluster.pyuncovered.They are covered now, so the module reaches 100% statement and 100% branch coverage, with 275 unit tests instead of 228 and still no cluster. SonarCloud overall goes from 66.4% to 73.3%.
The last two commits fix the two CLI bugs #94 found and left as strict xfail.
enableandlive_migrationare never argparse dests: they are the destinationsmain()fills from--disableand--enable-live-migration. Thecreatebranch guarded both assignments behind"enable" in argsand"live_migration" in args, always false, and theclonebranch never assignedenableat all. Socreate --disable,clone --disableandcreate --enable-live-migrationdid nothing. Both branches now do whatadd-to-clusteralready did, and the three xfail markers are gone.No flag is added, removed or renamed. What changes is that three flags start having the effect their help text advertises.