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

change unit ordering to allow iscsi+multipath to work #2987

Merged
merged 2 commits into from
Apr 26, 2024

Conversation

dustymabe
Copy link
Member

We want multipath on top of iscsi to work. We need to drop some unit
dependencies in order to get this to work. See
#2984 for more
context.

This was tested with an iscsi disk that was installed with:

sudo coreos-installer install /path/to/disk \
    --append-karg rd.multipath=default      \
    --append-karg root=/dev/disk/by-label/dm-mpath-root \
    --append-karg rw --append-karg rd.iscsi.firmware=1  \
    --console ttyS0,115200n8 -i config.ign --insecure   \
    --image-file fedora-coreos-40.20240424.dev.1-metal.x86_64.raw.xz

and then shared over iPXE with:

set initiator-iqn iqn.68cc69b9-1b54-4ff1-9d61-eedb570da8fd
sanboot iscsi:192.168.122.137::::iqn.2024-04.com.coreos:0 \
        iscsi:192.168.122.137::::iqn.2024-04.com.coreos:1

This isn't strictly required and is causing a dependency
timeout in early boot when iscsi+multipath is used. See
coreos#2984
for some more context.
…wait.target

We don't think it's strictly required and will enable
multipath to work on top of iscsi.

This was tested with an iscsi disk that was installed with:

```
sudo coreos-installer install /path/to/disk \
    --append-karg rd.multipath=default      \
    --append-karg root=/dev/disk/by-label/dm-mpath-root \
    --append-karg rw --append-karg rd.iscsi.firmware=1  \
    --console ttyS0,115200n8 -i config.ign --insecure   \
    --image-file fedora-coreos-40.20240424.dev.1-metal.x86_64.raw.xz
```

and then shared over iPXE with:

```
set initiator-iqn iqn.68cc69b9-1b54-4ff1-9d61-eedb570da8fd
sanboot iscsi:192.168.122.137::::iqn.2024-04.com.coreos:0 \
        iscsi:192.168.122.137::::iqn.2024-04.com.coreos:1
```
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Thanks for digging into this!

@@ -2,7 +2,6 @@
Description=CoreOS Enable Network
ConditionPathExists=/etc/initrd-release
DefaultDependencies=false
After=basic.target
Copy link
Member

Choose a reason for hiding this comment

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

This definitely looked suspicious how it was before since basic.target is a default dependency and you'd use DefaultDependencies=false to not inherit that.

@@ -1,7 +1,6 @@
[Unit]
Description=CoreOS Wait For Multipathed Boot
DefaultDependencies=false
Before=dracut-initqueue.service
Copy link
Member

Choose a reason for hiding this comment

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

There's one case where I think this might be load-bearing: FIPS. The FIPS module needs to mount the bootfs to be able to get the HMAC of the kernel and verify it. So in theory, it could try to be doing I/O to the bootfs before multipathd takes over. And of course, dracut being dracut, there's no way to order our unit before just this FIPS code without also blocking the iSCSI code.

I think one way to work around this is to have rdcore inject the boot karg as boot=/dev/disk/by-label/dm-mpath-boot in the multipath case instead of the UUID. That'll force the FIPS code to wait until the multipathed-version actually shows up.

Anyway, we don't have to tackle this right now or block on this. The larger issue is that we don't have any CI for the 'non-optimized paths' case, so we have no way of verifying that scenario. Ideally, we'd add that, and then we could verify that the problem above is real, and verify the fix for it.

@jlebon jlebon merged commit 83f419c into coreos:testing-devel Apr 26, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants