Bug 1853924

relevant for 4.3 and 4.4

As per our discussions concerning BZ 1848586, we are reusing the disk identifiers from the ovf file to insert the disks which means we are not supporting this feature where the disks with the same identifiers already exist. Unless we want to support cloning for this request we may consider closing this issue.

Well, we should avoid the exception.
We need to return a proper validation error in this case.

still happens in ovirt-engine-4.4.7.4-0.9.el8ev.noarch

Although one validation is fixed already in this bug (disks existing), the problem here is that we import a VM as a template.
The imported template uses the same ID as the VM, which cause the vm_static key violation. The validation we use is:
VmTemplate duplicateTemplate = vmTemplateDao.get(getParameters().getVmTemplate().getId());
        // check that the template does not exists in the target domain
        if (duplicateTemplate != null) {
            return failValidation(EngineMessage.VMT_CANNOT_IMPORT_TEMPLATE_EXISTS,
                    String.format("$TemplateName %1$s", duplicateTemplate.getName()));
        }
It won't find a thing, since the source is a VM. But if we will search the ID in vmStaticDao for example, we will see it's already there.
We should validate it as well. Since both templates and vms can be found in vm_static, it should be enough to check for duplicate in this table.

Liran posted https://gerrit.ovirt.org/115362 but it seems like we'd need another part to solve this bz
so moving back to ASSIGNED

Created attachment 1898029 [details]
Modified upload_ova_as_template.py script

1. When a template is imported from an OVA, all disks should be uploaded at the beginning and IDs of the disks should be referenced in the OVF.
2. Import with preloaded disks uses the template ID from the OVF, so a new template ID should be generated.

After these two modifications (see the script attached), the import finishes successfully. No changes in the Engine required.

Polina, Arik, do you consider such behaviour of REST API as correct?

(In reply to Shmuel Melamud from comment #9)
> Polina, Arik, do you consider such behaviour of REST API as correct?

yes, for the import-as-clone flow
for ordinary import flow, we need to keep the id since there might be VMs that are imported later on which are based on that template

(In reply to Arik from comment #10)
> (In reply to Shmuel Melamud from comment #9)
> > Polina, Arik, do you consider such behaviour of REST API as correct?
> 
> yes, for the import-as-clone flow
> for ordinary import flow, we need to keep the id since there might be VMs
> that are imported later on which are based on that template

agree :)

If so, I consider this bz as solved.

(In reply to Shmuel Melamud from comment #12)
> If so, I consider this bz as solved.

Shmuel, which patch addresses the last part of comment 5?

(In reply to Arik from comment #13)
> Shmuel, which patch addresses the last part of comment 5?

You mean checking the existence of the same ID so the command will fail in validation instead of DB error? You're right, let me fix that.

The last patch has been merged and it looks like everything is resolved now, so moving to MODIFIED.

verified on ovirt-engine-4.5.2-0.3.el8ev.noarch by running automation test rhevmtests.compute.virt.external_providers.ova.test_ova_sanity.TestUploadOVAasTemplate (commenting out the VM removing part  assert helper.remove_vm_sampler(self.vm_name), f"Failed to remove VM {self.vm_name} before uploading")

This bugzilla is included in oVirt 4.5.2 release, published on August 10th 2022.
Since the problem described in this bug report should be resolved in oVirt 4.5.2 release, it has been closed with a resolution of CURRENT RELEASE.
If the solution does not work for you, please open a new bug report.