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 vendor-data in proxmoxve #2014

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jdoss
Copy link

@jdoss jdoss commented Feb 6, 2025

This PR adds support for reading Ignition data out of the vendor-data file so we can leave the cloud-config in the user-data file. It will always use the user-data file if it has an ignition config over using the vendor-data file.

This works as expected:

# /home/jdoss/ignition --platform proxmoxve --stage fetch -log-to-stdout
INFO     : Ignition v2.10.1-1315-gaf19b7d7-dirty
INFO     : Stage: fetch
INFO     : no config dir at "/usr/lib/ignition/base.d"
INFO     : no config dir at "/usr/lib/ignition/base.platform.d/proxmoxve"
DEBUG    : parsed url from cmdline: ""
INFO     : no config URL provided
INFO     : reading system config file "/usr/lib/ignition/user.ign"
INFO     : no config at "/usr/lib/ignition/user.ign"
DEBUG    : creating temporary mount point
INFO     : op(1): [started]  mounting config drive
DEBUG    : op(1): executing: "mount" "-o" "ro" "-t" "auto" "/dev/disk/by-label/cidata" "/tmp/ignition-configdrive2469853404"
INFO     : op(1): [finished] mounting config drive
DEBUG    : config drive ("/tmp/ignition-configdrive2469853404/user-data") contains a cloud-config configuration, ignoring
DEBUG    : config drive ("/tmp/ignition-configdrive2469853404/vendor-data") contains a ignition configuration
INFO     : op(2): [started]  unmounting "/dev/disk/by-label/cidata" at "/tmp/ignition-configdrive2469853404"
INFO     : op(2): [finished] unmounting "/dev/disk/by-label/cidata" at "/tmp/ignition-configdrive2469853404"
DEBUG    : parsing config with SHA512: d45cc6bd9291debbfbd47365a802208a027b77407d39caf42f873ffd9183cc20dbc01803da2c17cfba09e1c753bff9168a5865bff30f9428088ae7f7b87d7ae0
INFO     : fetch: fetch complete
INFO     : fetch: fetch passed
INFO     : Ignition finished successfully

@jdoss jdoss force-pushed the jdoss/Proxmoxve_vendor-data_support branch from 0d7040f to f60d7f9 Compare February 25, 2025 00:21
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.

One more comment, otherwise LGTM.

Can you squash all your commits into the first?

@jdoss jdoss force-pushed the jdoss/Proxmoxve_vendor-data_support branch from 3ce4656 to 4849d9c Compare February 28, 2025 19:23
@jdoss
Copy link
Author

jdoss commented Feb 28, 2025

Feedback applied and commits squashed.

Copy link
Collaborator

@prestist prestist left a comment

Choose a reason for hiding this comment

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

Small nits; lgtm

For the commit message I think it might sound clearer if we remove the details about the pr.

wdyt about

"Add support for reading Ignition data out of the
vendor-data file. No longer clobber the user-data file."

?

vendor-data file. No longer clobber the user-data file."
@jdoss jdoss force-pushed the jdoss/Proxmoxve_vendor-data_support branch from 4849d9c to 9238e0f Compare March 5, 2025 17:40
@jdoss
Copy link
Author

jdoss commented Mar 5, 2025

Feedback applied. Please let's merge this. :)

@prestist
Copy link
Collaborator

prestist commented Mar 5, 2025

Thank you, I am trying to work through the failing CI. Locally having some issues with golang so it might take me a few moments.

@prestist
Copy link
Collaborator

prestist commented Mar 6, 2025

deviceLabel = "cidata" needs some spaces gofmt is not allowing builds.

You can fix this by running gofmt on the proxmoxve file.

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.

3 participants