From ae0505fbb0efe816f29b183686a70d4063236e8b Mon Sep 17 00:00:00 2001 From: Lasith Koswatta Gamage Date: Tue, 28 Nov 2023 01:08:46 +0000 Subject: [PATCH] (feat): Big overhaul of layer and actions --- cmd/ebs-bootstrap.go | 5 ++- configs/ubuntu.yml | 6 ++-- internal/action/action.go | 13 ++------ internal/action/file.go | 12 ------- internal/action/format.go | 4 --- internal/action/label.go | 4 --- internal/action/mount.go | 8 ----- internal/backend/device.go | 25 +++++++++----- internal/backend/resize.go | 8 +++-- internal/layer/directory.go | 8 +++-- internal/layer/format.go | 10 ++++-- internal/layer/label.go | 8 +++-- internal/layer/layer.go | 11 +++++++ internal/layer/mount.go | 27 +++++++++------ internal/layer/owner.go | 8 +++-- internal/layer/permissions.go | 8 +++-- internal/layer/resize.go | 4 +++ internal/layer/umount.go | 60 ---------------------------------- internal/service/filesystem.go | 15 +++++++-- 19 files changed, 105 insertions(+), 139 deletions(-) delete mode 100644 internal/layer/umount.go diff --git a/cmd/ebs-bootstrap.go b/cmd/ebs-bootstrap.go index f26f39f..f411c99 100644 --- a/cmd/ebs-bootstrap.go +++ b/cmd/ebs-bootstrap.go @@ -55,17 +55,16 @@ func main() { }) checkError(bv.Validate(c)) // Layers - layers := layer.NewExponentialBackoffLayerExecutor([]layer.Layer{ + le := layer.NewExponentialBackoffLayerExecutor([]layer.Layer{ layer.NewFormatDeviceLayer(db), layer.NewLabelDeviceLayer(db), - layer.NewUnmountDeviceLayer(db, fb), layer.NewCreateDirectoryLayer(db, fb), layer.NewMountDeviceLayer(db, fb), layer.NewResizeDeviceLayer(drb), layer.NewChangeOwnerLayer(ub, fb), layer.NewChangePermissionsLayer(fb), }, ae) - checkError(layers.Execute(c)) + checkError(le.Execute(c)) } func checkError(err error) { diff --git a/configs/ubuntu.yml b/configs/ubuntu.yml index 46b03d0..ec5e0bd 100644 --- a/configs/ubuntu.yml +++ b/configs/ubuntu.yml @@ -2,11 +2,11 @@ defaults: mode: healthcheck devices: /dev/vdb: - fs: ext4 + fs: xfs label: lasith-rules - mountPoint: /ifmx/dev/james + mountPoint: /ifmx/tempp mountOptions: defaults group: ubuntu user: ubuntu - permissions: 644 + permissions: 0644 resizeFs: true \ No newline at end of file diff --git a/internal/action/action.go b/internal/action/action.go index b224a41..fb566c5 100644 --- a/internal/action/action.go +++ b/internal/action/action.go @@ -9,18 +9,12 @@ import ( "github.com/reecetech/ebs-bootstrap/internal/utils" ) -const ( - DisabledWarning = "" - FormatActionWarning = "Formatting larger disks can take several seconds ⌛ Please wait patiently 🙇" -) - type Action interface { Execute() error GetDeviceName() string Success() string Prompt() string Refuse() string - Warning() string } type ActionExecutor struct { @@ -53,9 +47,6 @@ func (ae *ActionExecutor) ExecuteAction(action Action) error { } func (ae *ActionExecutor) executeAction(action Action) error { - if w := action.Warning(); w != DisabledWarning { - log.Printf("🟠 %s: %s", action.GetDeviceName(), w) - } if err := action.Execute(); err != nil { return err } @@ -64,8 +55,8 @@ func (ae *ActionExecutor) executeAction(action Action) error { } func (ae *ActionExecutor) ExecuteActions(actions []Action) error { - for _, action := range actions { - err := ae.ExecuteAction(action) + for _, a := range actions { + err := ae.ExecuteAction(a) if err != nil { return err } diff --git a/internal/action/file.go b/internal/action/file.go index 4db9290..e6a1cf4 100644 --- a/internal/action/file.go +++ b/internal/action/file.go @@ -44,10 +44,6 @@ func (a *CreateDirectoryAction) Success() string { return fmt.Sprintf("Successfully created directory %s", a.path) } -func (a *CreateDirectoryAction) Warning() string { - return DisabledWarning -} - type ChangeOwnerAction struct { deviceName string path string @@ -84,10 +80,6 @@ func (a *ChangeOwnerAction) Success() string { return fmt.Sprintf("Successfully changed ownership (%d:%d) of %s", a.uid, a.gid, a.path) } -func (a *ChangeOwnerAction) Warning() string { - return DisabledWarning -} - type ChangePermissionsAction struct { deviceName string path string @@ -129,7 +121,3 @@ func (a *ChangePermissionsAction) Refuse() string { func (a *ChangePermissionsAction) Success() string { return fmt.Sprintf("Successfully change permissions of %s to %s", a.path, a.perms) } - -func (a *ChangePermissionsAction) Warning() string { - return DisabledWarning -} diff --git a/internal/action/format.go b/internal/action/format.go index 6c21ecf..2998f89 100644 --- a/internal/action/format.go +++ b/internal/action/format.go @@ -37,7 +37,3 @@ func (a *FormatDeviceAction) Refuse() string { func (a *FormatDeviceAction) Success() string { return fmt.Sprintf("Successfully formated to %s", a.fileSystemService.GetFileSystem()) } - -func (a *FormatDeviceAction) Warning() string { - return FormatActionWarning -} diff --git a/internal/action/label.go b/internal/action/label.go index 631e936..5dfb593 100644 --- a/internal/action/label.go +++ b/internal/action/label.go @@ -39,7 +39,3 @@ func (a *LabelDeviceAction) Refuse() string { func (a *LabelDeviceAction) Success() string { return fmt.Sprintf("Successfully labelled to %s", a.label) } - -func (a *LabelDeviceAction) Warning() string { - return DisabledWarning -} diff --git a/internal/action/mount.go b/internal/action/mount.go index e517960..05ced37 100644 --- a/internal/action/mount.go +++ b/internal/action/mount.go @@ -46,10 +46,6 @@ func (a *MountDeviceAction) Success() string { return fmt.Sprintf("Successfully mounted %s to %s (%s)", a.source, a.target, a.options) } -func (a *MountDeviceAction) Warning() string { - return DisabledWarning -} - type UnmountDeviceAction struct { source string target string @@ -87,7 +83,3 @@ func (a *UnmountDeviceAction) Refuse() string { func (a *UnmountDeviceAction) Success() string { return fmt.Sprintf("Successfully unmounted %s from %s", a.source, a.target) } - -func (a *UnmountDeviceAction) Warning() string { - return DisabledWarning -} diff --git a/internal/backend/device.go b/internal/backend/device.go index cfcbf41..b4cc7d4 100644 --- a/internal/backend/device.go +++ b/internal/backend/device.go @@ -11,7 +11,7 @@ import ( type DeviceBackend interface { GetBlockDevice(device string) (*model.BlockDevice, error) - Label(bd *model.BlockDevice, label string) (action.Action, error) + Label(bd *model.BlockDevice, label string) ([]action.Action, error) Format(bd *model.BlockDevice, fileSystem model.FileSystem) (action.Action, error) Mount(bd *model.BlockDevice, target string, options model.MountOptions) action.Action Remount(bd *model.BlockDevice, target string, options model.MountOptions) action.Action @@ -41,26 +41,35 @@ func (db *LinuxDeviceBackend) GetBlockDevice(device string) (*model.BlockDevice, return blockDevice, nil } -func (db *LinuxDeviceBackend) Label(bd *model.BlockDevice, label string) (action.Action, error) { - fs, err := db.fileSystemServiceFactory.Select(bd.FileSystem) +// This method is unique in the sense that some file-systems like xfs might require +// that the device be unmounted before any labelling operations can commence. For these +// file systems, we would preprend any label action with an unmount action (if the device is already mounted) +func (db *LinuxDeviceBackend) Label(bd *model.BlockDevice, label string) ([]action.Action, error) { + actions := make([]action.Action, 0) + fss, err := db.fileSystemServiceFactory.Select(bd.FileSystem) if err != nil { return nil, err } - return action.NewLabelDeviceAction( + if fss.DoesLabelRequireUnmount() && bd.MountPoint != "" { + a := db.Umount(bd) + actions = append(actions, a) + } + a := action.NewLabelDeviceAction( bd.Name, label, - fs, - ), nil + fss, + ) + return append(actions, a), nil } func (db *LinuxDeviceBackend) Format(bd *model.BlockDevice, fileSystem model.FileSystem) (action.Action, error) { - fs, err := db.fileSystemServiceFactory.Select(fileSystem) + fss, err := db.fileSystemServiceFactory.Select(fileSystem) if err != nil { return nil, err } return action.NewFormatDeviceAction( bd.Name, - fs, + fss, ), nil } diff --git a/internal/backend/resize.go b/internal/backend/resize.go index 039e571..a4eb385 100644 --- a/internal/backend/resize.go +++ b/internal/backend/resize.go @@ -59,7 +59,7 @@ func (dmb *LinuxDeviceResizeBackend) Resize(bd *model.BlockDevice) (action.Actio return nil, err } target := bd.Name - if fss.DoesResizeRequiresMount() { + if fss.DoesResizeRequireMount() { if bd.MountPoint == "" { return nil, fmt.Errorf("🔴 %s: To resize the %s file system, device must be mounted", fss.GetFileSystem(), bd.Name) } @@ -87,7 +87,11 @@ func (dmb *LinuxDeviceResizeBackend) From(config *config.Config) error { return err } // Can not fetch file system metrics from a device with - // no file system. Therefore, we continue if this is the case + // no file system. Therefore, we exit with error if this is the case + // The reason we exit early, rather than continuing is because we want + // to simplify the data view of the Device Resize Backend as much as possible + // With this check, we ensure that all devices in the resize backend have a + // valid file system if bd.FileSystem == model.Unformatted { return fmt.Errorf("🔴 %s: Can not resize a device with no file system", bd.Name) } diff --git a/internal/layer/directory.go b/internal/layer/directory.go index 070a5be..dc3c81b 100644 --- a/internal/layer/directory.go +++ b/internal/layer/directory.go @@ -50,8 +50,8 @@ func (fdl *CreateDirectoryLayer) Modify(c *config.Config) ([]action.Action, erro if d != nil { continue } - action := fdl.fileBackend.CreateDirectory(bd.Name, cd.MountPoint) - actions = append(actions, action) + a := fdl.fileBackend.CreateDirectory(bd.Name, cd.MountPoint) + actions = append(actions, a) } return actions, nil } @@ -68,3 +68,7 @@ func (fdl *CreateDirectoryLayer) Validate(c *config.Config) error { log.Println("🟢 Passed directory validation checks") return nil } + +func (fdl *CreateDirectoryLayer) Warning() string { + return DisableWarning +} diff --git a/internal/layer/format.go b/internal/layer/format.go index 5ebcf7f..b897ff8 100644 --- a/internal/layer/format.go +++ b/internal/layer/format.go @@ -34,12 +34,12 @@ func (fdl *FormatDeviceLayer) Modify(c *config.Config) ([]action.Action, error) if bd.FileSystem == cd.Fs { continue } + if cd.Fs == model.Unformatted { + 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) } - if bd.MountPoint != "" { - return nil, fmt.Errorf("🔴 %s: Can not format a device that is already mounted to %s", bd.Name, bd.MountPoint) - } action, err := fdl.deviceBackend.Format(bd, cd.Fs) if err != nil { return nil, err @@ -62,3 +62,7 @@ func (fdl *FormatDeviceLayer) Validate(c *config.Config) error { log.Println("🟢 Passed file system validation checks") return nil } + +func (fdl *FormatDeviceLayer) Warning() string { + return "Formatting larger disks can take several seconds ⌛" +} diff --git a/internal/layer/label.go b/internal/layer/label.go index 9bfb250..8bf80da 100644 --- a/internal/layer/label.go +++ b/internal/layer/label.go @@ -40,11 +40,11 @@ func (fdl *LabelDeviceLayer) Modify(c *config.Config) ([]action.Action, error) { if bd.FileSystem == model.Unformatted { return nil, fmt.Errorf("🔴 %s: Can not label a device with no file system", bd.Name) } - action, err := fdl.deviceBackend.Label(bd, cd.Label) + a, err := fdl.deviceBackend.Label(bd, cd.Label) if err != nil { return nil, err } - actions = append(actions, action) + actions = append(actions, a...) } return actions, nil } @@ -65,3 +65,7 @@ func (fdl *LabelDeviceLayer) Validate(c *config.Config) error { log.Println("🟢 Passed label validation checks") return nil } + +func (fdl *LabelDeviceLayer) Warning() string { + return "Certain file systems require that devices be unmounted prior to labeling" +} diff --git a/internal/layer/layer.go b/internal/layer/layer.go index d88f6f9..bc65b77 100644 --- a/internal/layer/layer.go +++ b/internal/layer/layer.go @@ -1,6 +1,7 @@ package layer import ( + "log" "math" "time" @@ -15,10 +16,15 @@ const ( MaxRetries = 3 ) +const ( + DisableWarning = "" +) + type Layer interface { From(config *config.Config) error Modify(c *config.Config) ([]action.Action, error) Validate(config *config.Config) error + Warning() string } type LayerExecutor interface { @@ -58,6 +64,11 @@ func (le *ExponentialBackoffLayerExecutor) Execute(config *config.Config) error if err != nil { return err } + // Only print warning if actions are detected and a valid warning + // message is provided + if warning := layer.Warning(); len(actions) > 0 && warning != DisableWarning { + log.Printf("🟠 %s", warning) + } err = le.actionExecutor.ExecuteActions(actions) if err != nil { return err diff --git a/internal/layer/mount.go b/internal/layer/mount.go index f3d39fe..3d49328 100644 --- a/internal/layer/mount.go +++ b/internal/layer/mount.go @@ -7,6 +7,7 @@ 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 MountDeviceLayer struct { @@ -39,16 +40,14 @@ func (fdl *MountDeviceLayer) Modify(c *config.Config) ([]action.Action, error) { if cd.MountPoint == "" { continue } - if bd.MountPoint != "" && bd.MountPoint != cd.MountPoint { - return nil, fmt.Errorf("🔴 %s: Device is already mounted to %s. Thus cannot be mounted to %s", name, bd.MountPoint, cd.MountPoint) + if bd.FileSystem == model.Unformatted { + return nil, fmt.Errorf("🔴 %s: Can not mount a device with no file system", bd.Name) } - - var a action.Action if bd.MountPoint == cd.MountPoint { - if !c.GetRemount(name) { - continue + if c.GetRemount(name) { + a := fdl.deviceBackend.Remount(bd, cd.MountPoint, cd.MountOptions) + actions = append(actions, a) } - a = fdl.deviceBackend.Remount(bd, cd.MountPoint, cd.MountOptions) } else { _, err = fdl.fileBackend.GetDirectory(cd.MountPoint) if err != nil { @@ -60,10 +59,14 @@ func (fdl *MountDeviceLayer) Modify(c *config.Config) ([]action.Action, error) { if fdl.fileBackend.IsMount(cd.MountPoint) { return nil, fmt.Errorf("🔴 %s: %s is already mounted by another device", name, cd.MountPoint) } - a = fdl.deviceBackend.Mount(bd, cd.MountPoint, cd.MountOptions) + // If mount point already exists, then lets unmount it first + if bd.MountPoint != "" { + a := fdl.deviceBackend.Umount(bd) + actions = append(actions, a) + } + a := fdl.deviceBackend.Mount(bd, cd.MountPoint, cd.MountOptions) + actions = append(actions, a) } - - actions = append(actions, a) } return actions, nil } @@ -84,3 +87,7 @@ func (fdl *MountDeviceLayer) Validate(c *config.Config) error { log.Printf("🟢 Passed mountpoint validation checks") return nil } + +func (fdl *MountDeviceLayer) Warning() string { + return "Devices mounted to a location not specified in the configuration will be unmounted" +} diff --git a/internal/layer/owner.go b/internal/layer/owner.go index 4367234..27d7b43 100644 --- a/internal/layer/owner.go +++ b/internal/layer/owner.go @@ -68,8 +68,8 @@ func (fdl *ChangeOwnerLayer) Modify(c *config.Config) ([]action.Action, error) { if d.Uid == uid && d.Gid == gid { continue } - action := fdl.fileBackend.ChangeOwner(name, cd.MountPoint, uid, gid) - actions = append(actions, action) + a := fdl.fileBackend.ChangeOwner(name, cd.MountPoint, uid, gid) + actions = append(actions, a) } return actions, nil } @@ -119,3 +119,7 @@ func (fdl *ChangeOwnerLayer) Validate(c *config.Config) error { log.Printf("🟢 Passed ownership validation checks") return nil } + +func (fdl *ChangeOwnerLayer) Warning() string { + return DisableWarning +} diff --git a/internal/layer/permissions.go b/internal/layer/permissions.go index 5eda5fd..31d1b21 100644 --- a/internal/layer/permissions.go +++ b/internal/layer/permissions.go @@ -42,8 +42,8 @@ func (fdl *ChangePermissionsLayer) Modify(c *config.Config) ([]action.Action, er continue } - action := fdl.fileBackend.ChangePermissions(name, cd.MountPoint, cd.Permissions) - actions = append(actions, action) + a := fdl.fileBackend.ChangePermissions(name, cd.MountPoint, cd.Permissions) + actions = append(actions, a) } return actions, nil } @@ -70,3 +70,7 @@ func (fdl *ChangePermissionsLayer) Validate(c *config.Config) error { log.Printf("🟢 Passed permissions validation checks") return nil } + +func (fdl *ChangePermissionsLayer) Warning() string { + return DisableWarning +} diff --git a/internal/layer/resize.go b/internal/layer/resize.go index 42a485c..117723f 100644 --- a/internal/layer/resize.go +++ b/internal/layer/resize.go @@ -64,3 +64,7 @@ func (fdl *ResizeDeviceLayer) Validate(c *config.Config) error { log.Println("🟢 Passed file system resize checks") return nil } + +func (fdl *ResizeDeviceLayer) Warning() string { + return DisableWarning +} diff --git a/internal/layer/umount.go b/internal/layer/umount.go deleted file mode 100644 index 9c36d27..0000000 --- a/internal/layer/umount.go +++ /dev/null @@ -1,60 +0,0 @@ -package layer - -import ( - "fmt" - "log" - - "github.com/reecetech/ebs-bootstrap/internal/action" - "github.com/reecetech/ebs-bootstrap/internal/backend" - "github.com/reecetech/ebs-bootstrap/internal/config" -) - -type UnmountDeviceLayer struct { - deviceBackend backend.DeviceBackend - fileBackend backend.FileBackend -} - -func NewUnmountDeviceLayer(db backend.DeviceBackend, fb backend.FileBackend) *UnmountDeviceLayer { - return &UnmountDeviceLayer{ - deviceBackend: db, - fileBackend: fb, - } -} - -func (fdl *UnmountDeviceLayer) From(c *config.Config) error { - err := fdl.deviceBackend.From(c) - if err != nil { - return err - } - return fdl.fileBackend.From(c) -} - -func (fdl *UnmountDeviceLayer) Modify(c *config.Config) ([]action.Action, error) { - actions := make([]action.Action, 0) - for name, cd := range c.Devices { - bd, err := fdl.deviceBackend.GetBlockDevice(name) - if err != nil { - return nil, err - } - if bd.MountPoint == "" || bd.MountPoint == cd.MountPoint { - continue - } - action := fdl.deviceBackend.Umount(bd) - actions = append(actions, action) - } - return actions, nil -} - -func (fdl *UnmountDeviceLayer) Validate(c *config.Config) error { - for name, cd := range c.Devices { - bd, err := fdl.deviceBackend.GetBlockDevice(name) - if err != nil { - return err - } - if bd.MountPoint != "" && bd.MountPoint != cd.MountPoint { - return fmt.Errorf("🔴 %s: Failed unmount validation checks. Device mounted to %s", name, bd.MountPoint) - } - } - log.Printf("🟢 Passed unmount validation checks") - return nil -} diff --git a/internal/service/filesystem.go b/internal/service/filesystem.go index a3600d9..cff7ee7 100644 --- a/internal/service/filesystem.go +++ b/internal/service/filesystem.go @@ -15,7 +15,8 @@ type FileSystemService interface { Format(name string) error Label(name string, label string) error Resize(name string) error - DoesResizeRequiresMount() bool + DoesResizeRequireMount() bool + DoesLabelRequireUnmount() bool } type FileSystemServiceFactory interface { @@ -125,7 +126,11 @@ func (es *Ext4Service) GetSize(name string) (int, error) { return bs * bc, nil } -func (es *Ext4Service) DoesResizeRequiresMount() bool { +func (es *Ext4Service) DoesResizeRequireMount() bool { + return false +} + +func (es *Ext4Service) DoesLabelRequireUnmount() bool { return false } @@ -201,6 +206,10 @@ func (xs *XfsService) GetSize(name string) (int, error) { return bs * bc, nil } -func (es *XfsService) DoesResizeRequiresMount() bool { +func (es *XfsService) DoesResizeRequireMount() bool { + return true +} + +func (es *XfsService) DoesLabelRequireUnmount() bool { return true }