diff --git a/pkg/controller/volume/attachdetach/util/util.go b/pkg/controller/volume/attachdetach/util/util.go index 034bce8fc33..c811e94b4cd 100644 --- a/pkg/controller/volume/attachdetach/util/util.go +++ b/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 diff --git a/pkg/controller/volume/ephemeral/controller.go b/pkg/controller/volume/ephemeral/controller.go index e4fb9749fdf..b5ad0cf685e 100644 --- a/pkg/controller/volume/ephemeral/controller.go +++ b/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{}) diff --git a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go b/pkg/controller/volume/pvcprotection/pvc_protection_controller.go index cd971c395f9..b85961383fe 100644 --- a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go +++ b/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)) } } } diff --git a/staging/src/k8s.io/component-helpers/storage/ephemeral/ephemeral.go b/staging/src/k8s.io/component-helpers/storage/ephemeral/ephemeral.go new file mode 100644 index 00000000000..67dddaf5c31 --- /dev/null +++ b/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 +} diff --git a/staging/src/k8s.io/component-helpers/storage/ephemeral/ephemeral_test.go b/staging/src/k8s.io/component-helpers/storage/ephemeral/ephemeral_test.go new file mode 100644 index 00000000000..09f1d5844eb --- /dev/null +++ b/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) + } + } + }) + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index a527c330f66..446f4f8925d 100644 --- a/vendor/modules.txt +++ b/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