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

providers: support for proxmoxve #1023

Merged
merged 1 commit into from
Jun 19, 2024
Merged

providers: support for proxmoxve #1023

merged 1 commit into from
Jun 19, 2024

Conversation

arcln
Copy link
Contributor

@arcln arcln commented Dec 19, 2023

Hello,

This PR adds support for reading the cloud init drive from a ProxmoxVE instance.
The code mounts /dev/disk/by-label/cidata to a temprary location and then parses the YAML files present inside.
It supports : setting hostname, writing ssh keys, and writing systemd network configuration.

The changes introduces a new OEM ID proxmoxve that will be matched from the kernel command line. As time of writing, this OEM ID is not yet recognized by Ignition or other tools.

We currently maintain a build script that modifies a Flatcar image to inject this version of Afterburn : https://github.com/enix/flatcar-proxmoxve

Feedback is welcome :)

@jlebon
Copy link
Member

jlebon commented Jan 15, 2024

Thanks for the patch! For reference, Proxmox support has also been discussed in Fedora CoreOS in coreos/fedora-coreos-tracker#736. And while it's not currently being actively pursued, I don't think that should prevent support for it to land in Afterburn. That said, I've tagged the ticket to be discussed again in the next community meeting, at the very least so that we can agree on the platform ID. Feel free to join if you're able!

Are you planning to also implement the Ignition side of this?

@travier
Copy link
Member

travier commented Jan 22, 2024

@abuisine
Copy link

abuisine commented Jan 22, 2024

Are you planning to also implement the Ignition side of this?

Hi @jlebon, can you please precise what would be required on the ignition side ?
We would be happy to help (with @arcln)

@abuisine
Copy link

Hi @jlebon,
Do you think this PR can be merged "as is" as is does not prevent other implementations like ignition / Fedora support of proxmox ?
It is my impression that the provider id is settled and that it was the principal show stopper.

Cheers.

@abuisine
Copy link

for the record we are implementing the ignition support of the proxmoxve provider id cc @jlebon

pothos added a commit to flatcar/bootengine that referenced this pull request Mar 22, 2024
This makes use of coreos/afterburn#1023
to set up any static networking from the initrd (for Ignition) and the
hostname (early enough so that Ignition could overwrite it).
pothos added a commit to flatcar/init that referenced this pull request Mar 22, 2024
This makes use of coreos/afterburn#1023
to set up the ssh keys from the metadata.
pothos added a commit to flatcar/scripts that referenced this pull request Mar 22, 2024
This uses coreos/afterburn#1023 to provide
Proxmox images. TODO: Add Ignition PR.
pothos added a commit to flatcar/scripts that referenced this pull request Mar 22, 2024
This uses coreos/afterburn#1023 to provide
Proxmox images. TODO: Add Ignition PR.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup.
pothos added a commit to flatcar/scripts that referenced this pull request Mar 22, 2024
This uses coreos/afterburn#1023 to provide
Proxmox images. TODO: Add Ignition PR.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup.
pothos added a commit to flatcar/scripts that referenced this pull request Mar 22, 2024
This uses coreos/afterburn#1023 to provide
Proxmox images. TODO: Add Ignition PR.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup.
@pothos
Copy link
Contributor

pothos commented Mar 27, 2024

From the Flatcar side we would also be interested to have this.
I don't know if there is any specification about config drive contents from proxmox, but it looks like the user-data at least here is expected to be cloud-config to extract the ssh key and hostname. This conflicts with Ignition. Could this get moved to meta-data? I don't have a clue on what API proxmox provides to set this up. If none, then I guess we have to live with no ssh and hostname support in afterburn and let the user specify this in Ignition.

One more note about network units: This seems to be netplan - do you want to focus on a subset or support the full netplan spec? Recently I added a --netplan-config flag which only has the scope of copying this to, e.g., /run/netplan/ and then we could run netplan to convert it to NetworkManager/networkd units.

@abuisine
Copy link

From the Flatcar side we would also be interested to have this. I don't know if there is any specification about config drive contents from proxmox, but it looks like the user-data at least here is expected to be cloud-config to extract the ssh key and hostname. This conflicts with Ignition. Could this get moved to meta-data? I don't have a clue on what API proxmox provides to set this up. If none, then I guess we have to live with no ssh and hostname support in afterburn and let the user specify this in Ignition.

