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

os: add initial full-disk RAID proposal #3

Merged
merged 1 commit into from
Nov 26, 2020
Merged

os: add initial full-disk RAID proposal #3

merged 1 commit into from
Nov 26, 2020

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Nov 3, 2020

os/20201103-full-disk-raid.md Outdated Show resolved Hide resolved
Comment on lines 24 to 29
# Optional, defaults to 0 (entire disk)
# Do we want this?
root_size_mib: 8192
# Optional, defaults to xfs
# Do we want this?
root_format: xfs
Copy link
Member

Choose a reason for hiding this comment

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

Is the thinking with these that there's no easy way for users to specify it themselves via storage.disks.partitions[]? (Because IIUC, they'd have to know the label names the sugar generates.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem great to expect users to know the labels, but we could document them. And users could still use the "create a subsequent partition with a specified starting offset" trick.

os/20201103-full-disk-raid.md Show resolved Hide resolved
os/20201103-full-disk-raid.md Outdated Show resolved Hide resolved
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Overall seems sane


## Configuring boot disk RAID in OCP

Since FCCs aren't yet used directly in MachineConfigs and FCCT is not shipped with OCP, we'll need a workaround to allow FCC sugar to be used in 4.7. The MCO could:
Copy link
Member

Choose a reason for hiding this comment

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

xref openshift/enhancements#525

I'm uncertain though that the MCO is required here; I think in some cases this may need to be per-machine state and actually be part of the pointer config, which argues for having oc ignition fcct/oc ignition rhcct or something, see also openshift/oc#628

IOW wherever we say fcct we can say oc ignition fcct for OKD/FCOS too or so.

Alternatively we could add this functionality directly to MachineConfig, something like an even simpler

storage:
  boot_raid:
    - /dev/vda
    - /dev/vdb

Basically this path is making MachineConfig a peer of FCC.

(Or we could just punt this section for a second phase)

os/20201103-full-disk-raid.md Outdated Show resolved Hide resolved
@bgilbert
Copy link
Contributor Author

bgilbert commented Nov 5, 2020

Updated to address most of the comments.

@bgilbert
Copy link
Contributor Author

Updated based on refinement session. PTAL.

Based on a discussion with @arithx, the FCC sugar now provides a unified mechanism for configuring LUKS root, mirrored boot disks, or both. In an effort to keep the sugar as simple as possible, I've dropped the sugared properties for setting the root filesystem type and partition size for now. We can always re-add them later, and/or advanced users can use overrides or avoid the sugar entirely.

@bgilbert
Copy link
Contributor Author

Moved boot_device sugar to the top level per an OOB discussion with @arithx.

@bgilbert
Copy link
Contributor Author

Moved layout directly under boot_device to handle the case that some platform/architecture requires different handling for LUKS.

@bgilbert
Copy link
Contributor Author

Okay, made a final set of updates to reflect the current plan.

@bgilbert bgilbert merged commit 8373474 into coreos:master Nov 26, 2020
@bgilbert bgilbert deleted the raid branch November 26, 2020 01:56
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.

7 participants