suse-edge/Factory
SHA256
13
0
Fork 14
Code Issues Pull Requests 7 Actions 8 Packages Projects Releases Wiki Activity

Install both ramdisks in the ipa downloader #84

Merged
nbelouin merged 1 commits from nbelouin/Factory:multi-arch-ipa into main 2025-04-03 15:36:02 +02:00
Conversation 7 Commits 1 Files Changed 25 +1310 -108
nbelouin commented 2025-02-28 12:37:23 +01:00
Owner
Copy Link
  • Make the different ipa-ramdisk packages installable side by side
  • Clean the ipa-downloader Dockerfile from what seems to be unneeded
  • Get both images in
  • Use zstd instead of xz for better speed
  • Check sums before redoing certs integration
  • Add value to metal3 chart to select between architectures

Signed-off-by: Nicolas Belouin nicolas.belouin@suse.com

- Make the different ipa-ramdisk packages installable side by side - Clean the ipa-downloader Dockerfile from what seems to be unneeded - Get both images in - Use zstd instead of xz for better speed - Check sums before redoing certs integration - Add value to metal3 chart to select between architectures Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
nbelouin added 1 commit 2025-02-28 12:37:24 +01:00
Install both ramdisks in the ipa downloader
Some checks failed
Build PR in OBS / Build PR in OBS (pull_request_target) Failing after 25m15s
Details
730aaf84ec 
- Make the different ipa-ramdisk packages installable side by side
- Clean the ipa-downloader Dockerfile from what seems to be unneeded
- Get both images in
- Use zstd instead of xz for better speed
- Check sums before redoing certs integration
- Add value to metal3 chart to select between architectures

Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
steven.hardy reviewed 2025-02-28 12:49:22 +01:00
ironic-ipa-downloader-image/get-resource.sh Outdated
@@ -12,0 +24,4 @@
else
if [ "${CERT_CHANGED:-0}" = "1" ]; then
# Image has changed we need to refresh certs anyway
CERT_CHANGED=0
steven.hardy commented 2025-02-28 12:49:22 +01:00
Owner
Copy Link

IMO it would be easier to understand if we had two variables e.g CERT_CHANGED and IMAGE_CHANGED then rebuild if either one is non-zero below?

IMO it would be easier to understand if we had two variables e.g `CERT_CHANGED` and `IMAGE_CHANGED` then rebuild if either one is non-zero below?
nbelouin marked this conversation as resolved
steven.hardy reviewed 2025-02-28 13:00:42 +01:00
metal3-chart/values.yaml Outdated
@@ -64,2 +64,4 @@
databaseServiceName: metal3-mariadb
# Architecture for IPA (either x86_64 or arm64)
ipaArchitecture: x86_64
steven.hardy commented 2025-02-28 13:00:42 +01:00
Owner
Copy Link

I think there are some arch specific things also in ironic-image:

https://build.opensuse.org/projects/isv:SUSE:Edge:Metal3:Ironic:2024.2/packages/ironic-image/files/prepare-efi.sh?expand=1

So we may need to give this a different name perhaps - I guess the most intuitive behavior would be by default we deploy downstream clusters with the same arch as the management cluster, with the ability to override it with some architectureOverride variable or similar?

Note that in future we expect to wire this is via the BaremetalHost resource which does contain an Architecture field, but that will need some adjustments upstream e.g see here - we'd have to enable some additional variables e.g DEPLOY_RAMDISK_URL_AARCH64 etc, and adjust so when available these are used depending on the BMH spec.architecture

I think there are some arch specific things also in ironic-image: https://build.opensuse.org/projects/isv:SUSE:Edge:Metal3:Ironic:2024.2/packages/ironic-image/files/prepare-efi.sh?expand=1 So we may need to give this a different name perhaps - I guess the most intuitive behavior would be by default we deploy downstream clusters with the same arch as the management cluster, with the ability to override it with some `architectureOverride` variable or similar? Note that in future we expect to wire this is via the BaremetalHost resource which does contain an [Architecture field](https://github.com/metal3-io/baremetal-operator/blob/main/apis/metal3.io/v1alpha1/baremetalhost_types.go#L479), but that will need some adjustments upstream e.g see [here](https://github.com/metal3-io/baremetal-operator/blob/main/pkg/provisioner/ironic/factory.go#L114) - we'd have to enable some additional variables e.g `DEPLOY_RAMDISK_URL_AARCH64` etc, and adjust so when available these are used depending on the BMH `spec.architecture`
nbelouin commented 2025-03-04 14:54:05 +01:00
Author
Owner
Copy Link

Well I missed the prepare-efi stuff, and I got bad news here, with our current (provided by SLES) shim, we cannot have an ESP file that works for both x86 and arm (as it blindly loads grub.efi that is not an arch specific name), I think I may be able to come with a workaround for this iteration, but for the future it will be near impossible without a change here (the other way would be a change in Ironic itself to introduce a bootloader_by_arch parameter).

About the name, I'll go for deployArchitecture, since there is more than just the IPA things.

Regarding the explicit arch rather than arch override, well the chart has no knowledge of what is the current arch, hence I went for this, we can go for an override if we get some additional symlinks/files without a suffix for "current architecture", but it feels a bit wrong to me.

For the future, I guess the Architecture field gets forwarded to Ironic by the BMO, thus we should just change it from using the deploy_ramdisk parameter to the deploy_ramdisk_by_arch one.

