From ffa2375bb645cc8d86264d832b64d1a4e9d7b961 Mon Sep 17 00:00:00 2001 From: VinayKumarHavanur <54576364+VinayKumarHavanur@users.noreply.github.com> Date: Wed, 8 Jan 2025 14:59:01 +0530 Subject: [PATCH] Zoning check for Fibre Channel --- .../mock_fcp/mock_reconcile_utils.go | 15 +++++ utils/fcp/fcp.go | 5 ++ utils/fcp/reconcile_utils.go | 60 +++++++++++++++-- utils/fcp/utils.go | 18 +++++ utils/fcp/utils_test.go | 67 +++++++++++++++++++ 5 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 utils/fcp/utils_test.go diff --git a/mocks/mock_utils/mock_fcp/mock_reconcile_utils.go b/mocks/mock_utils/mock_fcp/mock_reconcile_utils.go index f29dd46fc..3edf03c69 100644 --- a/mocks/mock_utils/mock_fcp/mock_reconcile_utils.go +++ b/mocks/mock_utils/mock_fcp/mock_reconcile_utils.go @@ -40,6 +40,21 @@ func (m *MockFcpReconcileUtils) EXPECT() *MockFcpReconcileUtilsMockRecorder { return m.recorder } +// CheckZoningExistsWithTarget mocks base method. +func (m *MockFcpReconcileUtils) CheckZoningExistsWithTarget(arg0 context.Context, arg1 string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckZoningExistsWithTarget", arg0, arg1) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CheckZoningExistsWithTarget indicates an expected call of CheckZoningExistsWithTarget. +func (mr *MockFcpReconcileUtilsMockRecorder) CheckZoningExistsWithTarget(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckZoningExistsWithTarget", reflect.TypeOf((*MockFcpReconcileUtils)(nil).CheckZoningExistsWithTarget), arg0, arg1) +} + // GetDevicesForLUN mocks base method. func (m *MockFcpReconcileUtils) GetDevicesForLUN(arg0 []string) ([]string, error) { m.ctrl.T.Helper() diff --git a/utils/fcp/fcp.go b/utils/fcp/fcp.go index 0dde1ddfd..db657bd2a 100644 --- a/utils/fcp/fcp.go +++ b/utils/fcp/fcp.go @@ -350,6 +350,11 @@ func (client *Client) AttachVolume( return mpathSize, err } + // Check if zoning exists for the target + if isZoned, err := client.fcpUtils.CheckZoningExistsWithTarget(ctx, publishInfo.FCTargetWWNN); !isZoned { + return mpathSize, err + } + // First attempt to fix invalid serials by rescanning them err = client.handleInvalidSerials(ctx, lunID, publishInfo.FCTargetWWNN, publishInfo.FCPLunSerial, rescanOneLun) if err != nil { diff --git a/utils/fcp/reconcile_utils.go b/utils/fcp/reconcile_utils.go index d384366fd..faa4b822a 100644 --- a/utils/fcp/reconcile_utils.go +++ b/utils/fcp/reconcile_utils.go @@ -21,6 +21,7 @@ type FcpReconcileUtils interface { GetSysfsBlockDirsForLUN(int, []map[string]int) []string GetDevicesForLUN(paths []string) ([]string, error) ReconcileFCPVolumeInfo(ctx context.Context, trackingInfo *models.VolumeTrackingInfo) (bool, error) + CheckZoningExistsWithTarget(context.Context, string) (bool, error) } type FcpReconcileHelper struct { @@ -103,11 +104,7 @@ func (h *FcpReconcileHelper) GetFCPHostSessionMapForTarget( continue } - tgName := strings.TrimPrefix(string(targetName), "0x") - tgName = strings.TrimSuffix(tgName, "\n") - nname := strings.ReplaceAll(fcpNodeName, ":", "") - - if tgName == nname { + if MatchWorldWideNames(string(targetName), fcpNodeName, false) { fcHostPath := h.chrootPathPrefix + "/sys/class/fc_host/" var hostNumber string @@ -201,3 +198,56 @@ func (h *FcpReconcileHelper) GetDevicesForLUN(paths []string) ([]string, error) } return devices, nil } + +// CheckZoningExistsWithTarget checks if the target is zoned with the initiator. +func (h *FcpReconcileHelper) CheckZoningExistsWithTarget(ctx context.Context, targetNodeName string) (bool, error) { + fields := LogFields{"targetNodeName": targetNodeName} + Logc(ctx).WithFields(fields).Debug(">>>> fcp.CheckZoningExistsWithTarget") + defer Logc(ctx).WithFields(fields).Debug("<<<< fcp.CheckZoningExistsWithTarget") + + sysPath := h.chrootPathPrefix + "/sys/class/fc_remote_ports/" + rportDirs, err := os.ReadDir(sysPath) + if err != nil { + Logc(ctx).WithField("error", err).Errorf("Could not read %s", sysPath) + return false, err + } + + for _, rportDir := range rportDirs { + rportDirName := rportDir.Name() + if !strings.HasPrefix(rportDirName, "rport") { + continue + } + + devicePath := sysPath + rportDirName + nodeNamePath := devicePath + "/node_name" + nodeName, err := os.ReadFile(nodeNamePath) + if err != nil { + Logc(ctx).WithFields(LogFields{ + "path": nodeNamePath, + "error": err, + }).Error("Could not read target name file") + continue + } + + if !MatchWorldWideNames(string(nodeName), targetNodeName, false) { + // Skip the check for non-relevant target + continue + } + + portStatus, err := os.ReadFile(devicePath + "/port_state") + if err != nil { + Logc(ctx).WithFields(LogFields{ + "path": devicePath + "/port_state", + "error": err, + }).Error("Could not read port state file") + continue + } + + portStatusStr := strings.TrimSpace(string(portStatus)) + if portStatusStr == "Online" { + return true, nil + } + } + + return false, fmt.Errorf("no zoned ports found") +} diff --git a/utils/fcp/utils.go b/utils/fcp/utils.go index 4bb6649df..18a4f9919 100644 --- a/utils/fcp/utils.go +++ b/utils/fcp/utils.go @@ -151,3 +151,21 @@ func ConvertStrToWWNFormat(wwnStr string) string { } return wwn } + +// MatchWorldWideNames compares two WWNs and returns true if they match. +func MatchWorldWideNames(wwn1, wwn2 string, identicalSearch bool) bool { + if identicalSearch { + return wwn1 == wwn2 + } + + // Sanitize the WWN strings + wwn1Str := strings.TrimPrefix(wwn1, "0x") + wwn1Str = strings.TrimSpace(wwn1Str) + wwn1Str = strings.ReplaceAll(wwn1Str, ":", "") + + wwn2Str := strings.TrimPrefix(wwn2, "0x") + wwn2Str = strings.TrimSpace(wwn2Str) + wwn2Str = strings.ReplaceAll(wwn2Str, ":", "") + + return wwn1Str == wwn2Str +} diff --git a/utils/fcp/utils_test.go b/utils/fcp/utils_test.go new file mode 100644 index 000000000..bfbd3985c --- /dev/null +++ b/utils/fcp/utils_test.go @@ -0,0 +1,67 @@ +package fcp + +import ( + "testing" +) + +func TestMatchWorldWideNames(t *testing.T) { + tests := []struct { + name string + wwn1 string + wwn2 string + identicalSearch bool + expected bool + }{ + { + name: "Identical WWNs with identicalSearch true", + wwn1: "0x5005076801401b3f", + wwn2: "0x5005076801401b3f", + identicalSearch: true, + expected: true, + }, + { + name: "Different WWNs with identicalSearch true", + wwn1: "0x5005076801401b3f", + wwn2: "0x5005076801401b40", + identicalSearch: true, + expected: false, + }, + { + name: "Identical WWNs with identicalSearch false", + wwn1: "0x5005076801401b3f", + wwn2: "5005076801401b3f", + identicalSearch: false, + expected: true, + }, + { + name: "Different WWNs with identicalSearch false", + wwn1: "0x5005076801401b3f", + wwn2: "5005076801401b40", + identicalSearch: false, + expected: false, + }, + { + name: "WWNs with colons and identicalSearch false", + wwn1: "0x50:05:07:68:01:40:1b:3f", + wwn2: "5005076801401b3f", + identicalSearch: false, + expected: true, + }, + { + name: "WWNs with colons and identicalSearch false", + wwn1: "0x5005076801401b3f", + wwn2: "50:05:07:68:01:40:1b:3f", + identicalSearch: false, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := MatchWorldWideNames(tt.wwn1, tt.wwn2, tt.identicalSearch) + if result != tt.expected { + t.Errorf("MatchWorldWideNames(%s, %s, %v) = %v; want %v", tt.wwn1, tt.wwn2, tt.identicalSearch, result, tt.expected) + } + }) + } +}