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

Update disk resize to work with lvms-storage-operator/topolvm #4114

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions pkg/crc/config/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ func RegisterSettings(cfg *Config) {
return ValidateBool(value)
}

validPVSize := func(value interface{}) (bool, string) {
return validatePersistentVolumeSize(value, GetPreset(cfg))
}

// Preset setting should be on top because CPUs/Memory config depend on it.
cfg.AddSetting(Preset, version.GetDefaultPreset().String(), validatePreset, RequiresDeleteAndSetupMsg,
fmt.Sprintf("Virtual machine preset (valid values are: %s)", preset.AllPresets()))
Expand All @@ -93,8 +97,8 @@ func RegisterSettings(cfg *Config) {
"Enable experimental features (true/false, default: false)")
cfg.AddSetting(EmergencyLogin, false, ValidateBool, SuccessfullyApplied,
"Enable emergency login for 'core' user. Password is randomly generated. (true/false, default: false)")
cfg.AddSetting(PersistentVolumeSize, constants.DefaultPersistentVolumeSize, validatePersistentVolumeSize, SuccessfullyApplied,
fmt.Sprintf("Total size in GiB of the persistent volume used by the CSI driver for %s preset (must be greater than or equal to '%d')", preset.Microshift, constants.DefaultPersistentVolumeSize))
cfg.AddSetting(PersistentVolumeSize, defaultPVSize(cfg), validPVSize, SuccessfullyApplied,
fmt.Sprintf("Total size in GiB of the persistent volume used by the CSI driver (must be greater than or equal to '%d')", defaultPVSize(cfg)))

