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 installation disk space limit to 180Gi when using different disk for data. #528

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

masteryyh
Copy link
Contributor

@masteryyh masteryyh commented Jul 6, 2023

Problem:

When selecting different disk for data, 250Gi limit for installation disk is too large.

Solution:

Change the limit to 180Gi when using different disk for data.

Related Issue:
harvester/harvester#4133

Test plan:

ISO Installation

Single Disk

  1. Build an ISO from this branch;
  2. Boot into the installer, the installer should limit the installation disk at 250Gi minimum.

Multiple Disk

  1. Build an ISO from this branch;
  2. Boot into the installer, and use the same disk for installation disk and data disk;
  3. The installer should limit the installation disk at 250Gi minimum;
  4. When choosing different disk for installation and data, the installer should limit the installation disk at 180Gi minimum.

PXE Installation

Single disk

  1. Build artifacts from this branch;
  2. Use PXE to install Harvester artifacts you just built, when disk is smaller than 250Gi it should not continue.

Multiple disk

  1. Build artifacts from this branch;
  2. Prepare a Harvester config yaml, use another disk for data;
  3. Use PXE to install Harvester artifacts you just built, when installation disk is smaller than 180Gi or data disk is smaller than 50Gi the installer should not continue.

@masteryyh
Copy link
Contributor Author

FYI: It's ready for review.

Copy link
Contributor

@futuretea futuretea left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. Test Plan Passed. Just a small suggestion

pkg/console/install_panels.go Outdated Show resolved Hide resolved
Copy link
Contributor

@guangbochen guangbochen left a comment

Choose a reason for hiding this comment

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

LGTM, validated that both multi-disk and single-disk installation contains the correct partition.

vda    253:0    0 193273528320  0 disk
├─vda1 253:1    0      1048576  0 part
├─vda2 253:2    0     67108864  0 part /oem
├─vda3 253:3    0   4294967296  0 part
├─vda4 253:4    0   8589934592  0 part /run/initramfs/cos-state
└─vda5 253:5    0 180318371840  0 part /var/lib/longhorn
                                       /var/crash
                                       /var/lib/third-party
                                       /var/lib/cni
                                       /var/lib/wicked
                                       /var/lib/kubelet
                                       /var/lib/rancher
                                       /var/log
                                       /usr/libexec
                                       /root
                                       /opt
                                       /home
                                       /etc/pki/trust/anchors
                                       /etc/cni
                                       /etc/iscsi
                                       /etc/ssh
                                       /etc/rancher
                                       /etc/systemd
                                       /usr/local
vdb    253:16   0  53687091200  0 disk /var/lib/harvester/defaultdisk

pkg/console/install_panels.go Outdated Show resolved Hide resolved
Copy link
Contributor

@futuretea futuretea left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. need squash commits

@bk201
Copy link
Member

bk201 commented Jul 13, 2023

Excellent job; the merge makes the UX better. Can we wait for one more enter before proceeding to the next page when using a data disk? It's too rushed to jump to the next page immediately:

Screen.Recording.2023-07-13.at.4.18.24.PM.mov

Copy link
Member

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Contributor

@Vicente-Cheng Vicente-Cheng left a comment

Choose a reason for hiding this comment

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

Just some nits, others LGTM.
Tested with two disks (200G, 512G), the result as below
截圖 2023-07-17 上午10 42 19

pkg/console/install_panels.go Show resolved Hide resolved
pkg/console/install_panels.go Outdated Show resolved Hide resolved
pkg/console/install_panels.go Outdated Show resolved Hide resolved
pkg/console/install_panels.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Vicente-Cheng Vicente-Cheng left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for it! Let's wait for the CI.

@Vicente-Cheng Vicente-Cheng merged commit 60ad159 into harvester:master Jul 17, 2023
@Vicente-Cheng
Copy link
Contributor

@mergify backport v1.2

@mergify
Copy link

mergify bot commented Jul 17, 2023

backport v1.2

✅ Backports have been created

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.

5 participants