Skip to content

Commit

Permalink
feat(CSI-247): implement InterfaceGroup.GetRandomIpAddress() (#319)
Browse files Browse the repository at this point in the history
### TL;DR

Implemented random IP selection for NFS mounts and simplified mount reference counting.

### What changed?

- Added a new `GetRandomIpAddress` method to the `InterfaceGroup` struct, which selects a random IP address from the available IPs.
- Modified `GetNfsMountIp` to use the new `GetRandomIpAddress` method instead of `GetIpAddress`.
- Removed reference counting logic from the `nfsMount` struct, including the `refCount` field and associated `lock`.
- Simplified `incRef` and `decRef` methods to always perform mount and unmount operations, respectively.

### How to test?

1. Test the new `GetRandomIpAddress` method by calling it multiple times and verifying that different IPs are returned.
2. Verify that NFS mounts now use randomly selected IP addresses.
3. Test mounting and unmounting operations to ensure they work correctly without reference counting.
4. Check that concurrent mount operations are handled properly.

### Why make this change?

- Random IP selection for NFS mounts improves load balancing across available IPs.
- Simplifying the mount reference counting reduces complexity and potential race conditions.
- These changes aim to enhance the reliability and performance of NFS mount operations in the system.

---

feat(CSI-247): implement InterfaceGroup.GetRandomIpAddress()

feat(CSI-247): optimize NFS by utilizing multiple targets
  • Loading branch information
sergeyberezansky authored Sep 12, 2024
2 parents 3b53994 + 43c8d6a commit 286aac6
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 38 deletions.
17 changes: 16 additions & 1 deletion pkg/wekafs/apiclient/interfacegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"github.com/google/uuid"
"github.com/rs/zerolog/log"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/helm/pkg/urlutil"
"os"
"sort"
Expand Down Expand Up @@ -93,6 +94,20 @@ func (i *InterfaceGroup) GetIpAddress(ctx context.Context) (string, error) {
return i.Ips[idx], nil
}

func (i *InterfaceGroup) GetRandomIpAddress(ctx context.Context) (string, error) {
logger := log.Ctx(ctx)
if i == nil {
return "", errors.New("interface group is nil")
}
if len(i.Ips) == 0 {
return "", errors.New("no IP addresses found for interface group")
}
idx := rand.Intn(len(i.Ips))
ip := i.Ips[idx]
logger.Debug().Str("ip", ip).Msg("Selected random IP address")
return ip, nil
}

func (a *ApiClient) GetInterfaceGroups(ctx context.Context, interfaceGroups *[]InterfaceGroup) error {
ig := &InterfaceGroup{}

Expand Down Expand Up @@ -191,5 +206,5 @@ func (a *ApiClient) GetNfsMountIp(ctx context.Context, interfaceGroupName *strin
return "", errors.New("no IP addresses found for NFS interface group")
}

return ig.GetIpAddress(ctx)
return ig.GetRandomIpAddress(ctx)
}
42 changes: 5 additions & 37 deletions pkg/wekafs/nfsmount.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,12 @@ import (
"os"
"path/filepath"
"strings"
"sync"
"time"
)

type nfsMount struct {
fsName string
mountPoint string
refCount int
lock sync.Mutex
kMounter mount.Interface
debugPath string
mountOptions MountOptions
Expand All @@ -33,7 +30,7 @@ func (m *nfsMount) getMountPoint() string {
}

func (m *nfsMount) getRefCount() int {
return m.refCount
return 0
}

func (m *nfsMount) getMountOptions() MountOptions {
Expand All @@ -53,44 +50,15 @@ func (m *nfsMount) isMounted() bool {
}

func (m *nfsMount) incRef(ctx context.Context, apiClient *apiclient.ApiClient) error {
logger := log.Ctx(ctx)
m.lock.Lock()
defer m.lock.Unlock()
if m.refCount < 0 {
logger.Error().Str("mount_point", m.mountPoint).Int("refcount", m.refCount).Msg("During incRef negative refcount encountered")
m.refCount = 0 // to make sure that we don't have negative refcount later
}
if m.refCount == 0 {
if err := m.doMount(ctx, apiClient, m.mountOptions); err != nil {
return err
}
} else if !m.isMounted() {
logger.Warn().Str("mount_point", m.mountPoint).Int("refcount", m.refCount).Msg("Mount not exists although should!")
if err := m.doMount(ctx, apiClient, m.mountOptions); err != nil {
return err
}

if err := m.doMount(ctx, apiClient, m.mountOptions); err != nil {
return err
}
m.refCount++
logger.Trace().Int("refcount", m.refCount).Strs("mount_options", m.mountOptions.Strings()).Str("filesystem_name", m.fsName).Msg("RefCount increased")
return nil
}

func (m *nfsMount) decRef(ctx context.Context) error {
logger := log.Ctx(ctx)
m.lock.Lock()
defer m.lock.Unlock()
m.refCount--
m.lastUsed = time.Now()
logger.Trace().Int("refcount", m.refCount).Strs("mount_options", m.mountOptions.Strings()).Str("filesystem_name", m.fsName).Msg("RefCount decreased")
if m.refCount < 0 {
logger.Error().Int("refcount", m.refCount).Msg("During decRef negative refcount encountered")
m.refCount = 0 // to make sure that we don't have negative refcount later
}
if m.refCount == 0 {
if err := m.doUnmount(ctx); err != nil {
return err
}
if err := m.doUnmount(ctx); err != nil {
return err
}
return nil
}
Expand Down

0 comments on commit 286aac6

Please sign in to comment.