Error logging for duplicate host nqns

pkg/common/k8sutils/k8sutils.go

@@ -18,6 +18,8 @@ package k8sutils
 
 import (
 	"context"
+	"fmt"
+	"strings"
 
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -31,16 +33,29 @@ type NodeLabelsRetrieverInterface interface {
 	InClusterConfig() (*rest.Config, error)
 	NewForConfig(config *rest.Config) (*kubernetes.Clientset, error)
 	GetNodeLabels(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (map[string]string, error)
+	GetNVMeUUIDs(ctx context.Context, k8sclientset *kubernetes.Clientset) (map[string]string, error)
+}
+
+// NodeLabelsModifierInterface defines the methods for retrieving Kubernetes Node Labels
+type NodeLabelsModifierInterface interface {
+	AddNVMeLabels(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string, labelKey string, labelValue []string) error
 }
 
 // NodeLabelsRetrieverImpl provided the implementation for NodeLabelsRetrieverInterface
 type NodeLabelsRetrieverImpl struct{}
 
+// NodeLabelsModifierImpl provides the implementation for NodeLabelsModifierInterface
+type NodeLabelsModifierImpl struct{}
+
 // NodeLabelsRetriever is the actual instance of NodeLabelsRetrieverInterface which is used to retrieve the node labels
-var NodeLabelsRetriever NodeLabelsRetrieverInterface
+var (
+	NodeLabelsRetriever NodeLabelsRetrieverInterface
+	NodeLabelsModifier  NodeLabelsModifierInterface
+)
 
 func init() {
 	NodeLabelsRetriever = new(NodeLabelsRetrieverImpl)
+	NodeLabelsModifier = new(NodeLabelsModifierImpl)
 }
 
 // BuildConfigFromFlags is a method for building kubernetes client config
@@ -99,6 +114,65 @@ func CreateKubeClientSet(kubeconfig string) (*kubernetes.Clientset, error) {
 	return clientset, nil
 }
 
+// AddNVMeLabels adds a hostnqn uuid label to the specified Kubernetes node
+func (svc *NodeLabelsModifierImpl) AddNVMeLabels(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string, labelKey string, labelValue []string) error {
+	if k8sclientset == nil {
+		return fmt.Errorf("k8sclientset is nil")
+	}
+
+	// Get the current node
+	node, err := k8sclientset.CoreV1().Nodes().Get(ctx, kubeNodeName, v1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf("failed to get node %s: %v", kubeNodeName, err.Error())
+	}
+
+	// Initialize node labels if it is nil
+	if node.Labels == nil {
+		node.Labels = make(map[string]string)
+	}
+
+	// Fetch the uuids from hostnqns
+	var uuids []string
+	for _, nqn := range labelValue {
+		parts := strings.Split(nqn, ":")
+		if len(parts) == 3 { // nqn format is nqn.yyyy-mm.nvmexpress:uuid:xxxx-yyyy-zzzz
+			uuids = append(uuids, parts[2]) // Extract the UUID
+		}
+	}
+
+	// Update the node with the new labels
+	node.Labels[labelKey] = strings.Join(uuids, ",")
+	_, err = k8sclientset.CoreV1().Nodes().Update(ctx, node, v1.UpdateOptions{})
+	if err != nil {
+		return fmt.Errorf("failed to update node %s labels: %v", kubeNodeName, err.Error())
+	}
+	return nil
+}
+
+// GetNVMeUUIDs returns map of hosts with their hostnqn uuids
+func (svc *NodeLabelsRetrieverImpl) GetNVMeUUIDs(ctx context.Context, k8sclientset *kubernetes.Clientset) (map[string]string, error) {
+	nodeUUIDs := make(map[string]string)
+	if k8sclientset == nil {
+		return nodeUUIDs, fmt.Errorf("k8sclientset is nil")
+	}
+
+	// Retrieve the list of nodes
+	nodes, err := k8sclientset.CoreV1().Nodes().List(ctx, v1.ListOptions{})
+	if err != nil {
+		return nodeUUIDs, fmt.Errorf("failed to get node list: %v", err.Error())
+	}
+
+	// Iterate over all nodes to check their labels
+	for _, node := range nodes.Items {
+		labels := node.Labels
+		if uuid, exists := labels["hostnqn-uuid"]; exists {
+			nodeUUIDs[node.Name] = uuid
+		}
+	}
+
+	return nodeUUIDs, nil
+}
+
 // GetNodeLabels returns labels present in the k8s node
 func GetNodeLabels(ctx context.Context, kubeConfigPath string, kubeNodeName string) (map[string]string, error) {
 	k8sclientset, err := CreateKubeClientSet(kubeConfigPath)
@@ -108,3 +182,23 @@ func GetNodeLabels(ctx context.Context, kubeConfigPath string, kubeNodeName stri
 
 	return NodeLabelsRetriever.GetNodeLabels(ctx, k8sclientset, kubeNodeName)
 }
+
+// AddNVMeLabels adds a hostnqn uuid label in the k8s node
+func AddNVMeLabels(ctx context.Context, kubeConfigPath string, kubeNodeName string, labelKey string, labelValue []string) error {
+	k8sclientset, err := CreateKubeClientSet(kubeConfigPath)
+	if err != nil {
+		return err
+	}
+
+	return NodeLabelsModifier.AddNVMeLabels(ctx, k8sclientset, kubeNodeName, labelKey, labelValue)
+}
+
+// GetNVMeUUIDs checks for duplicate hostnqn uuid labels in the k8s node
+func GetNVMeUUIDs(ctx context.Context, kubeConfigPath string) (map[string]string, error) {
+	k8sclientset, err := CreateKubeClientSet(kubeConfigPath)
+	if err != nil {
+		return map[string]string{}, err
+	}
+
+	return NodeLabelsRetriever.GetNVMeUUIDs(ctx, k8sclientset)
+}

pkg/node/node.go

@@ -127,6 +127,13 @@ func (s *Service) Init() error {
 		return nil
 	}
 
+	if len(nvmeInitiators) != 0 {
+		err = k8sutils.AddNVMeLabels(ctx, s.opts.KubeConfigPath, s.opts.KubeNodeName, "hostnqn-uuid", nvmeInitiators)
+		if err != nil {
+			log.Warnf("Unable to add hostnqn uuid label for node %s: %v", s.opts.KubeNodeName, err.Error())
+		}
+	}
+
 	// Setup host on each of available arrays
 	for _, arr := range s.Arrays() {
 		if arr.BlockProtocol == common.NoneTransport {
@@ -243,6 +250,28 @@ func (s *Service) initConnectors() {
 	}
 }
 
+// Check for duplicate hostnqn uuids
+func (s *Service) checkForDuplicateUUIDs() {
+	nodeUUIDs := make(map[string]string)
+	duplicateUUIDs := make(map[string]string)
+
+	var err error
+	nodeUUIDs, err = k8sutils.GetNVMeUUIDs(context.Background(), s.opts.KubeConfigPath)
+	if err != nil {
+		log.Errorf("Unable to check uuids")
+		return
+	}
+
+	// Iterate over all nodes to check their uuid
+	for node, uuid := range nodeUUIDs {
+		if existingNode, found := duplicateUUIDs[uuid]; found {
+			log.Errorf("Duplicate hostnqn uuid %s found on nodes: %s and %s", uuid, existingNode, node)
+		} else {
+			duplicateUUIDs[uuid] = node
+		}
+	}
+}
+
 // NodeStageVolume prepares volume to be consumed by node publish by connecting volume to the node
 func (s *Service) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRequest) (*csi.NodeStageVolumeResponse, error) {
 	logFields := common.GetLogFields(ctx)
@@ -1433,7 +1462,13 @@ func (s *Service) setupHost(initiators []string, client gopowerstore.Client, arr
 		return fmt.Errorf("nodeID not set")
 	}
 
+	if s.useNVME[arrayID] {
+		// Check for duplicate hostnqn uuids
+		s.checkForDuplicateUUIDs()
+	}
+
 	reqInitiators := s.buildInitiatorsArray(initiators, arrayID)
+
 	var host *gopowerstore.Host
 	updateCHAP := false
 

pkg/node/node_test.go

@@ -273,6 +273,7 @@ func setDefaultNodeLabelsRetrieverMock() {
 	nodeLabelsRetrieverMock.On("GetNodeLabels", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
 	nodeLabelsRetrieverMock.On("InClusterConfig", mock.Anything).Return(nil, nil)
 	nodeLabelsRetrieverMock.On("NewForConfig", mock.Anything).Return(nil, nil)
+	nodeLabelsRetrieverMock.On("GetNVMeUUIDs", mock.Anything, mock.Anything).Return(nil, nil)
 }
 
 var _ = ginkgo.Describe("CSINodeService", func() {
@@ -318,6 +319,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 				clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil)
 				clientMock.On("CreateHost", mock.Anything, mock.Anything).
 					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
 				nodeSvc.opts.NodeNamePrefix = ""
 				err := nodeSvc.Init()
 				gomega.Expect(err).To(gomega.BeNil())
@@ -329,6 +331,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 				nodeSvc.nodeID = ""
 
 				fsMock.On("ReadFile", mock.Anything).Return([]byte("my-host-id"), errors.New("no such file"))
+				setDefaultNodeLabelsRetrieverMock()
 
 				err := nodeSvc.Init()
 				gomega.Expect(err.Error()).To(gomega.ContainSubstring("no such file"))
@@ -368,6 +371,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 					}}, nil)
 				clientMock.On("CreateHost", mock.Anything, mock.Anything).
 					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
 				nodeSvc.opts.NodeNamePrefix = ""
 				err := nodeSvc.Init()
 				gomega.Expect(err.Error()).To(gomega.ContainSubstring("Could not connect to PowerStore array"))
@@ -407,6 +411,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 					}}, nil)
 				clientMock.On("CreateHost", mock.Anything, mock.Anything).
 					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
 				nodeSvc.opts.NodeNamePrefix = ""
 				err := nodeSvc.Init()
 				gomega.Expect(err.Error()).To(gomega.ContainSubstring("node name prefix is too long"))
