Skip to content

Commit

Permalink
chore: move and initialize exec correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakobmoellerdev
jakobmoellerdev committed Sep 11, 2023
1 parent d1535e1 commit 3407add
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 42 deletions.
20 changes: 10 additions & 10 deletions pkg/filter/filter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"fmt"
"strings"

"github.com/openshift/lvm-operator/pkg/internal"
"github.com/openshift/lvm-operator/pkg/internal/exec"
"github.com/openshift/lvm-operator/pkg/lsblk"
"github.com/openshift/lvm-operator/pkg/lvm"
)
Expand Down Expand Up @@ -51,31 +51,31 @@ const (
usableDeviceType = "usableDeviceType"
)

type Filters map[string]func(lsblk.BlockDevice, internal.Executor) (bool, error)
type Filters map[string]func(lsblk.BlockDevice, exec.Executor) (bool, error)

func DefaultFilters(lsblkInstance lsblk.LSBLK) Filters {
return Filters{
notReadOnly: func(dev lsblk.BlockDevice, _ internal.Executor) (bool, error) {
notReadOnly: func(dev lsblk.BlockDevice, _ exec.Executor) (bool, error) {
return !dev.ReadOnly, nil
},

notSuspended: func(dev lsblk.BlockDevice, _ internal.Executor) (bool, error) {
notSuspended: func(dev lsblk.BlockDevice, _ exec.Executor) (bool, error) {
matched := dev.State != StateSuspended
return matched, nil
},

noBiosBootInPartLabel: func(dev lsblk.BlockDevice, _ internal.Executor) (bool, error) {
noBiosBootInPartLabel: func(dev lsblk.BlockDevice, _ exec.Executor) (bool, error) {
biosBootInPartLabel := strings.Contains(strings.ToLower(dev.PartLabel), strings.ToLower("bios")) ||
strings.Contains(strings.ToLower(dev.PartLabel), strings.ToLower("boot"))
return !biosBootInPartLabel, nil
},

noReservedInPartLabel: func(dev lsblk.BlockDevice, _ internal.Executor) (bool, error) {
noReservedInPartLabel: func(dev lsblk.BlockDevice, _ exec.Executor) (bool, error) {
reservedInPartLabel := strings.Contains(strings.ToLower(dev.PartLabel), "reserved")
return !reservedInPartLabel, nil
},

noValidFilesystemSignature: func(dev lsblk.BlockDevice, e internal.Executor) (bool, error) {
noValidFilesystemSignature: func(dev lsblk.BlockDevice, e exec.Executor) (bool, error) {
// if no fs type is set, it's always okay
if dev.FSType == "" {
return true, nil
Expand Down Expand Up @@ -108,17 +108,17 @@ func DefaultFilters(lsblkInstance lsblk.LSBLK) Filters {
return false, nil
},

noBindMounts: func(dev lsblk.BlockDevice, _ internal.Executor) (bool, error) {
noBindMounts: func(dev lsblk.BlockDevice, _ exec.Executor) (bool, error) {
hasBindMounts, _, err := lsblkInstance.HasBindMounts(dev)
return !hasBindMounts, err
},

noChildren: func(dev lsblk.BlockDevice, _ internal.Executor) (bool, error) {
noChildren: func(dev lsblk.BlockDevice, _ exec.Executor) (bool, error) {
hasChildren := dev.HasChildren()
return !hasChildren, nil
},

usableDeviceType: func(dev lsblk.BlockDevice, executor internal.Executor) (bool, error) {
usableDeviceType: func(dev lsblk.BlockDevice, executor exec.Executor) (bool, error) {
switch dev.Type {
case DeviceTypeLoop:
// check loop device isn't being used by kubernetes
Expand Down
2 changes: 1 addition & 1 deletion pkg/internal/exec.go → pkg/internal/exec/exec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package internal
package exec

import (
"fmt"
Expand Down
0 pkg/internal/test/mock_exec.go → pkg/internal/exec/test/mock_exec.go
View file
File renamed without changes.
8 changes: 4 additions & 4 deletions pkg/lsblk/lsblk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"path/filepath"
"strings"

"github.com/openshift/lvm-operator/pkg/internal"
"github.com/openshift/lvm-operator/pkg/internal/exec"
)

var (
Expand Down Expand Up @@ -49,17 +49,17 @@ type LSBLK interface {
}

type HostLSBLK struct {
internal.Executor
exec.Executor
lsblk string
mountInfo string
losetup string
}

func NewDefaultHostLSBLK() *HostLSBLK {
return NewHostLSBLK(&internal.CommandExecutor{}, DefaultLsblk, DefaultMountinfo, DefaultLosetup)
return NewHostLSBLK(&exec.CommandExecutor{}, DefaultLsblk, DefaultMountinfo, DefaultLosetup)
}

func NewHostLSBLK(executor internal.Executor, lsblk, mountInfo, losetup string) *HostLSBLK {
func NewHostLSBLK(executor exec.Executor, lsblk, mountInfo, losetup string) *HostLSBLK {
hostLsblk := &HostLSBLK{
lsblk: lsblk,
Executor: executor,
Expand Down
36 changes: 18 additions & 18 deletions pkg/lvm/lvm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import (
"encoding/json"
"errors"
"fmt"
"os/exec"
osexec "os/exec"
"strings"

"github.com/openshift/lvm-operator/pkg/internal"
"github.com/openshift/lvm-operator/pkg/internal/exec"
)

type lvmError string
Expand Down Expand Up @@ -119,7 +119,7 @@ type PhysicalVolume struct {
}

// Create creates a new volume group
func (vg VolumeGroup) Create(exec internal.Executor, pvs []string) error {
func (vg VolumeGroup) Create(exec exec.Executor, pvs []string) error {
if vg.Name == "" {
return fmt.Errorf("failed to create volume group. Volume group name is empty")
}
Expand All @@ -140,7 +140,7 @@ func (vg VolumeGroup) Create(exec internal.Executor, pvs []string) error {
}

// Extend extends the volume group only if new physical volumes are available
func (vg VolumeGroup) Extend(exec internal.Executor, pvs []string) error {
func (vg VolumeGroup) Extend(exec exec.Executor, pvs []string) error {
if vg.Name == "" {
return fmt.Errorf("failed to extend volume group. Volume group name is empty")
}
Expand All @@ -161,7 +161,7 @@ func (vg VolumeGroup) Extend(exec internal.Executor, pvs []string) error {
}

// CreateVG creates a new volume group
func (vg VolumeGroup) CreateVG(exec internal.Executor) error {
func (vg VolumeGroup) CreateVG(exec exec.Executor) error {
if vg.Name == "" {
return fmt.Errorf("failed to create volume group. Volume group name is empty")
}
Expand All @@ -185,7 +185,7 @@ func (vg VolumeGroup) CreateVG(exec internal.Executor) error {
}

// ExtendVG extends the volume group only if new physical volumes are available
func (vg VolumeGroup) ExtendVG(exec internal.Executor, pvs []string) (VolumeGroup, error) {
func (vg VolumeGroup) ExtendVG(exec exec.Executor, pvs []string) (VolumeGroup, error) {
if vg.Name == "" {
return VolumeGroup{}, fmt.Errorf("failed to extend volume group. Volume group name is empty")
}
Expand All @@ -210,7 +210,7 @@ func (vg VolumeGroup) ExtendVG(exec internal.Executor, pvs []string) (VolumeGrou
}

// Delete deletes a volume group and the physical volumes associated with it
func (vg VolumeGroup) Delete(e internal.Executor) error {
func (vg VolumeGroup) Delete(e exec.Executor) error {
// Remove Volume Group
vgArgs := []string{vg.Name}
_, err := e.ExecuteCommandWithOutputAsHost(vgRemoveCmd, vgArgs...)
Expand All @@ -231,7 +231,7 @@ func (vg VolumeGroup) Delete(e internal.Executor) error {
for _, pv := range vg.PVs {
_, err = e.ExecuteCommandWithOutput(lvmDevicesCmd, "--delpvid", pv.UUID)
if err != nil {
var exitError *exec.ExitError
var exitError *osexec.ExitError
if errors.As(err, &exitError) {
switch exitError.ExitCode() {
// Exit Code 5 On lvmdevices --delpvid means that the PV with that UUID no longer exists
Expand All @@ -247,7 +247,7 @@ func (vg VolumeGroup) Delete(e internal.Executor) error {
}

// GetVolumeGroup returns a volume group along with the associated physical volumes
func GetVolumeGroup(exec internal.Executor, name string) (*VolumeGroup, error) {
func GetVolumeGroup(exec exec.Executor, name string) (*VolumeGroup, error) {
res := new(vgsOutput)

args := []string{
Expand Down Expand Up @@ -285,7 +285,7 @@ func GetVolumeGroup(exec internal.Executor, name string) (*VolumeGroup, error) {
}

// ListPhysicalVolumes returns list of physical volumes used to create the given volume group
func ListPhysicalVolumes(exec internal.Executor, vgName string) ([]PhysicalVolume, error) {
func ListPhysicalVolumes(exec exec.Executor, vgName string) ([]PhysicalVolume, error) {
res := new(pvsOutput)
args := []string{
"pvs", "--units", "g", "-v", "--reportformat", "json",
Expand Down Expand Up @@ -317,7 +317,7 @@ func ListPhysicalVolumes(exec internal.Executor, vgName string) ([]PhysicalVolum
}

// ListVolumeGroups lists all volume groups and the physical volumes associated with them.
func ListVolumeGroups(exec internal.Executor) ([]VolumeGroup, error) {
func ListVolumeGroups(exec exec.Executor) ([]VolumeGroup, error) {
res := new(vgsOutput)
args := []string{
"vgs", "--reportformat", "json",
Expand Down Expand Up @@ -347,7 +347,7 @@ func ListVolumeGroups(exec internal.Executor) ([]VolumeGroup, error) {
}

// ListLogicalVolumes returns list of logical volumes for a volume group
func ListLogicalVolumes(exec internal.Executor, vgName string) ([]string, error) {
func ListLogicalVolumes(exec exec.Executor, vgName string) ([]string, error) {
res, err := GetLVSOutput(exec, vgName)
if err != nil {
return []string{}, err
Expand All @@ -363,7 +363,7 @@ func ListLogicalVolumes(exec internal.Executor, vgName string) ([]string, error)
}

// GetLVSOutput returns the output for `lvs` command in json format
func GetLVSOutput(exec internal.Executor, vgName string) (*lvsOutput, error) {
func GetLVSOutput(exec exec.Executor, vgName string) (*lvsOutput, error) {
res := new(lvsOutput)
args := []string{
"lvs", "-S", fmt.Sprintf("vgname=%s", vgName), "--units", "g", "--reportformat", "json",
Expand All @@ -376,7 +376,7 @@ func GetLVSOutput(exec internal.Executor, vgName string) (*lvsOutput, error) {
}

// LVExists checks if a logical volume exists in a volume group
func LVExists(exec internal.Executor, lvName, vgName string) (bool, error) {
func LVExists(exec exec.Executor, lvName, vgName string) (bool, error) {
lvs, err := ListLogicalVolumes(exec, vgName)
if err != nil {
return false, err
Expand All @@ -392,7 +392,7 @@ func LVExists(exec internal.Executor, lvName, vgName string) (bool, error) {
}

// DeleteLV deactivates the logical volume and deletes it
func DeleteLV(exec internal.Executor, lvName, vgName string) error {
func DeleteLV(exec exec.Executor, lvName, vgName string) error {
lv := fmt.Sprintf("%s/%s", vgName, lvName)

// deactivate logical volume
Expand All @@ -411,7 +411,7 @@ func DeleteLV(exec internal.Executor, lvName, vgName string) error {
}

// CreateLV creates the logical volume
func CreateLV(exec internal.Executor, lvName, vgName string, sizePercent int) error {
func CreateLV(exec exec.Executor, lvName, vgName string, sizePercent int) error {
args := []string{"-l", fmt.Sprintf("%d%%FREE", sizePercent),
"-c", defaultChunkSize, "-Z", "y", "-T", fmt.Sprintf("%s/%s", vgName, lvName)}

Expand All @@ -424,7 +424,7 @@ func CreateLV(exec internal.Executor, lvName, vgName string, sizePercent int) er
}

// ExtendLV extends the logical volume
func ExtendLV(exec internal.Executor, lvName, vgName string, sizePercent int) error {
func ExtendLV(exec exec.Executor, lvName, vgName string, sizePercent int) error {
args := []string{"-l", fmt.Sprintf("%d%%Vg", sizePercent), fmt.Sprintf("%s/%s", vgName, lvName)}

if _, err := exec.ExecuteCommandWithOutputAsHost(lvExtendCmd, args...); err != nil {
Expand All @@ -435,7 +435,7 @@ func ExtendLV(exec internal.Executor, lvName, vgName string, sizePercent int) er
return nil
}

func execute(exec internal.Executor, v interface{}, args ...string) error {
func execute(exec exec.Executor, v interface{}, args ...string) error {
output, err := exec.ExecuteCommandWithOutputAsHost(lvmCmd, args...)
if err != nil {
return fmt.Errorf("failed to execute command. %v", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/lvm/lvm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"strings"
"testing"

mockExec "github.com/openshift/lvm-operator/pkg/internal/test"
mockExec "github.com/openshift/lvm-operator/pkg/internal/exec/test"
"github.com/stretchr/testify/assert"
)

Expand Down
9 changes: 5 additions & 4 deletions pkg/vgmanager/vgmanager_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
"github.com/openshift/lvm-operator/controllers"
"github.com/openshift/lvm-operator/pkg/filter"
"github.com/openshift/lvm-operator/pkg/internal"
"github.com/openshift/lvm-operator/pkg/internal/exec"
"github.com/openshift/lvm-operator/pkg/lsblk"
"github.com/openshift/lvm-operator/pkg/lvm"
"github.com/openshift/lvm-operator/pkg/lvmd"
Expand Down Expand Up @@ -77,6 +77,7 @@ var (

// SetupWithManager sets up the controller with the Manager.
func (r *VGReconciler) SetupWithManager(mgr ctrl.Manager) error {
r.executor = &exec.CommandExecutor{}
return ctrl.NewControllerManagedBy(mgr).
For(&lvmv1alpha1.LVMVolumeGroup{}).
Owns(&lvmv1alpha1.LVMVolumeGroupNodeStatus{}, builder.MatchEveryOwner).
Expand All @@ -86,9 +87,9 @@ func (r *VGReconciler) SetupWithManager(mgr ctrl.Manager) error {
type VGReconciler struct {
client.Client
record.EventRecorder
Scheme *runtime.Scheme
executor internal.Executor
LVMD lvmd.Configurator
Scheme *runtime.Scheme
executor exec.Executor
LVMD lvmd.Configurator
lsblk.LSBLK
NodeName string
Namespace string
Expand Down
8 changes: 4 additions & 4 deletions pkg/vgmanager/vgmanager_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (

"github.com/go-logr/logr/testr"
lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
"github.com/openshift/lvm-operator/pkg/internal"
mockExec "github.com/openshift/lvm-operator/pkg/internal/test"
"github.com/openshift/lvm-operator/pkg/internal/exec"
mockExec "github.com/openshift/lvm-operator/pkg/internal/exec/test"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/strings/slices"
Expand Down Expand Up @@ -92,7 +92,7 @@ var mockLvsOutputRAID = `

func TestVGReconciler_validateLVs(t *testing.T) {
type fields struct {
executor internal.Executor
executor exec.Executor
}
type args struct {
volumeGroup *lvmv1alpha1.LVMVolumeGroup
Expand All @@ -108,7 +108,7 @@ func TestVGReconciler_validateLVs(t *testing.T) {
"json",
}

mockExecutorForLVSOutput := func(output string) internal.Executor {
mockExecutorForLVSOutput := func(output string) exec.Executor {
return &mockExec.MockExecutor{
MockExecuteCommandWithOutputAsHost: func(command string, args ...string) (string, error) {
if !slices.Equal(args, lvsCommandForVG1) {
Expand Down

0 comments on commit 3407add

Please sign in to comment.