diff --git a/cmd/ebs-bootstrap.go b/cmd/ebs-bootstrap.go index 680e45a..6dc9a6a 100644 --- a/cmd/ebs-bootstrap.go +++ b/cmd/ebs-bootstrap.go @@ -46,7 +46,6 @@ func main() { validators := []config.Validator{ config.NewFileSystemValidator(), config.NewModeValidator(), - config.NewResizeThresholdValidator(), config.NewMountPointValidator(), config.NewMountOptionsValidator(), config.NewOwnerValidator(uos), diff --git a/configs/ubuntu.yml b/configs/ubuntu.yml index e305eb2..7278ac8 100644 --- a/configs/ubuntu.yml +++ b/configs/ubuntu.yml @@ -1,6 +1,5 @@ defaults: - resizeFs: true - resizeThreshold: 99 + resize: true devices: /dev/vdb: fs: xfs @@ -17,4 +16,4 @@ devices: user: ubuntu group: ubuntu permissions: 755 - lvmConsumption: 30 + lvmConsumption: 100 diff --git a/internal/backend/lvm.go b/internal/backend/lvm.go index 6145bf0..aa54b4e 100644 --- a/internal/backend/lvm.go +++ b/internal/backend/lvm.go @@ -10,6 +10,44 @@ import ( "github.com/reecetech/ebs-bootstrap/internal/service" ) +const ( + // The % tolerance to expect the logical volume size to be within + // ------------------------------------------------------- + // If the (logical volume / volume group size) * 100 is less than + // (lvmConsumption% - tolerance%) then we perform a resize operation + // ------------------------------------------------------- + // If the (logical volume / volume group size) * 100 is greater than + // (lvmConsumption% + tolerance%) then the user is attempting a downsize + // operation. We outright deny this as downsizing can be a destructive + // operation + // ------------------------------------------------------- + // Why implement a tolernace-based policy for resizing? + // - When creating a Logical Volume, `ebs-bootstrap` issues a command like + // `lvcreate -l 20%VG -n lv_name vg_name` + // - When we calculate how much percentage of the volume group has been + // consumed by the logical volume, the value would look like 20.0052096... + // - A tolerance establishes a window of acceptable values for avoiding a + // resizing operation + LogicalVolumeResizeTolerance = float64(0.1) + // The % threshold at which to resize a physical volume + // ------------------------------------------------------- + // If the (physical volume / device size) * 100 falls + // under this threshold then we perform a resize operation + // ------------------------------------------------------- + // The smallest gp3 EBS volume you can create is 1GiB (1073741824 bytes). + // The default size of the extent of a PV is 4 MiB (4194304 bytes). + // Typically, the first extent of a PV is reserved for metadata. This + // produces a PV of size 1069547520 bytes (Usage=99.6093%). We ensure + // that we set the resize threshold to 99.6% to ensure that a 1 GiB EBS + // volume won't be always resized + // ------------------------------------------------------- + // Why not just look for a difference of 4194304 bytes? + // - The size of the extent can be changed by the user + // - Therefore we may not always see a difference of 4194304 bytes between + // the block device and physical volume size + PhysicalVolumeResizeThreshold = float64(99.6) +) + type LvmBackend interface { CreatePhysicalVolume(name string) action.Action CreateVolumeGroup(name string, physicalVolume string) action.Action @@ -19,9 +57,9 @@ type LvmBackend interface { GetLogicalVolume(name string, volumeGroup string) (*model.LogicalVolume, error) SearchLogicalVolumes(volumeGroup string) ([]*model.LogicalVolume, error) SearchVolumeGroup(physicalVolume string) (*model.VolumeGroup, error) - ShouldResizePhysicalVolume(name string, threshold float64) (bool, error) + ShouldResizePhysicalVolume(name string) (bool, error) ResizePhysicalVolume(name string) action.Action - ShouldResizeLogicalVolume(name string, volumeGroup string, volumeGroupPercent int, tolerance float64) (bool, error) + ShouldResizeLogicalVolume(name string, volumeGroup string, volumeGroupPercent int) (bool, error) ResizeLogicalVolume(name string, volumeGroup string, volumeGroupPercent int) action.Action From(config *config.Config) error } @@ -124,7 +162,7 @@ func (lb *LinuxLvmBackend) ActivateLogicalVolume(name string, volumeGroup string return action.NewActivateLogicalVolumeAction(name, volumeGroup, lb.lvmService) } -func (lb *LinuxLvmBackend) ShouldResizePhysicalVolume(name string, threshold float64) (bool, error) { +func (lb *LinuxLvmBackend) ShouldResizePhysicalVolume(name string) (bool, error) { pvn, err := lb.lvmGraph.GetPhysicalVolume(name) if err != nil { return false, nil @@ -133,16 +171,16 @@ func (lb *LinuxLvmBackend) ShouldResizePhysicalVolume(name string, threshold flo if len(dn) == 0 { return false, nil } - return (float64(pvn.Size) / float64(dn[0].Size) * 100) < threshold, nil + return (float64(pvn.Size) / float64(dn[0].Size) * 100) < PhysicalVolumeResizeThreshold, nil } func (lb *LinuxLvmBackend) ResizePhysicalVolume(name string) action.Action { return action.NewResizePhysicalVolumeAction(name, lb.lvmService) } -func (lb *LinuxLvmBackend) ShouldResizeLogicalVolume(name string, volumeGroup string, volumeGroupPercent int, tolerance float64) (bool, error) { - left := float64(volumeGroupPercent) - tolerance - right := float64(volumeGroupPercent) + tolerance +func (lb *LinuxLvmBackend) ShouldResizeLogicalVolume(name string, volumeGroup string, volumeGroupPercent int) (bool, error) { + left := float64(volumeGroupPercent) - LogicalVolumeResizeTolerance + right := float64(volumeGroupPercent) + LogicalVolumeResizeTolerance lvn, err := lb.lvmGraph.GetLogicalVolume(name, volumeGroup) if err != nil { return false, err diff --git a/internal/backend/metrics.go b/internal/backend/metrics.go index 6243bb5..a4f3e97 100644 --- a/internal/backend/metrics.go +++ b/internal/backend/metrics.go @@ -8,8 +8,13 @@ import ( "github.com/reecetech/ebs-bootstrap/internal/service" ) +const ( + FileSystemResizeThreshold = float64(99.9) +) + type DeviceMetricsBackend interface { GetBlockDeviceMetrics(name string) (*model.BlockDeviceMetrics, error) + ShouldResize(bdm *model.BlockDeviceMetrics) bool From(config *config.Config) error } @@ -43,6 +48,10 @@ func (dmb *LinuxDeviceMetricsBackend) GetBlockDeviceMetrics(name string) (*model return metrics, nil } +func (dmb *LinuxDeviceMetricsBackend) ShouldResize(bdm *model.BlockDeviceMetrics) bool { + return (float64(bdm.FileSystemSize) / float64(bdm.BlockDeviceSize) * 100) < FileSystemResizeThreshold +} + func (dmb *LinuxDeviceMetricsBackend) From(config *config.Config) error { dmb.blockDeviceMetrics = nil blockDeviceMetrics := map[string]*model.BlockDeviceMetrics{} diff --git a/internal/backend/metrics_test.go b/internal/backend/metrics_test.go index 1a7f4f7..37e999f 100644 --- a/internal/backend/metrics_test.go +++ b/internal/backend/metrics_test.go @@ -51,6 +51,42 @@ func TestGetBlockDeviceMetrics(t *testing.T) { } } +func TestLinuxDeviceMetricsBackendShouldResize(t *testing.T) { + subtests := []struct { + Name string + BlockDeviceMetrics *model.BlockDeviceMetrics + ExpectedOutput bool + }{ + // FileSystemThreshold = 99.9% + // 9989 / 10000 → 99.89% < 99.9% → true + { + Name: "Should Resize", + BlockDeviceMetrics: &model.BlockDeviceMetrics{ + FileSystemSize: 9989, + BlockDeviceSize: 10000, + }, + ExpectedOutput: true, + }, + // FileSystemThreshold = 99.9% + // 9999 / 10000 → 99.9% < 99.9% → false + { + Name: "Should Not Resize", + BlockDeviceMetrics: &model.BlockDeviceMetrics{ + FileSystemSize: 9990, + BlockDeviceSize: 10000, + }, + ExpectedOutput: false, + }, + } + for _, subtest := range subtests { + t.Run(subtest.Name, func(t *testing.T) { + dmb := NewMockLinuxDeviceMetricsBackend(nil) + shouldResize := dmb.ShouldResize(subtest.BlockDeviceMetrics) + utils.CheckOutput("dmb.ShouldResize()", t, subtest.ExpectedOutput, shouldResize) + }) + } +} + func TestLinuxDeviceMetricsBackendFrom(t *testing.T) { fssf := service.NewLinuxFileSystemServiceFactory(nil) diff --git a/internal/config/config.go b/internal/config/config.go index 87ac001..c4b3283 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -17,13 +17,12 @@ const ( ) type Flag struct { - Config string - Mode string - Remount bool - MountOptions string - ResizeFs bool - ResizeThreshold float64 - LvmConsumption int + Config string + Mode string + Remount bool + MountOptions string + Resize bool + LvmConsumption int } type Device struct { @@ -38,12 +37,11 @@ type Device struct { } type Options struct { - Mode model.Mode `yaml:"mode"` - Remount bool `yaml:"remount"` - MountOptions model.MountOptions `yaml:"mountOptions"` - ResizeFs bool `yaml:"resizeFs"` - ResizeThreshold float64 `yaml:"resizeThreshold"` - LvmConsumption int `yaml:"lvmConsumption"` + Mode model.Mode `yaml:"mode"` + Remount bool `yaml:"remount"` + MountOptions model.MountOptions `yaml:"mountOptions"` + Resize bool `yaml:"resize"` + LvmConsumption int `yaml:"lvmConsumption"` } // We don't export "overrides" as this is an attribute that is used @@ -100,8 +98,7 @@ func parseFlags(program string, args []string) (*Flag, error) { flags.StringVar(&f.Mode, "mode", "", "override for mode") flags.BoolVar(&f.Remount, "remount", false, "override for remount") flags.StringVar(&f.MountOptions, "mount-options", "", "override for mount options") - flags.BoolVar(&f.ResizeFs, "resize-fs", false, "override for resize filesystem") - flags.Float64Var(&f.ResizeThreshold, "resize-threshold", 0, "override for resize threshold") + flags.BoolVar(&f.Resize, "resize", false, "override for resize filesystem") flags.IntVar(&f.LvmConsumption, "lvm-consumption", 0, "override for lvm consumption") // Actually parse the flag @@ -117,8 +114,8 @@ func (c *Config) setOverrides(f *Flag) *Config { c.overrides.Mode = model.Mode(f.Mode) c.overrides.Remount = f.Remount c.overrides.MountOptions = model.MountOptions(f.MountOptions) - c.overrides.ResizeFs = f.ResizeFs - c.overrides.ResizeThreshold = f.ResizeThreshold + c.overrides.Resize = f.Resize + c.overrides.LvmConsumption = f.LvmConsumption return c } @@ -164,26 +161,12 @@ func (c *Config) GetMountOptions(name string) model.MountOptions { return DefaultMountOptions } -func (c *Config) GetResizeFs(name string) bool { +func (c *Config) GetResize(name string) bool { cd, found := c.Devices[name] if !found { return false } - return c.overrides.ResizeFs || c.Defaults.ResizeFs || cd.ResizeFs -} - -func (c *Config) GetResizeThreshold(name string) float64 { - cd, found := c.Devices[name] - if !found { - return 0 - } - if c.overrides.ResizeThreshold > 0 { - return c.overrides.ResizeThreshold - } - if cd.ResizeThreshold > 0 { - return cd.ResizeThreshold - } - return c.Defaults.ResizeThreshold + return c.overrides.Resize || c.Defaults.Resize || cd.Resize } func (c *Config) GetLvmConsumption(name string) int { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a9cb2d6..ddac289 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -30,8 +30,7 @@ devices: group: root permissions: 755 label: external-vol - resizeFs: true - resizeThreshold: 95 + resize: true remount: true`), ExpectedOutput: &Config{ Defaults: Options{ @@ -46,9 +45,8 @@ devices: Permissions: model.FilePermissions(0755), Label: "external-vol", Options: Options{ - ResizeFs: true, - ResizeThreshold: 95, - Remount: true, + Resize: true, + Remount: true, }, }, }, @@ -133,14 +131,12 @@ devices: mode: prompt remount: true mountOptions: nouuid - resizeFs: true - resizeThreshold: 95`, device)), + resize: true`, device)), ExpectedOutput: &Options{ - Mode: model.Prompt, - Remount: true, - MountOptions: "nouuid", - ResizeFs: true, - ResizeThreshold: 95, + Mode: model.Prompt, + Remount: true, + MountOptions: "nouuid", + Resize: true, }, ExpectedError: nil, }, @@ -150,11 +146,10 @@ devices: devices: /dev/nonexist: ~`), ExpectedOutput: &Options{ - Mode: model.Healthcheck, - Remount: false, - MountOptions: "defaults", - ResizeFs: false, - ResizeThreshold: 0, + Mode: model.Healthcheck, + Remount: false, + MountOptions: "defaults", + Resize: false, }, ExpectedError: nil, }, @@ -169,11 +164,10 @@ devices: utils.CheckError("config.New()", t, subtest.ExpectedError, err) d := &Options{ - Mode: c.GetMode(device), - Remount: c.GetRemount(device), - MountOptions: c.GetMountOptions(device), - ResizeFs: c.GetResizeFs(device), - ResizeThreshold: c.GetResizeThreshold(device), + Mode: c.GetMode(device), + Remount: c.GetRemount(device), + MountOptions: c.GetMountOptions(device), + Resize: c.GetResize(device), } utils.CheckOutput("config.New()", t, subtest.ExpectedOutput, d) }) @@ -197,11 +191,10 @@ devices: Name: "Mode Flag Options", Args: []string{"ebs-bootstrap", "-config", c, "-mode", string(model.Force)}, ExpectedOutput: &Options{ - Mode: model.Force, - Remount: false, - MountOptions: "defaults", - ResizeFs: false, - ResizeThreshold: 0, + Mode: model.Force, + Remount: false, + MountOptions: "defaults", + Resize: false, }, ExpectedError: nil, }, @@ -209,23 +202,21 @@ devices: Name: "Mount Flag Options", Args: []string{"ebs-bootstrap", "-config", c, "-remount", "-mount-options", "nouuid"}, ExpectedOutput: &Options{ - Mode: model.Healthcheck, - Remount: true, - MountOptions: "nouuid", - ResizeFs: false, - ResizeThreshold: 0, + Mode: model.Healthcheck, + Remount: true, + MountOptions: "nouuid", + Resize: false, }, ExpectedError: nil, }, { Name: "Resize Flag Options", - Args: []string{"ebs-bootstrap", "-config", c, "-resize-fs", "-resize-threshold", "95"}, + Args: []string{"ebs-bootstrap", "-config", c, "-resize"}, ExpectedOutput: &Options{ - Mode: model.Healthcheck, - Remount: false, - MountOptions: "defaults", - ResizeFs: true, - ResizeThreshold: 95, + Mode: model.Healthcheck, + Remount: false, + MountOptions: "defaults", + Resize: true, }, ExpectedError: nil, }, @@ -236,11 +227,10 @@ devices: utils.CheckError("config.New()", t, subtest.ExpectedError, err) o := &Options{ - Mode: c.GetMode(device), - Remount: c.GetRemount(device), - MountOptions: c.GetMountOptions(device), - ResizeFs: c.GetResizeFs(device), - ResizeThreshold: c.GetResizeThreshold(device), + Mode: c.GetMode(device), + Remount: c.GetRemount(device), + MountOptions: c.GetMountOptions(device), + Resize: c.GetResize(device), } utils.CheckOutput("config.New()", t, subtest.ExpectedOutput, o) }) @@ -263,11 +253,10 @@ defaults: devices: %s: ~`, device)), ExpectedOutput: &Options{ - Mode: model.Force, - Remount: false, - MountOptions: "defaults", - ResizeFs: false, - ResizeThreshold: 0, + Mode: model.Force, + Remount: false, + MountOptions: "defaults", + Resize: false, }, ExpectedError: nil, }, @@ -280,11 +269,10 @@ defaults: devices: %s: ~`, device)), ExpectedOutput: &Options{ - Mode: model.Healthcheck, - Remount: true, - MountOptions: "nouuid", - ResizeFs: false, - ResizeThreshold: 0, + Mode: model.Healthcheck, + Remount: true, + MountOptions: "nouuid", + Resize: false, }, ExpectedError: nil, }, @@ -292,16 +280,14 @@ devices: Name: "Resize Default Options", Data: []byte(fmt.Sprintf(`--- defaults: - resizeFs: true - resizeThreshold: 95 + resize: true devices: %s: ~`, device)), ExpectedOutput: &Options{ - Mode: model.Healthcheck, - Remount: false, - MountOptions: "defaults", - ResizeFs: true, - ResizeThreshold: 95, + Mode: model.Healthcheck, + Remount: false, + MountOptions: "defaults", + Resize: true, }, ExpectedError: nil, }, @@ -316,11 +302,10 @@ devices: utils.CheckError("config.New()", t, subtest.ExpectedError, err) d := &Options{ - Mode: c.GetMode(device), - Remount: c.GetRemount(device), - MountOptions: c.GetMountOptions(device), - ResizeFs: c.GetResizeFs(device), - ResizeThreshold: c.GetResizeThreshold(device), + Mode: c.GetMode(device), + Remount: c.GetRemount(device), + MountOptions: c.GetMountOptions(device), + Resize: c.GetResize(device), } utils.CheckOutput("config.New()", t, subtest.ExpectedOutput, d) }) diff --git a/internal/config/validator.go b/internal/config/validator.go index 7e9f70b..ba89ecf 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -163,31 +163,6 @@ func (ov *OwnerValidator) Validate(c *Config) error { return nil } -type ResizeThresholdValidator struct{} - -func NewResizeThresholdValidator() *ResizeThresholdValidator { - return &ResizeThresholdValidator{} -} - -func (rtv *ResizeThresholdValidator) Validate(c *Config) error { - if !rtv.isValid(c.Defaults.ResizeThreshold) { - return fmt.Errorf("🔴 '%g' (default) must be a floating point between 0 and 100 (inclusive)", c.Defaults.ResizeThreshold) - } - if !rtv.isValid(c.overrides.ResizeThreshold) { - return fmt.Errorf("🔴 '%g' (-resize-threshold) must be a floating point between 0 and 100 (inclusive)", c.overrides.ResizeThreshold) - } - for name, device := range c.Devices { - if !rtv.isValid(device.ResizeThreshold) { - return fmt.Errorf("🔴 %s: '%g' must be a floating point between 0 and 100 (inclusive)", name, device.ResizeThreshold) - } - } - return nil -} - -func (rtv *ResizeThresholdValidator) isValid(rt float64) bool { - return rt >= 0 && rt <= 100 -} - type LvmConsumptionValidator struct{} func NewLvmConsumptionValidator() *LvmConsumptionValidator { diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 67c773b..20493f7 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -395,75 +395,3 @@ func TestOwnerValidator(t *testing.T) { }) } } - -func TestResizeThresholdValidator(t *testing.T) { - subtests := []struct { - Name string - Config *Config - ExpectedError error - }{ - { - Name: "Valid Resize Thresholds", - Config: &Config{ - Defaults: Options{ - ResizeThreshold: 100, - }, - Devices: map[string]Device{ - "/dev/xvdf": { - Options: Options{ - ResizeThreshold: 75.0, - }, - }, - }, - overrides: Options{ - ResizeThreshold: 0, - }, - }, - ExpectedError: nil, - }, - { - Name: "Invalid Resize Threshold (Defaults)", - Config: &Config{ - Defaults: Options{ - ResizeThreshold: -5.0, - }, - Devices: map[string]Device{ - "/dev/xvdf": {}, - }, - }, - ExpectedError: fmt.Errorf("🔴 '-5' (default) must be a floating point between 0 and 100 (inclusive)"), - }, - { - Name: "Invalid Resize Threshold (Overrides)", - Config: &Config{ - Devices: map[string]Device{ - "/dev/xvdf": {}, - }, - overrides: Options{ - ResizeThreshold: 110.0, - }, - }, - ExpectedError: fmt.Errorf("🔴 '110' (-resize-threshold) must be a floating point between 0 and 100 (inclusive)"), - }, - { - Name: "Invalid Resize Threshold (Device)", - Config: &Config{ - Devices: map[string]Device{ - "/dev/xvdf": { - Options: Options{ - ResizeThreshold: -1.0, - }, - }, - }, - }, - ExpectedError: fmt.Errorf("🔴 /dev/xvdf: '-1' must be a floating point between 0 and 100 (inclusive)"), - }, - } - for _, subtest := range subtests { - t.Run(subtest.Name, func(t *testing.T) { - rtv := NewResizeThresholdValidator() - err := rtv.Validate(subtest.Config) - utils.CheckError("rtv.Validate()", t, subtest.ExpectedError, err) - }) - } -} diff --git a/internal/layer/lv_resize.go b/internal/layer/lv_resize.go index 3303369..a462b71 100644 --- a/internal/layer/lv_resize.go +++ b/internal/layer/lv_resize.go @@ -8,27 +8,6 @@ import ( "github.com/reecetech/ebs-bootstrap/internal/config" ) -const ( - // The % tolerance to expect the logical volume size to be within - // ------------------------------------------------------- - // If the (logical volume / volume group size) * 100 is less than - // (lvmConsumption% - tolerance%) then we perform a resize operation - // ------------------------------------------------------- - // If the (logical volume / volume group size) * 100 is greater than - // (lvmConsumption% + tolerance%) then the user is attempting a downsize - // operation. We outright deny this as downsizing can be a destructive - // operation - // ------------------------------------------------------- - // Why implement a tolernace-based policy for resizing? - // - When creating a Logical Volume, `ebs-bootstrap` issues a command like - // `lvcreate -l 20%VG -n lv_name vg_name` - // - When we calculate how much percentage of the volume group has been - // consumed by the logical volume, the value would look like 20.0052096... - // - A tolerance establishes a window of acceptable values for avoiding a - // resizing operation - ResizeTolerance = float64(0.1) -) - type ResizeLogicalVolumeLayer struct { lvmBackend backend.LvmBackend } @@ -45,10 +24,10 @@ func (rpvl *ResizeLogicalVolumeLayer) Modify(c *config.Config) ([]action.Action, if len(cd.Lvm) == 0 { continue } - if !c.GetResizeFs(name) { + if !c.GetResize(name) { continue } - shouldResize, err := rpvl.lvmBackend.ShouldResizeLogicalVolume(cd.Lvm, cd.Lvm, c.GetLvmConsumption(name), ResizeTolerance) + shouldResize, err := rpvl.lvmBackend.ShouldResizeLogicalVolume(cd.Lvm, cd.Lvm, c.GetLvmConsumption(name)) if err != nil { return nil, err } @@ -67,10 +46,10 @@ func (rpvl *ResizeLogicalVolumeLayer) Validate(c *config.Config) error { if len(cd.Lvm) == 0 { continue } - if !c.GetResizeFs(name) { + if !c.GetResize(name) { continue } - shouldResize, err := rpvl.lvmBackend.ShouldResizeLogicalVolume(cd.Lvm, cd.Lvm, c.GetLvmConsumption(name), ResizeTolerance) + shouldResize, err := rpvl.lvmBackend.ShouldResizeLogicalVolume(cd.Lvm, cd.Lvm, c.GetLvmConsumption(name)) if err != nil { return err } @@ -91,7 +70,7 @@ func (rpvl *ResizeLogicalVolumeLayer) From(c *config.Config) error { func (rpvl *ResizeLogicalVolumeLayer) ShouldProcess(c *config.Config) bool { for name, cd := range c.Devices { - if len(cd.Lvm) > 0 && c.GetResizeFs(name) { + if len(cd.Lvm) > 0 && c.GetResize(name) { return true } } diff --git a/internal/layer/pv_resize.go b/internal/layer/pv_resize.go index 7bf9c04..a00fb60 100644 --- a/internal/layer/pv_resize.go +++ b/internal/layer/pv_resize.go @@ -8,26 +8,6 @@ import ( "github.com/reecetech/ebs-bootstrap/internal/config" ) -const ( - // The % threshold at which to resize a physical volume - // ------------------------------------------------------- - // If the (physical volume / device size) * 100 falls - // under this threshold then we perform a resize operation - // ------------------------------------------------------- - // The smallest gp3 EBS volume you can create is 1GiB (1073741824 bytes). - // The default size of the extent of a PV is 4 MiB (4194304 bytes). - // Typically, the first extent of a PV is reserved for metadata. This - // produces a PV of size 1069547520 bytes (Usage=99.6093%). We ensure - // that we set the resize threshold to 99.6% to ensure that a 1 GiB EBS - // volume won't be always resized - // ------------------------------------------------------- - // Why not just look for a difference of 4194304 bytes? - // - The size of the extent can be changed by the user - // - Therefore we may not always see a difference of 4194304 bytes between - // the block device and physical volume size - ResizeThreshold = float64(99.6) -) - type ResizePhysicalVolumeLayer struct { lvmBackend backend.LvmBackend } @@ -44,10 +24,10 @@ func (rpvl *ResizePhysicalVolumeLayer) Modify(c *config.Config) ([]action.Action if len(cd.Lvm) == 0 { continue } - if !c.GetResizeFs(name) { + if !c.GetResize(name) { continue } - shouldResize, err := rpvl.lvmBackend.ShouldResizePhysicalVolume(name, ResizeThreshold) + shouldResize, err := rpvl.lvmBackend.ShouldResizePhysicalVolume(name) if err != nil { return nil, err } @@ -66,10 +46,10 @@ func (rpvl *ResizePhysicalVolumeLayer) Validate(c *config.Config) error { if len(cd.Lvm) == 0 { continue } - if !c.GetResizeFs(name) { + if !c.GetResize(name) { continue } - shouldResize, err := rpvl.lvmBackend.ShouldResizePhysicalVolume(name, ResizeThreshold) + shouldResize, err := rpvl.lvmBackend.ShouldResizePhysicalVolume(name) if err != nil { return err } @@ -90,7 +70,7 @@ func (rpvl *ResizePhysicalVolumeLayer) From(c *config.Config) error { func (rpvl *ResizePhysicalVolumeLayer) ShouldProcess(c *config.Config) bool { for name, cd := range c.Devices { - if len(cd.Lvm) > 0 && c.GetResizeFs(name) { + if len(cd.Lvm) > 0 && c.GetResize(name) { return true } } diff --git a/internal/layer/resize.go b/internal/layer/resize.go index 0d2042a..b9b84d8 100644 --- a/internal/layer/resize.go +++ b/internal/layer/resize.go @@ -31,7 +31,7 @@ func (fdl *ResizeDeviceLayer) From(c *config.Config) error { func (fdl *ResizeDeviceLayer) Modify(c *config.Config) ([]action.Action, error) { actions := make([]action.Action, 0) for name := range c.Devices { - if !c.GetResizeFs(name) { + if !c.GetResize(name) { continue } @@ -44,36 +44,30 @@ func (fdl *ResizeDeviceLayer) Modify(c *config.Config) ([]action.Action, error) return nil, err } + if !fdl.deviceMetricsBackend.ShouldResize(metrics) { + continue + } mode := c.GetMode(name) - rt := c.GetResizeThreshold(name) - // If the resize threshold is set to 0, always attempt to resize the block device. - // For the currently supported file systems xfs and ext4, the commands to - // resize the block device are idempotent - if rt == 0 || metrics.ShouldResize(rt) { - a, err := fdl.deviceBackend.Resize(bd) - if err != nil { - return nil, err - } - a = a.SetMode(mode) - actions = append(actions, a) + a, err := fdl.deviceBackend.Resize(bd) + if err != nil { + return nil, err } + a = a.SetMode(mode) + actions = append(actions, a) } return actions, nil } func (fdl *ResizeDeviceLayer) Validate(c *config.Config) error { for name := range c.Devices { - if !c.GetResizeFs(name) { + if !c.GetResize(name) { continue } metrics, err := fdl.deviceMetricsBackend.GetBlockDeviceMetrics(name) if err != nil { return err } - rt := c.GetResizeThreshold(name) - // If the resize threshold is 0, then the device would always be resized - // Therefore, lets ignore that case from our validation checks - if rt > 0 && metrics.ShouldResize(rt) { + if fdl.deviceMetricsBackend.ShouldResize(metrics) { return fmt.Errorf("🔴 %s: Failed to resize file system. File System=%d Block Device=%d (bytes)", name, metrics.FileSystemSize, metrics.BlockDeviceSize) } } @@ -86,7 +80,7 @@ func (fdl *ResizeDeviceLayer) Warning() string { func (fdl *ResizeDeviceLayer) ShouldProcess(c *config.Config) bool { for name := range c.Devices { - if c.GetResizeFs(name) { + if c.GetResize(name) { return true } } diff --git a/internal/layer/resize_test.go b/internal/layer/resize_test.go index e99f26a..e7dfbdd 100644 --- a/internal/layer/resize_test.go +++ b/internal/layer/resize_test.go @@ -24,14 +24,13 @@ func TestResizeDeviceLayerModify(t *testing.T) { ExpectedError error }{ { - Name: "Resize Not Required: File System Size Within Threshold", + Name: "Resize Not Required: File System Size Within FileSystemResizeThreshold", Config: &config.Config{ Devices: map[string]config.Device{ "/dev/xvdf": { Fs: model.Ext4, Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 90, + Resize: true, }, }, }, @@ -44,8 +43,8 @@ func TestResizeDeviceLayerModify(t *testing.T) { }, BlockDeviceMetrics: map[string]*model.BlockDeviceMetrics{ "/dev/xvdf": { - FileSystemSize: 95, - BlockDeviceSize: 100, + FileSystemSize: 9990, + BlockDeviceSize: 10000, }, }, CmpOption: cmp.AllowUnexported(), @@ -53,14 +52,13 @@ func TestResizeDeviceLayerModify(t *testing.T) { ExpectedError: nil, }, { - Name: "Resize Block Device + Resize Threshold=90% + Target=Device", + Name: "Resize Block Device", Config: &config.Config{ Devices: map[string]config.Device{ "/dev/xvdf": { Fs: model.Ext4, Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 90, + Resize: true, }, }, }, @@ -73,8 +71,8 @@ func TestResizeDeviceLayerModify(t *testing.T) { }, BlockDeviceMetrics: map[string]*model.BlockDeviceMetrics{ "/dev/xvdf": { - FileSystemSize: 80, - BlockDeviceSize: 100, + FileSystemSize: 9989, + BlockDeviceSize: 10000, }, }, CmpOption: cmp.AllowUnexported( @@ -86,16 +84,16 @@ func TestResizeDeviceLayerModify(t *testing.T) { }, ExpectedError: nil, }, + // These devices must be { - Name: "Resize Block Device + Resize Threshold=90% + Target=Mount Point", + Name: "Resize Block Device: Must be Mounted", Config: &config.Config{ Devices: map[string]config.Device{ "/dev/xvdf": { Fs: model.Xfs, MountPoint: "/mnt/foo", Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 90, + Resize: true, }, }, }, @@ -123,14 +121,13 @@ func TestResizeDeviceLayerModify(t *testing.T) { ExpectedError: nil, }, { - Name: "Resize Block Device (With Mount Point) + But No Mount Point Provided", + Name: "Resize Block Device: Must be Mounted But No Mount Point Provided", Config: &config.Config{ Devices: map[string]config.Device{ "/dev/xvdf": { Fs: model.Xfs, Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 90, + Resize: true, }, }, }, @@ -151,40 +148,6 @@ func TestResizeDeviceLayerModify(t *testing.T) { ExpectedOuput: nil, ExpectedError: fmt.Errorf("🔴 /dev/xvdf: To resize the xfs file system, device must be mounted"), }, - { - Name: "Resize Block Device + Resize Threshold=0% (Always Resize) + Target=Device", - Config: &config.Config{ - Devices: map[string]config.Device{ - "/dev/xvdf": { - Fs: model.Ext4, - Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 0, - }, - }, - }, - }, - Devices: map[string]*model.BlockDevice{ - "/dev/xvdf": { - Name: "/dev/xvdf", - FileSystem: model.Ext4, - }, - }, - BlockDeviceMetrics: map[string]*model.BlockDeviceMetrics{ - "/dev/xvdf": { - FileSystemSize: 100, - BlockDeviceSize: 100, - }, - }, - CmpOption: cmp.AllowUnexported( - action.ResizeDeviceAction{}, - service.Ext4Service{}, - ), - ExpectedOuput: []action.Action{ - action.NewResizeDeviceAction("/dev/xvdf", "/dev/xvdf", service.NewExt4Service(nil)).SetMode(config.DefaultMode), - }, - ExpectedError: nil, - }, { Name: "Skip + Resize Disabled", Config: &config.Config{ @@ -192,7 +155,7 @@ func TestResizeDeviceLayerModify(t *testing.T) { "/dev/xvdf": { Fs: model.Ext4, Options: config.Options{ - ResizeFs: false, + Resize: false, }, }, }, @@ -241,8 +204,7 @@ func TestResizeDeviceLayerValidate(t *testing.T) { "/dev/xvdf": { Fs: model.Ext4, Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 90, + Resize: true, }, }, }, @@ -255,35 +217,8 @@ func TestResizeDeviceLayerValidate(t *testing.T) { }, BlockDeviceMetrics: map[string]*model.BlockDeviceMetrics{ "/dev/xvdf": { - FileSystemSize: 95, - BlockDeviceSize: 100, - }, - }, - ExpectedError: nil, - }, - { - Name: "Skip + Resize Threshold=0%", - Config: &config.Config{ - Devices: map[string]config.Device{ - "/dev/xvdf": { - Fs: model.Ext4, - Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 0, - }, - }, - }, - }, - Devices: map[string]*model.BlockDevice{ - "/dev/xvdf": { - Name: "/dev/xvdf", - FileSystem: model.Ext4, - }, - }, - BlockDeviceMetrics: map[string]*model.BlockDeviceMetrics{ - "/dev/xvdf": { - FileSystemSize: 80, - BlockDeviceSize: 100, + FileSystemSize: 9990, + BlockDeviceSize: 10000, }, }, ExpectedError: nil, @@ -295,7 +230,7 @@ func TestResizeDeviceLayerValidate(t *testing.T) { "/dev/xvdf": { Fs: model.Ext4, Options: config.Options{ - ResizeFs: false, + Resize: false, }, }, }, @@ -308,8 +243,8 @@ func TestResizeDeviceLayerValidate(t *testing.T) { }, BlockDeviceMetrics: map[string]*model.BlockDeviceMetrics{ "/dev/xvdf": { - FileSystemSize: 80, - BlockDeviceSize: 100, + FileSystemSize: 9989, + BlockDeviceSize: 10000, }, }, ExpectedError: nil, @@ -321,8 +256,7 @@ func TestResizeDeviceLayerValidate(t *testing.T) { "/dev/xvdf": { Fs: model.Ext4, Options: config.Options{ - ResizeFs: true, - ResizeThreshold: 90, + Resize: true, }, }, }, @@ -335,11 +269,11 @@ func TestResizeDeviceLayerValidate(t *testing.T) { }, BlockDeviceMetrics: map[string]*model.BlockDeviceMetrics{ "/dev/xvdf": { - FileSystemSize: 80, - BlockDeviceSize: 100, + FileSystemSize: 9989, + BlockDeviceSize: 10000, }, }, - ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Failed to resize file system. File System=80 Block Device=100 (bytes)"), + ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Failed to resize file system. File System=9989 Block Device=10000 (bytes)"), }, } for _, subtest := range subtests { diff --git a/internal/model/device.go b/internal/model/device.go index a23daa8..98cc984 100644 --- a/internal/model/device.go +++ b/internal/model/device.go @@ -32,9 +32,3 @@ type BlockDeviceMetrics struct { FileSystemSize uint64 BlockDeviceSize uint64 } - -func (bdm *BlockDeviceMetrics) ShouldResize(threshold float64) bool { - // Minimum File System Size (mfss) - mfss := float64(bdm.BlockDeviceSize) * (threshold / 100) - return float64(bdm.FileSystemSize) < mfss -} diff --git a/internal/model/device_test.go b/internal/model/device_test.go index a8d999a..a5a60c7 100644 --- a/internal/model/device_test.go +++ b/internal/model/device_test.go @@ -33,45 +33,3 @@ func TestRemount(t *testing.T) { utils.CheckOutput("ParseRemount()", t, subtest.ExpectedOutput, mo) } } - -func TestShouldResize(t *testing.T) { - subtests := []struct { - Name string - ResizeThreshold float64 - FileSystemSize uint64 - BlockDeviceSize uint64 - ExpectedOutput bool - }{ - { - Name: "Resize=✅", - ResizeThreshold: 95, - FileSystemSize: 90, - BlockDeviceSize: 100, - ExpectedOutput: true, - }, - { - Name: "Resize=❌", - ResizeThreshold: 85, - FileSystemSize: 90, - BlockDeviceSize: 100, - ExpectedOutput: false, - }, - { - Name: "Resize=❌ ", - ResizeThreshold: 100, - FileSystemSize: 100, - BlockDeviceSize: 100, - ExpectedOutput: false, - }, - } - for _, subtest := range subtests { - t.Run(subtest.Name, func(t *testing.T) { - bdm := &BlockDeviceMetrics{ - subtest.FileSystemSize, - subtest.BlockDeviceSize, - } - sr := bdm.ShouldResize(subtest.ResizeThreshold) - utils.CheckOutput("bdm.ShouldResize()", t, subtest.ExpectedOutput, sr) - }) - } -}