One more note about network units: This seems to be netplan - do you want to focus on a subset or support the full netplan spec? Recently I added a --netplan-config flag which only has the scope of copying this to, e.g., /run/netplan/ and then we could run netplan to convert it to NetworkManager/networkd units.

We should find a way to support ignition in user_data but we should keep at the same time compatibility with "cloud-init settings" provided through the proxmox API.
I guess there is a way to have afterburn consume hostname and ssh keys if there are present, with a fallback to ignition if user_data is actually an ignition json.

@jlebon
Copy link
Member

jlebon commented Apr 12, 2024

According to the discussions in coreos/fedora-coreos-tracker#1652 and coreos/fedora-coreos-tracker#736, it's possible to provide an Ignition config but currently quite awkward to do.

I think this path is probably the best compromise, though maybe let's discuss in coreos/fedora-coreos-tracker#1652 since that's where all the context currently is?

@jlebon
Copy link
Member

jlebon commented Apr 23, 2024

Still need to review this, but can we add a new docs page (e.g. under https://coreos.github.io/afterburn/usage/) to describe the situation on this platform a bit more? Basically:

  • we're parsing the cloud-init config provided by Proxmox
  • we don't generically support custom cloud-init data
  • we no-op if the data isn't cloud-init
  • we only support specific keys from the cloud-init config
  • any limitations re. network-config that you may think of as it relates to systemd-networkd

@pothos
Copy link
Contributor

pothos commented Apr 24, 2024

FYI: I don't think we can use this in Flatcar when it relies on cloud-init userdata because Flatcar will run coreos-cloudinit anyway if it doesn't see Ignition user-data and then we would have it processed twice. For the network config that can be quite confusing because the afterburn network unit would only support a subset.

@abuisine
Copy link

abuisine commented Apr 24, 2024

FYI: I don't think we can use this in Flatcar when it relies on cloud-init userdata because Flatcar will run coreos-cloudinit anyway if it doesn't see Ignition user-data and then we would have it processed twice. For the network config that can be quite confusing because the afterburn network unit would only support a subset.

I will let @arcln answer for coreos-cloudinit and focus my answer on the network side of things.

Proxmoxve has a pretty limited Software Defined Networking (since the v7.4 as far as I can remember). To my knowledge, this is not widely used. This will probably improve in the future.
Most users today :

  1. either build a standalone network with DHCP server (not provided by PromoxVE)
  2. or provide static ip configuration through the cloud-init mechanism, or any other medium.

For this second option, on the ignition side of things ... my understanding is that network should be (must be ?) configured prior to ignition being launched. I remember having a conversation on systemd-sysext extensions for flatcar where an http url was used. Ignition was processing it so early that it was done before static network configuration. Systemd-sysext where therefore not installable from a network repo through Ignition.
We finally had to add kernel command lines in order to configure the network before ... which is not very practical to say the least.

A better option may be:
The network part of the cloud-config is applied through Afterburn (or another mechanism) prior to Ignition being run.
This PR includes network configuration for this reason.

It is not clear to me if Flatcar fully endorse Afterburn, could you confirm ?

@pothos
Copy link
Contributor

pothos commented Apr 24, 2024

The static IP addr setup through the kernel args inside Ignition is only needed if Ignition should download something. Using afterburn for that would only be possible if there was support for proper metadata at Proxmox VE. With the proposed approach it wouldn't work to use afterburn because the userdata is Ignition JSON then ;)

If you have a chance of getting into discussions with Proxmox to define metadata separate of userdata, that would be nice. Even if it would be cloud-config-based metadata as done for VMware, because there one can use the netplan CLI option in afterburn to extract this (A supported afterburn subset might also be ok as alternative) and one could then get initrd networking that way while using Ignition userdata.

@abuisine
Copy link

The static IP addr setup through the kernel args inside Ignition is only needed if Ignition should download something. Using afterburn for that would only be possible if there was support for proper metadata at Proxmox VE. With the proposed approach it wouldn't work to use afterburn because the userdata is Ignition JSON then ;)

If you have a chance of getting into discussions with Proxmox to define metadata separate of userdata, that would be nice. Even if it would be cloud-config-based metadata as done for VMware, because there one can use the netplan CLI option in afterburn to extract this (A supported afterburn subset might also be ok as alternative) and one could then get initrd networking that way while using Ignition userdata.

I do not specifically agree, let me precise my throughts :
The iso generated by ProxmoxVE contains multiple files including userdata AND network-config, so you can have ignition content in userdata and network settings from ui or api in network-config. This would work ... and is actually exactly what we are trying to achieve. This PR consumes the network-config file.

Also some users would override userdata with ignition configuration, but some others won't. Again this PR should work ok in both cases.

@arcln
Copy link
Contributor Author

arcln commented Apr 24, 2024

From my understanding, coreos-cloudinit is only executed if the OEM ID matches a white list. In our case the OEM ID is proxmoxve so it shouldn't run.

Also coreos-cloudinit may not read cloud init config from Proxmox VE because it looks for a file called user_data instead of user-data.

@pothos
Copy link
Contributor

pothos commented Apr 24, 2024

The iso generated by ProxmoxVE contains multiple files including userdata AND network-config

Ah, right, it has two separate files and already roughly follows https://cloudinit.readthedocs.io/en/latest/reference/datasources/vmware.html#walkthrough-of-guestinfo-keys-transport similar to the case with the special vmware guestinfo key. One could ask proxmox to set local-hostname: (or instance-id:?) in the network-config so that this would work from Afterburn if Ignition user-data is used. Only ssh keys would then have to be put into Ignition user-data.

Also coreos-cloudinit may not read cloud init config from Proxmox VE because it looks for a file called user_data instead of user-data.

Thanks for checking that, yes, the config drive mechanism will be triggered on Flatcar if not explicitly turned off. We could decide to stick to your afterburn support instead of handling this in coreos-cloudinit and thus say that coreos-cloudinit won't be supported on this platform. What I think would be a compromise for Flatcar is to use afterburn for the network-config file and leave the user-data file to either Ignition or coreos-cloudinit.

Anyway, since afterburn is always opt-in for distros it won't hurt to merge the PR (after the DHCP=yes case is fixed). Note that FCOS doesn't use networkd and thus it would be nice to also wire up the netplan flag here in case someone wants to use afterburn for network setup later, see

pub fn parse_netplan_config(&self) -> Result<Option<String>> {

@arcln
Copy link
Contributor Author

arcln commented Apr 29, 2024

Note that FCOS doesn't use networkd and thus it would be nice to also wire up the netplan flag here in case someone wants to use afterburn for network setup later

We agree that netplan support is important for Afterburn to be working on various OS. I will work on that in the next few days.

pothos added a commit to flatcar/scripts that referenced this pull request May 6, 2024
This uses coreos/afterburn#1023 and
coreos/ignition#1790 to provide
Proxmox images.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup. The afterburn
support for the SSH key and hostname parses the user-data when it's
cloud-init. The coreos-cloudinit support is not there but can be added
in addition: We need to add a new provider that varies from the existing
config drive support because the file is called user-data and not
user_data, and it needs to look for a filesystem label cidata and not
config-2.
pothos added a commit to flatcar/init that referenced this pull request May 6, 2024
This makes use of coreos/afterburn#1023
to set up the ssh keys from the metadata.
pothos added a commit to flatcar/bootengine that referenced this pull request May 6, 2024
This makes use of coreos/afterburn#1023
to set up any static networking from the initrd (for Ignition) and the
hostname (early enough so that Ignition could overwrite it).
pothos added a commit to flatcar/scripts that referenced this pull request May 6, 2024
This uses coreos/afterburn#1023 and
coreos/ignition#1790 to provide
Proxmox images.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup. The afterburn
support for the SSH key and hostname parses the user-data when it's
cloud-init. The coreos-cloudinit support is not there but can be added
in addition: We need to add a new provider that varies from the existing
config drive support because the file is called user-data and not
user_data, and it needs to look for a filesystem label cidata and not
config-2.
tormath1 pushed a commit to flatcar/scripts that referenced this pull request May 27, 2024
This uses coreos/afterburn#1023 and
coreos/ignition#1790 to provide
Proxmox images.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup. The afterburn
support for the SSH key and hostname parses the user-data when it's
cloud-init. The coreos-cloudinit support is not there but can be added
in addition: We need to add a new provider that varies from the existing
config drive support because the file is called user-data and not
user_data, and it needs to look for a filesystem label cidata and not
config-2.
tormath1 pushed a commit to flatcar/scripts that referenced this pull request May 30, 2024
This uses coreos/afterburn#1023 and
coreos/ignition#1790 to provide
Proxmox images.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup. The afterburn
support for the SSH key and hostname parses the user-data when it's
cloud-init. The coreos-cloudinit support is not there but can be added
in addition: We need to add a new provider that varies from the existing
config drive support because the file is called user-data and not
user_data, and it needs to look for a filesystem label cidata and not
config-2.
tormath1 pushed a commit to flatcar/scripts that referenced this pull request May 30, 2024
This uses coreos/afterburn#1023 and
coreos/ignition#1790 to provide
Proxmox images.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup. The afterburn
support for the SSH key and hostname parses the user-data when it's
cloud-init. The coreos-cloudinit support is not there but can be added
in addition: We need to add a new provider that varies from the existing
config drive support because the file is called user-data and not
user_data, and it needs to look for a filesystem label cidata and not
config-2.
tormath1 pushed a commit to flatcar/scripts that referenced this pull request Jun 7, 2024
This uses coreos/afterburn#1023 and
coreos/ignition#1790 to provide
Proxmox images.

This pulls in flatcar/bootengine#91
and flatcar/init#115 to run afterburn for
hostname, network, SSH key, and metadata attribute setup. The afterburn
support for the SSH key and hostname parses the user-data when it's
cloud-init. The coreos-cloudinit support is not there but can be added
in addition: We need to add a new provider that varies from the existing
config drive support because the file is called user-data and not
user_data, and it needs to look for a filesystem label cidata and not
config-2.
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments, but this looks sane overall.

src/providers/proxmoxve/configdrive.rs Outdated Show resolved Hide resolved
src/providers/proxmoxve/configdrive.rs Outdated Show resolved Hide resolved
src/providers/proxmoxve/cloudconfig.rs Outdated Show resolved Hide resolved
src/providers/proxmoxve/cloudconfig.rs Outdated Show resolved Hide resolved
src/providers/proxmoxve/tests.rs Show resolved Hide resolved
@arcln
Copy link
Contributor Author

arcln commented Jun 17, 2024

Everything looks good to me, I think this can be merged.

@pothos asked for netplan support. I implemented it, should I push to this PR ?

@jlebon
Copy link
Member

jlebon commented Jun 18, 2024

Can you squash it down to one commit and also address the lints from CI?

@pothos asked for netplan support. I implemented it, should I push to this PR ?

Let's do that in a follow-up PR? We'll want to add docs as well for this (see the similar VMware-related netplan support).

@arcln
Copy link
Contributor Author

arcln commented Jun 19, 2024

Fixed clippy errors and squashed commits.

I'll open a netplan support PR after this one is merged.

@jlebon jlebon enabled auto-merge June 19, 2024 17:06
@jlebon
Copy link
Member

jlebon commented Jun 19, 2024

Hmm, Jenkins is having trouble merging the release notes for testing:

[2024-06-19T17:06:21.573Z] Also:   org.jenkinsci.plugins.workflow.actions.ErrorAction$ErrorId: b18e670c-7d20-4916-b2af-bda4ac6213a3
[2024-06-19T17:06:21.573Z] hudson.plugins.git.GitException: Command "git merge a9a70f2ee7d96f90aeaa7c2ca96fc8a5e2df7834" returned status code 1:
[2024-06-19T17:06:21.573Z] stdout: Auto-merging src/providers/ibmcloud_classic/mod.rs
[2024-06-19T17:06:21.573Z] Auto-merging docs/release-notes.md
[2024-06-19T17:06:21.573Z] CONFLICT (content): Merge conflict in docs/release-notes.md
[2024-06-19T17:06:21.573Z] Automatic merge failed; fix conflicts and then commit the result.

Can you rebase onto the latest main?

@jlebon
Copy link
Member

jlebon commented Jun 19, 2024

(I tried rebasing it myself, but it looks like that option to allow maintainers to push was disabled.)

allow reading cloud-init drive from proxmoxve. supports writing hostname, sshkeys and networkd configuration.
auto-merge was automatically disabled June 19, 2024 18:06

Head branch was pushed to by a user without write access

@arcln
Copy link
Contributor Author

arcln commented Jun 19, 2024

done

@jlebon jlebon enabled auto-merge June 19, 2024 18:20
@jlebon jlebon merged commit b95fcd1 into coreos:main Jun 19, 2024
9 checks passed
@abuisine
Copy link

Cool ! We will work on the netplan PR + focus on the ignition and flatcar side of things :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants