Skip to content

Commit b7cf453

Browse files
committed
Support efc default arguments for bmcpfs
1 parent ef8b4e7 commit b7cf453

2 files changed

Lines changed: 180 additions & 0 deletions

File tree

pkg/bmcpfs/nodeserver.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"errors"
2525
"fmt"
2626
"os"
27+
"strings"
2728

2829
"github.com/container-storage-interface/spec/lib/go/csi"
2930
"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/cloud/metadata"
@@ -134,6 +135,11 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
134135
}
135136
klog.InfoS("Mounting mount target", "targetPath", req.TargetPath, "source", source)
136137

138+
// Default g_lease_Enable=false unless user explicitly specified it
139+
if !hasMountOption(mountOptions, "g_lease_Enable") {
140+
mountOptions = append(mountOptions, "g_lease_Enable=false")
141+
}
142+
137143
mountOptions = append(mountOptions, "efc,protocol=efc,fstype=cpfs")
138144
err = ns.mounter.Mount(source, req.TargetPath, "alinas", mountOptions)
139145
if err != nil {
@@ -159,3 +165,14 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu
159165
klog.InfoS("NodeUnpublishVolume: succeeded to umount", "targetPath", req.TargetPath)
160166
return &csi.NodeUnpublishVolumeResponse{}, nil
161167
}
168+
169+
// hasMountOption checks whether any option in the slice has the given key prefix (e.g. "g_lease_Enable").
170+
func hasMountOption(options []string, key string) bool {
171+
prefix := key + "="
172+
for _, opt := range options {
173+
if opt == key || strings.HasPrefix(opt, prefix) {
174+
return true
175+
}
176+
}
177+
return false
178+
}

pkg/bmcpfs/nodeserver_test.go

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
//go:build !windows
2+
3+
/*
4+
Copyright 2019 The Kubernetes Authors.
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
*/
18+
19+
package bmcpfs
20+
21+
import (
22+
"context"
23+
"testing"
24+
25+
"github.com/container-storage-interface/spec/lib/go/csi"
26+
"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/utils"
27+
"github.com/stretchr/testify/assert"
28+
k8smount "k8s.io/mount-utils"
29+
)
30+
31+
func TestHasMountOption(t *testing.T) {
32+
tests := []struct {
33+
name string
34+
options []string
35+
key string
36+
expected bool
37+
}{
38+
{
39+
name: "empty options",
40+
options: nil,
41+
key: "g_lease_Enable",
42+
expected: false,
43+
},
44+
{
45+
name: "key not present",
46+
options: []string{"net=tcp", "efc"},
47+
key: "g_lease_Enable",
48+
expected: false,
49+
},
50+
{
51+
name: "key present with value false",
52+
options: []string{"net=tcp", "g_lease_Enable=false"},
53+
key: "g_lease_Enable",
54+
expected: true,
55+
},
56+
{
57+
name: "key present with value true",
58+
options: []string{"g_lease_Enable=true", "efc"},
59+
key: "g_lease_Enable",
60+
expected: true,
61+
},
62+
{
63+
name: "key present as bare key (no value)",
64+
options: []string{"g_lease_Enable"},
65+
key: "g_lease_Enable",
66+
expected: true,
67+
},
68+
{
69+
name: "similar prefix but different key",
70+
options: []string{"g_lease_EnableX=true"},
71+
key: "g_lease_Enable",
72+
expected: false,
73+
},
74+
{
75+
name: "key is a substring of an option without = separator",
76+
options: []string{"g_lease_Enable_extra"},
77+
key: "g_lease_Enable",
78+
expected: false,
79+
},
80+
}
81+
82+
for _, tt := range tests {
83+
t.Run(tt.name, func(t *testing.T) {
84+
got := hasMountOption(tt.options, tt.key)
85+
assert.Equal(t, tt.expected, got)
86+
})
87+
}
88+
}
89+
90+
func TestNodePublishVolume_GLeaseEnable(t *testing.T) {
91+
tests := []struct {
92+
name string
93+
mountFlags []string
94+
expectLeaseInMount bool // whether g_lease_Enable=false appears in mount args
95+
}{
96+
{
97+
name: "default: g_lease_Enable=false injected",
98+
mountFlags: nil,
99+
expectLeaseInMount: true,
100+
},
101+
{
102+
name: "user specified g_lease_Enable=true, not injected",
103+
mountFlags: []string{"g_lease_Enable=true"},
104+
expectLeaseInMount: false,
105+
},
106+
{
107+
name: "user specified g_lease_Enable=false explicitly",
108+
mountFlags: []string{"g_lease_Enable=false"},
109+
expectLeaseInMount: false, // already present, we don't double-add
110+
},
111+
}
112+
113+
for _, tt := range tests {
114+
t.Run(tt.name, func(t *testing.T) {
115+
targetPath := t.TempDir() + "/target"
116+
mounter := k8smount.NewFakeMounter(nil)
117+
ns := &nodeServer{
118+
mounter: mounter,
119+
locks: utils.NewVolumeLocks(),
120+
}
121+
122+
req := &csi.NodePublishVolumeRequest{
123+
VolumeId: "vol-test",
124+
TargetPath: targetPath,
125+
VolumeCapability: &csi.VolumeCapability{
126+
AccessType: &csi.VolumeCapability_Mount{
127+
Mount: &csi.VolumeCapability_MountVolume{
128+
MountFlags: tt.mountFlags,
129+
},
130+
},
131+
},
132+
PublishContext: map[string]string{
133+
_networkType: networkTypeVPC,
134+
_vpcMountTarget: "10.0.0.1:/",
135+
},
136+
VolumeContext: map[string]string{},
137+
}
138+
139+
resp, err := ns.NodePublishVolume(context.Background(), req)
140+
assert.NoError(t, err)
141+
assert.NotNil(t, resp)
142+
143+
// Verify mount was called with the expected options via MountPoints
144+
assert.Len(t, mounter.MountPoints, 1)
145+
opts := mounter.MountPoints[0].Opts
146+
147+
if tt.expectLeaseInMount {
148+
assert.Contains(t, opts, "g_lease_Enable=false",
149+
"expected g_lease_Enable=false to be injected, got opts: %v", opts)
150+
} else {
151+
// When user already specified it, we should not find a duplicate
152+
count := 0
153+
for _, opt := range opts {
154+
if opt == "g_lease_Enable=false" || opt == "g_lease_Enable=true" {
155+
count++
156+
}
157+
}
158+
assert.Equal(t, 1, count,
159+
"expected exactly one g_lease_Enable option, got opts: %v", opts)
160+
}
161+
})
162+
}
163+
}

0 commit comments

Comments
 (0)