cephfs: remove redundant client eviction in fencing workflow

The MDS automatically evicts clients whose IPs appear in the
OSD blocklist, making explicit `client ls` and `client evict`
commands redundant. These MDS admin socket commands also require
wildcard (`*`) caps that cannot be narrowed down, causing EACCES
errors when the provisioner client lacks full MDS permissions.

Remove the redundant eviction step and use only OSD blocklisting
for CephFS fencing, consistent with how RBD fencing already works.

Signed-off-by: Praveen M <m.praveen@ibm.com>

Author: iPraveenParihar

internal/csi-addons/cephfs/network_fence.go

@@ -64,7 +64,8 @@ func validateNetworkFenceReq(fenceClients []*fence.CIDR, options map[string]stri
 }
 
 // FenceClusterNetwork blocks access to a CIDR block by creating a network fence.
-// It evicts the IP addresses of clients, which are in CIDR block.
+// The MDS automatically evicts clients whose IPs appear in the OSD blocklist,
+// so explicit client eviction via MDS commands is not needed.
 func (fcs *FenceControllerServer) FenceClusterNetwork(
 	ctx context.Context,
 	req *fence.FenceClusterNetworkRequest,
@@ -85,7 +86,7 @@ func (fcs *FenceControllerServer) FenceClusterNetwork(
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	err = nwFence.AddClientEviction(ctx)
+	err = nwFence.AddNetworkFence(ctx)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to fence CIDR block %q: %s", nwFence.Cidr, err.Error())
 	}

internal/csi-addons/networkfence/fencing.go

@@ -18,11 +18,9 @@ package networkfence
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"net"
-	"strconv"
 	"strings"
 	"time"
 
@@ -42,8 +40,6 @@ const (
 	// TODO: Make this configurable.
 	blockListCoolDownPeriod = 5 * time.Minute
 	invalidCommandStr       = "invalid command"
-	// we can always use mds rank 0, since all the clients have a session with rank-0.
-	mdsRank = 0
 )
 
 // NetworkFence contains the CIDR blocks to be blocked.
@@ -53,11 +49,6 @@ type NetworkFence struct {
 	cr       *util.Credentials
 }
 
-// activeClient represents the structure of an active client.
-type activeClient struct {
-	Inst string `json:"inst"`
-}
-
 // NewNetworkFence returns a networkFence struct object from the Network fence/unfence request.
 func NewNetworkFence(
 	ctx context.Context,
@@ -88,51 +79,6 @@ func NewNetworkFence(
 	return nwFence, nil
 }
 
-// AddClientEviction blocks access for all the IPs in the CIDR block
-// using client eviction, it also blocks the entire CIDR.
-func (nf *NetworkFence) AddClientEviction(ctx context.Context) error {
-	evictedIPs := make(map[string]bool)
-	// fetch active clients
-	activeClients, err := nf.listActiveClients(ctx)
-	if err != nil {
-		return err
-	}
-	// iterate through CIDR blocks and check if any active client matches
-	for _, cidr := range nf.Cidr {
-		for _, client := range activeClients {
-			var clientIP string
-			clientIP, err = client.fetchIP()
-			if err != nil {
-				return fmt.Errorf("error fetching client IP: %w", err)
-			}
-			// check if the clientIP is in the CIDR block
-			if isIPInCIDR(ctx, clientIP, cidr) {
-				var clientID int
-				clientID, err = client.fetchID()
-				if err != nil {
-					return fmt.Errorf("error fetching client ID: %w", err)
-				}
-				// evict the client
-				err = nf.evictCephFSClient(ctx, clientID)
-				if err != nil {
-					return fmt.Errorf("error evicting client %d: %w", clientID, err)
-				}
-				log.DebugLog(ctx, "client %d has been evicted\n", clientID)
-				// add the CIDR to the list of blocklisted IPs
-				evictedIPs[clientIP] = true
-			}
-		}
-	}
-
-	// add the range based blocklist for CIDR
-	err = nf.AddNetworkFence(ctx)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // RemoveNetworkFence unblocks access for all the IPs in the IP range mentioned via the CIDR block
 // using a network fence.
 // Unfencing one of the protocols(CephFS or RBD) suggests the node is expected to be recovered, so
@@ -220,91 +166,6 @@ func (nf *NetworkFence) addCephBlocklist(ctx context.Context, ip string, useRang
 	return util.AddCephBlocklist(ctx, nf.Monitors, nf.cr, ip, useRange)
 }
 
-func (nf *NetworkFence) listActiveClients(ctx context.Context) ([]activeClient, error) {
-	arg := []string{
-		"--id", nf.cr.ID,
-		"--keyfile=" + nf.cr.KeyFile,
-		"-m", nf.Monitors,
-	}
-	// FIXME: replace the ceph command with go-ceph API in future
-	cmd := []string{"tell", fmt.Sprintf("mds.%d", mdsRank), "client", "ls"}
-	cmd = append(cmd, arg...)
-	stdout, stdErr, err := util.ExecCommandWithTimeout(ctx, 2*time.Minute, "ceph", cmd...)
-	if err != nil {
-		return nil, fmt.Errorf("failed to list active clients: %w, stderr: %q", err, stdErr)
-	}
-
-	var activeClients []activeClient
-	if err := json.Unmarshal([]byte(stdout), &activeClients); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal JSON: %w", err)
-	}
-
-	return activeClients, nil
-}
-
-func (nf *NetworkFence) evictCephFSClient(ctx context.Context, clientID int) error {
-	arg := []string{
-		"--id", nf.cr.ID,
-		"--keyfile=" + nf.cr.KeyFile,
-		"-m", nf.Monitors,
-	}
-	// FIXME: replace the ceph command with go-ceph API in future
-	cmd := []string{"tell", fmt.Sprintf("mds.%d", mdsRank), "client", "evict", fmt.Sprintf("id=%d", clientID)}
-	cmd = append(cmd, arg...)
-	_, stdErr, err := util.ExecCommandWithTimeout(ctx, 2*time.Minute, "ceph", cmd...)
-	if err != nil {
-		return fmt.Errorf("failed to evict client %d: %w, stderr: %q", clientID, err, stdErr)
-	}
-	log.DebugLog(ctx, "client %s has been evicted from CephFS\n", clientID)
-
-	return nil
-}
-
-func isIPInCIDR(ctx context.Context, ip, cidr string) bool {
-	// Parse the CIDR block
-	_, ipCidr, err := net.ParseCIDR(cidr)
-	if err != nil {
-		log.ErrorLog(ctx, "error parsing CIDR block %s: %w\n", cidr, err)
-
-		return false
-	}
-
-	// Parse the IP address
-	ipAddress := net.ParseIP(ip)
-	if ipAddress == nil {
-		log.ErrorLog(ctx, "error parsing IP address %s\n", ip)
-
-		return false
-	}
-
-	// Check if the IP address is within the CIDR block
-	return ipCidr.Contains(ipAddress)
-}
-
-func (ac *activeClient) fetchIP() (string, error) {
-	// example: "inst": "client.4305 172.21.9.34:0/422650892",
-	// then returning value will be 172.21.9.34
-	return util.ParseClientIP(ac.Inst)
-}
-
-func (ac *activeClient) fetchID() (int, error) {
-	// example: "inst": "client.4305 172.21.9.34:0/422650892",
-	// then returning value will be 4305
-	clientInfo := ac.Inst
-	parts := strings.Fields(clientInfo)
-	if len(parts) >= 1 {
-		clientIDStr := strings.TrimPrefix(parts[0], "client.")
-		clientID, err := strconv.Atoi(clientIDStr)
-		if err != nil {
-			return 0, fmt.Errorf("failed to convert client ID to int: %w", err)
-		}
-
-		return clientID, nil
-	}
-
-	return 0, fmt.Errorf("failed to extract client ID, incorrect format: %s", clientInfo)
-}
-
 // getIPRange returns a list of IPs from the IP range
 // corresponding to a CIDR block.
 func getIPRange(cidr string) ([]string, error) {

internal/csi-addons/networkfence/fencing_test.go

@@ -58,91 +58,6 @@ func TestGetIPRange(t *testing.T) {
 	}
 }
 
-func TestFetchIP(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		clientInfo  string
-		expectedIP  string
-		expectedErr bool
-	}{
-		{
-			clientInfo:  "client.4305 172.21.9.34:0/422650892",
-			expectedIP:  "172.21.9.34",
-			expectedErr: false,
-		},
-		{
-			clientInfo:  "client.4305 [2001:0db8:85a3:0000:0000:8a2e:0370:7334]:0/422650892",
-			expectedIP:  "2001:db8:85a3::8a2e:370:7334",
-			expectedErr: false,
-		},
-		{
-			clientInfo:  "client.24152 v1:100.64.0.7:0/3658550259",
-			expectedIP:  "100.64.0.7",
-			expectedErr: false,
-		},
-		{
-			clientInfo:  "",
-			expectedIP:  "",
-			expectedErr: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.clientInfo, func(t *testing.T) {
-			t.Parallel()
-
-			client := activeClient{Inst: tt.clientInfo}
-			ip, actualErr := client.fetchIP()
-
-			if (actualErr != nil) != tt.expectedErr {
-				t.Errorf("expected error %v but got %v", tt.expectedErr, actualErr)
-			}
-
-			if ip != tt.expectedIP {
-				t.Errorf("expected IP %s but got %s", tt.expectedIP, ip)
-			}
-		})
-	}
-}
-
-func TestFetchID(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		clientInfo  string
-		expectedID  int
-		expectedErr bool
-	}{
-		{
-			clientInfo:  "client.4305 172.21.9.34:0/422650892",
-			expectedID:  4305,
-			expectedErr: false,
-		},
-		{
-			clientInfo:  "",
-			expectedID:  0,
-			expectedErr: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.clientInfo, func(t *testing.T) {
-			t.Parallel()
-			ac := &activeClient{Inst: tt.clientInfo}
-			actualID, actualErr := ac.fetchID()
-
-			if (actualErr != nil) != tt.expectedErr {
-				t.Errorf("expected error %v but got %v", tt.expectedErr, actualErr)
-			}
-
-			if actualID != tt.expectedID {
-				t.Errorf("expected ID %d but got %d", tt.expectedID, actualID)
-			}
-		})
-	}
-}
-
 func Test_containsMatchingBlockListEntry(t *testing.T) {
 	t.Parallel()
 	type args struct {