From c94c28c183b93cd7036a0714dea34073d8a14930 Mon Sep 17 00:00:00 2001 From: Anish Ramasekar Date: Wed, 13 May 2026 11:49:54 -0700 Subject: [PATCH 1/2] feat(volume): add IsRemount to MounterArgs Thread the reconciler's existing isRemount signal into MounterArgs so volume plugins can distinguish an initial publish from a republish (e.g. CSIDriver.spec.requiresRepublish=true). No behavior change. --- .../util/operationexecutor/operation_generator.go | 1 + pkg/volume/volume.go | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 6e21109451e..8fb37ad1c9f 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -588,6 +588,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc( Recorder: og.recorder, SELinuxLabel: volumeToMount.SELinuxLabel, ReconstructedVolume: actualStateOfWorld.IsVolumeReconstructed(volumeToMount.VolumeName, volumeToMount.PodName), + IsRemount: isRemount, }) // Update actual state of world markOpts := MarkVolumeOpts{ diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 152c7bc68ef..4a166f000f2 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -139,6 +139,16 @@ type MounterArgs struct { // mainly used by unit tests VolumeOwnershipApplicator VolumeOwnershipChanger ReconstructedVolume bool + + // IsRemount is true when SetUp is being invoked on a volume that the + // reconciler considers already mounted to the pod, e.g. a periodic + // republish triggered by CSIDriver.spec.requiresRepublish=true. Volume + // plugins should use this to avoid destroying state (e.g. mount + // directories, volume metadata files) that the pod is currently + // observing through an existing bind mount, since teardown on a + // failed remount cannot be repaired by a subsequent successful + // remount and would leave the pod with stale contents. + IsRemount bool } type VolumeOwnershipChanger interface { From 12a654c8b02ec38c6077fe9efe6e28c5e9adbe2f Mon Sep 17 00:00:00 2001 From: Anish Ramasekar Date: Wed, 13 May 2026 11:55:18 -0700 Subject: [PATCH 2/2] fix(csi): preserve mount dir on NodePublish error during remount 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. --- pkg/volume/csi/csi_mounter.go | 6 +++- pkg/volume/csi/csi_mounter_test.go | 54 +++++++++++++++++++++--------- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go index 83af5dc0ecf..f915179e786 100644 --- a/pkg/volume/csi/csi_mounter.go +++ b/pkg/volume/csi/csi_mounter.go @@ -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)) } diff --git a/pkg/volume/csi/csi_mounter_test.go b/pkg/volume/csi/csi_mounter_test.go index 3fbe9361f34..8af454a1bbc 100644 --- a/pkg/volume/csi/csi_mounter_test.go +++ b/pkg/volume/csi/csi_mounter_test.go @@ -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) } }) }