OSInside / kiwi

KIWI - Appliance Builder Next Generation
https://osinside.github.io/kiwi
GNU General Public License v3.0
284 stars 144 forks source link

No support for bundle_format and multiple images with same extension as result to bundle #2123

Open gmoro opened 2 years ago

gmoro commented 2 years ago

Problem description

When using vmdk with the diskmode set to monolithicFlat qemu-img generates two vmdk files, one is the disk descriptor and the other with a suffix "-flat.vmdk" that is the actual image. With bundle_format in the xml we end up packing just one of the files and overwriting because both will have the same final name.

I'm trying to work in a fix, but need input from upstream on how to best manage that, I was thinking in extending the NamedTuple that is used for the results, possibly creating a new NamedTuple and using Generics to control the return typing that is in place, but of course would like suggestions.

schaefi commented 2 years ago

In case of monolithicFlat vmdk format I suggest the method store_to_result to add the two result files. See kiwi/storage/subformat/vmdk.py for the implementation. This will make the result files aware of the Result class and the bundle format should apply to both of them

gmoro commented 2 years ago
        if vmdisk_section:
            disk_mode = vmdisk_section.get_diskmode()
            if disk_mode == "monolithicFlat":
                filename_flat=self.get_target_file_path_for_format('vmdk')[:-5]+"-flat.vmdk"
                result.add(
                    key='disk_format_backing_image',
                    filename=filename_flat,
                    use_for_bundle=True,
                    compress=self.runtime_config.get_bundle_compression(
                        default=True
                    ),
                    shasum=True
                )

I did this, to add the file to the results, the problem is at result_bundle.py where it will use

        for result_file in list(ordered_results.values()):
            if result_file.use_for_bundle:
                extension = result_file.filename.split('.').pop()
                if bundle_file_format_name:
                    bundle_file_basename = '.'.join(
                        [bundle_file_format_name, extension]
                    )

and we end just renaming image_name-flat.vmdk to bundle_format.vmdk and then image_name.vmdk also to bundle_format.vmdk, overwriting the previous one. So the implementation in bundle_format does not understand that there are two files and not both should be renamed the same

schaefi commented 2 years ago

So the implementation in bundle_format does not understand that there are two files and not both should be renamed the same

Hrm, yes this is true. I was under the impression that all result files should apply to the same pattern and differ only in their extension. If you look at kiwi/tasks/result_bundle.py you will find this code

extension = result_file.filename.split('.').pop()
if bundle_file_format_name:
    bundle_file_basename = '.'.join(
        [bundle_file_format_name, extension]
    )

So the filename as stored in the result instance is split by '.' and the last element of it is taken as extension. If you would use ".flat_vmdk" instead of "-flat.vmdk" I could imagine it might work.

But I agree this would be a nasty workaround

gmoro commented 1 year ago

Hi, can you assign this issue to me, also, I should have a PR coming today or tomorrow

schaefi commented 1 year ago

@gmoro yep, thanks much for your effort :+1:

gmoro commented 3 months ago

I saw some new commits around this in kiwi-10 , is it fixed? :)

schaefi commented 2 months ago

Hi @gmoro, happy to hear from you, hope you are doing good ?

I saw some new commits around this in kiwi-10 , is it fixed? :)

Parts are fixed. What I fixed is that the name of files matching the bundle_format are also correctly referenced in any output files. What is not fixed is the original issue you reported here. Actually there was no work ongoing to support monolithicFlat which produces more than one output format image file.

It would be great if we can get that fixed but I'm currently busy with public cloud stuff