@@ -438,6 +443,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 							},
 							Name: "host-name",
 						}, nil)
+					setDefaultNodeLabelsRetrieverMock()
 					err := nodeSvc.Init()
 					gomega.Expect(err).To(gomega.BeNil())
 				})
@@ -462,6 +468,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 
 					clientMock.On("ModifyHost", mock.Anything, mock.Anything, "host-id").
 						Return(gopowerstore.CreateResponse{}, nil)
+					setDefaultNodeLabelsRetrieverMock()
 
 					err := nodeSvc.Init()
 					gomega.Expect(err).To(gomega.BeNil())
@@ -501,6 +508,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 						}}, nil)
 					clientMock.On("ModifyHost", mock.Anything, mock.Anything, "host-id").
 						Return(gopowerstore.CreateResponse{ID: "host-id"}, nil)
+					setDefaultNodeLabelsRetrieverMock()
 
 					err := nodeSvc.Init()
 					gomega.Expect(err).To(gomega.BeNil())
@@ -540,6 +548,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 						}}, nil)
 					clientMock.On("ModifyHost", mock.Anything, mock.Anything, "host-id").
 						Return(gopowerstore.CreateResponse{ID: "host-id"}, nil)
+					setDefaultNodeLabelsRetrieverMock()
 
 					err := nodeSvc.Init()
 					gomega.Expect(err).To(gomega.BeNil())
