bpg / terraform-provider-proxmox

Terraform Provider for Proxmox
https://registry.terraform.io/providers/bpg/proxmox
Mozilla Public License 2.0
830 stars 137 forks source link

Intent to contribute: ISO generation for cloud-init disks #1389

Open ZauberNerd opened 4 months ago

ZauberNerd commented 4 months ago

Is your feature request related to a problem? Please describe. The proxmox_virtual_environment_file resource requires SSH access to the node when creating and uploading a cloud-init snippet.

Describe the solution you'd like I would like to reduce usage of SSH for provisioning resources, thus I propose to use the https://github.com/kdomanski/iso9660 library to create an ISO image on the fly (basically what's happening here: https://github.com/Telmate/terraform-provider-proxmox/blob/186ec3f23bf4a62fcad35f6292fa1350b8e1183b/proxmox/resource_cloud_init_disk.go#L77-L122)

Describe alternatives you've considered The information:

Consider using proxmox_virtual_environment_download_file resource instead. [...]

On the proxmox_virtual_environment_file docs is good advice for downloading OS installer ISOs directly onto the node, but for cloud-init disks you usually would want to create them locally from a yaml file or inline specification.

Additional context I'm happy to create a pull request for this feature, if desirable.

bpg commented 4 months ago

Hi @ZauberNerd! 👋🏼

That's an interesting idea! While I'm not opposed to this approach, it won't solve the main problem you're aiming at: eliminating SSH access. We still need SSH to run the custom import command for VM disks, as the PVE API does not allow importing, for example, a qcow2 image stored in the iso storage using import-disk attribute.

Would you mind sketching an example of a VM resource using this new approach, so we can discuss the details?

I'm also about to start experimenting with the implementation of the initialization section for the new VM2 resource. It would be nice to flesh out some design points around cloud-init.

ZauberNerd commented 4 months ago

Yes, unfortunately it doesn't solve other file upload issues. As far as I understand, only .iso and .vztmpl can be uploaded, right (https://pve.proxmox.com/pve-docs/api-viewer/#/nodes/{node}/storage/{storage}/upload)? I just checked and the UI creates a raw disk, which can then be configured via the UI. This won't be the case with my proposed solution.

My idea was to specifically target the cloud-init generation:

This would allow to at least eliminate SSH access for a common use case of provisioning cloud-init based VMs.

bpg commented 4 months ago

Hey @ZauberNerd !

I just checked and the UI creates a raw disk, which can then be configured via the UI. This won't be the case with my proposed solution.

Yes, and there will be a possibility of having two conflicting cloud-init configs, one from the custom build iso file you're proposing, and another that comes from the initialization section.

I would prefer to have the new "cloud-init-iso" configuration wrapped up in the initialization as well. That way we can have some schema rules that could prevent declaring PVE-native cloud-init params when a custom coloud-init files are there.

I'm thinking about a structure like:

initailization {
  datastore_id = ...
  interface = ...

  # all other existing attributes
  # but if present alongside the `iso` then provider will throw an error

  iso {
    meta_data = <<-EOT
    ...
    EOT

    user_data = ....
    vendor_data = ....
    ... 
  }

WDYT?

ZauberNerd commented 4 months ago

@bpg I like it. But do we then still need the iso block or could we make meta_data, user_data and vendor_data mutually exclusive to other parameters? I'm not too well versed in terraform api design, so not sure, if that is a good pattern or not?

bpg commented 3 months ago

Yes, we can define constraints on the attribute to make them mutually exclusive. Actually that's what I meant by "throw an error". Provider could do the schema validation when parsing a config, and flag attributes that overwrite each other.

bpg commented 3 months ago

There is also a good thread with lots of details about cloud-init behaviour in different cases.