From b5ddccc0c241d018a2a83f377ba41cce463af905 Mon Sep 17 00:00:00 2001 From: Surya Date: Wed, 23 Oct 2024 18:43:42 +0000 Subject: [PATCH 01/13] add nvme hostnqn labels --- pkg/common/k8sutils/k8sutils.go | 48 +++++++++++++++++++++++++++++++++ pkg/node/node.go | 5 ++++ 2 files changed, 53 insertions(+) diff --git a/pkg/common/k8sutils/k8sutils.go b/pkg/common/k8sutils/k8sutils.go index 8f52f4a8..4ca438e3 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" @@ -33,14 +35,24 @@ type NodeLabelsRetrieverInterface interface { GetNodeLabels(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (map[string]string, error) } +// NodeLabelsRetrieverInterface 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 NodeLabelsModifier NodeLabelsModifierInterface func init() { NodeLabelsRetriever = new(NodeLabelsRetrieverImpl) + NodeLabelsModifier = new(NodeLabelsModifierImpl) } // BuildConfigFromFlags is a method for building kubernetes client config @@ -99,6 +111,32 @@ 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) + } + + // Update the node with the new label + node.Labels[labelKey] = strings.Join(labelValue, ",") + _, 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 +} + // 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 +146,13 @@ 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) +} diff --git a/pkg/node/node.go b/pkg/node/node.go index 2b844a51..f6850a44 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -125,6 +125,11 @@ func (s *Service) Init() error { return nil } + 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 { From 94b3f28e96f18d6bfc77f86e54cb85230ea3133b Mon Sep 17 00:00:00 2001 From: Surya Date: Wed, 23 Oct 2024 19:04:11 +0000 Subject: [PATCH 02/13] fetch only uuids --- pkg/common/k8sutils/k8sutils.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/common/k8sutils/k8sutils.go b/pkg/common/k8sutils/k8sutils.go index 4ca438e3..650d936f 100644 --- a/pkg/common/k8sutils/k8sutils.go +++ b/pkg/common/k8sutils/k8sutils.go @@ -128,8 +128,17 @@ func (svc *NodeLabelsModifierImpl) AddNVMeLabels(ctx context.Context, k8sclients node.Labels = make(map[string]string) } - // Update the node with the new label - node.Labels[labelKey] = strings.Join(labelValue, ",") + // 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()) From 57a78147eb2fe2a088e281a4417a34def91ad46c Mon Sep 17 00:00:00 2001 From: Surya Date: Wed, 23 Oct 2024 19:49:51 +0000 Subject: [PATCH 03/13] add checkForDuplicateUUIDs --- pkg/node/node.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/node/node.go b/pkg/node/node.go index f6850a44..d259314f 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -32,6 +32,8 @@ import ( "strings" "github.com/dell/gonvme" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/dell/csi-powerstore/v2/pkg/array" @@ -130,6 +132,15 @@ func (s *Service) Init() error { log.Warnf("Unable to add hostnqn uuid label for node %s: %v", s.opts.KubeNodeName, err.Error()) } + // Initialize the Kubernetes client + k8sclientset, err := k8sutils.CreateKubeClientSet(s.opts.KubeConfigPath) + if err != nil { + return fmt.Errorf("failed to create Kubernetes clientset: %v", err.Error()) + } + + // Check for duplicate uuids + s.checkForDuplicateUUIDs(k8sclientset) + // Setup host on each of available arrays for _, arr := range s.Arrays() { if arr.BlockProtocol == common.NoneTransport { @@ -241,6 +252,30 @@ func (s *Service) initConnectors() { } } +// Check for duplicate hostnqn uuids +func (s *Service) checkForDuplicateUUIDs(k8sclientset *kubernetes.Clientset) { + nodeUUIDs := make(map[string]string) + + // Retrieve the list of nodes + nodes, err := k8sclientset.CoreV1().Nodes().List(context.Background(), v1.ListOptions{}) + if err != nil { + log.Errorf("failed to get node list: %v", err.Error()) + return + } + + // Iterate over all nodes to check their labels + for _, node := range nodes.Items { + labels := node.Labels + if uuid, exists := labels["hostnqn-uuid"]; exists { + if existingNode, found := nodeUUIDs[uuid]; found { + log.Errorf("Duplicate hostnqn uuid %s found on nodes: %s and %s", uuid, existingNode, node.Name) + } else { + nodeUUIDs[uuid] = node.Name + } + } + } +} + // 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) From 43bb2b15fa17398bbe85ed816221001d7d538941 Mon Sep 17 00:00:00 2001 From: Surya Date: Thu, 24 Oct 2024 07:38:23 +0000 Subject: [PATCH 04/13] add & check labels when nvme is used --- pkg/node/node.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/pkg/node/node.go b/pkg/node/node.go index d259314f..2a18147e 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -127,20 +127,13 @@ func (s *Service) Init() error { return nil } - 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()) - } - - // Initialize the Kubernetes client - k8sclientset, err := k8sutils.CreateKubeClientSet(s.opts.KubeConfigPath) - if err != nil { - return fmt.Errorf("failed to create Kubernetes clientset: %v", err.Error()) + 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()) + } } - // Check for duplicate uuids - s.checkForDuplicateUUIDs(k8sclientset) - // Setup host on each of available arrays for _, arr := range s.Arrays() { if arr.BlockProtocol == common.NoneTransport { @@ -1461,6 +1454,17 @@ func (s *Service) setupHost(initiators []string, client gopowerstore.Client, arr return fmt.Errorf("nodeID not set") } + if s.useNVME { + // Initialize the Kubernetes client + k8sclientset, err := k8sutils.CreateKubeClientSet(s.opts.KubeConfigPath) + if err != nil { + return fmt.Errorf("failed to create Kubernetes clientset: %v", err.Error()) + } + + // Check for duplicate hostnqn uuids + s.checkForDuplicateUUIDs(k8sclientset) + } + reqInitiators := s.buildInitiatorsArray(initiators) var host *gopowerstore.Host updateCHAP := false From 328dc009dc6c4f3bc5b33b335bdc52cd77835472 Mon Sep 17 00:00:00 2001 From: Surya Date: Thu, 24 Oct 2024 09:10:04 +0000 Subject: [PATCH 05/13] gofumpt fix --- pkg/common/k8sutils/k8sutils.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/common/k8sutils/k8sutils.go b/pkg/common/k8sutils/k8sutils.go index 650d936f..82c79b8f 100644 --- a/pkg/common/k8sutils/k8sutils.go +++ b/pkg/common/k8sutils/k8sutils.go @@ -47,8 +47,10 @@ type NodeLabelsRetrieverImpl struct{} type NodeLabelsModifierImpl struct{} // NodeLabelsRetriever is the actual instance of NodeLabelsRetrieverInterface which is used to retrieve the node labels -var NodeLabelsRetriever NodeLabelsRetrieverInterface -var NodeLabelsModifier NodeLabelsModifierInterface +var ( + NodeLabelsRetriever NodeLabelsRetrieverInterface + NodeLabelsModifier NodeLabelsModifierInterface +) func init() { NodeLabelsRetriever = new(NodeLabelsRetrieverImpl) From e2a2d6580276dc27569a6d065526f0d6336339f8 Mon Sep 17 00:00:00 2001 From: Surya Date: Thu, 24 Oct 2024 09:16:03 +0000 Subject: [PATCH 06/13] update useNVME field --- pkg/node/node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/node/node.go b/pkg/node/node.go index d1242a33..92188737 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -1466,7 +1466,7 @@ func (s *Service) setupHost(initiators []string, client gopowerstore.Client, arr return fmt.Errorf("nodeID not set") } - if s.useNVME { + if s.useNVME[arrayID] { // Initialize the Kubernetes client k8sclientset, err := k8sutils.CreateKubeClientSet(s.opts.KubeConfigPath) if err != nil { From 1e3a3eb95222717c02d506b66eed184bc326e08b Mon Sep 17 00:00:00 2001 From: Surya Date: Thu, 24 Oct 2024 09:21:56 +0000 Subject: [PATCH 07/13] update comment --- pkg/common/k8sutils/k8sutils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/common/k8sutils/k8sutils.go b/pkg/common/k8sutils/k8sutils.go index 82c79b8f..3569398a 100644 --- a/pkg/common/k8sutils/k8sutils.go +++ b/pkg/common/k8sutils/k8sutils.go @@ -35,7 +35,7 @@ type NodeLabelsRetrieverInterface interface { GetNodeLabels(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (map[string]string, error) } -// NodeLabelsRetrieverInterface defines the methods for retrieving Kubernetes Node Labels +// 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 } From 0925cabe86a5da93fef705b01a000bc538638b54 Mon Sep 17 00:00:00 2001 From: Akshay Saini Date: Thu, 24 Oct 2024 17:07:11 +0530 Subject: [PATCH 08/13] Fix UT --- pkg/node/node_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/node/node_test.go b/pkg/node/node_test.go index d2a79269..6f22baa6 100644 --- a/pkg/node/node_test.go +++ b/pkg/node/node_test.go @@ -318,6 +318,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 +330,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 +370,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 +410,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 +442,7 @@ var _ = ginkgo.Describe("CSINodeService", func() { }, Name: "host-name", }, nil) + setDefaultNodeLabelsRetrieverMock() err := nodeSvc.Init() gomega.Expect(err).To(gomega.BeNil()) }) @@ -462,6 +467,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 +507,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 +547,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 +598,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 +644,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()) @@ -676,6 +686,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 +730,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 +4093,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() From 97055c2f6c1fa70234e3a637e86e135386f3488d Mon Sep 17 00:00:00 2001 From: Surya Date: Fri, 25 Oct 2024 05:53:51 +0000 Subject: [PATCH 09/13] code restructure --- pkg/common/k8sutils/k8sutils.go | 35 +++++++++++++++++++++++++++++++++ pkg/node/node.go | 34 +++++++++++--------------------- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/pkg/common/k8sutils/k8sutils.go b/pkg/common/k8sutils/k8sutils.go index 3569398a..c5c431cc 100644 --- a/pkg/common/k8sutils/k8sutils.go +++ b/pkg/common/k8sutils/k8sutils.go @@ -33,6 +33,7 @@ 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, kubeNodeName string) (map[string]string, error) } // NodeLabelsModifierInterface defines the methods for retrieving Kubernetes Node Labels @@ -148,6 +149,30 @@ func (svc *NodeLabelsModifierImpl) AddNVMeLabels(ctx context.Context, k8sclients return nil } +// GetNVMeUUIDs returns map of hosts with their hostnqn uuids +func (svc *NodeLabelsRetrieverImpl) GetNVMeUUIDs(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (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(context.Background(), 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) @@ -167,3 +192,13 @@ func AddNVMeLabels(ctx context.Context, kubeConfigPath string, kubeNodeName stri 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, kubeNodeName string) (map[string]string, error) { + k8sclientset, err := CreateKubeClientSet(kubeConfigPath) + if err != nil { + return map[string]string{}, err + } + + return NodeLabelsRetriever.GetNVMeUUIDs(ctx, k8sclientset, kubeNodeName) +} diff --git a/pkg/node/node.go b/pkg/node/node.go index 92188737..2fa6bade 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -32,8 +32,6 @@ import ( "strings" "github.com/dell/gonvme" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/dell/csi-powerstore/v2/pkg/array" @@ -253,25 +251,23 @@ func (s *Service) initConnectors() { } // Check for duplicate hostnqn uuids -func (s *Service) checkForDuplicateUUIDs(k8sclientset *kubernetes.Clientset) { +func (s *Service) checkForDuplicateUUIDs() { nodeUUIDs := make(map[string]string) + duplicateUUIDs := make(map[string]string) - // Retrieve the list of nodes - nodes, err := k8sclientset.CoreV1().Nodes().List(context.Background(), v1.ListOptions{}) + var err error + nodeUUIDs, err = k8sutils.GetNVMeUUIDs(context.Background(), s.opts.KubeConfigPath, s.opts.KubeNodeName) if err != nil { - log.Errorf("failed to get node list: %v", err.Error()) + log.Errorf("Unable to check uuids") return } - // Iterate over all nodes to check their labels - for _, node := range nodes.Items { - labels := node.Labels - if uuid, exists := labels["hostnqn-uuid"]; exists { - if existingNode, found := nodeUUIDs[uuid]; found { - log.Errorf("Duplicate hostnqn uuid %s found on nodes: %s and %s", uuid, existingNode, node.Name) - } else { - nodeUUIDs[uuid] = node.Name - } + // 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 } } } @@ -1467,14 +1463,8 @@ func (s *Service) setupHost(initiators []string, client gopowerstore.Client, arr } if s.useNVME[arrayID] { - // Initialize the Kubernetes client - k8sclientset, err := k8sutils.CreateKubeClientSet(s.opts.KubeConfigPath) - if err != nil { - return fmt.Errorf("failed to create Kubernetes clientset: %v", err.Error()) - } - // Check for duplicate hostnqn uuids - s.checkForDuplicateUUIDs(k8sclientset) + s.checkForDuplicateUUIDs() } reqInitiators := s.buildInitiatorsArray(initiators, arrayID) From ce4ecef500344053f4877864b5d3c76cbb6f5f1a Mon Sep 17 00:00:00 2001 From: Surya Gupta Date: Fri, 25 Oct 2024 01:40:59 -0500 Subject: [PATCH 10/13] added mock function --- mocks/NodeLabelsRetriever.go | 27 +++++++++++++++++++++++++++ pkg/node/node_test.go | 1 + 2 files changed, 28 insertions(+) diff --git a/mocks/NodeLabelsRetriever.go b/mocks/NodeLabelsRetriever.go index 4dca0b74..0ac9ef73 100644 --- a/mocks/NodeLabelsRetriever.go +++ b/mocks/NodeLabelsRetriever.go @@ -59,6 +59,33 @@ func (_m *NodeLabelsRetrieverInterface) BuildConfigFromFlags(masterURL string, k return r0, r1 } +// GetNVMeUUIDs provides a mock function with given fields: ctx, k8sclientset, kubeNodeName +func (_m *NodeLabelsRetrieverInterface) GetNVMeUUIDs(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (map[string]string, error) { + ret := _m.Called(k8sclientset, ctx, kubeNodeName) + + var r0 map[string]string + var r1 error + if rf, ok := ret.Get(0).(func(*kubernetes.Clientset, context.Context, string) (map[string]string, error)); ok { + return rf(k8sclientset, ctx, kubeNodeName) + } + if rf, ok := ret.Get(0).(func(*kubernetes.Clientset, context.Context, string) map[string]string); ok { + r0 = rf(k8sclientset, ctx, kubeNodeName) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(map[string]string) + } + } + + if rf, ok := ret.Get(1).(func(*kubernetes.Clientset, context.Context, string) error); ok { + r1 = rf(k8sclientset, ctx, kubeNodeName) + } 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/node/node_test.go b/pkg/node/node_test.go index 6f22baa6..10627a05 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, mock.Anything).Return(map[string]string{}, nil) } var _ = ginkgo.Describe("CSINodeService", func() { From 21454438c41bc055be292aa938be6448534765df Mon Sep 17 00:00:00 2001 From: Surya Gupta Date: Fri, 25 Oct 2024 02:13:09 -0500 Subject: [PATCH 11/13] gofumpt fix --- mocks/NodeLabelsRetriever.go | 1 - 1 file changed, 1 deletion(-) diff --git a/mocks/NodeLabelsRetriever.go b/mocks/NodeLabelsRetriever.go index 0ac9ef73..83c2409f 100644 --- a/mocks/NodeLabelsRetriever.go +++ b/mocks/NodeLabelsRetriever.go @@ -85,7 +85,6 @@ func (_m *NodeLabelsRetrieverInterface) GetNVMeUUIDs(ctx context.Context, k8scli 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) From 28300f66bf3a8ed1bac1e52018ddc475e2cbcb0c Mon Sep 17 00:00:00 2001 From: Surya Gupta Date: Fri, 25 Oct 2024 02:32:48 -0500 Subject: [PATCH 12/13] updated GetNVMeUUIDs definition --- mocks/NodeLabelsRetriever.go | 18 +++++++++--------- pkg/common/k8sutils/k8sutils.go | 10 +++++----- pkg/node/node.go | 2 +- pkg/node/node_test.go | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/mocks/NodeLabelsRetriever.go b/mocks/NodeLabelsRetriever.go index 83c2409f..6c487c3e 100644 --- a/mocks/NodeLabelsRetriever.go +++ b/mocks/NodeLabelsRetriever.go @@ -59,25 +59,25 @@ func (_m *NodeLabelsRetrieverInterface) BuildConfigFromFlags(masterURL string, k return r0, r1 } -// GetNVMeUUIDs provides a mock function with given fields: ctx, k8sclientset, kubeNodeName -func (_m *NodeLabelsRetrieverInterface) GetNVMeUUIDs(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (map[string]string, error) { - ret := _m.Called(k8sclientset, ctx, kubeNodeName) +// 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, string) (map[string]string, error)); ok { - return rf(k8sclientset, ctx, kubeNodeName) + 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, string) map[string]string); ok { - r0 = rf(k8sclientset, ctx, kubeNodeName) + 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, string) error); ok { - r1 = rf(k8sclientset, ctx, kubeNodeName) + if rf, ok := ret.Get(1).(func(*kubernetes.Clientset, context.Context) error); ok { + r1 = rf(k8sclientset, ctx) } else { r1 = ret.Error(1) } diff --git a/pkg/common/k8sutils/k8sutils.go b/pkg/common/k8sutils/k8sutils.go index c5c431cc..798118cd 100644 --- a/pkg/common/k8sutils/k8sutils.go +++ b/pkg/common/k8sutils/k8sutils.go @@ -33,7 +33,7 @@ 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, 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 @@ -150,14 +150,14 @@ func (svc *NodeLabelsModifierImpl) AddNVMeLabels(ctx context.Context, k8sclients } // GetNVMeUUIDs returns map of hosts with their hostnqn uuids -func (svc *NodeLabelsRetrieverImpl) GetNVMeUUIDs(ctx context.Context, k8sclientset *kubernetes.Clientset, kubeNodeName string) (map[string]string, error) { +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(context.Background(), v1.ListOptions{}) + nodes, err := k8sclientset.CoreV1().Nodes().List(ctx, v1.ListOptions{}) if err != nil { return nodeUUIDs, fmt.Errorf("failed to get node list: %v", err.Error()) } @@ -194,11 +194,11 @@ func AddNVMeLabels(ctx context.Context, kubeConfigPath string, kubeNodeName stri } // GetNVMeUUIDs checks for duplicate hostnqn uuid labels in the k8s node -func GetNVMeUUIDs(ctx context.Context, kubeConfigPath string, kubeNodeName string) (map[string]string, error) { +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, kubeNodeName) + return NodeLabelsRetriever.GetNVMeUUIDs(ctx, k8sclientset) } diff --git a/pkg/node/node.go b/pkg/node/node.go index 2fa6bade..34307506 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -256,7 +256,7 @@ func (s *Service) checkForDuplicateUUIDs() { duplicateUUIDs := make(map[string]string) var err error - nodeUUIDs, err = k8sutils.GetNVMeUUIDs(context.Background(), s.opts.KubeConfigPath, s.opts.KubeNodeName) + nodeUUIDs, err = k8sutils.GetNVMeUUIDs(context.Background(), s.opts.KubeConfigPath) if err != nil { log.Errorf("Unable to check uuids") return diff --git a/pkg/node/node_test.go b/pkg/node/node_test.go index 10627a05..57aa7221 100644 --- a/pkg/node/node_test.go +++ b/pkg/node/node_test.go @@ -273,7 +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, mock.Anything).Return(map[string]string{}, nil) + nodeLabelsRetrieverMock.On("GetNVMeUUIDs", mock.Anything, mock.Anything).Return(map[string]string{}, nil) } var _ = ginkgo.Describe("CSINodeService", func() { From a89f21433b2226032f1f6c6fd9647065eea5d1e0 Mon Sep 17 00:00:00 2001 From: Surya Gupta Date: Fri, 25 Oct 2024 07:11:57 -0500 Subject: [PATCH 13/13] add UT for duplicate uuid scenario --- pkg/node/node_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/pkg/node/node_test.go b/pkg/node/node_test.go index 57aa7221..6f718367 100644 --- a/pkg/node/node_test.go +++ b/pkg/node/node_test.go @@ -273,7 +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(map[string]string{}, nil) + nodeLabelsRetrieverMock.On("GetNVMeUUIDs", mock.Anything, mock.Anything).Return(nil, nil) } var _ = ginkgo.Describe("CSINodeService", func() { @@ -650,6 +650,60 @@ var _ = ginkgo.Describe("CSINodeService", func() { 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()) + }) }) ginkgo.When("protocol flag initialization", func() {