From 50fec6fc577ababc85a2fe3077ed3c807f224523 Mon Sep 17 00:00:00 2001 From: huweiwen Date: Sun, 17 Dec 2023 21:28:20 +0800 Subject: [PATCH] kubelet/volumeManager: (Get -> Has)PossiblyMountedVolumesForPod This is intended to remove dependency on mountedVolume.OuterVolumeSpecNames --- pkg/kubelet/kubelet_test.go | 12 +++------- pkg/kubelet/kubelet_volumes.go | 4 +--- pkg/kubelet/kubelet_volumes_test.go | 17 +++++++------- pkg/kubelet/volumemanager/volume_manager.go | 23 +++++-------------- .../volumemanager/volume_manager_fake.go | 6 ++--- 5 files changed, 21 insertions(+), 41 deletions(-) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 0c1b4ecc224..815fea5cd88 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -3049,25 +3049,19 @@ func TestSyncLabels(t *testing.T) { func waitForVolumeUnmount( volumeManager kubeletvolume.VolumeManager, pod *v1.Pod) error { - var podVolumes kubecontainer.VolumeMap err := retryWithExponentialBackOff( time.Duration(50*time.Millisecond), func() (bool, error) { // Verify volumes detached - podVolumes = volumeManager.GetPossiblyMountedVolumesForPod( + hasVolumes := volumeManager.HasPossiblyMountedVolumesForPod( util.GetUniquePodName(pod)) - - if len(podVolumes) != 0 { - return false, nil - } - - return true, nil + return !hasVolumes, nil }, ) if err != nil { return fmt.Errorf( - "Expected volumes to be unmounted. But some volumes are still mounted: %#v", podVolumes) + "Expected volumes to be unmounted. But some volumes are still mounted") } return nil diff --git a/pkg/kubelet/kubelet_volumes.go b/pkg/kubelet/kubelet_volumes.go index 16a9daaddc7..40320dcdf55 100644 --- a/pkg/kubelet/kubelet_volumes.go +++ b/pkg/kubelet/kubelet_volumes.go @@ -76,9 +76,7 @@ func (kl *Kubelet) ListBlockVolumesForPod(podUID types.UID) (map[string]volume.B // podVolumesExist checks with the volume manager and returns true any of the // pods for the specified volume are mounted or are uncertain. func (kl *Kubelet) podVolumesExist(podUID types.UID) bool { - if mountedVolumes := - kl.volumeManager.GetPossiblyMountedVolumesForPod( - volumetypes.UniquePodName(podUID)); len(mountedVolumes) > 0 { + if kl.volumeManager.HasPossiblyMountedVolumesForPod(volumetypes.UniquePodName(podUID)) { return true } // TODO: This checks pod volume paths and whether they are mounted. If checking returns error, podVolumesExist will return true diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index 04eda105349..4f8bf7c45ef 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -472,11 +472,10 @@ func TestVolumeUnmountAndDetachControllerDisabled(t *testing.T) { assert.NoError(t, kubelet.volumeManager.WaitForUnmount(context.Background(), pod)) // Verify volumes unmounted - podVolumes = kubelet.volumeManager.GetPossiblyMountedVolumesForPod( + hasMountedVolumes := kubelet.volumeManager.HasPossiblyMountedVolumesForPod( util.GetUniquePodName(pod)) - assert.Empty(t, podVolumes, - "Expected volumes to be unmounted and detached. But some volumes are still mounted: %#v", podVolumes) + assert.False(t, hasMountedVolumes, "Expected volumes to be unmounted and detached. But some volumes are still mounted") assert.NoError(t, volumetest.VerifyTearDownCallCount( 1 /* expectedTearDownCallCount */, testKubelet.volumePlugin)) @@ -554,9 +553,9 @@ func TestVolumeAttachAndMountControllerEnabled(t *testing.T) { podVolumes := kubelet.volumeManager.GetMountedVolumesForPod( util.GetUniquePodName(pod)) - allPodVolumes := kubelet.volumeManager.GetPossiblyMountedVolumesForPod( + hasVolumes := kubelet.volumeManager.HasPossiblyMountedVolumesForPod( util.GetUniquePodName(pod)) - assert.Equal(t, podVolumes, allPodVolumes, "GetMountedVolumesForPod and GetPossiblyMountedVolumesForPod should return the same volumes") + assert.True(t, hasVolumes, "HasPossiblyMountedVolumesForPod should return true") expectedPodVolumes := []string{"vol1"} assert.Len(t, podVolumes, len(expectedPodVolumes), "Volumes for pod %+v", pod) @@ -641,9 +640,9 @@ func TestVolumeUnmountAndDetachControllerEnabled(t *testing.T) { podVolumes := kubelet.volumeManager.GetMountedVolumesForPod( util.GetUniquePodName(pod)) - allPodVolumes := kubelet.volumeManager.GetPossiblyMountedVolumesForPod( + hasVolumes := kubelet.volumeManager.HasPossiblyMountedVolumesForPod( util.GetUniquePodName(pod)) - assert.Equal(t, podVolumes, allPodVolumes, "GetMountedVolumesForPod and GetPossiblyMountedVolumesForPod should return the same volumes") + assert.True(t, hasVolumes, "HasPossiblyMountedVolumesForPod should return true") expectedPodVolumes := []string{"vol1"} assert.Len(t, podVolumes, len(expectedPodVolumes), "Volumes for pod %+v", pod) @@ -669,9 +668,9 @@ func TestVolumeUnmountAndDetachControllerEnabled(t *testing.T) { // Verify volumes unmounted podVolumes = kubelet.volumeManager.GetMountedVolumesForPod( util.GetUniquePodName(pod)) - allPodVolumes = kubelet.volumeManager.GetPossiblyMountedVolumesForPod( + hasVolumes = kubelet.volumeManager.HasPossiblyMountedVolumesForPod( util.GetUniquePodName(pod)) - assert.Equal(t, podVolumes, allPodVolumes, "GetMountedVolumesForPod and GetPossiblyMountedVolumesForPod should return the same volumes") + assert.False(t, hasVolumes, "HasPossiblyMountedVolumesForPod should return false") assert.Empty(t, podVolumes, "Expected volumes to be unmounted and detached. But some volumes are still mounted") diff --git a/pkg/kubelet/volumemanager/volume_manager.go b/pkg/kubelet/volumemanager/volume_manager.go index 7e40cffa4e0..5b2f9bc42c6 100644 --- a/pkg/kubelet/volumemanager/volume_manager.go +++ b/pkg/kubelet/volumemanager/volume_manager.go @@ -126,13 +126,11 @@ type VolumeManager interface { // volumes. GetMountedVolumesForPod(podName types.UniquePodName) container.VolumeMap - // GetPossiblyMountedVolumesForPod returns a VolumeMap containing the volumes - // referenced by the specified pod that are either successfully attached + // HasPossiblyMountedVolumesForPod returns whether the pod has + // any volumes that are either successfully attached // and mounted or are "uncertain", i.e. a volume plugin may be mounting - // them right now. The key in the map is the OuterVolumeSpecName (i.e. - // pod.Spec.Volumes[x].Name). It returns an empty VolumeMap if pod has no - // volumes. - GetPossiblyMountedVolumesForPod(podName types.UniquePodName) container.VolumeMap + // them right now. + HasPossiblyMountedVolumesForPod(podName types.UniquePodName) bool // GetExtraSupplementalGroupsForPod returns a list of the extra // supplemental groups for the Pod. These extra supplemental groups come @@ -332,17 +330,8 @@ func (vm *volumeManager) GetMountedVolumesForPod(podName types.UniquePodName) co return podVolumes } -func (vm *volumeManager) GetPossiblyMountedVolumesForPod(podName types.UniquePodName) container.VolumeMap { - podVolumes := make(container.VolumeMap) - for _, mountedVolume := range vm.actualStateOfWorld.GetPossiblyMountedVolumesForPod(podName) { - podVolumes[mountedVolume.OuterVolumeSpecName] = container.VolumeInfo{ - Mounter: mountedVolume.Mounter, - BlockVolumeMapper: mountedVolume.BlockVolumeMapper, - ReadOnly: mountedVolume.VolumeSpec.ReadOnly, - InnerVolumeSpecName: mountedVolume.InnerVolumeSpecName, - } - } - return podVolumes +func (vm *volumeManager) HasPossiblyMountedVolumesForPod(podName types.UniquePodName) bool { + return len(vm.actualStateOfWorld.GetPossiblyMountedVolumesForPod(podName)) > 0 } func (vm *volumeManager) GetExtraSupplementalGroupsForPod(pod *v1.Pod) []int64 { diff --git a/pkg/kubelet/volumemanager/volume_manager_fake.go b/pkg/kubelet/volumemanager/volume_manager_fake.go index 02c424c3801..b57f01779e9 100644 --- a/pkg/kubelet/volumemanager/volume_manager_fake.go +++ b/pkg/kubelet/volumemanager/volume_manager_fake.go @@ -78,9 +78,9 @@ func (f *FakeVolumeManager) GetMountedVolumesForPod(podName types.UniquePodName) return nil } -// GetPossiblyMountedVolumesForPod is not implemented -func (f *FakeVolumeManager) GetPossiblyMountedVolumesForPod(podName types.UniquePodName) container.VolumeMap { - return nil +// HasPossiblyMountedVolumesForPod is not implemented +func (f *FakeVolumeManager) HasPossiblyMountedVolumesForPod(podName types.UniquePodName) bool { + return false } // GetExtraSupplementalGroupsForPod is not implemented