VM rename test and fix

[justinc1]

Integration test is added to show that renaming VM a second time (idempotence test) does fail in current main.
Problem is (in most places) we search for VM by vm_name only, but we should by vm_name xor vm_name_new.
A helper function VM.get_by_old_or_new_name does this, and is used in VM module.

PR was draft:

• (partially done) unit tests need to be updated
• done - sanity tests
• todo - run all integration tests
• vm_nic, vm_disk, vm_params modules should also be updated.
  • I update vm_params module only, because only vm_params and vm modules have vm_name_new param
• maybe we can do extra cleanup, and stop using old VM.get_by_name - I'm not sure, need to check all affected modules.
  • I didn't even try this - unit test update would be painful

But there is enough code for first review and comments

[justinc1]

Unit test problems with TestManageVMDisks - I didn't check why I have problems with TestManageVMNics, but not with TestManageVMDisks. At first look, those two tests are not that much different.

[domendobnikar]

Looks good, I will add a backlog task / issue for VM code cleanup.

• Unit test cleanup
• Get method cleanup

[domendobnikar]

Should we add a cleanup issue/task here (not to be done in this PR)?
Feels like we have almost 10 different get methods for VM and they all do roughly the same thing ...
Maybe we can refactor this into one or two methods maximum.

[justinc1]

We can have a talk about this. I'm a bit confused by those options (find_by_name(must_exists=True) vs get_or_fail(), etc).

I have a felling a more robust way would be not to search for VM by name all the time, but search for it by name 1st time, then access it by UUID.

Also, I have a felling having VM.from_hypercore(vm_hypercore_dict) accessing HC3 to get also node info and snapshot schedule info is not ideal. Passing two extra params like hypercode_nodes and hypercore_snapshot_schedules might make unit test easier to write - less magic mocking, more like dependency injection. But whole team should agree this is something worth to try, and maybe first on some smaller class, not on VM.

[justinc1]

One more note, there is problem in VM.str() implementation. It will push branch problem-with-power-state-mapping, to show this. It seems to me that we store in VM.power_state both VM power_state as retrieved from HyperCore, and sometimes also power_state as set in ansible module. But ansible power_state is actually a command describing transition to HyperCore power_state, not HyperCore power_state (e.g. you can get into SHUTOFF state by nice shutdown or by force shutdown - two different actions, same end state).

I noticed this only because I added debug module.warn. Module was not misbehaving.

It would be a bit of extra work and testing, so I just added a small workaround to not throw this error to module users.