Build kernel param list on /bootscript with JSON URL param turned on

[aheil-lanl]

Pull Request Template

Thank you for your contribution! Please ensure the following before submitting:

Checklist

• My code follows the style guidelines of this project
• I have added/updated comments where needed
• I have added tests that prove my fix is effective or my feature works
• I have run make test (or equivalent) locally and all tests pass
• DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
• REUSE Compliance:
  • Each new/modified source file has SPDX copyright and license headers
  • Any non-commentable files include a <filename>.license sidecar
  • All referenced licenses are present in the LICENSES/ directory

Description

The JSON URL parameter added in #71 does not fully reflect how /bootscript works without it. Due to this, the kernel parameter list of the boot script is wrong when using the JSON URL parameter.

To fix this, I refactored the parameter list building into its own function. This allows the function to be called both when building an iPXE boot script and when building a JSON boot script, making the kernel parameter lists match.

I've also added tests and documentation, although both need more discussion for whether further changes are needed.

Why much of the checklist isn't checked

These are my explanations for all the items I left unchecked. I'm open to making changes to match these, but I don't have enough information to take action as I write this.

• style: I don't see anywhere this project has style guidelines. Lacking those, I've done my best to follow existing style I see in the file I edited.
• SPDX headers: not present in the file I edited (likely because it didn't originate from the OpenCHAMI project), but I could add them if needed.
• license dir: we don't have that, we have a single license file

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

---

For more info, see Contributing Guidelines.

[synackd]

@aheil-lanl Does this need additional testing? Would you mind posting how you tested this if you have?

[aheil-lanl]

@synackd I've tested this manually on a small scale. I made sure that nodes booted normally and the behavior of /bootscript serving iPXE scripts was unchanged, and also confirmed that nodes booted correctly when using kexec with the params, kernel, and initrd from /bootscript?json=1.

I'm not aware of any additional testing that needs to be done.

[synackd]

Sounds good. I think the only thing left to be tested is to make sure the parameters match with and without json enablement and they match those returned via /bootparameters.

[aheil-lanl]

I've also been able to test that manually, I forgot to mention it.

Without json enabled, the first line of the boot script looks something like this. I've generalized and removed some details here, but kept the structure the same.

kernel --name kernel http://192.168.0.254:8080/openchami/path/to/kernel initrd=initrd root=nfs:192.168.0.254:/path_to_nfs_root:ro,vers=4.2,sec=sys,nolock ochami_wg_ip=10.90.0.2 console=ttyS0,115200 intel_pstate=disable systemd.unified_cgroup_hierarchy=1 ipv6.disable=1 selinux=0 ochami=1 network-config=disabled xname=x9000c1s0b1n0 nid=3 ds=nocloud-net;s=localhost/ || goto boot_retry

When the node boots its /proc/cmdline looks like this, which strips off the kernel path and other iPXE-specific stuff.

kernel initrd=initrd root=nfs:192.168.0.254:/path_to_nfs_root:ro,vers=4.2,sec=sys,nolock ochami_wg_ip=10.90.0.2 console=ttyS0,115200 intel_pstate=disable systemd.unified_cgroup_hierarchy=1 ipv6.disable=1 selinux=0 ochami=1 network-config=disabled xname=x9000c1s0b1n0 nid=3 ds=nocloud-net;s=localhost/

With json enabled, the params returned look like this. This doesn't have the iPXE-specific text, but matches the actual kernel params of the node when it comes up using the iPXE boot script.

kernel initrd=initrd root=nfs:192.168.0.254:/path_to_nfs_root:ro,vers=4.2,sec=sys,nolock ochami_wg_ip=10.90.0.2 console=ttyS0,115200 intel_pstate=disable systemd.unified_cgroup_hierarchy=1 ipv6.disable=1 selinux=0 ochami=1 network-config=disabled xname=x9000c1s0b1n0 nid=3 ds=nocloud-net;s=localhost/

The node's /proc/cmdline matches as well when the node is booted with kexec and these parameters.

[synackd]

I've tested this with a VM.

curl -H "Authorization: Bearer ${DEMO_ACCESS_TOKEN}" 'https://demo.openchami.cluster:8443/boot/v1/bootscript?mac=52:54:00:be:ef:01&json=1' | jq

yields:

{
  "params": "kernel initrd=initrd nomodeset ro root=live:http://172.16.0.254:9000/boot-images/compute/base/rocky9.6-compute-base-rocky9 ip=dhcp overlayroot=tmpfs overlayroot_cfgdisk=disabled apparmor=0 selinux=0 console=ttyS0,115200 ip6=off cloud-init=enabled ds=nocloud-net;s=http://172.16.0.254:8081/cloud-init xname=x1000c0s0b0n0 nid=1",
  "kernel": {
    "path": "http://172.16.0.254:9000/boot-images/efi-images/compute/base/vmlinuz-5.14.0-570.58.1.el9_6.x86_64"
  },
  "initrd": {
    "path": "http://172.16.0.254:9000/boot-images/efi-images/compute/base/initramfs-5.14.0-570.58.1.el9_6.x86_64.img"
  }
}

curl -H "Authorization: Bearer ${DEMO_ACCESS_TOKEN}" 'https://demo.openchami.cluster:8443/boot/v1/bootscript?mac=52:54:00:be:ef:01&json=0'

yields:

#!ipxe
kernel --name kernel http://172.16.0.254:9000/boot-images/efi-images/compute/base/vmlinuz-5.14.0-570.58.1.el9_6.x86_64 initrd=initrd nomodeset ro root=live:http://172.16.0.254:9000/boot-images/compute/base/rocky9.6-compute-base-rocky9 ip=dhcp overlayroot=tmpfs overlayroot_cfgdisk=disabled apparmor=0 selinux=0 console=ttyS0,115200 ip6=off cloud-init=enabled ds=nocloud-net;s=http://172.16.0.254:8081/cloud-init xname=x1000c0s0b0n0 nid=1 || goto boot_retry
initrd --name initrd http://172.16.0.254:9000/boot-images/efi-images/compute/base/initramfs-5.14.0-570.58.1.el9_6.x86_64.img || goto boot_retry
boot || goto boot_retry
:boot_retry
sleep 30
chain https://${SYSTEM_URL}/apis/bss/boot/v1/bootscript?mac=52:54:00:be:ef:01&retry=1

The difference in the JSON is:

diff --git a/old.json b/new.json
index a93cde3..5b693bd 100644
--- a/old.json
+++ b/new.json
@@ -1,5 +1,5 @@
 {
-  "params": "nomodeset ro root=live:http://172.16.0.254:9000/boot-images/compute/base/rocky9.6-compute-base-rocky9 ip=dhcp overlayroot=tmpfs overlayroot_cfgdisk=disabled apparmor=0 selinux=0 console=ttyS0,115200 ip6=off cloud-init=enabled ds=nocloud-net;s=http://172.16.0.254:8081/cloud-init",
+  "params": "kernel initrd=initrd nomodeset ro root=live:http://172.16.0.254:9000/boot-images/compute/base/rocky9.6-compute-base-rocky9 ip=dhcp overlayroot=tmpfs overlayroot_cfgdisk=disabled apparmor=0 selinux=0 console=ttyS0,115200 ip6=off cloud-init=enabled ds=nocloud-net;s=http://172.16.0.254:8081/cloud-init xname=x1000c0s0b0n0 nid=1",
   "kernel": {
     "path": "http://172.16.0.254:9000/boot-images/efi-images/compute/base/vmlinuz-5.14.0-570.58.1.el9_6.x86_64"
   },

Looks like the parameters are preceded by kernel (as if this were in a bootloader directive, though without the --name kernel as in iPXE). Was this intended?

It looks like it works, but I'm not sure I fully understand the use case for this change. Would you mind elaborating?

[synackd]

I tested this and it looks good. Just one tiny change, and I think we can merge this.