fix(onboarding): allow 8GB dedicated boot devices#1978
Closed
Ajit-Mehrotra wants to merge 3 commits intomainfrom
Closed
fix(onboarding): allow 8GB dedicated boot devices#1978Ajit-Mehrotra wants to merge 3 commits intomainfrom
Ajit-Mehrotra wants to merge 3 commits intomainfrom
Conversation
- Purpose: let dedicated boot pools accept 8 GB class devices in the onboarding and internal boot wizard.\n- Before: eligibility used a MiB-based floor that effectively rejected marketed 8 GB drives because 8,000,000,000 bytes is below 8,192 MiB.\n- Why that was a problem: users could only pick 16 GB and larger devices for dedicated boot even though dedicated boot should support smaller media.\n- Now: dedicated boot checks against an 8,000,000,000-byte minimum while hybrid mode keeps the existing boot-reservation math.\n- How: make device eligibility depend on pool mode and add a regression test covering dedicated vs hybrid behavior for an 8 GB drive.
- Purpose: align the onboarding internal boot flow with Unraid's decimal GB/TB sizing convention. - Before: the wizard stored and validated hybrid boot sizes in MiB-based values like 16384 and converted capacities with 1024-based math. - Problem: the UI labels said GB while the underlying thresholds, presets, summaries, and GraphQL payloads were still using binary units, which made marketed drive sizes behave unexpectedly at the boundary. - Change: rename the onboarding boot-size contract to bootSizeMb and switch onboarding thresholds, presets, summaries, device eligibility math, and tests to decimal MB/GB semantics. - How: convert device sizes with 1000-based math in the web step, send decimal boot-size values through GraphQL and the internal boot service, regenerate schema/types, and update onboarding specs to assert the new 8,000 MB boundary.
- Purpose: align onboarding with the existing convention of showing decimal-sized disks to users while keeping internal boot reservation values in MiB. - Before: the follow-up decimal migration changed the onboarding boot-size contract, defaults, presets, and hybrid validation to decimal MB values like 16000 and 4000. - Problem: that no longer matched the prior internal contract, changed the hybrid 8 GB boundary behavior, and would send a different poolBootSize value to the backend than the feature previously expected. - Change: restore bootSizeMiB across onboarding state, GraphQL types, API inputs, service payloads, presets, and validation while preserving metric device labels and the dedicated 8,000,000,000-byte minimum from the earlier fix. - How: switch the onboarding boot-size pipeline back to MiB values such as 16384 and 4096, regenerate the web GraphQL artifacts, and update the onboarding specs to assert dedicated 8 GB eligibility with the original hybrid MiB threshold.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
Disabled knowledge base sources:
WalkthroughThis change refactors internal boot device eligibility logic to be pool-mode-aware (dedicated vs hybrid), replacing precomputed ineligibility codes with runtime calculations based on pool mode and normalized device sizes. Disk size metrics are updated from GiB (1024³) to GB (1000³) throughout tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
Closing this draft after verifying the reported issue was due to unit confusion: current main already uses an 8 GiB minimum, so there is no product change to ship here. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1978 +/- ##
==========================================
+ Coverage 52.08% 52.11% +0.03%
==========================================
Files 1031 1032 +1
Lines 71564 71628 +64
Branches 8090 8113 +23
==========================================
+ Hits 37275 37332 +57
- Misses 34164 34171 +7
Partials 125 125 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This updates the onboarding internal boot flow so dedicated boot can use marketed 8 GB devices, while keeping the existing internal sizing contract for hybrid boot reservations.
The final behavior in this branch is:
8,000,000,000bytes and largerGB)MiBinternally (16384,4096, etc.)MiB-based threshold behaviorProblem
The Setup Boot step and the internal boot wizard support both:
The wizard was still treating dedicated boot devices like the older split-boot flow and only allowing devices that met the larger internal minimum. That blocked marketed 8 GB devices from being used as dedicated boot devices even though they should be valid for the dedicated path.
At the same time, onboarding already follows a mixed sizing model:
MiBWe wanted to fix the dedicated 8 GB issue without changing that established internal contract.
What Changed
Dedicated boot eligibility
In web/src/components/Onboarding/steps/OnboardingInternalBootStep.vue, disk eligibility is now mode-aware:
8,000,000,000byte minimumMiB-based boot reservation thresholdThat means a marketed 8 GB device is now selectable for dedicated boot, but it is still rejected in hybrid mode if it does not satisfy the existing internal
MiBrequirement.Internal sizing contract preserved
The onboarding flow still uses
MiBinternally for boot reservation sizing:bootSizeMiB4096and16384poolBootSizeinMiBThis keeps the internal contract aligned with the existing implementation while still showing users metric disk sizes.
Tests updated
The onboarding tests were updated to cover the intended behavior:
Files of Interest
web/src/components/Onboarding/steps/OnboardingInternalBootStep.vueweb/src/components/Onboarding/store/onboardingDraft.tsweb/__test__/components/Onboarding/OnboardingInternalBootStep.test.tsweb/__test__/components/Onboarding/OnboardingSummaryStep.test.tsapi/src/unraid-api/cli/generated/graphql.tsValidation
Ran:
cd web && pnpm codegencd web && pnpm exec vitest run __test__/components/Onboarding/OnboardingInternalBootStep.test.ts __test__/components/Onboarding/internalBoot.test.ts __test__/components/Onboarding/OnboardingSummaryStep.test.ts __test__/components/Onboarding/OnboardingInternalBootStandalone.test.ts __test__/components/Onboarding/OnboardingNextStepsStep.test.tscd web && pnpm type-checkcd api && pnpm test -- src/unraid-api/graph/resolvers/onboarding/onboarding-internal-boot.service.spec.ts src/unraid-api/graph/resolvers/onboarding/onboarding.mutation.spec.tsNote: the API test command expands to a broader repo suite in this workspace, and that broader run completed successfully as well.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests