diff --git a/mocks/NodeLabelsRetriever.go b/mocks/NodeLabelsRetriever.go index 4dca0b74..6c487c3e 100644 --- a/mocks/NodeLabelsRetriever.go +++ b/mocks/NodeLabelsRetriever.go @@ -59,6 +59,32 @@ func (_m *NodeLabelsRetrieverInterface) BuildConfigFromFlags(masterURL string, k return r0, r1 } +// GetNVMeUUIDs provides a mock function with given fields: ctx, k8sclientset +func (_m *NodeLabelsRetrieverInterface) GetNVMeUUIDs(ctx context.Context, k8sclientset *kubernetes.Clientset) (map[string]string, error) { + ret := _m.Called(k8sclientset, ctx) + + var r0 map[string]string + var r1 error + if rf, ok := ret.Get(0).(func(*kubernetes.Clientset, context.Context) (map[string]string, error)); ok { + return rf(k8sclientset, ctx) + } + if rf, ok := ret.Get(0).(func(*kubernetes.Clientset, context.Context) map[string]string); ok { + r0 = rf(k8sclientset, ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(map[string]string) + } + } + + if rf, ok := ret.Get(1).(func(*kubernetes.Clientset, context.Context) error); ok { + r1 = rf(k8sclientset, ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetNodeLabels provides a mock function with given fields: ctx, k8sclientset, kubeNodeName func (_m *NodeLabelsRetrieverInterface) GetNodeLabels(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (map[string]string, error) { ret := _m.Called(k8sclientset, ctx, kubeNodeName) diff --git a/pkg/common/k8sutils/k8sutils.go b/pkg/common/k8sutils/k8sutils.go index 8f52f4a8..798118cd 100644 --- a/pkg/common/k8sutils/k8sutils.go +++ b/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) +} diff --git a/pkg/node/node.go b/pkg/node/node.go index eb556212..34307506 100644 --- a/pkg/node/node.go +++ b/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 diff --git a/pkg/node/node_test.go b/pkg/node/node_test.go index d2a79269..6f718367 100644 --- a/pkg/node/node_test.go +++ b/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()