Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ZauberNerd opened this issue Jun 12, 2024 · 6 comments
Open

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

ZauberNerd opened this issue Jun 12, 2024 · 6 comments
Labels
✨ enhancement New feature or request

Comments

@ZauberNerd
Copy link

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.

@ZauberNerd ZauberNerd added the ✨ enhancement New feature or request label Jun 12, 2024
@bpg
Copy link
Owner

bpg commented Jun 13, 2024

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
Copy link
Author

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:

  • add a new content_type to proxmox_virtual_environment_file (e.g. cloud-init or cloud-init-iso or something similar)
  • build an ISO9660 image on the fly via the above mentioned library with the content of source_file or source_raw and upload it

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

@bpg
Copy link
Owner

bpg commented Jun 22, 2024

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
Copy link
Author

@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
Copy link
Owner

bpg commented Jul 5, 2024

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
Copy link
Owner

bpg commented Jul 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants