Skip to content

Commit c4690d9

Browse files
authored
No controller fallback for fstype
PublishLUN will not default to a filesystem type. Import cases will continue to work by inspecting the volume to determine fstype before formatting.
1 parent efe85f8 commit c4690d9

13 files changed

Lines changed: 199 additions & 16 deletions

File tree

.github/workflows/github-actions.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,19 @@ jobs:
9494
mkdir ${{ runner.temp }}\${{ runner.os }}-coverage-binary.out
9595
go test -v ./... -covermode=count -- -test.gocoverdir=${{ runner.temp }}\${{ runner.os }}-coverage-binary.out
9696
go tool covdata textfmt -i=${{ runner.temp }}\${{ runner.os }}-coverage-binary.out -o ${{ runner.os }}-coverage.out
97+
- if: runner.os == 'Linux'
98+
name: Free Disk Space (Ubuntu)
99+
uses: jlumbroso/free-disk-space@v1.3.1
100+
with:
101+
# this might remove tools that are actually needed,
102+
# if set to "true" but frees about 6 GB
103+
tool-cache: false
104+
android: true
105+
dotnet: true
106+
haskell: true
107+
large-packages: true
108+
docker-images: false
109+
swap-storage: true
97110
- if: runner.os != 'Windows'
98111
name: Run the tests (not on Windows)
99112
run: |

frontend/csi/controller_helpers/kubernetes/helper.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
package kubernetes
44

@@ -140,6 +140,9 @@ func (h *helper) GetVolumeConfig(
140140
Logc(ctx).WithError(err).Error("Invalid storage class parameters for LUKS volume import.")
141141
return nil, err
142142
}
143+
if getAnnotation(annotations, AnnFileSystem) == "" {
144+
return nil, fmt.Errorf("imported LUKS encrypted volume must provide file system type annotation")
145+
}
143146
}
144147
}
145148

@@ -862,6 +865,9 @@ func getVolumeConfig(
862865
if _, err = strconv.ParseBool(luksEncryption); err != nil {
863866
Logc(ctx).WithError(err).Warning("Unable to parse luks annotation into bool.")
864867
}
868+
if getAnnotation(annotations, AnnFileSystem) == "" {
869+
Logc(ctx).Warning("Imported LUKS encrypted volume must provide file system type annotation.")
870+
}
865871
}
866872
}
867873

frontend/csi/controller_server.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
package csi
44

@@ -408,6 +408,7 @@ func (p *Plugin) ControllerPublishVolume(
408408
case tridentconfig.Block:
409409
publishInfo["LUKSEncryption"] = volumePublishInfo.LUKSEncryption
410410
publishInfo["sharedTarget"] = strconv.FormatBool(volumePublishInfo.SharedTarget)
411+
publishInfo["volumeMode"] = string(volume.Config.VolumeMode)
411412

412413
if volumePublishInfo.SANType == sa.NVMe {
413414
// fill in only NVMe specific fields in publishInfo

frontend/csi/controller_server_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,7 @@ func TestControllerPublishVolume(t *testing.T) {
10731073
"SANType": sa.NVMe,
10741074
"sharedTarget": "false",
10751075
"nvmeTargetIPs": "",
1076+
"volumeMode": "",
10761077
},
10771078
},
10781079
expErrCode: codes.OK,
@@ -1110,6 +1111,7 @@ func TestControllerPublishVolume(t *testing.T) {
11101111
"SANType": sa.ISCSI,
11111112
"sharedTarget": "false",
11121113
"useCHAP": "false",
1114+
"volumeMode": "",
11131115
},
11141116
},
11151117
expErrCode: codes.OK,
@@ -1141,6 +1143,7 @@ func TestControllerPublishVolume(t *testing.T) {
11411143
"protocol": "block",
11421144
"sharedTarget": "false",
11431145
"useCHAP": "false",
1146+
"volumeMode": "",
11441147
},
11451148
},
11461149
expErrCode: codes.OK,

frontend/csi/node_server.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
package csi
44

