diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index fe1ce5ac715..ba04fa60f0c 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -24,6 +24,7 @@ import ( "context" "errors" "fmt" + "slices" "sync" "time" @@ -564,15 +565,21 @@ func (dswp *desiredStateOfWorldPopulator) getPVCExtractPV( return nil, fmt.Errorf("failed to fetch PVC from API server: %v", err) } - // Pods that uses a PVC that is being deleted must not be started. + // Pods that uses a PVC that is being deleted and not protected by + // kubernetes.io/pvc-protection must not be started. // - // In case an old kubelet is running without this check or some kubelets - // have this feature disabled, the worst that can happen is that such - // pod is scheduled. This was the default behavior in 1.8 and earlier - // and users should not be that surprised. + // 1) In case an old kubelet is running without this check, the worst + // that can happen is that such pod is scheduled. This was the default + // behavior in 1.8 and earlier and users should not be that surprised. // It should happen only in very rare case when scheduler schedules // a pod and user deletes a PVC that's used by it at the same time. - if pvc.ObjectMeta.DeletionTimestamp != nil { + // + // 2) Adding a check for kubernetes.io/pvc-protection here to prevent + // the existing running pods from being affected during the rebuild of + // the desired state of the world cache when the kubelet is restarted. + // It is safe for kubelet to add this check here because the PVC will + // be stuck in Terminating state until the pod is deleted. + if pvc.ObjectMeta.DeletionTimestamp != nil && !slices.Contains(pvc.Finalizers, util.PVCProtectionFinalizer) { return nil, errors.New("PVC is being deleted") } diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 6c2d342e736..bb4c48717e2 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -64,7 +64,7 @@ import ( _ "k8s.io/kubernetes/test/e2e/node" _ "k8s.io/kubernetes/test/e2e/scheduling" _ "k8s.io/kubernetes/test/e2e/storage" - _ "k8s.io/kubernetes/test/e2e/storage/csi_mock" + _ "k8s.io/kubernetes/test/e2e/storage/csimock" _ "k8s.io/kubernetes/test/e2e/storage/external" _ "k8s.io/kubernetes/test/e2e/windows" diff --git a/test/e2e/storage/csi_mock/base.go b/test/e2e/storage/csimock/base.go similarity index 99% rename from test/e2e/storage/csi_mock/base.go rename to test/e2e/storage/csimock/base.go index c145bb0d284..423a3fdfce4 100644 --- a/test/e2e/storage/csi_mock/base.go +++ b/test/e2e/storage/csimock/base.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_attach_volume.go b/test/e2e/storage/csimock/csi_attach_volume.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_attach_volume.go rename to test/e2e/storage/csimock/csi_attach_volume.go index 9b263ea6cee..e1107f0dd21 100644 --- a/test/e2e/storage/csi_mock/csi_attach_volume.go +++ b/test/e2e/storage/csimock/csi_attach_volume.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_fsgroup_mount.go b/test/e2e/storage/csimock/csi_fsgroup_mount.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_fsgroup_mount.go rename to test/e2e/storage/csimock/csi_fsgroup_mount.go index 947ef88fa05..ca120b1f4be 100644 --- a/test/e2e/storage/csi_mock/csi_fsgroup_mount.go +++ b/test/e2e/storage/csimock/csi_fsgroup_mount.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_fsgroup_policy.go b/test/e2e/storage/csimock/csi_fsgroup_policy.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_fsgroup_policy.go rename to test/e2e/storage/csimock/csi_fsgroup_policy.go index 7a635deb6fb..14dab2c21f0 100644 --- a/test/e2e/storage/csi_mock/csi_fsgroup_policy.go +++ b/test/e2e/storage/csimock/csi_fsgroup_policy.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csimock/csi_kubelet_restart.go b/test/e2e/storage/csimock/csi_kubelet_restart.go new file mode 100644 index 00000000000..105ac427640 --- /dev/null +++ b/test/e2e/storage/csimock/csi_kubelet_restart.go @@ -0,0 +1,100 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package csimock + +import ( + "context" + "fmt" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/kubernetes/test/e2e/framework" + e2epod "k8s.io/kubernetes/test/e2e/framework/pod" + e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" + "k8s.io/kubernetes/test/e2e/storage/drivers" + "k8s.io/kubernetes/test/e2e/storage/utils" + admissionapi "k8s.io/pod-security-admission/api" +) + +var _ = utils.SIGDescribe("CSI Mock when kubelet restart", framework.WithSerial(), framework.WithDisruptive(), func() { + f := framework.NewDefaultFramework("csi-mock-when-kubelet-restart") + f.NamespacePodSecurityLevel = admissionapi.LevelPrivileged + m := newMockDriverSetup(f) + + ginkgo.BeforeEach(func() { + // These tests requires SSH to nodes, so the provider check should be identical to there + // (the limiting factor is the implementation of util.go's e2essh.GetSigner(...)). + + // Cluster must support node reboot + e2eskipper.SkipUnlessProviderIs(framework.ProvidersWithSSH...) + e2eskipper.SkipUnlessSSHKeyPresent() + }) + + ginkgo.It("should not umount volume when the pvc is terminating but still used by a running pod", func(ctx context.Context) { + + m.init(ctx, testParameters{ + registerDriver: true, + }) + ginkgo.DeferCleanup(m.cleanup) + + ginkgo.By("Creating a Pod with a PVC backed by a CSI volume") + _, pvc, pod := m.createPod(ctx, pvcReference) + + ginkgo.By("Waiting for the Pod to be running") + err := e2epod.WaitForPodRunningInNamespace(ctx, f.ClientSet, pod) + framework.ExpectNoError(err, "failed to wait for pod %s to be running", pod.Name) + pod, err = f.ClientSet.CoreV1().Pods(pod.Namespace).Get(ctx, pod.Name, metav1.GetOptions{}) + framework.ExpectNoError(err, "failed to get pod %s", pod.Name) + + ginkgo.By("Deleting the PVC") + err = f.ClientSet.CoreV1().PersistentVolumeClaims(pvc.Namespace).Delete(ctx, pvc.Name, metav1.DeleteOptions{}) + framework.ExpectNoError(err, "failed to delete PVC %s", pvc.Name) + + ginkgo.By("Restarting kubelet") + utils.KubeletCommand(ctx, utils.KRestart, f.ClientSet, pod) + ginkgo.DeferCleanup(utils.KubeletCommand, utils.KStart, f.ClientSet, pod) + + ginkgo.By("Verifying the PVC is terminating during kubelet restart") + pvc, err = f.ClientSet.CoreV1().PersistentVolumeClaims(pvc.Namespace).Get(ctx, pvc.Name, metav1.GetOptions{}) + framework.ExpectNoError(err, "failed to get PVC %s", pvc.Name) + gomega.Expect(pvc.DeletionTimestamp).NotTo(gomega.BeNil(), "PVC %s should have deletion timestamp", pvc.Name) + + ginkgo.By(fmt.Sprintf("Verifying that the driver didn't receive NodeUnpublishVolume call for PVC %s", pvc.Name)) + gomega.Consistently(ctx, + func(ctx context.Context) []drivers.MockCSICall { + calls, err := m.driver.GetCalls(ctx) + if err != nil { + if apierrors.IsUnexpectedServerError(err) { + // kubelet might not be ready yet when getting the calls + gomega.TryAgainAfter(framework.Poll).Wrap(err).Now() + return nil + } + return nil + } + return calls + }). + WithPolling(framework.Poll). + WithTimeout(framework.ClaimProvisionShortTimeout). + ShouldNot(gomega.ContainElement(gomega.HaveField("Method", gomega.Equal("NodeUnpublishVolume")))) + + ginkgo.By("Verifying the Pod is still running") + err = e2epod.WaitForPodRunningInNamespace(ctx, f.ClientSet, pod) + framework.ExpectNoError(err, "failed to wait for pod %s to be running", pod.Name) + }) +}) diff --git a/test/e2e/storage/csi_mock/csi_node_stage_error_cases.go b/test/e2e/storage/csimock/csi_node_stage_error_cases.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_node_stage_error_cases.go rename to test/e2e/storage/csimock/csi_node_stage_error_cases.go index fbfe6cc2b2b..daa621be407 100644 --- a/test/e2e/storage/csi_mock/csi_node_stage_error_cases.go +++ b/test/e2e/storage/csimock/csi_node_stage_error_cases.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_selinux_mount.go b/test/e2e/storage/csimock/csi_selinux_mount.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_selinux_mount.go rename to test/e2e/storage/csimock/csi_selinux_mount.go index f20b2a6df2f..c1554014f2d 100644 --- a/test/e2e/storage/csi_mock/csi_selinux_mount.go +++ b/test/e2e/storage/csimock/csi_selinux_mount.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_service_account_token.go b/test/e2e/storage/csimock/csi_service_account_token.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_service_account_token.go rename to test/e2e/storage/csimock/csi_service_account_token.go index fcd647575c2..e8b06f9a093 100644 --- a/test/e2e/storage/csi_mock/csi_service_account_token.go +++ b/test/e2e/storage/csimock/csi_service_account_token.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_snapshot.go b/test/e2e/storage/csimock/csi_snapshot.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_snapshot.go rename to test/e2e/storage/csimock/csi_snapshot.go index 60c3f4589c3..377b53245a1 100644 --- a/test/e2e/storage/csi_mock/csi_snapshot.go +++ b/test/e2e/storage/csimock/csi_snapshot.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_storage_capacity.go b/test/e2e/storage/csimock/csi_storage_capacity.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_storage_capacity.go rename to test/e2e/storage/csimock/csi_storage_capacity.go index b2cb03668b1..adb350c405c 100644 --- a/test/e2e/storage/csi_mock/csi_storage_capacity.go +++ b/test/e2e/storage/csimock/csi_storage_capacity.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_volume_expansion.go b/test/e2e/storage/csimock/csi_volume_expansion.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_volume_expansion.go rename to test/e2e/storage/csimock/csi_volume_expansion.go index 24e135af51b..495bfb0464d 100644 --- a/test/e2e/storage/csi_mock/csi_volume_expansion.go +++ b/test/e2e/storage/csimock/csi_volume_expansion.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_volume_limit.go b/test/e2e/storage/csimock/csi_volume_limit.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_volume_limit.go rename to test/e2e/storage/csimock/csi_volume_limit.go index b3799d5af3c..be8db44415f 100644 --- a/test/e2e/storage/csi_mock/csi_volume_limit.go +++ b/test/e2e/storage/csimock/csi_volume_limit.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context" diff --git a/test/e2e/storage/csi_mock/csi_workload.go b/test/e2e/storage/csimock/csi_workload.go similarity index 99% rename from test/e2e/storage/csi_mock/csi_workload.go rename to test/e2e/storage/csimock/csi_workload.go index 5ad053c861b..265e38a1c7b 100644 --- a/test/e2e/storage/csi_mock/csi_workload.go +++ b/test/e2e/storage/csimock/csi_workload.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package csi_mock +package csimock import ( "context"