From 3a79466ddde88e5161f1ca9754bebc62ef4ac8b9 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 2 Nov 2022 13:58:24 +0100 Subject: [PATCH] Reshuffle functions between reconstruct and reconstruc_common Move common functions to reconstruc_common.go and functions used only for the current (old) reconstruction to reconstruct.go --- .../reconciler/reconciler_common.go | 16 ++ .../volumemanager/reconciler/reconstruct.go | 163 ++--------------- .../reconciler/reconstruct_common.go | 164 ++++++++++++++++-- 3 files changed, 179 insertions(+), 164 deletions(-) diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_common.go b/pkg/kubelet/volumemanager/reconciler/reconciler_common.go index 26b38b0725d..c88bb156b63 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_common.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler_common.go @@ -1,3 +1,19 @@ +/* +Copyright 2022 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 ( diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct.go b/pkg/kubelet/volumemanager/reconciler/reconstruct.go index 6c0d2b1a4dc..ba6a0453031 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconstruct.go +++ b/pkg/kubelet/volumemanager/reconciler/reconstruct.go @@ -18,15 +18,11 @@ package reconciler import ( "context" - "fmt" - "path/filepath" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" - volumepkg "k8s.io/kubernetes/pkg/volume" - "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/nestedpendingoperations" "k8s.io/kubernetes/pkg/volume/util/operationexecutor" ) @@ -117,149 +113,6 @@ func (rc *reconciler) syncStates(kubeletPodDir string) { } } -// Reconstruct volume data structure by reading the pod's volume directories -func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume, error) { - // plugin initializations - plugin, err := rc.volumePluginMgr.FindPluginByName(volume.pluginName) - if err != nil { - return nil, err - } - - // Create pod object - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - UID: types.UID(volume.podName), - }, - } - mapperPlugin, err := rc.volumePluginMgr.FindMapperPluginByName(volume.pluginName) - if err != nil { - return nil, err - } - if volume.volumeMode == v1.PersistentVolumeBlock && mapperPlugin == nil { - return nil, fmt.Errorf("could not find block volume plugin %q (spec.Name: %q) pod %q (UID: %q)", volume.pluginName, volume.volumeSpecName, volume.podName, pod.UID) - } - - reconstructed, err := rc.operationExecutor.ReconstructVolumeOperation( - volume.volumeMode, - plugin, - mapperPlugin, - pod.UID, - volume.podName, - volume.volumeSpecName, - volume.volumePath, - volume.pluginName) - if err != nil { - return nil, err - } - volumeSpec := reconstructed.Spec - - // We have to find the plugins by volume spec (NOT by plugin name) here - // in order to correctly reconstruct ephemeral volume types. - // Searching by spec checks whether the volume is actually attachable - // (i.e. has a PV) whereas searching by plugin name can only tell whether - // the plugin supports attachable volumes. - attachablePlugin, err := rc.volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) - if err != nil { - return nil, err - } - deviceMountablePlugin, err := rc.volumePluginMgr.FindDeviceMountablePluginBySpec(volumeSpec) - if err != nil { - return nil, err - } - - var uniqueVolumeName v1.UniqueVolumeName - if attachablePlugin != nil || deviceMountablePlugin != nil { - uniqueVolumeName, err = util.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) - if err != nil { - return nil, err - } - } else { - uniqueVolumeName = util.GetUniqueVolumeNameFromSpecWithPod(volume.podName, plugin, volumeSpec) - } - - var volumeMapper volumepkg.BlockVolumeMapper - var volumeMounter volumepkg.Mounter - var deviceMounter volumepkg.DeviceMounter - // Path to the mount or block device to check - var checkPath string - - if volume.volumeMode == v1.PersistentVolumeBlock { - var newMapperErr error - volumeMapper, newMapperErr = mapperPlugin.NewBlockVolumeMapper( - volumeSpec, - pod, - volumepkg.VolumeOptions{}) - if newMapperErr != nil { - return nil, fmt.Errorf( - "reconstructVolume.NewBlockVolumeMapper failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", - uniqueVolumeName, - volumeSpec.Name(), - volume.podName, - pod.UID, - newMapperErr) - } - mapDir, linkName := volumeMapper.GetPodDeviceMapPath() - checkPath = filepath.Join(mapDir, linkName) - } else { - var err error - volumeMounter, err = plugin.NewMounter( - volumeSpec, - pod, - volumepkg.VolumeOptions{}) - if err != nil { - return nil, fmt.Errorf( - "reconstructVolume.NewMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", - uniqueVolumeName, - volumeSpec.Name(), - volume.podName, - pod.UID, - err) - } - checkPath = volumeMounter.GetPath() - if deviceMountablePlugin != nil { - deviceMounter, err = deviceMountablePlugin.NewDeviceMounter() - if err != nil { - return nil, fmt.Errorf("reconstructVolume.NewDeviceMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", - uniqueVolumeName, - volumeSpec.Name(), - volume.podName, - pod.UID, - err) - } - } - } - - // Check existence of mount point for filesystem volume or symbolic link for block volume - isExist, checkErr := rc.operationExecutor.CheckVolumeExistenceOperation(volumeSpec, checkPath, volumeSpec.Name(), rc.mounter, uniqueVolumeName, volume.podName, pod.UID, attachablePlugin) - if checkErr != nil { - return nil, checkErr - } - // If mount or symlink doesn't exist, volume reconstruction should be failed - if !isExist { - return nil, fmt.Errorf("volume: %q is not mounted", uniqueVolumeName) - } - - reconstructedVolume := &reconstructedVolume{ - volumeName: uniqueVolumeName, - podName: volume.podName, - volumeSpec: volumeSpec, - // volume.volumeSpecName is actually InnerVolumeSpecName. It will not be used - // for volume cleanup. - // in case pod is added back to desired state, outerVolumeSpecName will be updated from dsw information. - // See issue #103143 and its fix for details. - outerVolumeSpecName: volume.volumeSpecName, - pod: pod, - deviceMounter: deviceMounter, - volumeGidValue: "", - // devicePath is updated during updateStates() by checking node status's VolumesAttached data. - // TODO: get device path directly from the volume mount path. - devicePath: "", - mounter: volumeMounter, - blockVolumeMapper: volumeMapper, - } - return reconstructedVolume, nil -} - // updateDevicePath gets the node status to retrieve volume device path information. func (rc *reconciler) updateDevicePath(volumesNeedUpdate map[v1.UniqueVolumeName]*globalVolumeInfo) { node, fetchErr := rc.kubeClient.CoreV1().Nodes().Get(context.TODO(), string(rc.nodeName), metav1.GetOptions{}) @@ -318,3 +171,19 @@ func (rc *reconciler) updateStates(volumesNeedUpdate map[v1.UniqueVolumeName]*gl } return nil } + +func (rc *reconciler) markVolumeState(volume *reconstructedVolume, volumeState operationexecutor.VolumeMountState) error { + markVolumeOpts := operationexecutor.MarkVolumeOpts{ + PodName: volume.podName, + PodUID: types.UID(volume.podName), + VolumeName: volume.volumeName, + Mounter: volume.mounter, + BlockVolumeMapper: volume.blockVolumeMapper, + OuterVolumeSpecName: volume.outerVolumeSpecName, + VolumeGidVolume: volume.volumeGidValue, + VolumeSpec: volume.volumeSpec, + VolumeMountState: volumeState, + } + err := rc.actualStateOfWorld.MarkVolumeAsMounted(markVolumeOpts) + return err +} diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go b/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go index 88a6581f232..e5a33aa735a 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go +++ b/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go @@ -1,5 +1,5 @@ /* -Copyright 2016 The Kubernetes Authors. +Copyright 2022 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. @@ -21,13 +21,16 @@ import ( "io/fs" "os" "path" + "path/filepath" "time" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/kubelet/config" volumepkg "k8s.io/kubernetes/pkg/volume" + "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/operationexecutor" volumetypes "k8s.io/kubernetes/pkg/volume/util/types" utilpath "k8s.io/utils/path" @@ -115,22 +118,6 @@ func getDeviceMountPath(gvi *globalVolumeInfo) (string, error) { } } -func (rc *reconciler) markVolumeState(volume *reconstructedVolume, volumeState operationexecutor.VolumeMountState) error { - markVolumeOpts := operationexecutor.MarkVolumeOpts{ - PodName: volume.podName, - PodUID: types.UID(volume.podName), - VolumeName: volume.volumeName, - Mounter: volume.mounter, - BlockVolumeMapper: volume.blockVolumeMapper, - OuterVolumeSpecName: volume.outerVolumeSpecName, - VolumeGidVolume: volume.volumeGidValue, - VolumeSpec: volume.volumeSpec, - VolumeMountState: volumeState, - } - err := rc.actualStateOfWorld.MarkVolumeAsMounted(markVolumeOpts) - return err -} - // getVolumesFromPodDir scans through the volumes directories under the given pod directory. // It returns a list of pod volume information including pod's uid, volume's plugin name, mount path, // and volume spec name. @@ -188,3 +175,146 @@ func getVolumesFromPodDir(podDir string) ([]podVolume, error) { klog.V(4).InfoS("Get volumes from pod directory", "path", podDir, "volumes", volumes) return volumes, nil } + +// Reconstruct volume data structure by reading the pod's volume directories +func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume, error) { + // plugin initializations + plugin, err := rc.volumePluginMgr.FindPluginByName(volume.pluginName) + if err != nil { + return nil, err + } + + // Create pod object + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(volume.podName), + }, + } + mapperPlugin, err := rc.volumePluginMgr.FindMapperPluginByName(volume.pluginName) + if err != nil { + return nil, err + } + if volume.volumeMode == v1.PersistentVolumeBlock && mapperPlugin == nil { + return nil, fmt.Errorf("could not find block volume plugin %q (spec.Name: %q) pod %q (UID: %q)", volume.pluginName, volume.volumeSpecName, volume.podName, pod.UID) + } + + reconstructed, err := rc.operationExecutor.ReconstructVolumeOperation( + volume.volumeMode, + plugin, + mapperPlugin, + pod.UID, + volume.podName, + volume.volumeSpecName, + volume.volumePath, + volume.pluginName) + if err != nil { + return nil, err + } + volumeSpec := reconstructed.Spec + + // We have to find the plugins by volume spec (NOT by plugin name) here + // in order to correctly reconstruct ephemeral volume types. + // Searching by spec checks whether the volume is actually attachable + // (i.e. has a PV) whereas searching by plugin name can only tell whether + // the plugin supports attachable volumes. + attachablePlugin, err := rc.volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) + if err != nil { + return nil, err + } + deviceMountablePlugin, err := rc.volumePluginMgr.FindDeviceMountablePluginBySpec(volumeSpec) + if err != nil { + return nil, err + } + + var uniqueVolumeName v1.UniqueVolumeName + if attachablePlugin != nil || deviceMountablePlugin != nil { + uniqueVolumeName, err = util.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) + if err != nil { + return nil, err + } + } else { + uniqueVolumeName = util.GetUniqueVolumeNameFromSpecWithPod(volume.podName, plugin, volumeSpec) + } + + var volumeMapper volumepkg.BlockVolumeMapper + var volumeMounter volumepkg.Mounter + var deviceMounter volumepkg.DeviceMounter + // Path to the mount or block device to check + var checkPath string + + if volume.volumeMode == v1.PersistentVolumeBlock { + var newMapperErr error + volumeMapper, newMapperErr = mapperPlugin.NewBlockVolumeMapper( + volumeSpec, + pod, + volumepkg.VolumeOptions{}) + if newMapperErr != nil { + return nil, fmt.Errorf( + "reconstructVolume.NewBlockVolumeMapper failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", + uniqueVolumeName, + volumeSpec.Name(), + volume.podName, + pod.UID, + newMapperErr) + } + mapDir, linkName := volumeMapper.GetPodDeviceMapPath() + checkPath = filepath.Join(mapDir, linkName) + } else { + var err error + volumeMounter, err = plugin.NewMounter( + volumeSpec, + pod, + volumepkg.VolumeOptions{}) + if err != nil { + return nil, fmt.Errorf( + "reconstructVolume.NewMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", + uniqueVolumeName, + volumeSpec.Name(), + volume.podName, + pod.UID, + err) + } + checkPath = volumeMounter.GetPath() + if deviceMountablePlugin != nil { + deviceMounter, err = deviceMountablePlugin.NewDeviceMounter() + if err != nil { + return nil, fmt.Errorf("reconstructVolume.NewDeviceMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", + uniqueVolumeName, + volumeSpec.Name(), + volume.podName, + pod.UID, + err) + } + } + } + + // Check existence of mount point for filesystem volume or symbolic link for block volume + isExist, checkErr := rc.operationExecutor.CheckVolumeExistenceOperation(volumeSpec, checkPath, volumeSpec.Name(), rc.mounter, uniqueVolumeName, volume.podName, pod.UID, attachablePlugin) + if checkErr != nil { + return nil, checkErr + } + // If mount or symlink doesn't exist, volume reconstruction should be failed + if !isExist { + return nil, fmt.Errorf("volume: %q is not mounted", uniqueVolumeName) + } + + reconstructedVolume := &reconstructedVolume{ + volumeName: uniqueVolumeName, + podName: volume.podName, + volumeSpec: volumeSpec, + // volume.volumeSpecName is actually InnerVolumeSpecName. It will not be used + // for volume cleanup. + // in case pod is added back to desired state, outerVolumeSpecName will be updated from dsw information. + // See issue #103143 and its fix for details. + outerVolumeSpecName: volume.volumeSpecName, + pod: pod, + deviceMounter: deviceMounter, + volumeGidValue: "", + // devicePath is updated during updateStates() by checking node status's VolumesAttached data. + // TODO: get device path directly from the volume mount path. + devicePath: "", + mounter: volumeMounter, + blockVolumeMapper: volumeMapper, + } + return reconstructedVolume, nil +}