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

[BUG] Can't use data disk measured in terabytes v1.2.1 #4734

Closed
MikeTolkachev opened this issue Nov 16, 2023 · 8 comments
Closed

[BUG] Can't use data disk measured in terabytes v1.2.1 #4734

MikeTolkachev opened this issue Nov 16, 2023 · 8 comments
Assignees
Labels
backport-needed/1.2.2 kind/bug Issues that are defects reported by users or that we know have reached a real release priority/2 Nice to fix in this release reproduce/always Reproducible 100% of the time severity/4 Function working but has a minor issue (a minor incident with low impact)
Milestone

Comments

@MikeTolkachev
Copy link

I'm trying to use 4TB nvme drive for data disk and i receive a message:
Disk size is to small. Minimum 100Gi is required.

photo_2023-11-16_20-15-38 (2)

@MikeTolkachev MikeTolkachev added kind/bug Issues that are defects reported by users or that we know have reached a real release reproduce/needed Reminder to add a reproduce label and to remove this one severity/needed Reminder to add a severity label and to remove this one labels Nov 16, 2023
@MikeTolkachev MikeTolkachev changed the title [BUG] Can't use data disk measured in terabytes [BUG] Can't use data disk measured in terabytes v1.2.1 Nov 16, 2023
@connorkuehl
Copy link
Contributor

What size is your install disk?

@MikeTolkachev
Copy link
Author

Disk is 120Gi (111 on picture).
Installation is available if I select the first disk (120) for both installation and data.
photo_2023-11-16_20-15-38

@tserong
Copy link
Contributor

tserong commented Nov 21, 2023

I've tried to reproduce this, and I believe you're seeing that error not because of the 4TiB data disk, but because the installation disk is too small. In my testing, v1.2.1 needs at least a 180GiB installation disk if you have a separate data disk.

Here's what I got when I tried a 4TiB data disk:

harvester-4tb-data-disk

I realise that in your second screenshot (with the installation disk and data disk the same) it doesn't show an error, but what happens if you hit ENTER at that point to try to commence the install? In my case, I don't see an error immediately, but once I hit ENTER on the persistent size field, with a single 120GiB disk, I get the error "Disk size is too small. Minimum 250Gi is required":

harvester-120g-install-disk

@tserong
Copy link
Contributor

tserong commented Nov 21, 2023

Assuming I'm correct about the above, I suspect we need to make two changes:

  1. For clarity, make the error say "Installation disk is too small", rather than "Disk is too small".
  2. When using the installation disk for data, ensure the error message appears early, rather than waiting 'til the user hits ENTER at the end.

tserong added a commit to tserong/harvester-installer that referenced this issue Nov 21, 2023
This commit adds disk size validation in the calls to diskConfirm()
and dataDiskConfirm(), so that as you walk through the fields you'll
see potential error messages as you go, rather than having to wait
until leaving the last field on the page.  The error message clears
if you press the up arrow to go to previous fields.  This commit
also changes the wording of the error message to explicitly state
when the installation disk is too small.

Fixes: harvester/harvester#4734
Signed-off-by: Tim Serong <[email protected]>
@tserong tserong self-assigned this Nov 21, 2023
@bk201 bk201 added this to the v1.3.0 milestone Nov 22, 2023
@bk201 bk201 added priority/2 Nice to fix in this release severity/4 Function working but has a minor issue (a minor incident with low impact) reproduce/always Reproducible 100% of the time and removed severity/needed Reminder to add a severity label and to remove this one reproduce/needed Reminder to add a reproduce label and to remove this one labels Nov 22, 2023
@harvesterhci-io-github-bot
Copy link
Collaborator

harvesterhci-io-github-bot commented Nov 22, 2023

Pre Ready-For-Testing Checklist

  • If labeled: require/HEP Has the Harvester Enhancement Proposal PR submitted?
    The HEP PR is at:

  • Where is the reproduce steps/test steps documented?
    The reproduce steps/test steps are at: [e2e] [BUG] Can't use data disk measured in terabytes v1.2.1 tests#987

  • Is there a workaround for the issue? If so, where is it documented?
    The workaround is at:

  • Have the backend code been merged (harvester, harvester-installer, etc) (including backport-needed/*)?
    The PR is at: console: validate disk size earlier, update error wording harvester-installer#596

    • Does the PR include the explanation for the fix or the feature?

    • Does the PR include deployment change (YAML/Chart)? If so, where are the PRs for both YAML file and Chart?
      The PR for the YAML change is at:
      The PR for the chart change is at:

  • If labeled: area/ui Has the UI issue filed or ready to be merged?
    The UI issue/PR is at:

  • If labeled: require/doc, require/knowledge-base Has the necessary document PR submitted or merged?
    The documentation/KB PR is at:

  • If NOT labeled: not-require/test-plan Has the e2e test plan been merged? Have QAs agreed on the automation test case? If only test case skeleton w/o implementation, have you created an implementation issue?

    • The automation skeleton PR is at:
    • The automation test case PR is at:
  • If the fix introduces the code for backward compatibility Has a separate issue been filed with the label release/obsolete-compatibility?
    The compatibility issue is filed at:

@harvesterhci-io-github-bot
Copy link
Collaborator

Automation e2e test issue: harvester/tests#987

@harvesterhci-io-github-bot
Copy link
Collaborator

added backport-needed/1.2.2 issue: #4793.

@irishgordo
Copy link
Contributor

This looks good on v1.3.0-rc1 😄 👍
I'll go ahead and close this out:
Screenshot from 2024-01-29 10-48-07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-needed/1.2.2 kind/bug Issues that are defects reported by users or that we know have reached a real release priority/2 Nice to fix in this release reproduce/always Reproducible 100% of the time severity/4 Function working but has a minor issue (a minor incident with low impact)
Projects
None yet
Development

No branches or pull requests

6 participants