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

Add Cloudstack provider support for Flatcar #1064

Closed
wants to merge 1 commit into from

Conversation

hrak
Copy link
Contributor

@hrak hrak commented Jan 26, 2023

What this PR does / why we need it:

This PR adds Cloudstack provider support for Flatcar Linux. Flatcar requires some support scripts to be present in the oem partition (/usr/share/oem) to support the Cloudstack metadata service. These files were taken from Flatcar's own Cloudstack image

Additional context
Add any other context for the reviewers

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 26, 2023
@k8s-ci-robot
Copy link
Contributor

Welcome @hrak!

It looks like this is your first PR to kubernetes-sigs/image-builder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/image-builder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 26, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @hrak. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 26, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hrak
Once this PR has been reviewed and has the lgtm label, please assign sbueringer for approval by writing /assign @sbueringer in a comment. For more information see the Kubernetes Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mboersma
Copy link
Contributor

/ok-to-test
/assign @invidian

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 26, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 26, 2023
@@ -24,6 +24,15 @@
owner: root
group: root
mode: 0644
when: ansible_os_family != "Flatcar"

# For Flatcar, copy the cloudstack support files into the oem partition
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this comment could probably be left out, the block is self-explanatory.

@invidian
Copy link
Member

@mboersma thanks for assigning me, but perhaps we should get something more knowledgeable about Flatcar internals to look into this one. Maybe @jepio or @pothos would find a minute to have a look?

@hrak thanks for the PR. As you mentioned Flatcar's own Cloudstack image, I wonder, could we customize iso_url perhaps when building an image for different provider? This way, we wouldn't have to copy-paste and play keep-up-to-date game with Flatcar images, but just inherit what they provide. I don't know how feasible that is though.

I'm talking about these:

$ git grep flatcar-linux.net
packer/ova/flatcar.json:  "iso_checksum": "https://{{env `FLATCAR_CHANNEL`}}.release.flatcar-linux.net/amd64-usr/{{env `FLATCAR_VERSION`}}/flatcar_production_iso_image.iso.DIGESTS.asc",
packer/ova/flatcar.json:  "iso_url": "https://{{env `FLATCAR_CHANNEL`}}.release.flatcar-linux.net/amd64-usr/{{env `FLATCAR_VERSION`}}/flatcar_production_iso_image.iso",
packer/qemu/qemu-flatcar.json:  "iso_checksum": "https://{{env `FLATCAR_CHANNEL`}}.release.flatcar-linux.net/amd64-usr/{{env `FLATCAR_VERSION`}}/flatcar_production_iso_image.iso.DIGESTS.asc",
packer/qemu/qemu-flatcar.json:  "iso_url": "https://{{env `FLATCAR_CHANNEL`}}.release.flatcar-linux.net/amd64-usr/{{env `FLATCAR_VERSION`}}/flatcar_production_iso_image.iso",
packer/raw/raw-flatcar.json:  "iso_checksum": "https://{{env `FLATCAR_CHANNEL`}}.release.flatcar-linux.net/amd64-usr/{{env `FLATCAR_VERSION`}}/flatcar_production_iso_image.iso.DIGESTS.asc",
packer/raw/raw-flatcar.json:  "iso_url": "https://{{env `FLATCAR_CHANNEL`}}.release.flatcar-linux.net/amd64-usr/{{env `FLATCAR_VERSION`}}/flatcar_production_iso_image.iso",

Could you give it a try?

@hrak
Copy link
Contributor Author

hrak commented Jan 27, 2023

@hrak thanks for the PR. As you mentioned Flatcar's own Cloudstack image, I wonder, could we customize iso_url perhaps when building an image for different provider? This way, we wouldn't have to copy-paste and play keep-up-to-date game with Flatcar images, but just inherit what they provide. I don't know how feasible that is though.

Could you give it a try?

Flatcar does not supply provider-specific iso images, only VM images. What they do is build a VM image based on the iso and then build a OEM-specific ebuild into the oem partition in the 'image_to_vm' build stage (see here)

The ebuild containing the oem-specific files can be found here. The files have not really changed in a long time.

So actually, this does not just apply to Cloudstack, but to any provider supported by image-builder. Which makes me wonder how people actually currently use Flatcar based capi images on any of the supported cloud providers, since i don't see oem-azure, oem-gce etc. referenced anywhere.

btw, building based on an image instead of an ISO could be an option if the "disk_image": "true" flag would be supported as suggested in #924.

@invidian
Copy link
Member

Flatcar does not supply provider-specific iso images, only VM images. What they do is build a VM image based on the iso and then build a OEM-specific ebuild into the oem partition in the 'image_to_vm' build stage (see here)

The ebuild containing the oem-specific files can be found here. The files have not really changed in a long time.

TIL, thanks!

So actually, this does not just apply to Cloudstack, but to any provider supported by image-builder. Which makes me wonder how people actually currently use Flatcar based capi images on any of the supported cloud providers, since i don't see oem-azure, oem-gce etc. referenced anywhere.

