Proxmox VE provider #1844
Proxmox VE provider #1844
Conversation
|
Hello! I just noticed you had "draft" in your PR title. You can make this a draft PR rather then a full on pr. and when ready you can set it to be no longer a draft. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft |
| } | ||
|
|
||
| config, report, err := util.ParseConfig(f.Logger, data) | ||
| if err != nil { |
There was a problem hiding this comment.
Thanks for the PR!
I think we can keep it in line with other providers and drop this by check and directly return util.ParseConfig(f.Logger, data). This should work because at least in Flatcar the cloud-init detection is present and will result in Ignition skipping this. I don't know how FCOS with vanilla Ignition handles unknown user data content and what the desired behavior there is - e.g., maybe it makes sense to require the user to pass an empty config because the default cloud-init contents won't be supported.
| if err != nil { | ||
| // Proxmox VE will populate user-data with a cloud-init YAML config by default. | ||
| // If such config is present, we should not return an error, | ||
| // and instead just ignore it and let Afterburn pick it up later. |
There was a problem hiding this comment.
About afterburn supporting parts of the cloud-init config: I have left a comment on the afterburn PR and I think it would be more consistent and less controversial if such a feature is not introduced. To get this merged without a large discussion I would recommend to stick to the established behavior.
There was a problem hiding this comment.
Hi Pothos, cloud-init is really the by default behavior of ProxmoxVE, and is even the only supported option through the GUI.
I understand the preference for minimal disturbance, but these would basically mean that afterburn/ignition based OSes will not support ProxmoxVE. Or at the very least that API only configuration is supported.
Could we further discuss this on the Afterburn PR ?
My understanding is that this Ignition PR to support ProxmoxVE makes sense from your point of view ?
|
See also #1790. The approach here was suggested in coreos/fedora-coreos-tracker#1652 (comment) also and... maybe we'll have to do that but yuck it's really unfortunate. Can we keep discussing in coreos/fedora-coreos-tracker#1652? Let's agree on the final approach there and then make sure we don't have multiple people working on the Ignition part. |
Hello,
This PR implements the
proxmoxveprovider.The provider mounts the cloud-init drive and parses the user-data file assuming it is an ignition config.
If the file is not an ignition config, it will not fail. It will just ignore it and say that the config is empty.
That is because Proxmox VE will create a user-data by default containing some cloud-init config in YAML.
This config will be picked up by afterburn anyway, so we don't need ignition to crash for that.
This PR is the missing puzzle piece to provide Proxmox VE images for Flatcar (see flatcar/scripts#1783)
Whats missing for the PR to be ready :
I will work on that soon.