Allow predefined VM UUIDs

[dahoat-sprecher]

Hello to All,

Although guest.xml.j2 read the variable vm.uuid, this value was later discarded by vm_manager. This behavior is fixed, permitting redeployment of applications binding their licenses to this UUID.

Best regards,
Daniel

[dupremathieu]

We regenerate the UUID to avoid conflicts with existing VM.

If we want to be sure there is no conflict we have to list all VM UUID in all machines to check if the UUID is available. We could do it easily with SEAPATH VM but for VM deploy with libvirt directly we cannot detect it unless to have a libvirt connection to each machine which is for the moment optional.

I know that latest version of libvirt allow overriding the UUID which is accessible inside the VM without changing the VM UUID but as far as know it is not supported in our libvirt version.

@eroussy @insatomcat is it ok for you to only check for SEAPATH UUID? I've made a POC of this solution: 0001-Add-UUID-collision-detection-for-predefined-VM-UUIDs.patch

After testing this can lead to unclear error if there is already a VM with the UUID deployed in the cluster.

[eroussy]

@eroussy @insatomcat is it ok for you to only which for SEAPATH UUID? I've made a POC of this solution: 0001-Add-UUID-collision-detection-for-predefined-VM-UUIDs.patch

I'm not sure to understand your question but direct Libvirt deployment should not be a problem because

• We should use vm-manager even in standalone (still to test)
• Libvirt would fail with a meaningful message

IMO, the real use case/problem is vm-manager deployement in cluster mode

[dupremathieu]

@eroussy @insatomcat is it ok for you to only which for SEAPATH UUID? I've made a POC of this solution: 0001-Add-UUID-collision-detection-for-predefined-VM-UUIDs.patch

I'm not sure to understand your question but direct Libvirt deployment should not be a problem because

* We should use vm-manager even in standalone (still to test)

* Libvirt would fail with a meaningful message

IMO, the real use case/problem is vm-manager deployement in cluster mode

@eroussy The only use case that cause trouble is in cluster if you have deployed outside SEAPATH a libvirt VM with the case UUID that the VM we want to deploy.

If you have use vm-manager we could detect it (cf my patch).

My question is do we accept this limitation?

[insatomcat]

Thanks for the discussion and the POC patch @dupremathieu.

I think we should keep this simple: if a user explicitly sets a vm.uuid in their configuration, they are opting into manual UUID management and therefore take full responsibility for ensuring that UUID is unique across the cluster. This is a deliberate, advanced use case — the whole point is to preserve a specific UUID for license-binding purposes, which implies the user already knows exactly what UUID they are working with.

Adding collision detection adds complexity and, as noted, cannot be made fully reliable anyway (e.g. VMs deployed directly via libvirt outside of SEAPATH). A partial check could even give users a false sense of safety.

I'd suggest we document this clearly — something like: "When a predefined UUID is specified, the user is responsible for guaranteeing its uniqueness in the target environment. No collision detection is performed." — and proceed with merging the current PR as-is.

[eroussy]

I'd suggest we document this clearly — something like: "When a predefined UUID is specified, the user is responsible for guaranteeing its uniqueness in the target environment. No collision detection is performed." — and proceed with merging the current PR as-is.

I can agree with that. But we should then also document that in the deploy_vms_cluster ansible role.
@dupremathieu is it ok for you ?

[eroussy]

@dahoat-sprecher You have one test failing in CI about uuid replacement https://github.com/seapath/vm_manager/blob/main/tests/test_vm_manager_libvirt.py#L22

Can you replace this test with two tests:

• One verifying that uuid is correct when the user specifically provides one
• Another to verify that no uuid is present when the user doesn't define any

[dahoat-sprecher]

@eroussy The current test verifies that a UUID is set which is different to the old one. Should my second test verify the same behavior if no UUID is provided by the user (UUID by vm_manager) or do we actually want no UUID so libvirt generates a new one?

[eroussy]

@eroussy The current test verifies that a UUID is set which is different to the old one. Should my second test verify the same behavior if no UUID is provided by the user (UUID by vm_manager) or do we actually want no UUID so libvirt generates a new one?

For me, the default would be for libvirt to generate a uuid when no one is provided by the user (that necessitate one test)
And then we need another test for your new feature about providing the uuid manually

[dahoat-sprecher]

I reviewed the previous state of vm_manager again to double-check (

vm_manager/vm_manager/vm_manager_libvirt.py

Line 39 in 66fa141

name_element.text = str(uuid.uuid4())

). The generation of a fresh UUID was already handled by vm_manger. Therefore, I kept this code and also rewrote the tests preserving as much original logic as possible.

Best regards,
Daniel

[eroussy]

Thanks @dahoat-sprecher
I rebased the branch on main and launched the CI

There is one last change I would like you to make on this test that was just merged yesterday : https://github.com/seapath/vm_manager/blob/main/tests/test_vm_manager_cluster.py#L255
(This test does not run on the CI, that's why it is not failing)
Then we're good to go and merging that

@dupremathieu I propose to ignore the code duplication error thrown by SonarCloud, as vm_manager_cluster.py and vm_manager_libvirt.py are already pretty similar and I don't see a simple way to change that.

[dahoat-sprecher]

Oh, i missed the file as it was introduced after I branched. 😅

Apart from test_uuid_replaced, the changes also failed test_add_to_cluster_with_new_name (https://github.com/seapath/vm_manager/blob/main/tests/test_vm_manager_cluster.py#L542) for me.
My solution would be to explicitly remove the UUID in the add_to_cluster call (https://github.com/seapath/vm_manager/blob/main/vm_manager/vm_manager_cluster.py#L380)

[sonarqubecloud]

Quality Gate failed

Failed conditions
36.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud