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 63c7152040e..75d1a1e833b 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" @@ -528,15 +529,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/storage/csimock/csi_kubelet_restart.go b/test/e2e/storage/csimock/csi_kubelet_restart.go index 9d6ffdb3512..105ac427640 100644 --- a/test/e2e/storage/csimock/csi_kubelet_restart.go +++ b/test/e2e/storage/csimock/csi_kubelet_restart.go @@ -19,27 +19,35 @@ package csimock import ( "context" "fmt" - "os" - "os/exec" - "strings" "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/feature" "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", feature.Kind, framework.WithSerial(), framework.WithDisruptive(), func() { +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, }) @@ -51,16 +59,16 @@ var _ = utils.SIGDescribe("CSI Mock when kubelet restart", feature.Kind, framewo 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") - err = stopKindKubelet(ctx) - framework.ExpectNoError(err, "failed to stop kubelet") - err = startKindKubelet(ctx) - framework.ExpectNoError(err, "failed to start 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{}) @@ -69,7 +77,7 @@ var _ = utils.SIGDescribe("CSI Mock when kubelet restart", feature.Kind, framewo 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) interface{} { + func(ctx context.Context) []drivers.MockCSICall { calls, err := m.driver.GetCalls(ctx) if err != nil { if apierrors.IsUnexpectedServerError(err) { @@ -90,39 +98,3 @@ var _ = utils.SIGDescribe("CSI Mock when kubelet restart", feature.Kind, framewo framework.ExpectNoError(err, "failed to wait for pod %s to be running", pod.Name) }) }) - -func stopKindKubelet(ctx context.Context) error { - return kubeletExec("systemctl", "stop", "kubelet") -} - -func startKindKubelet(ctx context.Context) error { - return kubeletExec("systemctl", "start", "kubelet") -} - -// Run a command in container with kubelet (and the whole control plane as containers) -func kubeletExec(command ...string) error { - containerName := getKindContainerName() - args := []string{"exec", containerName} - args = append(args, command...) - cmd := exec.Command("docker", args...) - - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("command %q failed: %v\noutput:%s", prettyCmd(cmd), err, string(out)) - } - - framework.Logf("command %q succeeded:\n%s", prettyCmd(cmd), string(out)) - return nil -} - -func getKindContainerName() string { - clusterName := os.Getenv("KIND_CLUSTER_NAME") - if clusterName == "" { - clusterName = "kind" - } - return clusterName + "-control-plane" -} - -func prettyCmd(cmd *exec.Cmd) string { - return fmt.Sprintf("%s %s", cmd.Path, strings.Join(cmd.Args, " ")) -}