Skip to content

Commit 6f532a7

Browse files
gouthampachagouthampacha
authored
[manila-csi-plugin] Seed fsName to ceph-csi's node plugin (#2994)
* Seed fsName to ceph-csi's node plugin This option is now mandatory to use when there are multiple CephFS file systems in the ceph cluster. Without this, ceph won't be able to find the (sub)volume to mount. * Refactor splitter code, add unit tests/doc comments Signed-off-by: Goutham Pacha Ravi <[email protected]> --------- Signed-off-by: Goutham Pacha Ravi <[email protected]>
1 parent be3818a commit 6f532a7

File tree

4 files changed

+316
-0
lines changed

4 files changed

+316
-0
lines changed

‎pkg/csi/manila/nodeserver.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ func (ns *nodeServer) buildVolumeContext(ctx context.Context, volID volumeID, sh
133133
sa := getShareAdapter(ns.d.shareProto)
134134
opts := &shareadapters.VolumeContextArgs{
135135
Locations: availableExportLocations,
136+
Share: share,
136137
Options: shareOpts,
137138
}
138139
volumeContext, err = sa.BuildVolumeContext(opts)

‎pkg/csi/manila/shareadapters/cephfs.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ package shareadapters
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223
"time"
2324

2425
"github.com/gophercloud/gophercloud/v2"
2526
"github.com/gophercloud/gophercloud/v2/openstack/sharedfilesystems/v2/shares"
2627
"k8s.io/apimachinery/pkg/util/wait"
2728
manilautil "k8s.io/cloud-provider-openstack/pkg/csi/manila/util"
29+
"k8s.io/cloud-provider-openstack/pkg/util"
2830
"k8s.io/klog/v2"
2931
)
3032

@@ -133,9 +135,44 @@ func (Cephfs) BuildVolumeContext(args *VolumeContextArgs) (volumeContext map[str
133135
volCtx["fuseMountOptions"] = args.Options.CephfsFuseMountOptions
134136
}
135137

138+
// Extract fs_name from __mount_options metadata if available
139+
// This is used by the ceph-csi plugin:
140+
// https://github.com/ceph/ceph-csi/blob/521a90c041acbe0fc68db8ecb27ef84da5af87dc/docs/static-pvc.md?plain=1#L287
141+
if fsName := extractFsNameFromMountOptions(args.Share); fsName != "" {
142+
volCtx["fsName"] = fsName
143+
klog.V(4).Infof("Found fs_name in share metadata: %s", fsName)
144+
}
145+
136146
return volCtx, err
137147
}
138148

