diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_new.go b/pkg/kubelet/volumemanager/reconciler/reconciler.go similarity index 91% rename from pkg/kubelet/volumemanager/reconciler/reconciler_new.go rename to pkg/kubelet/volumemanager/reconciler/reconciler.go index 3f8ab539609..b5f1e424a62 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_new.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler.go @@ -21,10 +21,7 @@ import ( "k8s.io/klog/v2" ) -// TODO: move to reconciler.go and remove old code there when NewVolumeManagerReconstruction is GA - -// TODO: Replace Run() when NewVolumeManagerReconstruction is GA -func (rc *reconciler) runNew(stopCh <-chan struct{}) { +func (rc *reconciler) Run(stopCh <-chan struct{}) { rc.reconstructVolumes() klog.InfoS("Reconciler: start to sync state") wait.Until(rc.reconcileNew, rc.loopSleepDuration, stopCh) diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_common.go b/pkg/kubelet/volumemanager/reconciler/reconciler_common.go index c478ccada9e..4129247658b 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_common.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler_common.go @@ -146,10 +146,6 @@ type reconciler struct { volumesNeedReportedInUse []v1.UniqueVolumeName } -func (rc *reconciler) Run(stopCh <-chan struct{}) { - rc.runNew(stopCh) -} - func (rc *reconciler) unmountVolumes() { // Ensure volumes that should be unmounted are unmounted. for _, mountedVolume := range rc.actualStateOfWorld.GetAllMountedVolumes() { diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_new_test.go b/pkg/kubelet/volumemanager/reconciler/reconciler_new_test.go deleted file mode 100644 index 67f5bbba1cb..00000000000 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_new_test.go +++ /dev/null @@ -1,144 +0,0 @@ -/* -Copyright 2023 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 reconciler - -import ( - "testing" - - "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/record" - "k8s.io/kubernetes/pkg/kubelet/volumemanager/cache" - "k8s.io/kubernetes/pkg/volume" - volumetesting "k8s.io/kubernetes/pkg/volume/testing" - "k8s.io/kubernetes/pkg/volume/util" - "k8s.io/kubernetes/pkg/volume/util/hostutil" - "k8s.io/kubernetes/pkg/volume/util/operationexecutor" - "k8s.io/mount-utils" -) - -func TestReconcileWithUpdateReconstructedFromAPIServer(t *testing.T) { - // Calls Run() with two reconstructed volumes. - // Verifies the devicePaths + volume attachability are reconstructed from node.status. - - // Arrange - node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: string(nodeName), - }, - Status: v1.NodeStatus{ - VolumesAttached: []v1.AttachedVolume{ - { - Name: "fake-plugin/fake-device1", - DevicePath: "fake/path", - }, - }, - }, - } - volumePluginMgr, fakePlugin := volumetesting.GetTestKubeletVolumePluginMgrWithNode(t, node) - seLinuxTranslator := util.NewFakeSELinuxLabelTranslator() - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr, seLinuxTranslator) - asw := cache.NewActualStateOfWorld(nodeName, volumePluginMgr) - kubeClient := createTestClient() - fakeRecorder := &record.FakeRecorder{} - fakeHandler := volumetesting.NewBlockVolumePathHandler() - oex := operationexecutor.NewOperationExecutor(operationexecutor.NewOperationGenerator( - kubeClient, - volumePluginMgr, - fakeRecorder, - fakeHandler)) - rc := NewReconciler( - kubeClient, - true, /* controllerAttachDetachEnabled */ - reconcilerLoopSleepDuration, - waitForAttachTimeout, - nodeName, - dsw, - asw, - hasAddedPods, - oex, - mount.NewFakeMounter(nil), - hostutil.NewFakeHostUtil(nil), - volumePluginMgr, - kubeletPodsDir) - reconciler := rc.(*reconciler) - - // The pod has two volumes, fake-device1 is attachable, fake-device2 is not. - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod1", - UID: "pod1uid", - }, - Spec: v1.PodSpec{ - Volumes: []v1.Volume{ - { - Name: "volume-name", - VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device1", - }, - }, - }, - { - Name: "volume-name2", - VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device2", - }, - }, - }, - }, - }, - } - - volumeSpec1 := &volume.Spec{Volume: &pod.Spec.Volumes[0]} - volumeName1 := util.GetUniqueVolumeName(fakePlugin.GetPluginName(), "fake-device1") - volumeSpec2 := &volume.Spec{Volume: &pod.Spec.Volumes[1]} - volumeName2 := util.GetUniqueVolumeName(fakePlugin.GetPluginName(), "fake-device2") - - assert.NoError(t, asw.AddAttachUncertainReconstructedVolume(volumeName1, volumeSpec1, nodeName, "")) - assert.NoError(t, asw.MarkDeviceAsUncertain(volumeName1, "/dev/badly/reconstructed", "/var/lib/kubelet/plugins/global1", "")) - assert.NoError(t, asw.AddAttachUncertainReconstructedVolume(volumeName2, volumeSpec2, nodeName, "")) - assert.NoError(t, asw.MarkDeviceAsUncertain(volumeName2, "/dev/reconstructed", "/var/lib/kubelet/plugins/global2", "")) - - assert.False(t, reconciler.StatesHasBeenSynced()) - - reconciler.volumesNeedUpdateFromNodeStatus = append(reconciler.volumesNeedUpdateFromNodeStatus, volumeName1, volumeName2) - // Act - run reconcile loop just once. - // "volumesNeedUpdateFromNodeStatus" is not empty, so no unmount will be triggered. - reconciler.reconcileNew() - - // Assert - assert.True(t, reconciler.StatesHasBeenSynced()) - assert.Empty(t, reconciler.volumesNeedUpdateFromNodeStatus) - - attachedVolumes := asw.GetAttachedVolumes() - assert.Equalf(t, len(attachedVolumes), 2, "two volumes in ASW expected") - for _, vol := range attachedVolumes { - if vol.VolumeName == volumeName1 { - // devicePath + attachability must have been updated from node.status - assert.True(t, vol.PluginIsAttachable) - assert.Equal(t, vol.DevicePath, "fake/path") - } - if vol.VolumeName == volumeName2 { - // only attachability was updated from node.status - assert.False(t, vol.PluginIsAttachable) - assert.Equal(t, vol.DevicePath, "/dev/reconstructed") - } - } -} diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go index 21709d1ecb9..c57bba4793a 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go @@ -2292,3 +2292,114 @@ func getReconciler(kubeletDir string, t *testing.T, volumePaths []string, kubeCl tmpKubeletPodDir) return rc, fakePlugin } + +func TestReconcileWithUpdateReconstructedFromAPIServer(t *testing.T) { + // Calls Run() with two reconstructed volumes. + // Verifies the devicePaths + volume attachability are reconstructed from node.status. + + // Arrange + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: string(nodeName), + }, + Status: v1.NodeStatus{ + VolumesAttached: []v1.AttachedVolume{ + { + Name: "fake-plugin/fake-device1", + DevicePath: "fake/path", + }, + }, + }, + } + volumePluginMgr, fakePlugin := volumetesting.GetTestKubeletVolumePluginMgrWithNode(t, node) + seLinuxTranslator := util.NewFakeSELinuxLabelTranslator() + dsw := cache.NewDesiredStateOfWorld(volumePluginMgr, seLinuxTranslator) + asw := cache.NewActualStateOfWorld(nodeName, volumePluginMgr) + kubeClient := createTestClient() + fakeRecorder := &record.FakeRecorder{} + fakeHandler := volumetesting.NewBlockVolumePathHandler() + oex := operationexecutor.NewOperationExecutor(operationexecutor.NewOperationGenerator( + kubeClient, + volumePluginMgr, + fakeRecorder, + fakeHandler)) + rc := NewReconciler( + kubeClient, + true, /* controllerAttachDetachEnabled */ + reconcilerLoopSleepDuration, + waitForAttachTimeout, + nodeName, + dsw, + asw, + hasAddedPods, + oex, + mount.NewFakeMounter(nil), + hostutil.NewFakeHostUtil(nil), + volumePluginMgr, + kubeletPodsDir) + reconciler := rc.(*reconciler) + + // The pod has two volumes, fake-device1 is attachable, fake-device2 is not. + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod1", + UID: "pod1uid", + }, + Spec: v1.PodSpec{ + Volumes: []v1.Volume{ + { + Name: "volume-name", + VolumeSource: v1.VolumeSource{ + GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ + PDName: "fake-device1", + }, + }, + }, + { + Name: "volume-name2", + VolumeSource: v1.VolumeSource{ + GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ + PDName: "fake-device2", + }, + }, + }, + }, + }, + } + + volumeSpec1 := &volume.Spec{Volume: &pod.Spec.Volumes[0]} + volumeName1 := util.GetUniqueVolumeName(fakePlugin.GetPluginName(), "fake-device1") + volumeSpec2 := &volume.Spec{Volume: &pod.Spec.Volumes[1]} + volumeName2 := util.GetUniqueVolumeName(fakePlugin.GetPluginName(), "fake-device2") + + assert.NoError(t, asw.AddAttachUncertainReconstructedVolume(volumeName1, volumeSpec1, nodeName, "")) + assert.NoError(t, asw.MarkDeviceAsUncertain(volumeName1, "/dev/badly/reconstructed", "/var/lib/kubelet/plugins/global1", "")) + assert.NoError(t, asw.AddAttachUncertainReconstructedVolume(volumeName2, volumeSpec2, nodeName, "")) + assert.NoError(t, asw.MarkDeviceAsUncertain(volumeName2, "/dev/reconstructed", "/var/lib/kubelet/plugins/global2", "")) + + assert.False(t, reconciler.StatesHasBeenSynced()) + + reconciler.volumesNeedUpdateFromNodeStatus = append(reconciler.volumesNeedUpdateFromNodeStatus, volumeName1, volumeName2) + // Act - run reconcile loop just once. + // "volumesNeedUpdateFromNodeStatus" is not empty, so no unmount will be triggered. + reconciler.reconcileNew() + + // Assert + assert.True(t, reconciler.StatesHasBeenSynced()) + assert.Empty(t, reconciler.volumesNeedUpdateFromNodeStatus) + + attachedVolumes := asw.GetAttachedVolumes() + assert.Equalf(t, len(attachedVolumes), 2, "two volumes in ASW expected") + for _, vol := range attachedVolumes { + if vol.VolumeName == volumeName1 { + // devicePath + attachability must have been updated from node.status + assert.True(t, vol.PluginIsAttachable) + assert.Equal(t, vol.DevicePath, "fake/path") + } + if vol.VolumeName == volumeName2 { + // only attachability was updated from node.status + assert.False(t, vol.PluginIsAttachable) + assert.Equal(t, vol.DevicePath, "/dev/reconstructed") + } + } +} diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct_new.go b/pkg/kubelet/volumemanager/reconciler/reconstruct.go similarity index 100% rename from pkg/kubelet/volumemanager/reconciler/reconstruct_new.go rename to pkg/kubelet/volumemanager/reconciler/reconstruct.go diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct_new_test.go b/pkg/kubelet/volumemanager/reconciler/reconstruct_test.go similarity index 100% rename from pkg/kubelet/volumemanager/reconciler/reconstruct_new_test.go rename to pkg/kubelet/volumemanager/reconciler/reconstruct_test.go