harvester / harvester

Open source hyperconverged infrastructure (HCI) software
https://harvesterhci.io/
Apache License 2.0
3.84k stars 324 forks source link

[FEATURE] Support for iscsi boot disks #5150

Open ibrokethecloud opened 8 months ago

ibrokethecloud commented 8 months ago

Is your feature request related to a problem? Please describe.

Large customers running diskless systems may wish to use something like an iscsi disk as the boot device.

Most modern hardware allows definition of an iscsi target as a boot device.

As we have tested internally when booting of an iscsi disk additional kernel arguments rd.iscsi.firmware rd.iscsi.ibft are needed to ensure that the OS can boot successfully.

Describe the solution you'd like

We should have an additional check in the installer / upgrade path to detect if the install disk is an iscsi disk, and accordingly append these additional kernel arguments in the default entry.

These arguments will need to be retained post upgrade as well.

Describe alternatives you've considered

Additional context

abonillabeeche commented 8 months ago

We may need to include the support to use the OS and DATA mpio disks rather than the individual disk. This means the initramfs would need mpio support. See here

Otherwise, we'd need to disable mpio for the root/OS disk.

innobead commented 3 months ago

Let's add HEP for this, even though we have a KB for reference.

cc @bk201

harvesterhci-io-github-bot commented 3 months ago

Pre Ready-For-Testing Checklist

harvesterhci-io-github-bot commented 3 months ago

Automation e2e test issue: harvester/tests#1407

ibrokethecloud commented 2 months ago

For testing the changes:

The multipath support can be validated in a kvm setup as follows:

image image

noahgildersleeve commented 1 week ago

Validated multpath with Harvester v1.4.0-rc3.Verified that multpath was running properly after install and boot and ran the p0 volumes and images test cases from automation. I don't think the one failed automated test is relevant.

Screenshot_20241022_123502 Screenshot_20241022_140403

I'm checking if this covers the iscsi test case as well.

I ran the test according to the quoted steps.

For testing the changes:

The multipath support can be validated in a kvm setup as follows:

  • Create a raw disk image of atleast 250GB image
  • Add the same disk twice to an VM. Please ensure the following

    • The disk is marked sharable
    • A serial is provided, say disk1. The serial info needs to be same on both scsi disk additions

image image

  • Boot from the iso. Despite two paths being added to the same underlying disk, the harvester installer only shows the 1 installable disk. The serial info is important here as it is used to dedup disks with same serial info and/or wwn number
  • Proceed with installation as normal, and specify the following harvester config via a config url to enable multipathd and configure multipath.conf. The additionalKernelArguments are not needed for this example to work, but have been added to help validate the arguments being passed to kernel via grub post install
os:
  externalStorageConfig:
    enabled: true
    multiPathConfig:
    - vendor: "QEMU"
      product: "QEMU HARDDISK"
  additionalKernelArguments: "multipath=on"
  • Post install of harvester, create a VM
  • Shell into node and verify multipath is only showing pathing for the install disks, and longhorn disks are ignored
multipath:~ # multipath -ll
0QEMU_QEMU_HARDDISK_disk1 dm-0 QEMU,QEMU HARDDISK
size=250G features='0' hwhandler='0' wp=rw
|-+- policy='service-time 0' prio=1 status=active
| `- 0:0:0:1 sda 8:0  active ready running
`-+- policy='service-time 0' prio=1 status=enabled
  `- 0:0:0:2 sdb 8:16 active ready running
noahgildersleeve commented 1 week ago

I talked with @ibrokethecloud about this and the implementation of the multpath should cover the test case. It's checking for duped serial numbers and deduping them. This should affect anything that's being passed over to the host OS from the BIOS/UEFI. This includes duped serials from RAID controllers, iSCSI, and others.

I'm going to double check this on physical hardware then close it if it works as expected.

noahgildersleeve commented 1 week ago

I'm closing this as fixed and will re-open it if there are any issues on bare metal.

innobead commented 1 week ago

Do we have a doc for this feature?

bk201 commented 1 week ago

@ibrokethecloud we should have a doc PR before closing the issue.

ibrokethecloud commented 1 week ago

i will submit a docs PR with the additional config needed to leverage this

khushboo-rancher commented 3 days ago

Moving this to implement as this is waiting for doc PR.