Skip to content

Commit

Permalink
Os and io refactor
Browse files Browse the repository at this point in the history 
Refactor osutils into its own package
Add osutils unit tests
  • Loading branch information
@jharrod
jharrod authored Dec 20, 2024
1 parent 281c33f commit 395ba19
Show file tree
Hide file tree
Showing 41 changed files with 1,182 additions and 514 deletions.
5 changes: 2 additions & 3 deletions core/orchestrator_core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,8 @@ type TridentOrchestrator struct {

// NewTridentOrchestrator returns a storage orchestrator instance
func NewTridentOrchestrator(client persistentstore.Client) (*TridentOrchestrator, error) {
// TODO (vivintw) the adaptors are being plugged in here as a temporary measure to prevent cyclic dependencies.
// NewClient() must plugin default implementation of the various package clients.
iscsiClient, err := iscsi.New(utils.NewOSClient())
iscsiClient, err := iscsi.New()
if err != nil {
return nil, err
}
Expand All @@ -121,7 +120,7 @@ func NewTridentOrchestrator(client persistentstore.Client) (*TridentOrchestrator
return nil, err
}

fcpClent, err := fcp.New(utils.NewOSClient(), filesystem.New(mountClient))
fcpClent, err := fcp.New()
if err != nil {
return nil, err
}
Expand Down
17 changes: 9 additions & 8 deletions frontend/csi/node_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/netapp/trident/utils/filesystem"
"github.com/netapp/trident/utils/iscsi"
"github.com/netapp/trident/utils/models"
"github.com/netapp/trident/utils/osutils"
)

const (
Expand Down Expand Up @@ -263,7 +264,7 @@ func (p *Plugin) NodeUnpublishVolume(
return nil, status.Error(codes.InvalidArgument, "no target path provided")
}

isDir, err := utils.IsLikelyDir(targetPath)
isDir, err := p.osutils.IsLikelyDir(targetPath)
if err != nil {
if os.IsNotExist(err) {
Logc(ctx).WithFields(fields).Infof("target path (%s) not found; volume is not mounted.", targetPath)
Expand Down Expand Up @@ -305,7 +306,7 @@ func (p *Plugin) NodeUnpublishVolume(
// however today Kubernetes performs this deletion. Here we are making best efforts
// to delete the resource at target path. Sometimes this fails resulting CSI calling
// NodeUnpublishVolume again and usually deletion goes through in the second attempt.
if err = utils.DeleteResourceAtPath(ctx, targetPath); err != nil {
if err = p.osutils.DeleteResourceAtPath(ctx, targetPath); err != nil {
Logc(ctx).Debugf("Unable to delete resource at target path: %s; %s", targetPath, err)
}

Expand Down Expand Up @@ -340,7 +341,7 @@ func (p *Plugin) NodeGetVolumeStats(
}

// Ensure volume is published at path
exists, err := utils.PathExists(req.GetVolumePath())
exists, err := p.osutils.PathExists(req.GetVolumePath())
if !exists || err != nil {
return nil, status.Error(codes.NotFound,
fmt.Sprintf("could not find volume mount at path: %s; %v", req.GetVolumePath(), err))
Expand Down Expand Up @@ -655,7 +656,7 @@ func (p *Plugin) NodeGetInfo(
func (p *Plugin) nodeGetInfo(ctx context.Context) *models.Node {
// Only get the host system info if we don't have the info yet.
if p.hostInfo == nil {
host, err := utils.GetHostSystemInfo(ctx)
host, err := p.osutils.GetHostSystemInfo(ctx)
if err != nil {
p.hostInfo = &models.HostSystem{}
Logc(ctx).WithError(err).Warn("Unable to get host system information.")
Expand All @@ -680,7 +681,7 @@ func (p *Plugin) nodeGetInfo(ctx context.Context) *models.Node {
Logc(ctx).WithField("IQN", iscsiWWN).Info("Discovered iSCSI initiator name.")
}

ips, err := utils.GetIPAddresses(ctx)
ips, err := p.osutils.GetIPAddresses(ctx)
if err != nil {
Logc(ctx).WithField("error", err).Error("Could not get IP addresses.")
} else if len(ips) == 0 {
Expand All @@ -696,23 +697,23 @@ func (p *Plugin) nodeGetInfo(ctx context.Context) *models.Node {

// Discover active protocol services on the host.
var services []string
nfsActive, err := utils.NFSActiveOnHost(ctx)
nfsActive, err := p.osutils.NFSActiveOnHost(ctx)
if err != nil {
Logc(ctx).WithError(err).Warn("Error discovering NFS service on host.")
}
if nfsActive {
services = append(services, "NFS")
}

smbActive, err := utils.SMBActiveOnHost(ctx)
smbActive, err := osutils.SMBActiveOnHost(ctx)
if err != nil {
Logc(ctx).WithError(err).Warn("Error discovering SMB service on host.")
}
if smbActive {
services = append(services, "SMB")
}

iscsiActive, err := utils.ISCSIActiveOnHost(ctx, *p.hostInfo)
iscsiActive, err := p.iscsi.ISCSIActiveOnHost(ctx, *p.hostInfo)
if err != nil {
Logc(ctx).WithError(err).Warn("Error discovering iSCSI service on host.")
}
Expand Down
12 changes: 11 additions & 1 deletion frontend/csi/node_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/netapp/trident/utils/iscsi"
"github.com/netapp/trident/utils/models"
"github.com/netapp/trident/utils/mount"
"github.com/netapp/trident/utils/osutils"
)

func TestNodeStageVolume(t *testing.T) {
Expand Down Expand Up @@ -1025,7 +1026,7 @@ func TestFixISCSISessions(t *testing.T) {
},
}

iscsiClient, err := iscsi.New(utils.NewOSClient())
iscsiClient, err := iscsi.New()
assert.NoError(t, err)

nodeServer := &Plugin{
Expand Down Expand Up @@ -1750,6 +1751,7 @@ func TestNodeRegisterWithController_Success(t *testing.T) {
mockOrchestrator := mockcore.NewMockOrchestrator(mockCtrl)
mockNVMeHandler := mockUtils.NewMockNVMeInterface(mockCtrl)

iscsiClient, _ := iscsi.New()
// Create a node server plugin
nodeServer := &Plugin{
nodeName: nodeName,
Expand All @@ -1758,6 +1760,8 @@ func TestNodeRegisterWithController_Success(t *testing.T) {
restClient: mockClient,
nvmeHandler: mockNVMeHandler,
orchestrator: mockOrchestrator,
osutils: osutils.New(),
iscsi: iscsiClient,
}

// Create a fake node response to be returned by controller
Expand Down Expand Up @@ -1791,6 +1795,7 @@ func TestNodeRegisterWithController_TopologyLabels(t *testing.T) {
mockClient := mockControllerAPI.NewMockTridentController(mockCtrl)
mockOrchestrator := mockcore.NewMockOrchestrator(mockCtrl)
mockNVMeHandler := mockUtils.NewMockNVMeInterface(mockCtrl)
iscsiClient, _ := iscsi.New()

// Create a node server plugin
nodeServer := &Plugin{
Expand All @@ -1800,6 +1805,8 @@ func TestNodeRegisterWithController_TopologyLabels(t *testing.T) {
restClient: mockClient,
nvmeHandler: mockNVMeHandler,
orchestrator: mockOrchestrator,
osutils: osutils.New(),
iscsi: iscsiClient,
}

// Create set of cases with varying topology labels
Expand Down Expand Up @@ -1874,6 +1881,7 @@ func TestNodeRegisterWithController_Failure(t *testing.T) {
mockClient := mockControllerAPI.NewMockTridentController(mockCtrl)
mockOrchestrator := mockcore.NewMockOrchestrator(mockCtrl)
mockNVMeHandler := mockUtils.NewMockNVMeInterface(mockCtrl)
iscsiClient, _ := iscsi.New()

// Create a node server plugin
nodeServer := &Plugin{
Expand All @@ -1883,6 +1891,8 @@ func TestNodeRegisterWithController_Failure(t *testing.T) {
restClient: mockClient,
nvmeHandler: mockNVMeHandler,
orchestrator: mockOrchestrator,
iscsi: iscsiClient,
osutils: osutils.New(),
}

// Create a fake node response to be returned by controller
Expand Down
14 changes: 9 additions & 5 deletions frontend/csi/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/netapp/trident/utils/iscsi"
"github.com/netapp/trident/utils/models"
"github.com/netapp/trident/utils/mount"
"github.com/netapp/trident/utils/osutils"
)

const (
Expand Down Expand Up @@ -91,6 +92,7 @@ type Plugin struct {
mount mount.Mount
fs filesystem.Filesystem
fcp fcp.FCP
osutils osutils.Utils
}

func NewControllerPlugin(
Expand All @@ -112,6 +114,7 @@ func NewControllerPlugin(
enableForceDetach: enableForceDetach,
opCache: sync.Map{},
command: execCmd.NewCommand(),
osutils: osutils.New(),
}

var err error
Expand Down Expand Up @@ -164,9 +167,8 @@ func NewNodePlugin(
}
Logc(ctx).Info(msg)

// TODO (vivintw) the adaptors are being plugged in here as a temporary measure to prevent cyclic dependencies.
// NewClient() must plugin default implementation of the various package clients.
iscsiClient, err := iscsi.New(utils.NewOSClient())
iscsiClient, err := iscsi.New()
if err != nil {
return nil, err
}
Expand All @@ -178,7 +180,7 @@ func NewNodePlugin(

fs := filesystem.New(mountClient)

fcpClient, err := fcp.New(utils.NewOSClient(), fs)
fcpClient, err := fcp.New()
if err != nil {
return nil, err
}
Expand All @@ -205,6 +207,7 @@ func NewNodePlugin(
mount: mountClient,
fs: fs,
command: execCmd.NewCommand(),
osutils: osutils.New(),
}

if runtime.GOOS == "windows" {
Expand Down Expand Up @@ -275,7 +278,7 @@ func NewAllInOnePlugin(

// TODO (vivintw) the adaptors are being plugged in here as a temporary measure to prevent cyclic dependencies.
// NewClient() must plugin default implementation of the various package clients.
iscsiClient, err := iscsi.New(utils.NewOSClient())
iscsiClient, err := iscsi.New()
if err != nil {
return nil, err
}
Expand All @@ -287,7 +290,7 @@ func NewAllInOnePlugin(

fs := filesystem.New(mountClient)

fcpClient, err := fcp.New(utils.NewOSClient(), fs)
fcpClient, err := fcp.New()
if err != nil {
return nil, err
}
Expand All @@ -313,6 +316,7 @@ func NewAllInOnePlugin(
mount: mountClient,
fs: fs,
command: execCmd.NewCommand(),
osutils: osutils.New(),
}

// Define controller capabilities
Expand Down
24 changes: 13 additions & 11 deletions internal/nodeprep/instruction/instructions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"github.com/netapp/trident/internal/nodeprep/nodeinfo"
"github.com/netapp/trident/internal/nodeprep/protocol"
"github.com/netapp/trident/mocks/mock_internal/mock_nodeprep/mock_nodeinfo"
"github.com/netapp/trident/mocks/mock_utils/mock_osutils"
"github.com/netapp/trident/utils/models"
"github.com/netapp/trident/utils/osutils"
)

func TestInstructions(t *testing.T) {
Expand All @@ -32,7 +34,7 @@ func TestInstructions(t *testing.T) {
}

type parameters struct {
getOSClient func(controller *gomock.Controller) nodeinfo.OS
getOSClient func(controller *gomock.Controller) osutils.Utils
getBinaryClient func(controller *gomock.Controller) nodeinfo.Binary
expectedNodeInfo *nodeinfo.NodeInfo
setInstructions func()
Expand Down Expand Up @@ -62,8 +64,8 @@ func TestInstructions(t *testing.T) {

tests := map[string]parameters{
"node info returns supported distro ubuntu": {
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
os := mock_nodeinfo.NewMockOS(controller)
getOSClient: func(controller *gomock.Controller) osutils.Utils {
os := mock_osutils.NewMockUtils(controller)
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&ubuntuHostSystemResponse, nil)
return os
},
Expand All @@ -83,8 +85,8 @@ func TestInstructions(t *testing.T) {
assertError: assert.NoError,
},
"node info returns supported distro amazon linux": {
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
os := mock_nodeinfo.NewMockOS(controller)
getOSClient: func(controller *gomock.Controller) osutils.Utils {
os := mock_osutils.NewMockUtils(controller)
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&amazonHostSystemResponse, nil)
return os
},
Expand All @@ -103,8 +105,8 @@ func TestInstructions(t *testing.T) {
assertError: assert.NoError,
},
"node info returns unknown linux distro with package manager": {
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
os := mock_nodeinfo.NewMockOS(controller)
getOSClient: func(controller *gomock.Controller) osutils.Utils {
os := mock_osutils.NewMockUtils(controller)
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&fooHostSystemResponse, nil)
return os
},
Expand All @@ -123,8 +125,8 @@ func TestInstructions(t *testing.T) {
assertError: assert.NoError,
},
"node info returns unknown linux distro without package manager": {
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
os := mock_nodeinfo.NewMockOS(controller)
getOSClient: func(controller *gomock.Controller) osutils.Utils {
os := mock_osutils.NewMockUtils(controller)
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&fooHostSystemResponse, nil)
return os
},
Expand All @@ -144,8 +146,8 @@ func TestInstructions(t *testing.T) {
assertError: assert.Error,
},
"node info on host with no package manager": {
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
os := mock_nodeinfo.NewMockOS(controller)
getOSClient: func(controller *gomock.Controller) osutils.Utils {
os := mock_osutils.NewMockUtils(controller)
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&ubuntuHostSystemResponse, nil)
return os
},
Expand Down
12 changes: 6 additions & 6 deletions internal/nodeprep/nodeinfo/node_info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"strings"

. "github.com/netapp/trident/logging"
"github.com/netapp/trident/utils"
"github.com/netapp/trident/utils/models"
"github.com/netapp/trident/utils/osutils"
)

type NodeInfo struct {
Expand All @@ -25,7 +25,7 @@ type (
)

const (
DistroUbuntu Distro = utils.Ubuntu
DistroUbuntu Distro = osutils.Ubuntu
DistroAmzn Distro = "amzn"
DistroRhcos Distro = "rhcos"
DistroUnknown = "unknown"
Expand All @@ -40,17 +40,17 @@ type Node interface {
}

type NodeClient struct {
os OS
os osutils.Utils
binary Binary
}

func New() *NodeClient {
return NewDetailed(NewOSClient(), NewBinary())
return NewDetailed(osutils.New(), NewBinary())
}

func NewDetailed(os OS, binary Binary) *NodeClient {
func NewDetailed(osUtils osutils.Utils, binary Binary) *NodeClient {
return &NodeClient{
os: os,
os: osUtils,
binary: binary,
}
}
Expand Down
Loading

0 comments on commit 395ba19

Please sign in to comment.