@@ -590,6 +599,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 				clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil)
 				clientMock.On("CreateHost", mock.Anything, mock.Anything).
 					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
 
 				err := nodeSvc.Init()
 				gomega.Expect(err).To(gomega.BeNil())
@@ -635,6 +645,61 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 				clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil)
 				clientMock.On("CreateHost", mock.Anything, mock.Anything).
 					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
+
+				err := nodeSvc.Init()
+				gomega.Expect(err).To(gomega.BeNil())
+			})
+
+			ginkgo.It("should create NVMe host and check for duplicate UUIDs", func() {
+				nodeSvc.Arrays()[firstValidIP].BlockProtocol = common.NVMEFCTransport
+				nodeSvc.nodeID = ""
+				nodeSvc.useNVME[firstGlobalID] = true
+				fsMock.On("ReadFile", mock.Anything).Return([]byte("my-host-id"), nil)
+				conn, _ := net.Dial("udp", "127.0.0.1:80")
+				fsMock.On("NetDial", mock.Anything).Return(
+					conn,
+					nil,
+				)
+				iscsiConnectorMock.On("GetInitiatorName", mock.Anything).
+					Return(validISCSIInitiators, nil)
+				nvmeConnectorMock.On("GetInitiatorName", mock.Anything).
+					Return(validNVMEInitiators, nil)
+				fcConnectorMock.On("GetInitiatorPorts", mock.Anything).
+					Return(validFCTargetsWWPN, nil)
+
+				nodeSvc.opts.KubeNodeName = common.EnvKubeNodeName
+				nodeSvc.opts.KubeConfigPath = common.EnvKubeConfigPath
+				nodeLabelsRetrieverMock.On("GetNVMeUUIDs", mock.Anything, mock.Anything).Return(
+					map[string]string{
+						"node1": "duplicate-uuid",
+						"node2": "duplicate-uuid",
+					},
+					nil,
+				)
+
+				clientMock.On("GetHostByName", mock.Anything, mock.AnythingOfType("string")).
+					Return(gopowerstore.Host{}, gopowerstore.APIError{
+						ErrorMsg: &api.ErrorMsg{
+							StatusCode: http.StatusNotFound,
+						},
+					})
+				clientMock.On("GetHosts", mock.Anything).Return(
+					[]gopowerstore.Host{{
+						ID: "host-id",
+						Initiators: []gopowerstore.InitiatorInstance{{
+							PortName: "not-matching-port-name",
+							PortType: gopowerstore.InitiatorProtocolTypeEnumNVME,
+						}},
+						Name: "host-name",
+					}}, nil)
+
+				clientMock.On("GetCustomHTTPHeaders").Return(make(http.Header))
+				clientMock.On("GetSoftwareMajorMinorVersion", context.Background()).Return(float32(3.0), nil)
+				clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil)
+				clientMock.On("CreateHost", mock.Anything, mock.Anything).
+					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
 
 				err := nodeSvc.Init()
 				gomega.Expect(err).To(gomega.BeNil())
@@ -676,6 +741,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 				clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil)
 				clientMock.On("CreateHost", mock.Anything, mock.Anything).
 					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
 
 				err := nodeSvc.Init()
 				gomega.Expect(err).To(gomega.BeNil())
@@ -719,6 +785,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 				clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil)
 				clientMock.On("CreateHost", mock.Anything, mock.Anything).
 					Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+				setDefaultNodeLabelsRetrieverMock()
 
 				err := nodeSvc.Init()
 				gomega.Expect(err).To(gomega.BeNil())
@@ -4081,6 +4148,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
 			clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil)
 			clientMock.On("CreateHost", mock.Anything, mock.Anything).
 				Return(gopowerstore.CreateResponse{ID: validHostID}, nil)
+			setDefaultNodeLabelsRetrieverMock()
 			nodeSvc.opts.NodeNamePrefix = ""
 			nodeSvc.Init()