// Shared directories configs
if runtime.GOOS == "windows" {
Expand Down Expand Up @@ -155,6 +159,10 @@ func defaultCPUs(cfg Storage) int {
return constants.GetDefaultCPUs(GetPreset(cfg))
}

func defaultPVSize(cfg Storage) int {
return constants.GetDefaultPersistentVolumeSize(GetPreset(cfg))
}

func defaultMemory(cfg Storage) int {
return constants.GetDefaultMemory(GetPreset(cfg))
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/crc/config/validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ func validateDiskSize(value interface{}) (bool, string) {
}

// validatePersistentVolumeSize checks if provided disk size is valid in the config
func validatePersistentVolumeSize(value interface{}) (bool, string) {
func validatePersistentVolumeSize(value interface{}, preset crcpreset.Preset) (bool, string) {
diskSize, err := cast.ToIntE(value)
if err != nil {
return false, fmt.Sprintf("could not convert '%s' to integer", value)
}
if err := validation.ValidatePersistentVolumeSize(diskSize); err != nil {
if err := validation.ValidatePersistentVolumeSize(diskSize, preset); err != nil {
return false, err.Error()
}

Expand Down
14 changes: 12 additions & 2 deletions pkg/crc/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ const (
DefaultName = "crc"
DefaultDiskSize = 31

DefaultPersistentVolumeSize = 15

DefaultSSHUser = "core"
DefaultSSHPort = 22

Expand Down Expand Up @@ -223,6 +221,18 @@ func GetDefaultMemory(preset crcpreset.Preset) int {
}
}

func GetDefaultPersistentVolumeSize(preset crcpreset.Preset) int {
switch preset {
case crcpreset.OpenShift, crcpreset.OKD:
return 3
case crcpreset.Microshift:
return 15
default:
// should not be reached
return 3
Copy link
Member

Choose a reason for hiding this comment

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

Will it be enough? because registry also going to use the pvc which is going to part of this partition and if a user mirror an image to openshift or even build a image from openshift then this space is going to be filled very fast :(

Copy link
Member Author

Choose a reason for hiding this comment

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

right, we might have to increase the default disk-size, but there's very limited free space available, and currently that space is shared for both image-registry and host container image storage, with this since we have to create a separate partition this becomes more apparent:

[core@crc ~]$ df -h /dev/vda4
Filesystem      Size  Used Avail Use% Mounted on
/dev/vda4        31G   23G  7.6G  76% /

We only have 7.6G free space, of which now 4.6G is for host container images and 3G for PVs

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe we can increase the default disk-size to 40 and default persistent-volume-size to 7, these are just arbitrary numbers, but it'd leave around 7gb free space on the root partition (closer to the current situation)

(the bundle sizes are also increasing which would mean less free space for workloads, right? i saw the 4.16.3 bundle from CI in 400MB larger than the 4.16.0 bundle)

}
}

func GetDefaultBundleImageRegistry(preset crcpreset.Preset) string {
return fmt.Sprintf("//%s/%s:%s", RegistryURI, getImageName(preset), version.GetBundleVersion(preset))
}
Expand Down
196 changes: 196 additions & 0 deletions pkg/crc/machine/disks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
package machine

import (
"context"
"errors"
"fmt"
"strconv"
"strings"

"github.com/crc-org/crc/v2/pkg/crc/constants"
logging "github.com/crc-org/crc/v2/pkg/crc/logging"
"github.com/crc-org/crc/v2/pkg/crc/machine/types"
crcPreset "github.com/crc-org/crc/v2/pkg/crc/preset"
crcssh "github.com/crc-org/crc/v2/pkg/crc/ssh"
"golang.org/x/crypto/ssh"
)

func growPartition(sshRunner *crcssh.Runner, partition string) error {
if _, _, err := sshRunner.RunPrivileged(fmt.Sprintf("Growing %s partition", partition), "/usr/bin/growpart", partition[:len("/dev/.da")], partition[len("/dev/.da"):]); err != nil {
var exitErr *ssh.ExitError
if !errors.As(err, &exitErr) {
return err
}
if exitErr.ExitStatus() != 1 {
return err
}
logging.Debugf("No free space after %s, nothing to do", partition)
return nil
}
return nil
}

func ocpGetPVShiftSize(diskSize int, pvSize int) int {
defaultPvSize := constants.GetDefaultPersistentVolumeSize(crcPreset.OpenShift)
if pvSize > defaultPvSize {
return (diskSize - constants.DefaultDiskSize) - (pvSize - defaultPvSize)
}
return diskSize - constants.DefaultDiskSize
}

func moveTopolvmPartition(ctx context.Context, shiftSize int, vm *virtualMachine, sshRunner *crcssh.Runner) error {
pvPartition, err := getTopolvmPartition(sshRunner)
if err != nil {
return err
}
_, _, err = sshRunner.RunPrivileged("move topolvm partition to end of disk",
fmt.Sprintf("echo '+%dG,' | sudo sfdisk --move-data /dev/vda -N %s --force", shiftSize, pvPartition[len("/dev/.da"):]))
var exitErr *ssh.ExitError
if err != nil {
if !errors.As(err, &exitErr) {
return err
}
if exitErr.ExitStatus() != 1 {
return err
}
}
if err == nil {
logging.Info("Restart VM after moving topolvm partition to end")
if err := restartHost(ctx, vm, sshRunner); err != nil {
return fmt.Errorf("Failed to move topolvm partition to increase root partition size: %w", err)
}
}
return nil
}

func growRootFileSystem(ctx context.Context, startConfig types.StartConfig, vm *virtualMachine, sshRunner *crcssh.Runner) error {
if startConfig.Preset == crcPreset.OpenShift {
sizeToMove := ocpGetPVShiftSize(startConfig.DiskSize, startConfig.PersistentVolumeSize)
if err := moveTopolvmPartition(ctx, sizeToMove, vm, sshRunner); err != nil {
return err
}
}
rootPart, err := getrootPartition(sshRunner, startConfig.Preset)
if err != nil {
return err
}

if err := growPersistentVolume(sshRunner, startConfig.Preset, startConfig.PersistentVolumeSize); err != nil {
return fmt.Errorf("Unable to grow persistent volume partition: %w", err)
}

// with '/dev/[sv]da4' as input, run 'growpart /dev/[sv]da 4'
if err := growPartition(sshRunner, rootPart); err != nil {
return nil
}

logging.Infof("Resizing %s filesystem", rootPart)
rootFS := "/sysroot"
if _, _, err := sshRunner.RunPrivileged(fmt.Sprintf("Remounting %s read/write", rootFS), "mount -o remount,rw", rootFS); err != nil {
return err
}
if _, _, err = sshRunner.RunPrivileged(fmt.Sprintf("Growing %s filesystem", rootFS), "xfs_growfs", rootFS); err != nil {
return err
}

return nil
}

func growPersistentVolume(sshRunner *crcssh.Runner, preset crcPreset.Preset, persistentVolumeSize int) error {
if preset == crcPreset.Microshift {
rootPart, err := getrootPartition(sshRunner, preset)
if err != nil {
return err
}
lvFullName := "rhel/root"
if err := growLVForMicroshift(sshRunner, lvFullName, rootPart, persistentVolumeSize); err != nil {
return err
}
}

if preset == crcPreset.OpenShift {
pvPartition, err := getTopolvmPartition(sshRunner)
if err != nil {
return err
}
if _, _, err := sshRunner.RunPrivileged(fmt.Sprintf("Growing %s partition", pvPartition), "/usr/bin/growpart", pvPartition[:len("/dev/.da")], pvPartition[len("/dev/.da"):]); err != nil {
var exitErr *ssh.ExitError
if !errors.As(err, &exitErr) {
return err
}
if exitErr.ExitStatus() != 1 {
return err
}
logging.Debugf("No free space after %s, nothing to do", pvPartition)
return nil
}
}
return nil
}

func getrootPartition(sshRunner *crcssh.Runner, preset crcPreset.Preset) (string, error) {
query := "--label root"
if preset == crcPreset.Microshift {
query = "-t TYPE=LVM2_member"
}
return getDeviceID(sshRunner, query)
}

func getTopolvmPartition(sshRunner *crcssh.Runner) (string, error) {
return getDeviceID(sshRunner, "-t PARTLABEL=topolvm")
}

func getDeviceID(sshRunner *crcssh.Runner, query string) (string, error) {
part, _, err := sshRunner.RunPrivileged("Get device id", "/usr/sbin/blkid", query, "-o", "device")
if err != nil {
return "", err
}
parts := strings.Split(strings.TrimSpace(part), "\n")
if len(parts) != 1 {
return "", fmt.Errorf("Unexpected number of devices: %s", part)
}
part = strings.TrimSpace(parts[0])
if !strings.HasPrefix(part, "/dev/vda") && !strings.HasPrefix(part, "/dev/sda") {
return "", fmt.Errorf("Unexpected device: %s", part)
}
return part, nil
}

func growLVForMicroshift(sshRunner *crcssh.Runner, lvFullName string, rootPart string, persistentVolumeSize int) error {
if _, _, err := sshRunner.RunPrivileged("Resizing the physical volume(PV)", "/usr/sbin/pvresize", "--devices", rootPart, rootPart); err != nil {
return err
}

// Get the size of volume group
sizeVG, _, err := sshRunner.RunPrivileged("Get the volume group size", "/usr/sbin/vgs", "--noheadings", "--nosuffix", "--units", "b", "-o", "vg_size", "--devices", rootPart)
if err != nil {
return err
}
vgSize, err := strconv.Atoi(strings.TrimSpace(sizeVG))
if err != nil {
return err
}

// Get the size of root lv
sizeLV, _, err := sshRunner.RunPrivileged("Get the size of root logical volume", "/usr/sbin/lvs", "-S", fmt.Sprintf("lv_full_name=%s", lvFullName), "--noheadings", "--nosuffix", "--units", "b", "-o", "lv_size", "--devices", rootPart)
if err != nil {
return err
}
lvSize, err := strconv.Atoi(strings.TrimSpace(sizeLV))
if err != nil {
return err
}

GB := 1073741824
vgFree := persistentVolumeSize * GB
expectedLVSize := vgSize - vgFree
sizeToIncrease := expectedLVSize - lvSize
lvPath := fmt.Sprintf("/dev/%s", lvFullName)
if sizeToIncrease > 1 {
logging.Info("Extending and resizing '/dev/rhel/root' logical volume")
if _, _, err := sshRunner.RunPrivileged("Extending and resizing the logical volume(LV)", "/usr/sbin/lvextend", "-L", fmt.Sprintf("+%db", sizeToIncrease), lvPath, "--devices", rootPart); err != nil {
return err
}
}
return nil
}
102 changes: 9 additions & 93 deletions pkg/crc/machine/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
libmachinestate "github.com/crc-org/machine/libmachine/state"
"github.com/docker/go-units"
"github.com/pkg/errors"
"golang.org/x/crypto/ssh"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -112,99 +111,16 @@ func (client *client) updateVMConfig(startConfig types.StartConfig, vm *virtualM
return nil
}

func growRootFileSystem(sshRunner *crcssh.Runner, preset crcPreset.Preset, persistentVolumeSize int) error {
rootPart, err := getrootPartition(sshRunner, preset)
if err != nil {
return err
}

// with '/dev/[sv]da4' as input, run 'growpart /dev/[sv]da 4'
if _, _, err := sshRunner.RunPrivileged(fmt.Sprintf("Growing %s partition", rootPart), "/usr/bin/growpart", rootPart[:len("/dev/.da")], rootPart[len("/dev/.da"):]); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

The corresponding code in the new file has become

	if err := growPersistentVolume(sshRunner, startConfig.Preset, startConfig.PersistentVolumeSize); err != nil {
		return fmt.Errorf("Unable to grow persistent volume partition: %w", err)
	}

	// with '/dev/[sv]da4' as input, run 'growpart /dev/[sv]da 4'
	if err := growPartition(sshRunner, rootPart); err != nil {
		return nil
	}

While the addition of a helper is a good thing, this should not be hidden in a commit saying it's only moving code.
Fwiw, a helper for move topolvm partition to end of disk could also be useful to make growRootFilesystem easier to read.

var exitErr *ssh.ExitError
if !errors.As(err, &exitErr) {
return err
}
if exitErr.ExitStatus() != 1 {
return err
}
logging.Debugf("No free space after %s, nothing to do", rootPart)
return nil
}

if preset == crcPreset.Microshift {
lvFullName := "rhel/root"
if err := growLVForMicroshift(sshRunner, lvFullName, rootPart, persistentVolumeSize); err != nil {
return err
}
}

logging.Infof("Resizing %s filesystem", rootPart)
rootFS := "/sysroot"
if _, _, err := sshRunner.RunPrivileged(fmt.Sprintf("Remounting %s read/write", rootFS), "mount -o remount,rw", rootFS); err != nil {
return err
}
if _, _, err = sshRunner.RunPrivileged(fmt.Sprintf("Growing %s filesystem", rootFS), "xfs_growfs", rootFS); err != nil {
return err
}

return nil
}

func getrootPartition(sshRunner *crcssh.Runner, preset crcPreset.Preset) (string, error) {
diskType := "xfs"
if preset == crcPreset.Microshift {
diskType = "LVM2_member"
}
part, _, err := sshRunner.RunPrivileged("Get device id", "/usr/sbin/blkid", "-t", fmt.Sprintf("TYPE=%s", diskType), "-o", "device")
if err != nil {
return "", err
func restartHost(ctx context.Context, vm *virtualMachine, sshRunner *crcssh.Runner) error {
if err := vm.Driver.Stop(); err != nil {
return errors.Wrap(err, "Error re-starting machine at stop")
}
parts := strings.Split(strings.TrimSpace(part), "\n")
if len(parts) != 1 {
return "", fmt.Errorf("Unexpected number of devices: %s", part)
}
rootPart := strings.TrimSpace(parts[0])
if !strings.HasPrefix(rootPart, "/dev/vda") && !strings.HasPrefix(rootPart, "/dev/sda") {
return "", fmt.Errorf("Unexpected root device: %s", rootPart)
}
return rootPart, nil
}

func growLVForMicroshift(sshRunner *crcssh.Runner, lvFullName string, rootPart string, persistentVolumeSize int) error {
if _, _, err := sshRunner.RunPrivileged("Resizing the physical volume(PV)", "/usr/sbin/pvresize", "--devices", rootPart, rootPart); err != nil {
return err
}

// Get the size of volume group
sizeVG, _, err := sshRunner.RunPrivileged("Get the volume group size", "/usr/sbin/vgs", "--noheadings", "--nosuffix", "--units", "b", "-o", "vg_size", "--devices", rootPart)
if err != nil {
return err
}
vgSize, err := strconv.Atoi(strings.TrimSpace(sizeVG))
if err != nil {
return err
}

// Get the size of root lv
sizeLV, _, err := sshRunner.RunPrivileged("Get the size of root logical volume", "/usr/sbin/lvs", "-S", fmt.Sprintf("lv_full_name=%s", lvFullName), "--noheadings", "--nosuffix", "--units", "b", "-o", "lv_size", "--devices", rootPart)
if err != nil {
return err
}
lvSize, err := strconv.Atoi(strings.TrimSpace(sizeLV))
if err != nil {
return err
if err := startHost(ctx, vm); err != nil {
return errors.Wrap(err, "Error re-starting machine at start")
}

GB := 1073741824
vgFree := persistentVolumeSize * GB
expectedLVSize := vgSize - vgFree
sizeToIncrease := expectedLVSize - lvSize
lvPath := fmt.Sprintf("/dev/%s", lvFullName)
if sizeToIncrease > 1 {
logging.Info("Extending and resizing '/dev/rhel/root' logical volume")
if _, _, err := sshRunner.RunPrivileged("Extending and resizing the logical volume(LV)", "/usr/sbin/lvextend", "-L", fmt.Sprintf("+%db", sizeToIncrease), lvPath, "--devices", rootPart); err != nil {
return err
}
logging.Debug("Waiting until ssh is available")
if err := sshRunner.WaitForConnectivity(ctx, 300*time.Second); err != nil {
return errors.Wrap(err, "Failed to connect to the CRC VM with SSH -- virtual machine might be unreachable")
}
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

This restart means add more time to startup even it takes ~1 mins.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, its less than 1 min, but that's on linux, on windows i expect it to be even longer than 1 minute

Expand Down Expand Up @@ -430,7 +346,7 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
}

// Trigger disk resize, this will be a no-op if no disk size change is needed
if err := growRootFileSystem(sshRunner, startConfig.Preset, startConfig.PersistentVolumeSize); err != nil {
if err := growRootFileSystem(ctx, startConfig, vm, sshRunner); err != nil {
return nil, errors.Wrap(err, "Error updating filesystem size")
}

Expand Down
Loading
Loading