149+
// extractFsNameFromMountOptions extracts the fs from __mount_options metadata
150+
// The __mount_options metadata contains mount options including fs for CephFS
151+
func extractFsNameFromMountOptions(share *shares.Share) string {
152+
if share == nil || share.Metadata == nil {
153+
return ""
154+
}
155+
156+
mountOptions, exists := share.Metadata["__mount_options"]
157+
if !exists {
158+
klog.V(4).Infof("No __mount_options metadata found in share %s", share.ID)
159+
return ""
160+
}
161+
162+
// Mount options are typically comma-separated key=value pairs
163+
// Example: "fs=myfs,other_option=value"
164+
options := util.SplitTrim(mountOptions, ',')
165+
for _, option := range options {
166+
if strings.HasPrefix(option, "fs=") {
167+
fsName := strings.TrimPrefix(option, "fs=")
168+
return fsName
169+
}
170+
}
171+
172+
klog.V(4).Infof("No fs found in __mount_options metadata for share %s: %s", share.ID, mountOptions)
173+
return ""
174+
}
175+
139176
func (Cephfs) BuildNodeStageSecret(args *SecretArgs) (secret map[string]string, err error) {
140177
return map[string]string{
141178
"userID": args.AccessRight.AccessTo,

‎pkg/csi/manila/shareadapters/cephfs_test.go‎

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
/*
2+
Licensed under the Apache License, Version 2.0 (the "License");
3+
you may not use this file except in compliance with the License.
4+
You may obtain a copy of the License at
5+
6+
http://www.apache.org/licenses/LICENSE-2.0
7+
8+
Unless required by applicable law or agreed to in writing, software
9+
distributed under the License is distributed on an "AS IS" BASIS,
10+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
See the License for the specific language governing permissions and
12+
limitations under the License.
13+
*/
14+
15+
package shareadapters
16+
17+
import (
18+
"testing"
19+
20+
"github.com/gophercloud/gophercloud/v2/openstack/sharedfilesystems/v2/shares"
21+
"k8s.io/cloud-provider-openstack/pkg/csi/manila/options"
22+
)
23+
24+
func TestExtractFsNameFromMountOptions(t *testing.T) {
25+
testCases := []struct {
26+
name string
27+
share *shares.Share
28+
expected string
29+
}{
30+
{
31+
name: "Valid fs in mount options",
32+
share: &shares.Share{
33+
ID: "test-share-1",
34+
Metadata: map[string]string{
35+
"__mount_options": "fs=my_cephfs,rw,relatime",
36+
},
37+
},
38+
expected: "my_cephfs",
39+
},
40+
{
41+
name: "fs with spaces around comma",
42+
share: &shares.Share{
43+
ID: "test-share-2",
44+
Metadata: map[string]string{
45+
"__mount_options": "rw, fs=production_fs , relatime",
46+
},
47+
},
48+
expected: "production_fs",
49+
},
50+
{
51+
name: "fs at the end",
52+
share: &shares.Share{
53+
ID: "test-share-4",
54+
Metadata: map[string]string{
55+
"__mount_options": "rw,relatime,fs=end_fs",
56+
},
57+
},
58+
expected: "end_fs",
59+
},
60+
{
61+
name: "fs with underscores and numbers",
62+
share: &shares.Share{
63+
ID: "test-share-5",
64+
Metadata: map[string]string{
65+
"__mount_options": "fs=cephfs_vol_123,rw",
66+
},
67+
},
68+
expected: "cephfs_vol_123",
69+
},
70+
{
71+
name: "fs with hyphens",
72+
share: &shares.Share{
73+
ID: "test-share-6",
74+
Metadata: map[string]string{
75+
"__mount_options": "fs=my-ceph-fs,rw,relatime",
76+
},
77+
},
78+
expected: "my-ceph-fs",
79+
},
80+
{
81+
name: "fs with dots",
82+
share: &shares.Share{
83+
ID: "test-share-7",
84+
Metadata: map[string]string{
85+
"__mount_options": "fs=ceph.filesystem.name,rw",
86+
},
87+
},
88+
expected: "ceph.filesystem.name",
89+
},
90+
{
91+
name: "fs with mixed whitespace",
92+
share: &shares.Share{
93+
ID: "test-share-8",
94+
Metadata: map[string]string{
95+
"__mount_options": " rw , fs=whitespace_test , relatime ",
96+
},
97+
},
98+
expected: "whitespace_test",
99+
},
100+
{
101+
name: "fs with empty value",
102+
share: &shares.Share{
103+
ID: "test-share-9",
104+
Metadata: map[string]string{
105+
"__mount_options": "rw,fs=,relatime",
106+
},
107+
},
108+
expected: "",
109+
},
110+
{
111+
name: "fs with complex filesystem name",
112+
share: &shares.Share{
113+
ID: "test-share-10",
114+
Metadata: map[string]string{
115+
"__mount_options": "fs=production_cluster_01.cephfs_vol,rw,relatime",
116+
},
117+
},
118+
expected: "production_cluster_01.cephfs_vol",
119+
},
120+
{
121+
name: "No fs in mount options",
122+
share: &shares.Share{
123+
ID: "test-share-11",
124+
Metadata: map[string]string{
125+
"__mount_options": "rw,relatime,noatime",
126+
},
127+
},
128+
expected: "",
129+
},
130+
{
131+
name: "No __mount_options metadata",
132+
share: &shares.Share{
133+
ID: "test-share-14",
134+
Metadata: map[string]string{
135+
"other_key": "other_value",
136+
},
137+
},
138+
expected: "",
139+
},
140+
{
141+
name: "Empty __mount_options",
142+
share: &shares.Share{
143+
ID: "test-share-15",
144+
Metadata: map[string]string{
145+
"__mount_options": "",
146+
},
147+
},
148+
expected: "",
149+
},
150+
{
151+
name: "Nil share",
152+
share: nil,
153+
expected: "",
154+
},
155+
{
156+
name: "Nil metadata",
157+
share: &shares.Share{
158+
ID: "test-share-18",
159+
Metadata: nil,
160+
},
161+
expected: "",
162+
},
163+
{
164+
name: "fs with special characters",
165+
share: &shares.Share{
166+
ID: "test-share-19",
167+
Metadata: map[string]string{
168+
"__mount_options": "fs=fs@cluster#1,rw",
169+
},
170+
},
171+
expected: "fs@cluster#1",
172+
},
173+
{
174+
name: "fs with equals in value",
175+
share: &shares.Share{
176+
ID: "test-share-20",
177+
Metadata: map[string]string{
178+
"__mount_options": "fs=fs=with=equals,rw",
179+
},
180+
},
181+
expected: "fs=with=equals",
182+
},
183+
}
184+
185+
for _, tc := range testCases {
186+
t.Run(tc.name, func(t *testing.T) {
187+
result := extractFsNameFromMountOptions(tc.share)
188+
if result != tc.expected {
189+
t.Errorf("Expected '%s', got '%s'", tc.expected, result)
190+
}
191+
})
192+
}
193+
}
194+
195+
func TestCephfsBuildVolumeContextWithFsName(t *testing.T) {
196+
adapter := &Cephfs{}
197+
198+
// Test share with fs in metadata
199+
shareWithFsName := &shares.Share{
200+
ID: "test-share-with-fsname",
201+
Metadata: map[string]string{
202+
"__mount_options": "fs=test_cephfs,rw,relatime",
203+
},
204+
}
205+
206+
// Test share without fs
207+
shareWithoutFsName := &shares.Share{
208+
ID: "test-share-without-fsname",
209+
Metadata: map[string]string{
210+
"other_metadata": "value",
211+
},
212+
}
213+
214+
exportLocations := []shares.ExportLocation{
215+
{
216+
Path: "10.0.0.1:6789,10.0.0.2:6789:/volumes/_nogroup/test-volume-id",
217+
},
218+
}
219+
220+
testCases := []struct {
221+
name string
222+
share *shares.Share
223+
expectFsName bool
224+
expectedFsName string
225+
}{
226+
{
227+
name: "Share with fs",
228+
share: shareWithFsName,
229+
expectFsName: true,
230+
expectedFsName: "test_cephfs",
231+
},
232+
{
233+
name: "Share without fs",
234+
share: shareWithoutFsName,
235+
expectFsName: false,
236+
},
237+
}
238+
239+
for _, tc := range testCases {
240+
t.Run(tc.name, func(t *testing.T) {
241+
args := &VolumeContextArgs{
242+
Locations: exportLocations,
243+
Share: tc.share,
244+
Options: &options.NodeVolumeContext{},
245+
}
246+
247+
volCtx, err := adapter.BuildVolumeContext(args)
248+
if err != nil {
249+
t.Errorf("BuildVolumeContext failed: %v", err)
250+
return
251+
}
252+
253+
fsName, exists := volCtx["fsName"]
254+
if tc.expectFsName {
255+
if !exists {
256+
t.Error("Expected fsName in volume context, but it was not found")
257+
} else if fsName != tc.expectedFsName {
258+
t.Errorf("Expected fsName '%s', got '%s'", tc.expectedFsName, fsName)
259+
}
260+
} else {
261+
if exists {
262+
t.Errorf("Did not expect fsName in volume context, but found '%s'", fsName)
263+
}
264+
}
265+
266+
// Verify other expected fields are still present
267+
expectedFields := []string{"monitors", "rootPath", "mounter", "provisionVolume"}
268+
for _, field := range expectedFields {
269+
if _, exists := volCtx[field]; !exists {
270+
t.Errorf("Expected field '%s' not found in volume context", field)
271+
}
272+
}
273+
})
274+
}
275+
}

‎pkg/csi/manila/shareadapters/shareadapter.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ type VolumeContextArgs struct {
3535
// an export location when building a volume context.
3636
Locations []shares.ExportLocation
3737

38+
// Share object containing metadata and other share information
39+
Share *shares.Share
40+
3841
Options *options.NodeVolumeContext
3942
}
4043

0 commit comments

Comments
 (0)