Merge pull request #105345 from pohly/generic-ephemeral-volume-util

generic ephemeral volume util, base code and controller

pkg/controller/volume/attachdetach/util/util.go

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

pkg/controller/volume/ephemeral/controller.go

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

pkg/controller/volume/pvcprotection/pvc_protection_controller.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/component-base/metrics/prometheus/ratelimiter"
+	"k8s.io/component-helpers/storage/ephemeral"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/controller/volume/common"
 	"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 != "" {
 		for _, volume := range pod.Spec.Volumes {
 			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))
 				return true
 			}
@@ -407,7 +408,7 @@ func (c *Controller) enqueuePVCs(pod *v1.Pod, deleted bool) {
 		case volume.PersistentVolumeClaim != nil:
 			c.queue.Add(pod.Namespace + "/" + volume.PersistentVolumeClaim.ClaimName)
 		case c.genericEphemeralVolumeFeatureEnabled && volume.Ephemeral != nil:
-			c.queue.Add(pod.Namespace + "/" + pod.Name + "-" + volume.Name)
+			c.queue.Add(pod.Namespace + "/" + ephemeral.VolumeClaimName(pod, &volume))
 		}
 	}
 }

staging/src/k8s.io/component-helpers/storage/ephemeral/ephemeral.go

@@ -0,0 +1,57 @@
+/*
+Copyright 2021 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 ephemeral provides code that supports the usual pattern
+// for accessing the PVC that provides a generic ephemeral inline volume:
+//
+// - determine the PVC name that corresponds to the inline volume source
+// - retrieve the PVC
+// - verify that the PVC is owned by the pod
+// - use the PVC
+package ephemeral
+
+import (
+	"fmt"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// VolumeClaimName returns the name of the PersistentVolumeClaim
+// object that gets created for the generic ephemeral inline volume. The
+// name is deterministic and therefore this function does not need any
+// additional information besides the Pod name and volume name and it
+// will never fail.
+//
+// Before using the PVC for the Pod, the caller must check that it is
+// indeed the PVC that was created for the Pod by calling IsUsable.
+func VolumeClaimName(pod *v1.Pod, volume *v1.Volume) string {
+	return pod.Name + "-" + volume.Name
+}
+
+// VolumeIsForPod checks that the PVC is the ephemeral volume that
+// was created for the Pod. It returns an error that is informative
+// enough to be returned by the caller without adding further details
+// about the Pod or PVC.
+func VolumeIsForPod(pod *v1.Pod, pvc *v1.PersistentVolumeClaim) error {
+	// Checking the namespaces is just a precaution. The caller should
+	// never pass in a PVC that isn't from the same namespace as the
+	// Pod.
+	if pvc.Namespace != pod.Namespace || !metav1.IsControlledBy(pvc, pod) {
+		return fmt.Errorf("PVC %s/%s was not created for pod %s/%s (pod is not owner)", pvc.Namespace, pvc.Name, pod.Namespace, pod.Name)
+	}
+	return nil
+}

staging/src/k8s.io/component-helpers/storage/ephemeral/ephemeral_test.go

@@ -0,0 +1,126 @@
+/*
+Copyright 2021 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 ephemeral
+
+import (
+	"fmt"
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+)
+
+func TestVolumeIsForPod(t *testing.T) {
+	uid := 0
+	newUID := func() types.UID {
+		uid++
+		return types.UID(fmt.Sprintf("%d", uid))
+	}
+	isController := true
+
+	podNotOwner := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "kube-system",
+			Name:      "podNotOwner",
+			UID:       newUID(),
+		},
+	}
+	podOwner := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "kube-system",
+			Name:      "podOwner",
+			UID:       newUID(),
+		},
+	}
+	pvcNoOwner := &v1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "kube-system",
+			Name:      "pvcNoOwner",
+			UID:       newUID(),
+		},
+	}
+	pvcWithOwner := &v1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "kube-system",
+			Name:      "pvcNoOwner",
+			UID:       newUID(),
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					UID:        podOwner.UID,
+					Controller: &isController,
+				},
+			},
+		},
+	}
+	userPVCWithOwner := &v1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "user-namespace",
+			Name:      "userPVCWithOwner",
+			UID:       newUID(),
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					UID:        podOwner.UID,
+					Controller: &isController,
+				},
+			},
+		},
+	}
+
+	testcases := map[string]struct {
+		pod           *v1.Pod
+		pvc           *v1.PersistentVolumeClaim
+		expectedError string
+	}{
+		"owned": {
+			pod: podOwner,
+			pvc: pvcWithOwner,
+		},
+		"other-pod": {
+			pod:           podNotOwner,
+			pvc:           pvcWithOwner,
+			expectedError: `PVC kube-system/pvcNoOwner was not created for pod kube-system/podNotOwner (pod is not owner)`,
+		},
+		"no-owner": {
+			pod:           podOwner,
+			pvc:           pvcNoOwner,
+			expectedError: `PVC kube-system/pvcNoOwner was not created for pod kube-system/podOwner (pod is not owner)`,
+		},
+		"different-namespace": {
+			pod:           podOwner,
+			pvc:           userPVCWithOwner,
+			expectedError: `PVC user-namespace/userPVCWithOwner was not created for pod kube-system/podOwner (pod is not owner)`,
+		},
+	}
+
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			err := VolumeIsForPod(tc.pod, tc.pvc)
+			if tc.expectedError == "" {
+				if err != nil {
+					t.Errorf("expected no error, got %v", err)
+				}
+			} else {
+				if err == nil {
+					t.Errorf("expected error %q, got nil", tc.expectedError)
+				} else if tc.expectedError != err.Error() {
+					t.Errorf("expected error %q, got %v", tc.expectedError, err)
+				}
+			}
+		})
+	}
+}

vendor/modules.txt

@@ -1978,6 +1978,7 @@ k8s.io/component-helpers/node/utils/sysctl
 k8s.io/component-helpers/node/utils/sysctl/testing
 k8s.io/component-helpers/scheduling/corev1
 k8s.io/component-helpers/scheduling/corev1/nodeaffinity
+k8s.io/component-helpers/storage/ephemeral
 k8s.io/component-helpers/storage/volume
 # k8s.io/controller-manager v0.0.0 => ./staging/src/k8s.io/controller-manager
 ## explicit