From a98c72f2922247cfd98bd7c1bb28dbd639d39724 Mon Sep 17 00:00:00 2001 From: lasith-kg Date: Wed, 3 Jan 2024 04:44:04 +0000 Subject: [PATCH] (feat): Add test coverage for layer.LabelDeviceLayer() --- internal/layer/format.go | 2 +- internal/layer/format_test.go | 8 +- internal/layer/label.go | 6 - internal/layer/label_test.go | 226 ++++++++++++++++++++++++++++++++++ internal/layer/layer_test.go | 2 +- 5 files changed, 232 insertions(+), 12 deletions(-) create mode 100644 internal/layer/label_test.go diff --git a/internal/layer/format.go b/internal/layer/format.go index 34976bc..687eedf 100644 --- a/internal/layer/format.go +++ b/internal/layer/format.go @@ -37,7 +37,7 @@ func (fdl *FormatDeviceLayer) Modify(c *config.Config) ([]action.Action, error) return nil, fmt.Errorf("🔴 %s: Can not erase the file system of a device", bd.Name) } if bd.FileSystem != model.Unformatted { - return nil, fmt.Errorf("🔴 %s: Can not format a device that already has a %s file system", bd.Name, bd.FileSystem.String()) + return nil, fmt.Errorf("🔴 %s: Can not format a device with an existing %s file system", bd.Name, bd.FileSystem.String()) } a, err := fdl.deviceBackend.Format(bd, cd.Fs) if err != nil { diff --git a/internal/layer/format_test.go b/internal/layer/format_test.go index 813e014..7e1086d 100644 --- a/internal/layer/format_test.go +++ b/internal/layer/format_test.go @@ -88,7 +88,7 @@ func TestFormatDeviceLayerModify(t *testing.T) { ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Can not erase the file system of a device"), }, { - Name: "Attempting to Change the File System of a Block Device", + Name: "Attempting to Change the Existing File System of a Block Device", Config: &config.Config{ Defaults: config.Options{Mode: model.Force}, Devices: map[string]config.Device{ @@ -105,7 +105,7 @@ func TestFormatDeviceLayerModify(t *testing.T) { }, CmpOption: cmp.AllowUnexported(), ExpectedOutput: nil, - ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Can not format a device that already has a xfs file system"), + ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Can not format a device with an existing xfs file system"), }, } for _, subtest := range subtests { @@ -127,7 +127,7 @@ func TestFormatDeviceLayerValidate(t *testing.T) { ExpectedOutput error }{ { - Name: "Valid Block Device", + Name: "File System Matches Requested File System", Config: &config.Config{ Devices: map[string]config.Device{ "/dev/xvdf": { @@ -144,7 +144,7 @@ func TestFormatDeviceLayerValidate(t *testing.T) { ExpectedOutput: nil, }, { - Name: "Invalid Block Device", + Name: "File System Does Not Match Requested File System", Config: &config.Config{ Devices: map[string]config.Device{ "/dev/xvdf": { diff --git a/internal/layer/label.go b/internal/layer/label.go index bc4453d..44e2401 100644 --- a/internal/layer/label.go +++ b/internal/layer/label.go @@ -6,7 +6,6 @@ import ( "github.com/reecetech/ebs-bootstrap/internal/action" "github.com/reecetech/ebs-bootstrap/internal/backend" "github.com/reecetech/ebs-bootstrap/internal/config" - "github.com/reecetech/ebs-bootstrap/internal/model" ) type LabelDeviceLayer struct { @@ -36,12 +35,7 @@ func (fdl *LabelDeviceLayer) Modify(c *config.Config) ([]action.Action, error) { if bd.Label == cd.Label { continue } - if bd.FileSystem == model.Unformatted { - return nil, fmt.Errorf("🔴 %s: Can not label a device with no file system", bd.Name) - } mode := c.GetMode(name) - // Labelling a device can potentially require unmounting it first - // Therefore, multiple actions may be returned: Label Actions (las) las, err := fdl.deviceBackend.Label(bd, cd.Label) if err != nil { return nil, err diff --git a/internal/layer/label_test.go b/internal/layer/label_test.go new file mode 100644 index 0000000..049901c --- /dev/null +++ b/internal/layer/label_test.go @@ -0,0 +1,226 @@ +package layer + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/reecetech/ebs-bootstrap/internal/action" + "github.com/reecetech/ebs-bootstrap/internal/backend" + "github.com/reecetech/ebs-bootstrap/internal/config" + "github.com/reecetech/ebs-bootstrap/internal/model" + "github.com/reecetech/ebs-bootstrap/internal/service" + "github.com/reecetech/ebs-bootstrap/internal/utils" +) + +func TestLabelDeviceLayerModify(t *testing.T) { + subtests := []struct { + Name string + Config *config.Config + Devices map[string]*model.BlockDevice + CmpOption cmp.Option + ExpectedOuput []action.Action + ExpectedError error + }{ + { + Name: "Label Device With File System That Avoids Unmounting", + Config: &config.Config{ + Defaults: config.Options{Mode: model.Force}, + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Ext4, + Label: "label", + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + FileSystem: model.Ext4, + }, + }, + CmpOption: cmp.AllowUnexported( + action.LabelDeviceAction{}, + service.Ext4Service{}, + ), + ExpectedOuput: []action.Action{ + action.NewLabelDeviceAction("/dev/xvdf", "label", service.NewExt4Service(nil)).SetMode(model.Force), + }, + ExpectedError: nil, + }, + { + Name: "Label Device With File System That Requires Unmounting", + Config: &config.Config{ + Defaults: config.Options{Mode: model.Force}, + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Xfs, + Label: "label", + MountPoint: "/mnt/foo", + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + FileSystem: model.Xfs, + MountPoint: "/mnt/foo", + }, + }, + CmpOption: cmp.AllowUnexported( + action.UnmountDeviceAction{}, + action.LabelDeviceAction{}, + service.XfsService{}, + ), + ExpectedOuput: []action.Action{ + action.NewUnmountDeviceAction("/dev/xvdf", "/mnt/foo", nil).SetMode(model.Force), + action.NewLabelDeviceAction("/dev/xvdf", "label", service.NewXfsService(nil)).SetMode(model.Force), + }, + ExpectedError: nil, + }, + { + Name: "Label Matches Requested Label", + Config: &config.Config{ + Defaults: config.Options{Mode: model.Force}, + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Ext4, + Label: "label", + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + FileSystem: model.Ext4, + Label: "label", + }, + }, + CmpOption: cmp.AllowUnexported(), + ExpectedOuput: []action.Action{}, + ExpectedError: nil, + }, + { + Name: "Skip Labelling", + Config: &config.Config{ + Defaults: config.Options{Mode: model.Force}, + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Ext4, + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + FileSystem: model.Ext4, + }, + }, + CmpOption: cmp.AllowUnexported(), + ExpectedOuput: []action.Action{}, + ExpectedError: nil, + }, + { + Name: "Attempting to Label Block Device With No File System", + Config: &config.Config{ + Defaults: config.Options{Mode: model.Force}, + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Ext4, + Label: "label", + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + }, + }, + CmpOption: cmp.AllowUnexported(), + ExpectedOuput: nil, + ExpectedError: fmt.Errorf("🔴 /dev/xvdf: An unformatted file system can not be queried/modified"), + }, + } + for _, subtest := range subtests { + t.Run(subtest.Name, func(t *testing.T) { + ldb := backend.NewMockLinuxDeviceBackend(subtest.Devices) + layer := NewLabelDeviceLayer(ldb) + actions, err := layer.Modify(subtest.Config) + utils.CheckError("LabelDeviceLayer.Modify()", t, subtest.ExpectedError, err) + utils.CheckOutput("LabelDeviceLayer.Modify()", t, subtest.ExpectedOuput, actions, subtest.CmpOption) + }) + } +} + +func TestLabelDeviceLayerValidate(t *testing.T) { + subtests := []struct { + Name string + Config *config.Config + Devices map[string]*model.BlockDevice + ExpectedError error + }{ + { + Name: "Label Matches Requested Label", + Config: &config.Config{ + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Ext4, + Label: "label", + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + FileSystem: model.Ext4, + Label: "label", + }, + }, + ExpectedError: nil, + }, + { + Name: "Skipping Validation", + Config: &config.Config{ + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Ext4, + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + FileSystem: model.Ext4, + }, + }, + ExpectedError: nil, + }, + { + Name: "Label Does Not Match Requested Label", + Config: &config.Config{ + Devices: map[string]config.Device{ + "/dev/xvdf": { + Fs: model.Ext4, + Label: "label", + }, + }, + }, + Devices: map[string]*model.BlockDevice{ + "/dev/xvdf": { + Name: "/dev/xvdf", + FileSystem: model.Ext4, + Label: "not-label", + }, + }, + ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Failed label validation checks. Expected=label, Actual=not-label"), + }, + } + for _, subtest := range subtests { + t.Run(subtest.Name, func(t *testing.T) { + ldb := backend.NewMockLinuxDeviceBackend(subtest.Devices) + ldl := NewLabelDeviceLayer(ldb) + err := ldl.Validate(subtest.Config) + utils.CheckError("ldl.Validate()", t, subtest.ExpectedError, err) + }) + } +} diff --git a/internal/layer/layer_test.go b/internal/layer/layer_test.go index bd30c3b..b2a3e1d 100644 --- a/internal/layer/layer_test.go +++ b/internal/layer/layer_test.go @@ -43,7 +43,7 @@ func (ml *MockLayer) Warning() string { func TestExponentialBackoffLayerExecutor(t *testing.T) { mae := action.NewDefaultActionExecutor() // Lets generate ExponentialBackoffParameters with a custom - // InitialInterval. We do not want to slow down the test suite + // InitialInterval of 10 ms. We do not want to slow down the test suite // with an excessively long initial interval debp := DefaultExponentialBackoffParameters() ebp := &ExponentialBackoffParameters{