Skip to content

Commit 395ba19

Browse files
jharrodjharrod
authored
Os and io refactor
Refactor osutils into its own package Add osutils unit tests
1 parent 281c33f commit 395ba19

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+1182
-514
lines changed

‎core/orchestrator_core.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,8 @@ type TridentOrchestrator struct {
109109

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

124-
fcpClent, err := fcp.New(utils.NewOSClient(), filesystem.New(mountClient))
123+
fcpClent, err := fcp.New()
125124
if err != nil {
126125
return nil, err
127126
}

‎frontend/csi/node_server.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/netapp/trident/utils/filesystem"
3434
"github.com/netapp/trident/utils/iscsi"
3535
"github.com/netapp/trident/utils/models"
36+
"github.com/netapp/trident/utils/osutils"
3637
)
3738

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

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

@@ -340,7 +341,7 @@ func (p *Plugin) NodeGetVolumeStats(
340341
}
341342

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

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

697698
// Discover active protocol services on the host.
698699
var services []string
699-
nfsActive, err := utils.NFSActiveOnHost(ctx)
700+
nfsActive, err := p.osutils.NFSActiveOnHost(ctx)
700701
if err != nil {
701702
Logc(ctx).WithError(err).Warn("Error discovering NFS service on host.")
702703
}
703704
if nfsActive {
704705
services = append(services, "NFS")
705706
}
706707

707-
smbActive, err := utils.SMBActiveOnHost(ctx)
708+
smbActive, err := osutils.SMBActiveOnHost(ctx)
708709
if err != nil {
709710
Logc(ctx).WithError(err).Warn("Error discovering SMB service on host.")
710711
}
711712
if smbActive {
712713
services = append(services, "SMB")
713714
}
714715

715-
iscsiActive, err := utils.ISCSIActiveOnHost(ctx, *p.hostInfo)
716+
iscsiActive, err := p.iscsi.ISCSIActiveOnHost(ctx, *p.hostInfo)
716717
if err != nil {
717718
Logc(ctx).WithError(err).Warn("Error discovering iSCSI service on host.")
718719
}

‎frontend/csi/node_server_test.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/netapp/trident/utils/iscsi"
3434
"github.com/netapp/trident/utils/models"
3535
"github.com/netapp/trident/utils/mount"
36+
"github.com/netapp/trident/utils/osutils"
3637
)
3738

3839
func TestNodeStageVolume(t *testing.T) {
@@ -1025,7 +1026,7 @@ func TestFixISCSISessions(t *testing.T) {
10251026
},
10261027
}
10271028

1028-
iscsiClient, err := iscsi.New(utils.NewOSClient())
1029+
iscsiClient, err := iscsi.New()
10291030
assert.NoError(t, err)
10301031

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

1754+
iscsiClient, _ := iscsi.New()
17531755
// Create a node server plugin
17541756
nodeServer := &Plugin{
17551757
nodeName: nodeName,
@@ -1758,6 +1760,8 @@ func TestNodeRegisterWithController_Success(t *testing.T) {
17581760
restClient: mockClient,
17591761
nvmeHandler: mockNVMeHandler,
17601762
orchestrator: mockOrchestrator,
1763+
osutils: osutils.New(),
1764+
iscsi: iscsiClient,
17611765
}
17621766

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

17951800
// Create a node server plugin
17961801
nodeServer := &Plugin{
@@ -1800,6 +1805,8 @@ func TestNodeRegisterWithController_TopologyLabels(t *testing.T) {
18001805
restClient: mockClient,
18011806
nvmeHandler: mockNVMeHandler,
18021807
orchestrator: mockOrchestrator,
1808+
osutils: osutils.New(),
1809+
iscsi: iscsiClient,
18031810
}
18041811

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

18781886
// Create a node server plugin
18791887
nodeServer := &Plugin{
@@ -1883,6 +1891,8 @@ func TestNodeRegisterWithController_Failure(t *testing.T) {
18831891
restClient: mockClient,
18841892
nvmeHandler: mockNVMeHandler,
18851893
orchestrator: mockOrchestrator,
1894+
iscsi: iscsiClient,
1895+
osutils: osutils.New(),
18861896
}
18871897

18881898
// Create a fake node response to be returned by controller

‎frontend/csi/plugin.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/netapp/trident/utils/iscsi"
3131
"github.com/netapp/trident/utils/models"
3232
"github.com/netapp/trident/utils/mount"
33+
"github.com/netapp/trident/utils/osutils"
3334
)
3435

3536
const (
@@ -91,6 +92,7 @@ type Plugin struct {
9192
mount mount.Mount
9293
fs filesystem.Filesystem
9394
fcp fcp.FCP
95+
osutils osutils.Utils
9496
}
9597

9698
func NewControllerPlugin(
@@ -112,6 +114,7 @@ func NewControllerPlugin(
112114
enableForceDetach: enableForceDetach,
113115
opCache: sync.Map{},
114116
command: execCmd.NewCommand(),
117+
osutils: osutils.New(),
115118
}
116119

117120
var err error
@@ -164,9 +167,8 @@ func NewNodePlugin(
164167
}
165168
Logc(ctx).Info(msg)
166169

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

179181
fs := filesystem.New(mountClient)
180182

181-
fcpClient, err := fcp.New(utils.NewOSClient(), fs)
183+
fcpClient, err := fcp.New()
182184
if err != nil {
183185
return nil, err
184186
}
@@ -205,6 +207,7 @@ func NewNodePlugin(
205207
mount: mountClient,
206208
fs: fs,
207209
command: execCmd.NewCommand(),
210+
osutils: osutils.New(),
208211
}
209212

210213
if runtime.GOOS == "windows" {
@@ -275,7 +278,7 @@ func NewAllInOnePlugin(
275278

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

288291
fs := filesystem.New(mountClient)
289292

290-
fcpClient, err := fcp.New(utils.NewOSClient(), fs)
293+
fcpClient, err := fcp.New()
291294
if err != nil {
292295
return nil, err
293296
}
@@ -313,6 +316,7 @@ func NewAllInOnePlugin(
313316
mount: mountClient,
314317
fs: fs,
315318
command: execCmd.NewCommand(),
319+
osutils: osutils.New(),
316320
}
317321

318322
// Define controller capabilities

‎internal/nodeprep/instruction/instructions_test.go

+13-11
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import (
1010
"github.com/netapp/trident/internal/nodeprep/nodeinfo"
1111
"github.com/netapp/trident/internal/nodeprep/protocol"
1212
"github.com/netapp/trident/mocks/mock_internal/mock_nodeprep/mock_nodeinfo"
13+
"github.com/netapp/trident/mocks/mock_utils/mock_osutils"
1314
"github.com/netapp/trident/utils/models"
15+
"github.com/netapp/trident/utils/osutils"
1416
)
1517

1618
func TestInstructions(t *testing.T) {
@@ -32,7 +34,7 @@ func TestInstructions(t *testing.T) {
3234
}
3335

3436
type parameters struct {
35-
getOSClient func(controller *gomock.Controller) nodeinfo.OS
37+
getOSClient func(controller *gomock.Controller) osutils.Utils
3638
getBinaryClient func(controller *gomock.Controller) nodeinfo.Binary
3739
expectedNodeInfo *nodeinfo.NodeInfo
3840
setInstructions func()
@@ -62,8 +64,8 @@ func TestInstructions(t *testing.T) {
6264

6365
tests := map[string]parameters{
6466
"node info returns supported distro ubuntu": {
65-
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
66-
os := mock_nodeinfo.NewMockOS(controller)
67+
getOSClient: func(controller *gomock.Controller) osutils.Utils {
68+
os := mock_osutils.NewMockUtils(controller)
6769
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&ubuntuHostSystemResponse, nil)
6870
return os
6971
},
@@ -83,8 +85,8 @@ func TestInstructions(t *testing.T) {
8385
assertError: assert.NoError,
8486
},
8587
"node info returns supported distro amazon linux": {
86-
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
87-
os := mock_nodeinfo.NewMockOS(controller)
88+
getOSClient: func(controller *gomock.Controller) osutils.Utils {
89+
os := mock_osutils.NewMockUtils(controller)
8890
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&amazonHostSystemResponse, nil)
8991
return os
9092
},
@@ -103,8 +105,8 @@ func TestInstructions(t *testing.T) {
103105
assertError: assert.NoError,
104106
},
105107
"node info returns unknown linux distro with package manager": {
106-
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
107-
os := mock_nodeinfo.NewMockOS(controller)
108+
getOSClient: func(controller *gomock.Controller) osutils.Utils {
109+
os := mock_osutils.NewMockUtils(controller)
108110
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&fooHostSystemResponse, nil)
109111
return os
110112
},
@@ -123,8 +125,8 @@ func TestInstructions(t *testing.T) {
123125
assertError: assert.NoError,
124126
},
125127
"node info returns unknown linux distro without package manager": {
126-
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
127-
os := mock_nodeinfo.NewMockOS(controller)
128+
getOSClient: func(controller *gomock.Controller) osutils.Utils {
129+
os := mock_osutils.NewMockUtils(controller)
128130
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&fooHostSystemResponse, nil)
129131
return os
130132
},
@@ -144,8 +146,8 @@ func TestInstructions(t *testing.T) {
144146
assertError: assert.Error,
145147
},
146148
"node info on host with no package manager": {
147-
getOSClient: func(controller *gomock.Controller) nodeinfo.OS {
148-
os := mock_nodeinfo.NewMockOS(controller)
149+
getOSClient: func(controller *gomock.Controller) osutils.Utils {
150+
os := mock_osutils.NewMockUtils(controller)
149151
os.EXPECT().GetHostSystemInfo(gomock.Any()).Return(&ubuntuHostSystemResponse, nil)
150152
return os
151153
},

‎internal/nodeprep/nodeinfo/node_info.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
"strings"
1010

1111
. "github.com/netapp/trident/logging"
12-
"github.com/netapp/trident/utils"
1312
"github.com/netapp/trident/utils/models"
13+
"github.com/netapp/trident/utils/osutils"
1414
)
1515

1616
type NodeInfo struct {
@@ -25,7 +25,7 @@ type (
2525
)
2626

2727
const (
28-
DistroUbuntu Distro = utils.Ubuntu
28+
DistroUbuntu Distro = osutils.Ubuntu
2929
DistroAmzn Distro = "amzn"
3030
DistroRhcos Distro = "rhcos"
3131
DistroUnknown = "unknown"
@@ -40,17 +40,17 @@ type Node interface {
4040
}
4141

4242
type NodeClient struct {
43-
os OS
43+
os osutils.Utils
4444
binary Binary
4545
}
4646

4747
func New() *NodeClient {
48-
return NewDetailed(NewOSClient(), NewBinary())
48+
return NewDetailed(osutils.New(), NewBinary())
4949
}
5050

51-
func NewDetailed(os OS, binary Binary) *NodeClient {
51+
func NewDetailed(osUtils osutils.Utils, binary Binary) *NodeClient {
5252
return &NodeClient{
53-
os: os,
53+
os: osUtils,
5454
binary: binary,
5555
}
5656
}

0 commit comments

Comments
 (0)