Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect storage protocol usage when multiple appliances are defined in secret #361

Merged
merged 4 commits into from
Oct 23, 2024
Merged

Fix incorrect storage protocol usage when multiple appliances are defined in secret #361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e9e0e1
Initial code changes
santhoshatdell Oct 18, 2024
0f997b0
Merge branch 'main' into protocol-fix
santhoshatdell Oct 22, 2024
fdba537
Fixed failing unit tests
santhoshatdell Oct 22, 2024
9ca083b
Added unit tests
santhoshatdell Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 56 additions & 44 deletions pkg/node/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ type Service struct {
opts Opts
nodeID string

useFC bool
useNVME bool
useFC map[string]bool
useNVME map[string]bool
useNFS bool
initialized bool
reusedHost bool
Expand Down Expand Up @@ -109,6 +109,8 @@ func (s *Service) Init() error {
}
s.iscsiTargets = make(map[string][]string)
s.nvmeTargets = make(map[string][]string)
s.useFC = make(map[string]bool)
s.useNVME = make(map[string]bool)
iscsiInitiators, fcInitiators, nvmeInitiators, err := s.getInitiators()
if err != nil {
return fmt.Errorf("can't get initiators of the node: %s", err.Error())
Expand All @@ -132,52 +134,57 @@ func (s *Service) Init() error {
}

var initiators []string
var useNVME, useFC bool

switch arr.BlockProtocol {
case common.NVMETCPTransport:
if len(nvmeInitiators) == 0 {
log.Errorf("NVMeTCP transport was requested but NVMe initiator is not available")
}
s.useNVME = true
s.useFC = false
useNVME = true
useFC = false
case common.NVMEFCTransport:
if len(nvmeInitiators) == 0 {
log.Errorf("NVMeFC transport was requested but NVMe initiator is not available")
}
s.useNVME = true
s.useFC = true
useNVME = true
useFC = true
case common.ISCSITransport:
if len(iscsiInitiators) == 0 {
log.Errorf("iSCSI transport was requested but iSCSI initiator is not available")
}
s.useNVME = false
s.useFC = false
useNVME = false
useFC = false
case common.FcTransport:
if len(fcInitiators) == 0 {
log.Errorf("FC transport was requested but FC initiator is not available")
}
s.useNVME = false
s.useFC = true
useNVME = false
useFC = true
default:
s.useNVME = len(nvmeInitiators) > 0
s.useFC = len(fcInitiators) > 0
useNVME = len(nvmeInitiators) > 0
useFC = len(fcInitiators) > 0
}
if s.useNVME {
if useNVME {
initiators = nvmeInitiators
if s.useFC {
if useFC {
log.Infof("NVMeFC Protocol is requested")
} else {
log.Infof("NVMeTCP Protocol is requested")
}
} else if s.useFC {
} else if useFC {
initiators = fcInitiators
log.Infof("FC Protocol is requested")
} else {
initiators = iscsiInitiators
log.Infof("iSCSI Protocol is requested")
}

err = s.setupHost(initiators, arr.GetClient(), arr.GetIP())
// store the values in the array list for later use
s.useNVME[arr.GlobalID] = useNVME
s.useFC[arr.GlobalID] = useFC

err = s.setupHost(initiators, arr.GetClient(), arr.GetIP(), arr.GetGlobalID())
if err != nil {
log.Errorf("can't setup host on %s: %s", arr.Endpoint, err.Error())
}
Expand Down Expand Up @@ -270,8 +277,8 @@ func (s *Service) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeR
}
} else {
stager = &SCSIStager{
useFC: s.useFC,
useNVME: s.useNVME,
useFC: s.useFC[arr.GlobalID],
useNVME: s.useNVME[arr.GlobalID],
iscsiConnector: s.iscsiConnector,
nvmeConnector: s.nvmeConnector,
fcConnector: s.fcConnector,
Expand Down Expand Up @@ -305,7 +312,7 @@ func (s *Service) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVol
return nil, status.Error(codes.InvalidArgument, "staging target path is required")
}

id, _, protocol, remoteVolumeID, _, err := array.ParseVolumeID(ctx, id, s.DefaultArray(), nil)
id, arrayID, protocol, remoteVolumeID, _, err := array.ParseVolumeID(ctx, id, s.DefaultArray(), nil)
if err != nil {
if apiError, ok := err.(gopowerstore.APIError); ok && apiError.NotFound() {
return &csi.NodeUnstageVolumeResponse{}, nil
Expand All @@ -315,6 +322,11 @@ func (s *Service) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVol
err.Error())
}

arr, ok := s.Arrays()[arrayID]
if !ok {
return nil, status.Errorf(codes.Internal, "can't find array with ID %s", arrayID)
}

// append additional path to be able to do bind mounts
stagingPath := getStagingPath(ctx, req.GetStagingTargetPath(), id)

Expand Down Expand Up @@ -352,9 +364,9 @@ func (s *Service) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVol

connectorCtx := common.SetLogFields(context.Background(), logFields)

if s.useNVME {
if s.useNVME[arr.GlobalID] {
err = s.nvmeConnector.DisconnectVolumeByDeviceName(connectorCtx, device)
} else if s.useFC {
} else if s.useFC[arr.GlobalID] {
err = s.fcConnector.DisconnectVolumeByDeviceName(connectorCtx, device)
} else {
err = s.iscsiConnector.DisconnectVolumeByDeviceName(connectorCtx, device)
Expand Down Expand Up @@ -885,7 +897,7 @@ func (s *Service) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolum
}
log.WithFields(f).Info("Calling resize the file system")

if !s.useNVME {
if !s.useNVME[arr.GlobalID] {
// Rescan the device for the volume expanded on the array
for _, device := range devMnt.DeviceNames {
devicePath := sysBlock + device
Expand Down Expand Up @@ -1064,8 +1076,8 @@ func (s *Service) NodeGetInfo(ctx context.Context, _ *csi.NodeGetInfoRequest) (*
}

if arr.BlockProtocol != common.NoneTransport {
if s.useNVME {
if s.useFC {
if s.useNVME[arr.GlobalID] {
if s.useFC[arr.GlobalID] {
nvmefcInfo, err := common.GetNVMEFCTargetInfoFromStorage(arr.GetClient(), "")
if err != nil {
log.Errorf("couldn't get targets from the array: %s", err.Error())
Expand Down Expand Up @@ -1131,7 +1143,7 @@ func (s *Service) NodeGetInfo(ctx context.Context, _ *csi.NodeGetInfoRequest) (*
s.useNFS = true
}
}
} else if s.useFC {
} else if s.useFC[arr.GlobalID] {
// Check node initiators connection to array
nodeID := s.nodeID
if s.reusedHost {
Expand Down Expand Up @@ -1413,27 +1425,27 @@ func (s *Service) readFCPortsFilterFile() ([]string, error) {
return result, nil
}

func (s *Service) setupHost(initiators []string, client gopowerstore.Client, arrayIP string) error {
func (s *Service) setupHost(initiators []string, client gopowerstore.Client, arrayIP, arrayID string) error {
log.Infof("setting up host on %s", arrayIP)
defer log.Infof("finished setting up host on %s", arrayIP)

if s.nodeID == "" {
return fmt.Errorf("nodeID not set")
}

reqInitiators := s.buildInitiatorsArray(initiators)
reqInitiators := s.buildInitiatorsArray(initiators, arrayID)
var host *gopowerstore.Host
updateCHAP := false

h, err := client.GetHostByName(context.Background(), s.nodeID)
if err == nil {
err := s.updateHost(context.Background(), initiators, client, h)
err := s.updateHost(context.Background(), initiators, client, h, arrayID)
if err != nil {
return err
}
if s.opts.EnableCHAP && len(h.Initiators) > 0 && (h.Initiators[0].ChapSingleUsername == "" || h.Initiators[0].ChapSingleUsername == "admin") {
log.Debug("CHAP was enabled earlier, modifying credentials")
err := s.modifyHostInitiators(context.Background(), h.ID, client, nil, nil, initiators)
err := s.modifyHostInitiators(context.Background(), h.ID, client, nil, nil, initiators, arrayID)
if err != nil {
return fmt.Errorf("can't modify initiators CHAP credentials %s", err.Error())
}
Expand Down Expand Up @@ -1472,15 +1484,15 @@ func (s *Service) setupHost(initiators []string, client gopowerstore.Client, arr

if host == nil {
// register node on PowerStore
_, err := s.createHost(context.Background(), initiators, client)
_, err := s.createHost(context.Background(), initiators, client, arrayID)
if err != nil {
log.Error(err.Error())
return err
}
} else {
// node already registered
if updateCHAP { // add CHAP credentials if they aren't available
err := s.modifyHostInitiators(context.Background(), host.ID, client, nil, nil, initiators)
err := s.modifyHostInitiators(context.Background(), host.ID, client, nil, nil, initiators, arrayID)
if err != nil {
return fmt.Errorf("can't modify initiators CHAP credentials %s", err.Error())
}
Expand Down Expand Up @@ -1510,19 +1522,19 @@ func (s *Service) modifyHostName(ctx context.Context, client gopowerstore.Client
return nil
}

func (s *Service) buildInitiatorsArray(initiators []string) []gopowerstore.InitiatorCreateModify {
func (s *Service) buildInitiatorsArray(initiators []string, arrayID string) []gopowerstore.InitiatorCreateModify {
var portType gopowerstore.InitiatorProtocolTypeEnum
if s.useNVME {
if s.useNVME[arrayID] {
portType = gopowerstore.InitiatorProtocolTypeEnumNVME
} else if s.useFC {
} else if s.useFC[arrayID] {
portType = gopowerstore.InitiatorProtocolTypeEnumFC
} else {
portType = gopowerstore.InitiatorProtocolTypeEnumISCSI
}
initiatorsReq := make([]gopowerstore.InitiatorCreateModify, len(initiators))
for i, iqn := range initiators {
iqn := iqn
if !s.useFC && s.opts.EnableCHAP {
if !s.useFC[arrayID] && s.opts.EnableCHAP {
initiatorsReq[i] = gopowerstore.InitiatorCreateModify{
ChapSinglePassword: &s.opts.CHAPPassword,
ChapSingleUsername: &s.opts.CHAPUsername,
Expand All @@ -1540,15 +1552,15 @@ func (s *Service) buildInitiatorsArray(initiators []string) []gopowerstore.Initi
}

// create or update host on PowerStore array
func (s *Service) updateHost(ctx context.Context, initiators []string, client gopowerstore.Client, host gopowerstore.Host) (err error) {
func (s *Service) updateHost(ctx context.Context, initiators []string, client gopowerstore.Client, host gopowerstore.Host, arrayID string) (err error) {
initiatorsToAdd, initiatorsToDelete := checkIQNS(initiators, host)
return s.modifyHostInitiators(ctx, host.ID, client, initiatorsToAdd, initiatorsToDelete, nil)
return s.modifyHostInitiators(ctx, host.ID, client, initiatorsToAdd, initiatorsToDelete, nil, arrayID)
}

// register host
func (s *Service) createHost(ctx context.Context, initiators []string, client gopowerstore.Client) (id string, err error) {
func (s *Service) createHost(ctx context.Context, initiators []string, client gopowerstore.Client, arrayID string) (id string, err error) {
osType := gopowerstore.OSTypeEnumLinux
reqInitiators := s.buildInitiatorsArray(initiators)
reqInitiators := s.buildInitiatorsArray(initiators, arrayID)
description := fmt.Sprintf("k8s node: %s", s.opts.KubeNodeName)

var createParams gopowerstore.HostCreate
Expand Down Expand Up @@ -1595,7 +1607,7 @@ func (s *Service) createHost(ctx context.Context, initiators []string, client go

// add or remove initiators from host
func (s *Service) modifyHostInitiators(ctx context.Context, hostID string, client gopowerstore.Client,
initiatorsToAdd []string, initiatorsToDelete []string, initiatorsToModify []string,
initiatorsToAdd []string, initiatorsToDelete []string, initiatorsToModify []string, arrayID string,
) error {
if len(initiatorsToDelete) > 0 {
modifyParams := gopowerstore.HostModify{}
Expand All @@ -1607,7 +1619,7 @@ func (s *Service) modifyHostInitiators(ctx context.Context, hostID string, clien
}
if len(initiatorsToAdd) > 0 {
modifyParams := gopowerstore.HostModify{}
initiators := s.buildInitiatorsArray(initiatorsToAdd)
initiators := s.buildInitiatorsArray(initiatorsToAdd, arrayID)
modifyParams.AddInitiators = &initiators
_, err := client.ModifyHost(ctx, &modifyParams, hostID)
if err != nil {
Expand All @@ -1616,7 +1628,7 @@ func (s *Service) modifyHostInitiators(ctx context.Context, hostID string, clien
}
if len(initiatorsToModify) > 0 {
modifyParams := gopowerstore.HostModify{}
initiators := s.buildInitiatorsArrayModify(initiatorsToModify)
initiators := s.buildInitiatorsArrayModify(initiatorsToModify, arrayID)
modifyParams.ModifyInitiators = &initiators
_, err := client.ModifyHost(ctx, &modifyParams, hostID)
if err != nil {
Expand Down Expand Up @@ -1653,11 +1665,11 @@ func checkIQNS(IQNs []string, host gopowerstore.Host) (iqnToAdd, iqnToDelete []s
return
}

func (s *Service) buildInitiatorsArrayModify(initiators []string) []gopowerstore.UpdateInitiatorInHost {
func (s *Service) buildInitiatorsArrayModify(initiators []string, arrayID string) []gopowerstore.UpdateInitiatorInHost {
initiatorsReq := make([]gopowerstore.UpdateInitiatorInHost, len(initiators))
for i, iqn := range initiators {
iqn := iqn
if !s.useFC && s.opts.EnableCHAP {
if !s.useFC[arrayID] && s.opts.EnableCHAP {
initiatorsReq[i] = gopowerstore.UpdateInitiatorInHost{
ChapSinglePassword: &s.opts.CHAPPassword,
ChapSingleUsername: &s.opts.CHAPUsername,
Expand Down
12 changes: 6 additions & 6 deletions pkg/node/node_connectivity_checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright © 2022-2023 Dell Inc. or its subsidiaries. All Rights Reserved.
* Copyright © 2022-2024 Dell Inc. or its subsidiaries. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -228,7 +228,7 @@ func (s *Service) nodeProbe(_ context.Context, array *array.PowerStoreArray) err
log.Debugf("Successfully got Host on %s", array.GlobalID)
s.populateTargetsInCache(array)
// check if nvme sessions are active
if s.useNVME {
if s.useNVME[array.GlobalID] {
log.Debugf("Checking if nvme sessions are active on node or not")
sessions, _ := s.nvmeLib.GetSessions()
for _, target := range s.nvmeTargets[array.GlobalID] {
Expand All @@ -247,7 +247,7 @@ func (s *Service) nodeProbe(_ context.Context, array *array.PowerStoreArray) err
return nil
}
return fmt.Errorf("no active nvme sessions")
} else if s.useFC {
} else if s.useFC[array.GlobalID] {
log.Debugf("Checking if FC sessions are active on node or not")
for _, initiator := range host.Initiators {
if len(initiator.ActiveSessions) > 0 {
Expand Down Expand Up @@ -282,12 +282,12 @@ func (s *Service) nodeProbe(_ context.Context, array *array.PowerStoreArray) err
func (s *Service) populateTargetsInCache(array *array.PowerStoreArray) {
// if nvmeTargets in cache is empty
// this could be empty in 2 cases: Either container is getting restarted or discovery & login has failed in NodeGetInfo
if s.useNVME {
if s.useNVME[array.GlobalID] {
if len(s.nvmeTargets[array.GlobalID]) != 0 {
return
}
// for NVMeFC
if s.useFC {
if s.useFC[array.GlobalID] {
nvmefcInfo, err := common.GetNVMEFCTargetInfoFromStorage(array.GetClient(), "")
if err != nil {
log.Errorf("couldn't get targets from the array: %s", err.Error())
Expand Down Expand Up @@ -328,7 +328,7 @@ func (s *Service) populateTargetsInCache(array *array.PowerStoreArray) {
break
}
}
} else if !s.useFC && !s.useNFS {
} else if !s.useFC[array.GlobalID] && !s.useNFS {
// if iscsiTargets in cache is empty
if len(s.iscsiTargets[array.GlobalID]) != 0 {
return
Expand Down
Loading