@@ -1327,6 +1327,7 @@ func (p *Plugin) nodeStageFCPVolume(
13271327
publishInfo.FCPLunSerial = req.PublishContext["fcpLunSerial"]
13281328
publishInfo.FCPIgroup = req.PublishContext["fcpIgroup"]
13291329
publishInfo.SANType = req.PublishContext["SANType"]
1330+
publishInfo.VolumeMode = req.PublishContext["volumeMode"]
13301331

13311332
volumeId, stagingTargetPath, err := p.getVolumeIdAndStagingPath(req)
13321333
if err != nil {
@@ -1765,6 +1766,7 @@ func (p *Plugin) nodeStageISCSIVolume(
17651766
publishInfo.IscsiLunSerial = req.PublishContext["iscsiLunSerial"]
17661767
publishInfo.IscsiInterface = req.PublishContext["iscsiInterface"]
17671768
publishInfo.IscsiIgroup = req.PublishContext["iscsiIgroup"]
1769+
publishInfo.VolumeMode = req.PublishContext["volumeMode"]
17681770

17691771
if useCHAP {
17701772
publishInfo.IscsiUsername = req.PublishContext["iscsiUsername"]
@@ -2986,6 +2988,7 @@ func (p *Plugin) nodeStageNVMeVolume(
29862988
publishInfo.NVMeTargetIPs = strings.Split(req.PublishContext["nvmeTargetIPs"], ",")
29872989
publishInfo.SANType = req.PublishContext["SANType"]
29882990
publishInfo.FormatOptions = req.PublishContext["formatOptions"]
2991+
publishInfo.VolumeMode = req.PublishContext["volumeMode"]
29892992

29902993
err := p.nvmeHandler.AttachNVMeVolumeRetry(
29912994
ctx, req.VolumeContext["internalName"], "", publishInfo, req.GetSecrets(), nvme.NVMeAttachTimeout,

storage_drivers/ontap/ontap_common.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ func PublishLUN(
984984
}
985985

986986
// Prefer the LUN attribute fstype (authoritative, records what is on disk) over volConfig
987-
// to catch import mismatches. Fall back to volConfig, then driver default if absent.
987+
// to catch import mismatches. Fall back to volConfig.
988988
fstype := volConfig.FileSystem
989989
lunFstype, lunErr := clientAPI.LunGetFSType(ctx, lunPath)
990990
if lunErr != nil || lunFstype == "" {
@@ -995,14 +995,14 @@ func PublishLUN(
995995
Logc(ctx).WithFields(LogFields{
996996
"LUN": lunPath,
997997
"fstype": fstype,
998-
}).Error("LUN attribute fstype not found, using default.")
999-
fstype = drivers.DefaultFileSystemType
998+
}).Warning("LUN attribute fstype not found.")
1000999
}
10011000
// else: keep volConfig value ("raw", "ext4", "xfs", etc.) — LUN has no attribute
10021001
} else {
10031002
// LUN attribute is present; it takes precedence over volConfig.
10041003
fstype = lunFstype
10051004
}
1005+
publishInfo.VolumeMode = string(volConfig.VolumeMode)
10061006

10071007
// Get the format options
10081008
// An example of how formatOption may look like:

storage_drivers/ontap/ontap_common_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5929,7 +5929,7 @@ func TestPublishLun(t *testing.T) {
59295929
mockAPI.EXPECT().LunMapGetReportingNodes(ctx, igroupName, lunPath).Return([]string{"Node1"}, nil)
59305930
mockAPI.EXPECT().GetSLMDataLifs(ctx, ips, []string{"Node1"}).Return([]string{}, nil)
59315931

5932-
err := PublishLUN(ctx, mockAPI, config, ips, publishInfo, lunPath, igroupName, iSCSINodeName, volNoFsOrFmt)
5932+
err := PublishLUN(ctx, mockAPI, config, ips, publishInfo, lunPath, igroupName, iSCSINodeName, publishVolCfg("xfs", ""))
59335933

59345934
assert.NoError(t, err)
59355935

@@ -5942,7 +5942,7 @@ func TestPublishLun(t *testing.T) {
59425942

59435943
assert.Error(t, err)
59445944

5945-
// Test 3 - LunGetFSType returns error
5945+
// Test 3 - LunGetFSType returns error, fstype will fall back to fstype in vol config
59465946
mockAPI = mockapi.NewMockOntapAPI(mockCtrl)
59475947
publishInfo.HostIQN = []string{"host_iqn"}
59485948
mockAPI.EXPECT().LunGetFSType(ctx, lunPath).Return("", errors.New("LunGetFSType returned error"))
@@ -5956,7 +5956,7 @@ func TestPublishLun(t *testing.T) {
59565956
err = PublishLUN(ctx, mockAPI, config, ips, publishInfo, lunPath, igroupName, iSCSINodeName, volNoFsOrFmt)
59575957

59585958
assert.NoError(t, err)
5959-
assert.Equal(t, drivers.DefaultFileSystemType, publishInfo.FilesystemType)
5959+
assert.Equal(t, "", publishInfo.FilesystemType)
59605960

59615961
// Test 4 - LunGetAttribute returns error (fstype still from LUN)
59625962
mockAPI = mockapi.NewMockOntapAPI(mockCtrl)
@@ -6106,7 +6106,7 @@ func TestPublishLun(t *testing.T) {
61066106
assert.Equal(t, volFmtOpts, publishInfo.FormatOptions)
61076107
assert.Contains(t, publishInfo.MountOptions, "nouuid")
61086108

6109-
// Test 12 - volume fstype empty, LunGetFSType returns empty string with no error, use default
6109+
// Test 12 - volume fstype empty, LunGetFSType returns empty string with no error, expect empty
61106110
mockAPI = mockapi.NewMockOntapAPI(mockCtrl)
61116111
publishInfo = &tridentmodels.VolumePublishInfo{
61126112
BackendUUID: "fakeBackendUUID",
@@ -6127,9 +6127,9 @@ func TestPublishLun(t *testing.T) {
61276127
err = PublishLUN(ctx, mockAPI, config, ips, publishInfo, lunPath, igroupName, iSCSINodeName, volNoFsOrFmt)
61286128

61296129
assert.NoError(t, err)
6130-
assert.Equal(t, drivers.DefaultFileSystemType, publishInfo.FilesystemType)
6130+
assert.Equal(t, "", publishInfo.FilesystemType)
61316131

6132-
// Test 13 - volume fstype empty, LunGetFSType returns error, use default
6132+
// Test 13 - volume fstype empty, LunGetFSType returns error, will be empty
61336133
mockAPI = mockapi.NewMockOntapAPI(mockCtrl)
61346134
publishInfo = &tridentmodels.VolumePublishInfo{
61356135
BackendUUID: "fakeBackendUUID",
@@ -6150,7 +6150,7 @@ func TestPublishLun(t *testing.T) {
61506150
err = PublishLUN(ctx, mockAPI, config, ips, publishInfo, lunPath, igroupName, iSCSINodeName, volNoFsOrFmt)
61516151

61526152
assert.NoError(t, err)
6153-
assert.Equal(t, drivers.DefaultFileSystemType, publishInfo.FilesystemType)
6153+
assert.Equal(t, "", publishInfo.FilesystemType)
61546154

61556155
// Test 14 - volConfig "raw", LUN attribute returns "ext4": LUN attribute wins.
61566156
// Surfaces the real on-disk fstype so the node-side mismatch guard can fire.

utils/fcp/fcp.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
package fcp
44

@@ -507,6 +507,7 @@ func (client *Client) AttachVolume(
507507
devicePath = luksDevice.MappedDevicePath()
508508
}
509509

510+
// No filesystem work is required for raw block; return early.
510511
if publishInfo.FilesystemType == filesystem.Raw {
511512
return mpathSize, nil
512513
}
@@ -521,6 +522,17 @@ func (client *Client) AttachVolume(
521522
}
522523
}
523524

525+
fstype, err := filesystem.DetermineFSType(publishInfo.FilesystemType, existingFstype, publishInfo.VolumeMode)
526+
if err != nil {
527+
return mpathSize, fmt.Errorf("LUN %s, device %s is formatted with unknown filesystem type", name, devicePath)
528+
}
529+
publishInfo.FilesystemType = fstype
530+
531+
// No filesystem work is required for raw block; return early.
532+
if publishInfo.FilesystemType == filesystem.Raw {
533+
return mpathSize, nil
534+
}
535+
524536
if existingFstype == "" {
525537
if !isLUKSDevice {
526538
if unformatted, err := client.deviceClient.IsDeviceUnformatted(ctx, devicePath); err != nil {

utils/filesystem/utils.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
// Copyright 2025 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
package filesystem
44

55
import (
66
"fmt"
77
"regexp"
88
"strings"
9+
10+
"github.com/netapp/trident/config"
911
)
1012

1113
var permsRegex = regexp.MustCompile(`^[0-7]{4}$`)
@@ -27,3 +29,31 @@ func ValidateOctalUnixPermissions(perms string) error {
2729
}
2830
return nil
2931
}
32+
33+
// DetermineFSType resolves the effective filesystem type to use for a volume.
34+
// Priority order:
35+
// 1. publishInfoType — use it as-is when provided (explicit caller intent).
36+
// 2. existingType — adopt the type already on disk when it is known (non-empty,
37+
// non-UnknownFstype), so that subsequent format/mount logic stays consistent.
38+
// 3. volumeMode fallback — when no usable on-disk type is available, default to
39+
// ext4 for Filesystem volumes or raw for Block volumes. A Filesystem volume
40+
// whose on-disk type is UnknownFstype still defaults to ext4 but also returns
41+
// an error because the LUN's existing format is unrecognisable.
42+
func DetermineFSType(publishInfoType, existingType, volumeMode string) (string, error) {
43+
fsType := publishInfoType
44+
if publishInfoType == "" {
45+
switch {
46+
case existingType != "" && existingType != UnknownFstype:
47+
// Adopt whatever is on disk so subsequent format/mount logic uses it.
48+
fsType = existingType
49+
case volumeMode == string(config.Filesystem):
50+
if existingType == UnknownFstype {
51+
return "", fmt.Errorf("LUN is formatted with unknown filesystem type")
52+
}
53+
fsType = Ext4
54+
case volumeMode == string(config.RawBlock):
55+
fsType = Raw
56+
}
57+
}
58+
return fsType, nil
59+
}

utils/filesystem/utils_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"testing"
77

88
"github.com/stretchr/testify/assert"
9+
10+
"github.com/netapp/trident/config"
911
)
1012

1113
func TestVerifyFilesystemSupport(t *testing.T) {
@@ -66,3 +68,92 @@ func TestValidateOctalUnixPermissions(t *testing.T) {
6668
assert.Equal(t, test.errNotNil, err != nil)
6769
}
6870
}
71+
72+
func TestDetermineFSType(t *testing.T) {
73+
fsVolumeMode := string(config.Filesystem)
74+
blockVolumeMode := string(config.RawBlock)
75+
76+
tests := []struct {
77+
name string
78+
publishInfoType string
79+
existingType string
80+
volumeMode string
81+
expectedFSType string
82+
errNotNil bool
83+
}{
84+
// publishInfoType takes precedence regardless of other inputs.
85+
{
86+
name: "publishInfoType provided — returned directly",
87+
publishInfoType: Xfs,
88+
existingType: "",
89+
volumeMode: fsVolumeMode,
90+
expectedFSType: Xfs,
91+
},
92+
{
93+
name: "publishInfoType overrides existing on-disk type",
94+
publishInfoType: Ext4,
95+
existingType: Xfs,
96+
volumeMode: fsVolumeMode,
97+
expectedFSType: Ext4,
98+
},
99+
100+
// No publishInfoType — adopt the known on-disk type.
101+
{
102+
name: "existing xfs adopted when publishInfoType absent",
103+
existingType: Xfs,
104+
volumeMode: fsVolumeMode,
105+
expectedFSType: Xfs,
106+
},
107+
{
108+
name: "existing ext3 adopted when publishInfoType absent",
109+
existingType: Ext3,
110+
volumeMode: fsVolumeMode,
111+
expectedFSType: Ext3,
112+
},
113+
114+
// No publishInfoType, no usable existing type — fall back by volumeMode.
115+
{
116+
name: "filesystem volume with empty existing type defaults to ext4",
117+
existingType: "",
118+
volumeMode: fsVolumeMode,
119+
expectedFSType: Ext4,
120+
},
121+
{
122+
name: "raw-block volume with empty existing type defaults to raw",
123+
existingType: "",
124+
volumeMode: blockVolumeMode,
125+
expectedFSType: Raw,
126+
},
127+
{
128+
name: "raw-block volume with unknown existing type defaults to raw",
129+
existingType: UnknownFstype,
130+
volumeMode: blockVolumeMode,
131+
expectedFSType: Raw,
132+
},
133+
134+
// Filesystem volume with an unknown on-disk type: default to ext4 but surface an error.
135+
{
136+
name: "filesystem volume with unknown existing type returns ext4 and error",
137+
existingType: UnknownFstype,
138+
volumeMode: fsVolumeMode,
139+
expectedFSType: "",
140+
errNotNil: true,
141+
},
142+
143+
// No publishInfoType, no matching volumeMode — nothing to fall back to.
144+
{
145+
name: "empty inputs yield empty fsType and no error",
146+
existingType: "",
147+
volumeMode: "",
148+
expectedFSType: "",
149+
},
150+
}
151+
152+
for _, tt := range tests {
153+
t.Run(tt.name, func(t *testing.T) {
154+
fsType, err := DetermineFSType(tt.publishInfoType, tt.existingType, tt.volumeMode)
155+
assert.Equal(t, tt.expectedFSType, fsType)
156+
assert.Equal(t, tt.errNotNil, err != nil)
157+
})
158+
}
159+
}

0 commit comments

Comments
 (0)