At least on Azure, we use Azure-specific Flatcar images as a base, this is why there is no need for special handling of oem I think.

btw, building based on an image instead of an ISO could be an option if the "disk_image": "true" flag would be supported as suggested in #924.

Interesting. So then you would just point it to cloudstack disk image and that's it, correct? That seem like a nice win. Could you give it a try or at least leave a comment in #924 about this please? So we can collect more use cases for this, so it's maybe easier to prioritize to work on at some point.

Otherwise, I think the PR looks good then.

@jepio
Copy link
Contributor

jepio commented Jan 27, 2023

@hrak I'm not excited about copying files out of the Flatcar repos and keeping them here. Not that the files change often, but if they would no one would remember to keep them in sync. Here are two approaches that I think would be better:

  • fetching them from github at runtime from the proper tag that matches the flatcar version being built
  • passing the right OEM_ID to flatcar-install -o. You'd have to try this, but this should install the cloudstack image during the packer run.

@pothos
Copy link
Contributor

pothos commented Jan 27, 2023

Using Packer, and specially in the way it's done here with the ISO "bootstrap" phase makes things more complicated than needed^^ As was already said, I also think the best options would be to start the right image or at least pass the -o cloudstack flag to flatcar-install (which is what we also do with VMware).
By the way, since cloudstack isn't covered by the Flatcar release tests, things may be not in the best state and if you know what to improve please file an issue and if you can suggest an easy way how to cover it in the Flatcar release tests that would also help.

@hrak
Copy link
Contributor Author

hrak commented Jan 31, 2023

I'm back, and a little wiser after opening a related issue on the Flatcar project. It appears cloud-init on Flatcar does not support Jinja templates, which cluster-api requires for cloud-init, so its a dead end road.

Someone pointed out to me in that issue that ignition is the way to go for cluster-api + Flatcar. This is working for me now, including metadata and installing ssh keys! Description of the process can be found in kubernetes-sigs/cluster-api-provider-cloudstack#216 (comment)

Based on my findings, what could be implemented as a cloudstack provider for flatcar is those two drop-ins that i now send using ignition. They override the metadata provider name to be cloudstack-metadata. This is needed because coreos-metadata only knows providers cloudstack-configdrive and cloudstack-metadata not cloudstack like the oem id says.

@jepio
Copy link
Contributor

jepio commented Feb 1, 2023

Thanks, it looks like cloudstack could be switched to afterburn (coreos-metadata). A fix for the same issue was once necessary for openstack https://github.com/flatcar/bootengine/pull/21/files. Could you PR that for openstack?

@hrak
Copy link
Contributor Author

hrak commented Feb 3, 2023

Thanks, it looks like cloudstack could be switched to afterburn (coreos-metadata).

Indeed, the only thing is that using oem_id=cloudstack currently doesn't work because afterburn only knows about platforms cloudstack-configdrive and cloudstack-metadata. Hence the override systemd dropins that set Environment=COREOS_METADATA_OPT_PROVIDER=--provider=cloudstack-metadata in my ignition cfg.

This was recently addressed for openstack, which had a similar problem. For cloudstack this is still an open issue.

A fix for the same issue was once necessary for openstack https://github.com/flatcar/bootengine/pull/21/files. Could you PR that for openstack?

It doesn't look like this is needed for Cloudstack, i don't have any issues with the hostname being set with Flatcar using afterburn on Cloudstack.

@hrak
Copy link
Contributor Author

hrak commented Apr 5, 2023

Thanks, it looks like cloudstack could be switched to afterburn (coreos-metadata). A fix for the same issue was once necessary for openstack https://github.com/flatcar/bootengine/pull/21/files. Could you PR that for openstack?

@jepio Looking into this some more now. It looks like the hostname is currently being set by whatever is supplied by DHCP, which works, but i was wondering if there is any advantage in doing this same Openstack approach for Cloudstack?

@jepio
Copy link
Contributor

jepio commented Apr 5, 2023

Thanks, it looks like cloudstack could be switched to afterburn (coreos-metadata). A fix for the same issue was once necessary for openstack https://github.com/flatcar/bootengine/pull/21/files. Could you PR that for openstack?

@jepio Looking into this some more now. It looks like the hostname is currently being set by whatever is supplied by DHCP, which works, but i was wondering if there is any advantage in doing this same Openstack approach for Cloudstack?

What I intended to suggest was opening a PR to Flatcar to improve support for cloudstack, so that the OEM support does not need to be fixed downstream in image-builder. That way it works for anyone using cloudstack.

The openstack approach just seems similar to me, so you could reuse some of the same units/scripts after extending them a bit.

@jsturtevant
Copy link
Contributor

Checking in - Is this still a relevant PR?

@hrak
Copy link
Contributor Author

hrak commented Aug 2, 2023

Checking in - Is this still a relevant PR?

Sorry for the delay, no i think this can be closed, we have a solution now with some systemd drop-ins, and i will explore fixing this upstream.

@hrak hrak closed this Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants