Skip to content

Commit

Permalink
Merge pull request #364 from dell/1530_duplicate_hostnqn
Browse files Browse the repository at this point in the history 
Error logging for duplicate host nqns
  • Loading branch information
@suryagupta4
suryagupta4 authored Oct 25, 2024
2 parents f267ca8 + a89f214 commit 44479cf
Show file tree
Hide file tree
Showing 4 changed files with 224 additions and 1 deletion.
26 changes: 26 additions & 0 deletions mocks/NodeLabelsRetriever.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 95 additions & 1 deletion pkg/common/k8sutils/k8sutils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package k8sutils

import (
"context"
"fmt"
"strings"

v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
35 changes: 35 additions & 0 deletions pkg/node/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
68 changes: 68 additions & 0 deletions pkg/node/node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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())
Expand All @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -438,6 +443,7 @@ var _ = ginkgo.Describe("CSINodeService", func() {
},
Name: "host-name",
}, nil)
setDefaultNodeLabelsRetrieverMock()
err := nodeSvc.Init()
gomega.Expect(err).To(gomega.BeNil())
})
Expand All @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 44479cf

Please sign in to comment.