-
Notifications
You must be signed in to change notification settings - Fork 402
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
Add flatcar build for Proxmox #1589
base: main
Are you sure you want to change the base?
Conversation
The build is working, but let me double check for any findings. |
78fc974
to
7c0308c
Compare
WIP in CAPMOX but I will provide a link to how this works in the provider once it is released. |
8190ae6
to
b79e31a
Compare
/retest |
@mboersma @drew-viles What do y'all think about this currently requiring the use of the Alpha Flatcar builds? I'm kinda inclined to not include this in image-builder until it's available in the Stable channel but I'm open to being convinced otherwise. |
The PROXMOX VE image, was tested really well with a lot of parties. On the other hand The image builder has the ability to set Flatcar Channel, |
I'm more concerned about it defaulting to Alpha channel. If user chose to use non-stable channels that's fine but I think the out-of-the-box settings should encourage stability. (Especially true as we're not in a position to provide any support or testing for Proxmox and very limited for Flatcar so if anyone has any issues they are often on their own) |
@tormath1, if you want to add anything here. |
capmox will have the support soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Currently, Proxmox support is only available in the
alpha
channel of Flatcar.
I don't have a problem introducing this build target as an alpha feature, so long as the goal is to move this to a stable channel by default as @AverageMarcus suggested. Having said that, we don't have a way to test this in CI or locally, so all I can say is it looks good to me.
@drew-viles @jsturtevant do either of you have time to look at this as well? (@AverageMarcus is away until 2025, so I may ask for more review help, or just lgtm+approve a bunch of things myself, mea culpa.) |
/hold @mcbenjemaa could you squash the commits? |
In terms of running the alpha as the default, I have no major opinion on it as I don't use flatcar so can't really comment on the stability of the alpha. It seems a bit brave but if it's been tested and we can be sure it stays stable enough I'm happy to go with the crowd. I'm back from vacation now but will be out again in 2 weeks until Jan so get all the PRs you can in over those two weeks 🤣 I can approved once the squash has happened |
Hi, regarding Alpha channel concerns, this will not be true anymore. In a few days, we will promote the Proxmox feature to "Beta" and we are confident enough to encourage folks to run a few Beta nodes in their workloads. |
b79e31a
to
365ddf3
Compare
365ddf3
to
ecd5cd8
Compare
Squash is done, |
Now live. 🙏 |
/lgtm |
If everyone is happy now we can unhold and merge this. |
try flatcar try flatcar -2 fix comment sort jsonm Build Proxmox flatcar with openstack OEM Add scsi controller and write docs Flatcar for Proxmox VE
ecd5cd8
to
3311823
Compare
New changes are detected. LGTM label has been removed. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Documentation updated |
Change description
This PR adds:
Related issues
Additional context