github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2026-07-18 04:54:54 +00:00
Code Issues Projects Releases Wiki Activity

fix(csi): preserve mount dir on NodePublish error during remount

Browse Source 
On a remount (e.g. CSIDriver.spec.requiresRepublish=true), the volume is
already published and the pod is observing the existing bind mount.
Removing the mount dir on a NodePublish error left the pod with stale
contents that subsequent successful republishes could not repair.
This commit is contained in:
Anish Ramasekar
2026-05-13 11:55:18 -07:00
parent 238883be8d
commit 6c684db7a3
2 changed files with 43 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/volume/csi/csi_mounter.go
View File

@@ -315,7 +315,11 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error
if csiRPCError != nil {
// If operation finished with error then we can remove the mount directory.
if volumetypes.IsOperationFinishedError(csiRPCError) && !mounterArgs.ReconstructedVolume {
// Skip on remount (e.g. CSIDriver.spec.requiresRepublish=true): the volume
// was already published and the pod is observing the existing bind mount,
// so removing the mount dir here would leave the pod with stale contents
// that a subsequent successful republish cannot repair (#121271).
if volumetypes.IsOperationFinishedError(csiRPCError) && !mounterArgs.ReconstructedVolume && !mounterArgs.IsRemount {
if removeMountDirErr := removeMountDir(c.plugin, dir); removeMountDirErr != nil {
klog.Error(log("mounter.SetupAt failed to remove mount dir after a NodePublish() error [%s]: %v", dir, removeMountDirErr))
}

54
pkg/volume/csi/csi_mounter_test.go
View File

@@ -1177,11 +1177,13 @@ func TestMounterSetUpWithFSGroup(t *testing.T) {
func TestMounterSetUpFWithNodePublishFinalError(t *testing.T) {
testCases := []struct {
name string
podUID types.UID
options []string
spec func(string, []string) *volume.Spec
reconstructedVolume bool
name string
podUID types.UID
options []string
spec func(string, []string) *volume.Spec
reconstructedVolume bool
isRemount bool
expectDataFileExists bool
}{
{
name: "setup with reconstructed volume",
@@ -1192,7 +1194,8 @@ func TestMounterSetUpFWithNodePublishFinalError(t *testing.T) {
pvSrc.Spec.MountOptions = options
return volume.NewSpecFromPersistentVolume(pvSrc, false)
},
reconstructedVolume: true,
reconstructedVolume: true,
expectDataFileExists: true,
},
{
name: "setup with new volume",
@@ -1203,7 +1206,27 @@ func TestMounterSetUpFWithNodePublishFinalError(t *testing.T) {
pvSrc.Spec.MountOptions = options
return volume.NewSpecFromPersistentVolume(pvSrc, false)
},
reconstructedVolume: false,
reconstructedVolume: false,
expectDataFileExists: false,
},
{
// Regression test for https://github.com/kubernetes/kubernetes/issues/121271:
// when NodePublishVolume fails on a remount (e.g. a republish
// triggered by CSIDriver.spec.requiresRepublish=true), the
// mount directory and vol_data.json must be preserved so the
// pod continues to see the previously-published contents and
// a subsequent successful republish can refresh them in place.
name: "setup with remount preserves mount dir on final error",
podUID: types.UID(fmt.Sprintf("%08X", rand.Uint64())),
spec: func(fsType string, options []string) *volume.Spec {
pvSrc := makeTestPV("pv1", 20, testDriver, "vol1")
pvSrc.Spec.CSI.FSType = fsType
pvSrc.Spec.MountOptions = options
return volume.NewSpecFromPersistentVolume(pvSrc, false)
},
reconstructedVolume: false,
isRemount: true,
expectDataFileExists: true,
},
}
@@ -1240,21 +1263,20 @@ func TestMounterSetUpFWithNodePublishFinalError(t *testing.T) {
csiMounter.csiClient.(*fakeCsiDriverClient).nodeClient.SetNextError(status.Errorf(codes.InvalidArgument, "mount failed"))
// Mounter.SetUp()
if err := csiMounter.SetUp(volume.MounterArgs{ReconstructedVolume: tc.reconstructedVolume}); err == nil {
if err := csiMounter.SetUp(volume.MounterArgs{
ReconstructedVolume: tc.reconstructedVolume,
IsRemount: tc.isRemount,
}); err == nil {
t.Fatalf("mounter.Setup expected err but succeed")
}
mountPath := csiMounter.GetPath()
volPath := filepath.Dir(mountPath)
dataFile := filepath.Join(volPath, volDataFileName)
if tc.reconstructedVolume {
if _, err := os.Stat(dataFile); os.IsNotExist(err) {
t.Errorf("volume file [%s] expects to be exists, but removed", dataFile)
}
return
}
if _, err := os.Stat(dataFile); err == nil {
t.Errorf("volume file [%s] expects to be removed, but exists", dataFile)
_, statErr := os.Stat(dataFile)
exists := statErr == nil
if exists != tc.expectDataFileExists {
t.Errorf("volume file [%s]: exists=%v, want=%v (statErr=%v)", dataFile, exists, tc.expectDataFileExists, statErr)
}
})
}