Well I missed the `prepare-efi` stuff, and I got bad news here, with our current (provided by SLES) `shim`, we cannot have an ESP file that works for both x86 and arm (as it blindly loads `grub.efi` that is not an arch specific name), I think I may be able to come with a workaround for this iteration, but for the future it will be near impossible without a change here (the other way would be a change in Ironic itself to introduce a `bootloader_by_arch` parameter). About the name, I'll go for `deployArchitecture`, since there is more than just the IPA things. Regarding the explicit arch rather than arch override, well the chart has no knowledge of what is the current arch, hence I went for this, we can go for an override if we get some additional symlinks/files without a suffix for "current architecture", but it feels a bit wrong to me. For the future, I guess the Architecture field gets forwarded to Ironic by the BMO, thus we should just change it from using the `deploy_ramdisk` parameter to the `deploy_ramdisk_by_arch` one.
steven.hardy requested review from amorgante 2025-02-28 13:02:12 +01:00
steven.hardy requested review from steven.hardy 2025-02-28 13:02:12 +01:00
steven.hardy commented 2025-02-28 13:02:32 +01:00
Owner
Copy Link

This looks like a great start, thanks for working on it! :)

This looks like a great start, thanks for working on it! :)
nbelouin force-pushed multi-arch-ipa from 730aaf84ec to 780d38b356 2025-02-28 14:02:01 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from 780d38b356 to 19ca659670 2025-03-04 15:54:30 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from 19ca659670 to e6067ffda3 2025-03-04 15:57:07 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from e6067ffda3 to 087a1773ca 2025-03-04 16:02:04 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from 087a1773ca to a5d297f8d0 2025-03-04 16:06:26 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from a5d297f8d0 to 3179a9ddd2 2025-03-04 16:12:21 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from 3179a9ddd2 to 6b35d42d97 2025-03-04 16:16:02 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from 6b35d42d97 to e50206725e 2025-03-05 13:03:49 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from e50206725e to 4c97292e62 2025-03-05 13:44:08 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from 4c97292e62 to 8543583e4d 2025-03-05 15:00:34 +01:00 Compare
amorgante commented 2025-03-19 16:57:45 +01:00
Owner
Copy Link

Tested in VM with the following scenario:

  • MGMT cluster x86_64 VM including the PR helm chart + values.yaml to specify arm64 architecture
  • Downstream cluster using a aarch64 VM without telco stuff. CAPI manifest only deploy rke2 cluster. As we're using emulated VM it's taking a lot of time to deploy it, but at the end the vm is provisioned successfully
Tested in VM with the following scenario: - MGMT cluster x86_64 VM including the PR helm chart + values.yaml to specify arm64 architecture - Downstream cluster using a aarch64 VM without telco stuff. CAPI manifest only deploy rke2 cluster. As we're using emulated VM it's taking a lot of time to deploy it, but at the end the vm is provisioned successfully
amorgante reviewed 2025-03-19 16:59:19 +01:00
metal3-chart/values.yaml Outdated
@@ -63,6 +63,9 @@ global:
# Name for the MariaDB service
databaseServiceName: metal3-mariadb
# Architecture for deployed nodes (either x86_64 or arm64)
amorgante commented 2025-03-19 16:59:19 +01:00
Owner
Copy Link

I'd say to rename arm64 to aarch64 to be more specific

I'd say to rename arm64 to aarch64 to be more specific
nbelouin force-pushed multi-arch-ipa from 8543583e4d to 4974b00e4f 2025-03-24 17:02:03 +01:00 Compare
amorgante commented 2025-03-25 13:07:56 +01:00
Owner
Copy Link

LGTM we can merge it. It has been tested in Lab with VMs and Baremetal (both using mgt-cluster x86_64 and deploying arm64 downstream clusters)

LGTM we can merge it. It has been tested in Lab with VMs and Baremetal (both using mgt-cluster x86_64 and deploying arm64 downstream clusters)
nbelouin force-pushed multi-arch-ipa from 4974b00e4f to dfcba60da1 2025-03-26 11:23:57 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from dfcba60da1 to 80779f6f6c 2025-03-27 11:35:44 +01:00 Compare
nbelouin force-pushed multi-arch-ipa from 80779f6f6c to 329cfd4999 2025-03-28 13:29:22 +01:00 Compare
nbelouin changed title from WIP: Install both ramdisks in the ipa downloader to Install both ramdisks in the ipa downloader 2025-03-28 14:53:33 +01:00
amorgante approved these changes 2025-03-31 10:17:54 +02:00
amorgante left a comment
Owner
Copy Link

LGTM

LGTM
nbelouin force-pushed multi-arch-ipa from 329cfd4999 to 98fa8835f7 2025-04-03 15:34:31 +02:00 Compare
nbelouin merged commit 2b020e9bd7 into main 2025-04-03 15:36:02 +02:00
nbelouin deleted branch multi-arch-ipa 2025-04-03 15:36:02 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
steven.hardy
amorgante
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
amorgante atanasdinov atrendafilov dbekhit dprodanov eminguez fdegirmenci ipetrov117 jtomasek kzhelyazkov mchiappero nbelouin roxenham steven.hardy
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: suse-edge/Factory#84
No description provided.
Delete Branch "nbelouin/Factory:multi-arch-ipa"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?