controller: use generic ephemeral volume helper functions

The name concatenation and ownership check were originally considered small
enough to not warrant dedicated functions, but the intent of the code is more
readable with them.

There also was a missing owner check in the attach controller.
This commit is contained in:
Patrick Ohly 2021-08-31 08:39:55 +02:00
parent 650eba6904
commit 4ae0eecb34
3 changed files with 31 additions and 25 deletions

View File

@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
utilfeature "k8s.io/apiserver/pkg/util/feature" utilfeature "k8s.io/apiserver/pkg/util/feature"
corelisters "k8s.io/client-go/listers/core/v1" corelisters "k8s.io/client-go/listers/core/v1"
"k8s.io/component-helpers/storage/ephemeral"
"k8s.io/klog/v2" "k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache"
"k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/features"
@ -46,8 +47,10 @@ func CreateVolumeSpec(podVolume v1.Volume, pod *v1.Pod, nodeName types.NodeName,
claimName = pvcSource.ClaimName claimName = pvcSource.ClaimName
readOnly = pvcSource.ReadOnly readOnly = pvcSource.ReadOnly
} }
isEphemeral := false
if ephemeralSource := podVolume.VolumeSource.Ephemeral; ephemeralSource != nil && utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume) { if ephemeralSource := podVolume.VolumeSource.Ephemeral; ephemeralSource != nil && utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume) {
claimName = pod.Name + "-" + podVolume.Name claimName = ephemeral.VolumeClaimName(pod, &podVolume)
isEphemeral = true
} }
if claimName != "" { if claimName != "" {
klog.V(10).Infof( klog.V(10).Infof(
@ -56,8 +59,7 @@ func CreateVolumeSpec(podVolume v1.Volume, pod *v1.Pod, nodeName types.NodeName,
claimName) claimName)
// If podVolume is a PVC, fetch the real PV behind the claim // If podVolume is a PVC, fetch the real PV behind the claim
pvName, pvcUID, err := getPVCFromCacheExtractPV( pvc, err := getPVCFromCache(pod.Namespace, claimName, pvcLister)
pod.Namespace, claimName, pvcLister)
if err != nil { if err != nil {
return nil, fmt.Errorf( return nil, fmt.Errorf(
"error processing PVC %q/%q: %v", "error processing PVC %q/%q: %v",
@ -65,7 +67,13 @@ func CreateVolumeSpec(podVolume v1.Volume, pod *v1.Pod, nodeName types.NodeName,
claimName, claimName,
err) err)
} }
if isEphemeral {
if err := ephemeral.VolumeIsForPod(pod, pvc); err != nil {
return nil, err
}
}
pvName, pvcUID := pvc.Spec.VolumeName, pvc.UID
klog.V(10).Infof( klog.V(10).Infof(
"Found bound PV for PVC (ClaimName %q/%q pvcUID %v): pvName=%q", "Found bound PV for PVC (ClaimName %q/%q pvcUID %v): pvName=%q",
pod.Namespace, pod.Namespace,
@ -119,20 +127,19 @@ func CreateVolumeSpec(podVolume v1.Volume, pod *v1.Pod, nodeName types.NodeName,
return spec, nil return spec, nil
} }
// getPVCFromCacheExtractPV fetches the PVC object with the given namespace and // getPVCFromCache fetches the PVC object with the given namespace and
// name from the shared internal PVC store extracts the name of the PV it is // name from the shared internal PVC store.
// pointing to and returns it.
// This method returns an error if a PVC object does not exist in the cache // This method returns an error if a PVC object does not exist in the cache
// with the given namespace/name. // with the given namespace/name.
// This method returns an error if the PVC object's phase is not "Bound". // This method returns an error if the PVC object's phase is not "Bound".
func getPVCFromCacheExtractPV(namespace string, name string, pvcLister corelisters.PersistentVolumeClaimLister) (string, types.UID, error) { func getPVCFromCache(namespace string, name string, pvcLister corelisters.PersistentVolumeClaimLister) (*v1.PersistentVolumeClaim, error) {
pvc, err := pvcLister.PersistentVolumeClaims(namespace).Get(name) pvc, err := pvcLister.PersistentVolumeClaims(namespace).Get(name)
if err != nil { if err != nil {
return "", "", fmt.Errorf("failed to find PVC %s/%s in PVCInformer cache: %v", namespace, name, err) return nil, fmt.Errorf("failed to find PVC %s/%s in PVCInformer cache: %v", namespace, name, err)
} }
if pvc.Status.Phase != v1.ClaimBound || pvc.Spec.VolumeName == "" { if pvc.Status.Phase != v1.ClaimBound || pvc.Spec.VolumeName == "" {
return "", "", fmt.Errorf( return nil, fmt.Errorf(
"PVC %s/%s has non-bound phase (%q) or empty pvc.Spec.VolumeName (%q)", "PVC %s/%s has non-bound phase (%q) or empty pvc.Spec.VolumeName (%q)",
namespace, namespace,
name, name,
@ -140,7 +147,7 @@ func getPVCFromCacheExtractPV(namespace string, name string, pvcLister coreliste
pvc.Spec.VolumeName) pvc.Spec.VolumeName)
} }
return pvc.Spec.VolumeName, pvc.UID, nil return pvc, nil
} }
// getPVSpecFromCache fetches the PV object with the given name from the shared // getPVSpecFromCache fetches the PV object with the given name from the shared

View File

@ -37,10 +37,10 @@ import (
kcache "k8s.io/client-go/tools/cache" kcache "k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record" "k8s.io/client-go/tools/record"
"k8s.io/client-go/util/workqueue" "k8s.io/client-go/util/workqueue"
"k8s.io/component-helpers/storage/ephemeral"
"k8s.io/kubernetes/pkg/controller/volume/common" "k8s.io/kubernetes/pkg/controller/volume/common"
ephemeralvolumemetrics "k8s.io/kubernetes/pkg/controller/volume/ephemeral/metrics" ephemeralvolumemetrics "k8s.io/kubernetes/pkg/controller/volume/ephemeral/metrics"
"k8s.io/kubernetes/pkg/controller/volume/events" "k8s.io/kubernetes/pkg/controller/volume/events"
"k8s.io/kubernetes/pkg/volume/util"
) )
// Controller creates PVCs for ephemeral inline volumes in a pod spec. // Controller creates PVCs for ephemeral inline volumes in a pod spec.
@ -241,25 +241,23 @@ func (ec *ephemeralController) syncHandler(key string) error {
// handleEphemeralVolume is invoked for each volume of a pod. // handleEphemeralVolume is invoked for each volume of a pod.
func (ec *ephemeralController) handleVolume(pod *v1.Pod, vol v1.Volume) error { func (ec *ephemeralController) handleVolume(pod *v1.Pod, vol v1.Volume) error {
klog.V(5).Infof("ephemeral: checking volume %s", vol.Name) klog.V(5).Infof("ephemeral: checking volume %s", vol.Name)
ephemeral := vol.Ephemeral if vol.Ephemeral == nil {
if ephemeral == nil {
return nil return nil
} }
pvcName := pod.Name + "-" + vol.Name pvcName := ephemeral.VolumeClaimName(pod, &vol)
pvc, err := ec.pvcLister.PersistentVolumeClaims(pod.Namespace).Get(pvcName) pvc, err := ec.pvcLister.PersistentVolumeClaims(pod.Namespace).Get(pvcName)
if err != nil && !errors.IsNotFound(err) { if err != nil && !errors.IsNotFound(err) {
return err return err
} }
if pvc != nil { if pvc != nil {
if metav1.IsControlledBy(pvc, pod) { if err := ephemeral.VolumeIsForPod(pod, pvc); err != nil {
return err
}
// Already created, nothing more to do. // Already created, nothing more to do.
klog.V(5).Infof("ephemeral: volume %s: PVC %s already created", vol.Name, pvcName) klog.V(5).Infof("ephemeral: volume %s: PVC %s already created", vol.Name, pvcName)
return nil return nil
} }
return fmt.Errorf("PVC %q (uid: %q) was not created for the pod",
util.GetPersistentVolumeClaimQualifiedName(pvc), pvc.UID)
}
// Create the PVC with pod as owner. // Create the PVC with pod as owner.
isTrue := true isTrue := true
@ -276,10 +274,10 @@ func (ec *ephemeralController) handleVolume(pod *v1.Pod, vol v1.Volume) error {
BlockOwnerDeletion: &isTrue, BlockOwnerDeletion: &isTrue,
}, },
}, },
Annotations: ephemeral.VolumeClaimTemplate.Annotations, Annotations: vol.Ephemeral.VolumeClaimTemplate.Annotations,
Labels: ephemeral.VolumeClaimTemplate.Labels, Labels: vol.Ephemeral.VolumeClaimTemplate.Labels,
}, },
Spec: ephemeral.VolumeClaimTemplate.Spec, Spec: vol.Ephemeral.VolumeClaimTemplate.Spec,
} }
ephemeralvolumemetrics.EphemeralVolumeCreateAttempts.Inc() ephemeralvolumemetrics.EphemeralVolumeCreateAttempts.Inc()
_, err = ec.kubeClient.CoreV1().PersistentVolumeClaims(pod.Namespace).Create(context.TODO(), pvc, metav1.CreateOptions{}) _, err = ec.kubeClient.CoreV1().PersistentVolumeClaims(pod.Namespace).Create(context.TODO(), pvc, metav1.CreateOptions{})

View File

@ -32,6 +32,7 @@ import (
"k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue" "k8s.io/client-go/util/workqueue"
"k8s.io/component-base/metrics/prometheus/ratelimiter" "k8s.io/component-base/metrics/prometheus/ratelimiter"
"k8s.io/component-helpers/storage/ephemeral"
"k8s.io/klog/v2" "k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/controller/volume/common" "k8s.io/kubernetes/pkg/controller/volume/common"
"k8s.io/kubernetes/pkg/controller/volume/protectionutil" "k8s.io/kubernetes/pkg/controller/volume/protectionutil"
@ -299,7 +300,7 @@ func (c *Controller) podUsesPVC(pod *v1.Pod, pvc *v1.PersistentVolumeClaim) bool
if pod.Spec.NodeName != "" { if pod.Spec.NodeName != "" {
for _, volume := range pod.Spec.Volumes { for _, volume := range pod.Spec.Volumes {
if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.ClaimName == pvc.Name || if volume.PersistentVolumeClaim != nil && volume.PersistentVolumeClaim.ClaimName == pvc.Name ||
c.genericEphemeralVolumeFeatureEnabled && !podIsShutDown(pod) && volume.Ephemeral != nil && pod.Name+"-"+volume.Name == pvc.Name && metav1.IsControlledBy(pvc, pod) { c.genericEphemeralVolumeFeatureEnabled && !podIsShutDown(pod) && volume.Ephemeral != nil && ephemeral.VolumeClaimName(pod, &volume) == pvc.Name && ephemeral.VolumeIsForPod(pod, pvc) == nil {
klog.V(2).InfoS("Pod uses PVC", "pod", klog.KObj(pod), "PVC", klog.KObj(pvc)) klog.V(2).InfoS("Pod uses PVC", "pod", klog.KObj(pod), "PVC", klog.KObj(pvc))
return true return true
} }
@ -407,7 +408,7 @@ func (c *Controller) enqueuePVCs(pod *v1.Pod, deleted bool) {
case volume.PersistentVolumeClaim != nil: case volume.PersistentVolumeClaim != nil:
c.queue.Add(pod.Namespace + "/" + volume.PersistentVolumeClaim.ClaimName) c.queue.Add(pod.Namespace + "/" + volume.PersistentVolumeClaim.ClaimName)
case c.genericEphemeralVolumeFeatureEnabled && volume.Ephemeral != nil: case c.genericEphemeralVolumeFeatureEnabled && volume.Ephemeral != nil:
c.queue.Add(pod.Namespace + "/" + pod.Name + "-" + volume.Name) c.queue.Add(pod.Namespace + "/" + ephemeral.VolumeClaimName(pod, &volume))
} }
} }
} }