Skip to content

Commit

Permalink
fix(CSI-260): lookup of NFS interface group fails when empty name pro…
Browse files Browse the repository at this point in the history
…vided
  • Loading branch information
sergeyberezansky committed Sep 24, 2024
1 parent fcf2e14 commit fe238e2
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 35 deletions.
52 changes: 21 additions & 31 deletions pkg/wekafs/apiclient/interfacegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/helm/pkg/urlutil"
"os"
"sort"
)

type InterfaceGroupType string
Expand Down Expand Up @@ -143,7 +142,7 @@ func (a *ApiClient) GetInterfaceGroupByUid(ctx context.Context, uid uuid.UUID, i
return nil
}

func (a *ApiClient) fetchNfsInterfaceGroup(ctx context.Context, name *string, useDefault bool) error {
func (a *ApiClient) fetchNfsInterfaceGroup(ctx context.Context, name string) error {
igs := &[]InterfaceGroup{}
err := a.GetInterfaceGroupsByType(ctx, InterfaceGroupTypeNFS, igs)
if err != nil {
Expand All @@ -152,42 +151,33 @@ func (a *ApiClient) fetchNfsInterfaceGroup(ctx context.Context, name *string, us
if len(*igs) == 0 {
return errors.New("no nfs interface groups found")
}
if name != nil {
for _, ig := range *igs {
if ig.Name == *name {
a.NfsInterfaceGroups[*name] = &ig
igname := name
if name == "" {
igname = "default"
}
for _, ig := range *igs {
if ig.Name == name || name == "" {
if len(ig.Ips) == 0 {
if name == "" {
continue
}
return errors.New("no IP addresses found for nfs interface group \"" + name + "\"")
}
a.NfsInterfaceGroups[igname] = &ig
return nil
}
} else if useDefault {
a.NfsInterfaceGroups["default"] = &(*igs)[0]
}

ig := &InterfaceGroup{}
if name != nil {
ig = a.NfsInterfaceGroups[*name]
} else {
ig = a.NfsInterfaceGroups["default"]
}
if ig == nil {
return errors.New("no nfs interface group found")
}

if len(ig.Ips) == 0 {
return errors.New("no IP addresses found for nfs interface group")
}
// Make sure the IPs are always sorted
sort.Strings(ig.Ips)
return nil
return errors.New(fmt.Sprintf("no nfs interface group named '%s' found", name))
}

func (a *ApiClient) GetNfsInterfaceGroup(ctx context.Context, name *string) *InterfaceGroup {
igName := "default"
if name != nil {
igName = *name
func (a *ApiClient) GetNfsInterfaceGroup(ctx context.Context, name string) *InterfaceGroup {
igName := name
if name == "" {
igName = "default"
}
_, ok := a.NfsInterfaceGroups[igName]
if !ok {
err := a.fetchNfsInterfaceGroup(ctx, name, true)
err := a.fetchNfsInterfaceGroup(ctx, name)
if err != nil {
return nil
}
Expand All @@ -197,7 +187,7 @@ func (a *ApiClient) GetNfsInterfaceGroup(ctx context.Context, name *string) *Int

// GetNfsMountIp returns the IP address of the NFS interface group to be used for NFS mount
// TODO: need to do it much more sophisticated way to distribute load
func (a *ApiClient) GetNfsMountIp(ctx context.Context, interfaceGroupName *string) (string, error) {
func (a *ApiClient) GetNfsMountIp(ctx context.Context, interfaceGroupName string) (string, error) {
ig := a.GetNfsInterfaceGroup(ctx, interfaceGroupName)
if ig == nil {
return "", errors.New("no NFS interface group found")
Expand Down
44 changes: 44 additions & 0 deletions pkg/wekafs/apiclient/interfacegroup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package apiclient

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
)

func TestGetNfsInterfaceGroup(t *testing.T) {
apiClient := GetApiClientForTest(t)
// Mock GetInterfaceGroupsByType method

// Test case: Valid interface group
ig := apiClient.GetNfsInterfaceGroup(context.Background(), "")
assert.NotNil(t, ig)
assert.Contains(t, apiClient.NfsInterfaceGroups, "default")

// Test case: Invalid interface group
ig = apiClient.GetNfsInterfaceGroup(context.Background(), "NFS")
assert.Nil(t, ig)

// Test case: Invalid interface group
ig = apiClient.GetNfsInterfaceGroup(context.Background(), "Data")
assert.NotNil(t, ig)
assert.Contains(t, apiClient.NfsInterfaceGroups, "Data")

}

func TestGetNfsMountIp(t *testing.T) {
apiClient := GetApiClientForTest(t)
// Mock GetInterfaceGroupsByType method

// Test case: Valid interface group
ip, err := apiClient.GetNfsMountIp(context.Background(), "")
assert.NoError(t, err)
assert.NotEmpty(t, ip)

// Test case: Valid interface group
ip, err = apiClient.GetNfsMountIp(context.Background(), "NFS")
assert.NoError(t, err)
assert.NotEmpty(t, ip)

}
2 changes: 1 addition & 1 deletion pkg/wekafs/apiclient/nfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ var fsName string
var client *ApiClient

func TestMain(m *testing.M) {
flag.StringVar(&endpoint, "api-endpoint", "vm49-1723969301909816-0.lan:14000", "API endpoint for tests")
flag.StringVar(&endpoint, "api-endpoint", "vm125-1726039130891528-0.lan:14000", "API endpoint for tests")
flag.StringVar(&creds.Username, "api-username", "admin", "API username for tests")
flag.StringVar(&creds.Password, "api-password", "AAbb1234", "API password for tests")
flag.StringVar(&creds.Organization, "api-org", "Root", "API org for tests")
Expand Down
2 changes: 1 addition & 1 deletion pkg/wekafs/nfsmount.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type nfsMount struct {
mountOptions MountOptions
lastUsed time.Time
mountIpAddress string
interfaceGroupName *string
interfaceGroupName string
clientGroupName string
protocolVersion apiclient.NfsVersionString
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/wekafs/nfsmounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type nfsMounter struct {
debugPath string
selinuxSupport *bool
gc *innerPathVolGc
interfaceGroupName *string
interfaceGroupName string
clientGroupName string
nfsProtocolVersion string
exclusiveMountOptions []mutuallyExclusiveMountOptionSet
Expand All @@ -36,7 +36,7 @@ func newNfsMounter(driver *WekaFsDriver) *nfsMounter {
mounter := &nfsMounter{mountMap: make(nfsMountsMap), debugPath: driver.debugPath, selinuxSupport: selinuxSupport, exclusiveMountOptions: driver.config.mutuallyExclusiveOptions}
mounter.gc = initInnerPathVolumeGc(mounter)
mounter.schedulePeriodicMountGc()
mounter.interfaceGroupName = &driver.config.interfaceGroupName
mounter.interfaceGroupName = driver.config.interfaceGroupName
mounter.clientGroupName = driver.config.clientGroupName
mounter.nfsProtocolVersion = driver.config.nfsProtocolVersion

Expand Down

0 comments on commit fe238e2

Please sign in to comment.