canonical / cloud-init

Official upstream for the cloud-init: cloud instance initialization
https://cloud-init.io/
Other
3.01k stars 887 forks source link

feat(vultr): add override for network interface detection #5847

Closed andy191x closed 3 weeks ago

andy191x commented 4 weeks ago

Proposed Commit Message

feat(vultr): add override for network interface detection (#5847)

Additional Context

The purpose of this feature is to provide the Vultr team with a way to define custom NIC lists to scan for metadata. This is particularly useful when dealing with niche hardware deployments.

The feature checks for the presence of an /opt/vultr/find_candidate_nics.sh shell script on the running guest image. If detected, DataSourceVultr uses that as an optional source of truth before falling back to net.find_candidate_nics().

Test Steps

This feature was tested manually on Vultr's Ubuntu 22.04 and Ubuntu 24.04 images (as of 10/2024).

The added functionality is low risk, as it is both opt-in and limited to the Vultr cloud.

cp vultr.py /usr/lib/python3/dist-packages/cloudinit/sources/helpers/vultr.py
cloud-init clean -r

Merge type

TheRealFalcon commented 3 weeks ago

@andy191x , are you associated with the Vultr project? Does this find_candidate_nics.sh script already exist? Do you have any examples of it running? I'm not against running a script, but is there any reason the logic can't make its way into cloud-init rather than as an independent script?

Also, I noticed you added yourself to the CLA signers file, but have you signed the actual CLA too? I'm not seeing your information there. If you did, can you provide the date that you signed it?

andy191x commented 3 weeks ago

@TheRealFalcon, hello!

Yes, I'm from the Vultr project. Our prior maintainer is no longer with the org, therefore future patches from the Vultr side will be submitted by me.

Does this find_candidate_nics.sh script already exist? Do you have any examples of it running? I'm not against running a script, but is there any reason the logic can't make its way into cloud-init rather than as an independent script?

No, not currently. The idea is that find_candidate_nics.sh would be an open placeholder on the Vultr side where we can patch NIC detection without having to get your team involved. On our side, we build Ubuntu images for different hardware configurations with cloud-init installed. On machines that have many devices, the metadata detection in DataSourceVultr.py can take a long time to complete. In general, the DHCP logic that's already there works well, so we were hoping to leave that alone for now. We are able to work around the slow boots using udev, but having the ability to launch a shell script for NIC detection makes things more workable for our image building team.

To clarify: find_candidate_nics.sh wouldn't be a single script, but rather a script that varies on different server hardware deployments.

Also, I noticed you added yourself to the CLA signers file, but have you signed the actual CLA too?

Yes. It was on 10/30/2024. After submitting, there wasn't a confirmation code, I was redirected to a "Thanks for agreeing to contribute" page. If it helps, I submitted using the "I am signing on behalf of an organization, foundation, company or other entity, which may have multiple contributors." option with Vultr as the organization. Also, happy to verify identify if needed.

TheRealFalcon commented 3 weeks ago

Yes, I'm from the Vultr project. Our prior maintainer is no longer with the org, therefore future patches from the Vultr side will be submitted by me.

Nice to meet you and welcome to the project!

I was able to confirm that you signed the CLA.

Thanks for the info on the script usage. Are you able to confirm that the location of the script is only writable by root? If not, a user could use cloud-init to run scripts as root.

andy191x commented 3 weeks ago

Nice to meet you and welcome to the project!

Cheers!

Thanks for the info on the script usage. Are you able to confirm that the location of the script is only writable by root? If not, a user could use cloud-init to run scripts as root.

Yes, the parent path, /opt/vultr is only root-writeable. Here's a quick view from our Ubuntu 22.04 and Ubuntu 24.04 images:

linuxuser@u22:~$ ls -lh /opt/ | grep -i vultr
drwxr-xr-x 2 root root 4.0K Nov  7 18:43 vultr
linuxuser@u22:~$ touch /opt/vultr/test1
touch: cannot touch '/opt/vultr/test1': Permission denied

linuxuser@u24:~$ ls -lh /opt/ | grep -i vultr
drwxr-xr-x 2 root root 4.0K Nov  5 23:03 vultr
linuxuser@u24:~$ touch /opt/vultr/test1
touch: cannot touch '/opt/vultr/test1': Permission denied

Our imagebuilding team has a rule in place that scripts written to that folder are only writeable by root. In the rare event in which that policy fails, responsibility would be on the Vultr side. In this scenario, the affected scope should be very small, as we're hoping to only use find_candidate_nics.sh on bespoke hardware configurations.