Allows for the mounting of ISOs when a Proxmox VM is created. Useful …

[jahmelharris]

Allows for the mounting of additional ISOs when the VM is created. A new config option has been created which allows users to specify an array of bus names and filenames.

"cd_drive": [
{
"bus": "sata1",
"filename": "disk-images:iso/kali-linux-mate-2018.4-amd64.iso"
},
{
"bus": "sata2",
"filename": "disk-images:iso/unattended-winserver.iso"
}
],

Closes #7950

[hashicorp-cla]

All committers have signed the CLA.

[carlpett]

Hi @jahmelharris,
Thanks for the contribution! However, it doesn't quite interact with other iso-related functionality, like uploading iso files to the server, and ejecting files afterwards, which I think we'll need to cover.

One option would be looking at generalising the iso_file configuration to handle multiple drives. Another would be keeping the separate config for additional attachments, but then reimplementing the uploading and unmounting.

What are your thoughts?

[jahmelharris]

Hi @carlpett. I think upload is a good idea, but i'm not sure whether it makes sense to merge everything ISO related. While it makes sense in theory, as the proxmox API doesn't support creating multiple CD or disk drives when creating a VM, in my mind they're separate steps. Create VM; add drives. I'll look at adding the ability to upload ISOs and submit another PR.

[carlpett]

@jahmelharris Yes, when interacting with the Go library to create the VM, they will indeed need to be separated. However, the library has quite a few warts that we should try to avoid letting guide what we expose to the Packer user. At some point, it might be swapped (hopefully), and then it'd be sad to have to be "wart compatible".

I think it should mainly come down to if the configuration is better either way? Think out loud a bit, I guess it'd end up something like these two options:

"iso_file": "local:iso/foo.iso",
"additional_iso_files": [
  {
    "device": "sata1",
    "iso_file": "local:iso/bar.iso"
  }
]

vs

"iso_files": [
  {
    "device": "special-boot?",
    "iso_file": "local:iso/foo.iso",
  },
  {
    "device": "sata1",
    "iso_file": "local:iso/bar.iso"
  }
]

And from this look, keeping them separate does makes a bit more sense to me. Since there's special handling of the boot iso, it'd actually make the config a bit confusing.

[SecSamDev]

If this feature will be used to build Windows images, then the CD-ISO is not the correct option, because we cannot know the letter that Windows will assign to the CD drive. In contrast, a floppy disk always has the letter ** A **. Following the configuration set by the QEMU builder, a ** floppy_files ** key is the ideal choice.

The autounattend.xml needs to know the drive letter to load the drivers.

<component name="Microsoft-Windows-PnpCustomizationsWinPE" processorArchitecture="amd64" publicKeyToken="31bf3856ad364e35" language="neutral" versionScope="nonSxS" xmlns:wcm="http://schemas.microsoft.com/WMIConfig/2002/State" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <DriverPaths>
        <PathAndCredentials wcm:keyValue="1" wcm:action="add">
            <Path>A:\</Path>
        </PathAndCredentials>
    </DriverPaths>
</component>

The QEMU template needs:
args: -fda /path/to/floppy.img

[jahmelharris]

From what I could see when looking into this, proxmox doesn't support floppy disks (via the API). I see what you're saying about drive letters, but should it be a problem that packer tries to solve? If i'm building a Windows image, I should know what the driver letters would be and can use them appropriately.

[SecSamDev]

You cant know before hand the letter assigned by windows, but now that i think of it, i can replace the "Path" With a simple dot making the drivers relative to the autounattend location.
In that case, a multi iso loading will be okey.

[carlpett]

@jahmelharris I lost track of this one, sorry. From what I can tell, there's a build failure that needs to be addressed, at least. Would you mind adding the uploads/ejection in this PR? Since the Packer releases aren't paced based on the state of the Proxmox builder (as long as it builds), we'll risk releasing this feature half-complete otherwise.
I'll also leave some more comments in the code shortly.

[carlpett]

Some naming things

[carlpett]

Can we rename this something like additional_iso_files?

[carlpett]

This should be a device, not a bus

[nywilken]

Hi @jahmelharris just wanted to follow up on the changes requested by @carlpett. Do you have any questions? Please let us know if you still have bandwidth to work on this feature and if there is anything we can do to help push the PR forward. Thanks again for the contribution.

[jahmelharris]

Hi @nywilken. Unfortunately life got in the way, but will aim to pick this up again next week. I'll keep you updated.

[SwampDragons]

Should I close this PR for now?

[SwampDragons]

Closing since it's been a month with no activity.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.