[release-v1.56] Annotation to check for statically provisioned PVs when creating DataVolumes

cmd/cdi-controller/controller.go

@@ -204,6 +204,11 @@ func start(ctx context.Context, cfg *rest.Config) {
 		os.Exit(1)
 	}
 
+	if err := dvc.CreateCommonIndexes(mgr); err != nil {
+		klog.Errorf("Unable to create shared indexes: %v", err)
+		os.Exit(1)
+	}
+
 	// TODO: Current DV controller had threadiness 3, should we do the same here, defaults to one thread.
 	if _, err := dvc.NewImportController(ctx, mgr, log, installerLabels); err != nil {
 		klog.Errorf("Unable to setup datavolume import controller: %v", err)

go.mod

@@ -47,6 +47,7 @@ require (
 	k8s.io/client-go v12.0.0+incompatible
 	k8s.io/cluster-bootstrap v0.0.0
 	k8s.io/code-generator v0.23.3
+	k8s.io/component-helpers v0.23.0
 	k8s.io/klog/v2 v2.70.1
 	k8s.io/kube-aggregator v0.23.0
 	k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1

go.sum

@@ -1980,6 +1980,8 @@ k8s.io/code-generator v0.23.0 h1:lhyd2KJVCEmpjaCpuoooGs+e3xhPwpYvupnNRidO0Ds=
 k8s.io/code-generator v0.23.0/go.mod h1:vQvOhDXhuzqiVfM/YHp+dmg10WDZCchJVObc9MvowsE=
 k8s.io/component-base v0.23.0 h1:UAnyzjvVZ2ZR1lF35YwtNY6VMN94WtOnArcXBu34es8=
 k8s.io/component-base v0.23.0/go.mod h1:DHH5uiFvLC1edCpvcTDV++NKULdYYU6pR9Tt3HIKMKI=
+k8s.io/component-helpers v0.23.0 h1:qNbqN10QTefiWcCOPkHL/0nn81sdKVv6ZgEXcSyot/U=
+k8s.io/component-helpers v0.23.0/go.mod h1:liXMh6FZS4qamKtMJQ7uLHnFe3tlC86RX5mJEk/aerg=
 k8s.io/cri-api v0.23.0/go.mod h1:2edENu3/mkyW3c6fVPPPaVGEFbLRacJizBbSp7ZOLOo=
 k8s.io/gengo v0.0.0-20181113154421-fd15ee9cc2f7/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
 k8s.io/gengo v0.0.0-20201113003025-83324d819ded/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E=

pkg/apiserver/webhooks/datavolume-mutate.go

@@ -26,6 +26,7 @@ import (
 	snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
 	admissionv1 "k8s.io/api/admission/v1"
 	authv1 "k8s.io/api/authorization/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8sfield "k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/client-go/kubernetes"
@@ -85,7 +86,7 @@ func (p *sarProxy) Create(sar *authv1.SubjectAccessReview) (*authv1.SubjectAcces
 }
 
 func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admissionv1.AdmissionResponse {
-	var dataVolume, oldDataVolume cdiv1.DataVolume
+	dataVolume := &cdiv1.DataVolume{}
 
 	klog.V(3).Infof("Got AdmissionReview %+v", ar)
 
@@ -102,22 +103,7 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi
 		return allowedAdmissionResponse()
 	}
 
-	cloneSourceHandler, err := newCloneSourceHandler(&dataVolume, wh.cdiClient)
-	if err != nil {
-		return toAdmissionResponseError(err)
-	}
-
-	targetNamespace, targetName := dataVolume.Namespace, dataVolume.Name
-	if targetNamespace == "" {
-		targetNamespace = ar.Request.Namespace
-	}
-
-	if targetName == "" {
-		targetName = ar.Request.Name
-	}
-
 	modifiedDataVolume := dataVolume.DeepCopy()
-	modified := false
 
 	if ar.Request.Operation == admissionv1.Create {
 		config, err := wh.cdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
@@ -130,17 +116,41 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi
 			}
 			if modifiedDataVolume.Annotations[cc.AnnDeleteAfterCompletion] != "false" {
 				modifiedDataVolume.Annotations[cc.AnnDeleteAfterCompletion] = "true"
-				modified = true
 			}
 		}
 	}
 
-	if cloneSourceHandler.cloneType == noClone {
-		klog.V(3).Infof("DataVolume %s/%s not cloning", targetNamespace, targetName)
-		if modified {
+	_, prePopulated := dataVolume.Annotations[cc.AnnPrePopulated]
+	_, checkStaticVolume := dataVolume.Annotations[cc.AnnCheckStaticVolume]
+	noTokenOkay := prePopulated || checkStaticVolume
+
+	targetNamespace, targetName := dataVolume.Namespace, dataVolume.Name
+	if targetNamespace == "" {
+		targetNamespace = ar.Request.Namespace
+	}
+
+	if targetName == "" {
+		targetName = ar.Request.Name
+	}
+
+	cloneSourceHandler, err := newCloneSourceHandler(dataVolume, wh.cdiClient)
+	if err != nil {
+		if k8serrors.IsNotFound(err) && noTokenOkay {
+			// no token needed, likely since no datasource
+			klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, no datasource", targetNamespace, targetName)
 			return toPatchResponse(dataVolume, modifiedDataVolume)
 		}
-		return allowedAdmissionResponse()
+		return toAdmissionResponseError(err)
+	}
+
+	if cloneSourceHandler.cloneType == noClone {
+		klog.V(3).Infof("DataVolume %s/%s not cloning", targetNamespace, targetName)
+		return toPatchResponse(dataVolume, modifiedDataVolume)
+	}
+
+	// only add token at create time
+	if ar.Request.Operation != admissionv1.Create {
+		return toPatchResponse(dataVolume, modifiedDataVolume)
 	}
 
 	sourceName, sourceNamespace := cloneSourceHandler.sourceName, cloneSourceHandler.sourceNamespace
@@ -150,19 +160,12 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi
 
 	_, err = wh.k8sClient.CoreV1().Namespaces().Get(context.TODO(), sourceNamespace, metav1.GetOptions{})
 	if err != nil {
-		return toAdmissionResponseError(err)
-	}
-
-	if ar.Request.Operation == admissionv1.Update {
-		if err := json.Unmarshal(ar.Request.OldObject.Raw, &oldDataVolume); err != nil {
-			return toAdmissionResponseError(err)
-		}
-
-		_, ok := oldDataVolume.Annotations[cc.AnnCloneToken]
-		if ok {
-			klog.V(3).Infof("DataVolume %s/%s already has clone token", targetNamespace, targetName)
-			return allowedAdmissionResponse()
+		if k8serrors.IsNotFound(err) && noTokenOkay {
+			// no token needed, likely since no source namespace
+			klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, no source namespace", targetNamespace, targetName)
+			return toPatchResponse(dataVolume, modifiedDataVolume)
 		}
+		return toAdmissionResponseError(err)
 	}
 
 	ok, reason, err := cloneSourceHandler.cloneAuthFunc(wh.proxy, sourceNamespace, sourceName, targetNamespace, ar.Request.UserInfo)
@@ -171,6 +174,11 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi
 	}
 
 	if !ok {
+		if noTokenOkay {
+			klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, auth failed", targetNamespace, targetName)
+			return toPatchResponse(dataVolume, modifiedDataVolume)
+		}
+
 		causes := []metav1.StatusCause{
 			{
 				Type:    metav1.CauseTypeFieldValueInvalid,

pkg/apiserver/webhooks/datavolume-mutate_test.go

@@ -125,13 +125,40 @@ var _ = Describe("Mutating DataVolume Webhook", func() {
 			Expect(resp.Patch).To(BeNil())
 		})
 
+		DescribeTable("should accept a DataVolume with sourceRef to non-existing DataSource", func(anno string) {
+			dataVolume := newDataSourceDataVolume("testDV", nil, "test")
+			Expect(dataVolume.Annotations).To(BeNil())
+			dataVolume.Annotations = map[string]string{anno: "whatever"}
+			dvBytes, _ := json.Marshal(&dataVolume)
+			ar := &admissionv1.AdmissionReview{
+				Request: &admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
+					Resource: metav1.GroupVersionResource{
+						Group:    cdicorev1.SchemeGroupVersion.Group,
+						Version:  cdicorev1.SchemeGroupVersion.Version,
+						Resource: "datavolumes",
+					},
+					Object: runtime.RawExtension{
+						Raw: dvBytes,
+					},
+				},
+			}
+			resp := mutateDVs(key, ar, true)
+			Expect(resp.Allowed).To(BeTrue())
+			Expect(resp.Patch).ToNot(BeNil())
+		},
+			Entry("with static volume annotation", cc.AnnCheckStaticVolume),
+			Entry("with prePopulated volume annotation", cc.AnnPrePopulated),
+		)
+
 		It("should allow a DataVolume with sourceRef to existing DataSource", func() {
 			dataVolume := newDataSourceDataVolume("testDV", nil, "test")
 			Expect(dataVolume.Annotations).To(BeNil())
 			dvBytes, _ := json.Marshal(&dataVolume)
 
 			ar := &admissionv1.AdmissionReview{
 				Request: &admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
 					Resource: metav1.GroupVersionResource{
 						Group:    cdicorev1.SchemeGroupVersion.Group,
 						Version:  cdicorev1.SchemeGroupVersion.Version,
@@ -207,6 +234,7 @@ var _ = Describe("Mutating DataVolume Webhook", func() {
 
 			ar := &admissionv1.AdmissionReview{
 				Request: &admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
 					Resource: metav1.GroupVersionResource{
 						Group:    cdicorev1.SchemeGroupVersion.Group,
 						Version:  cdicorev1.SchemeGroupVersion.Version,
@@ -223,12 +251,41 @@ var _ = Describe("Mutating DataVolume Webhook", func() {
 			Expect(resp.Patch).To(BeNil())
 		})
 
+		DescribeTable("should accept a clone DataVolume", func(anno string) {
+			dataVolume := newPVCDataVolume("testDV", "testNamespace", "test")
+			Expect(dataVolume.Annotations).To(BeNil())
+			dataVolume.Annotations = map[string]string{anno: "whatever"}
+			dvBytes, _ := json.Marshal(&dataVolume)
+
+			ar := &admissionv1.AdmissionReview{
+				Request: &admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
+					Resource: metav1.GroupVersionResource{
+						Group:    cdicorev1.SchemeGroupVersion.Group,
+						Version:  cdicorev1.SchemeGroupVersion.Version,
+						Resource: "datavolumes",
+					},
+					Object: runtime.RawExtension{
+						Raw: dvBytes,
+					},
+				},
+			}
+
+			resp := mutateDVs(key, ar, false)
+			Expect(resp.Allowed).To(BeTrue())
+			Expect(resp.Patch).ToNot(BeNil())
+		},
+			Entry("with static volume annotation", cc.AnnCheckStaticVolume),
+			Entry("with prePopulated volume annotation", cc.AnnPrePopulated),
+		)
+
 		It("should reject a clone if the source PVC's namespace doesn't exist", func() {
 			dataVolume := newPVCDataVolume("testDV", "noNamespace", "test")
 			dvBytes, _ := json.Marshal(&dataVolume)
 
 			ar := &admissionv1.AdmissionReview{
 				Request: &admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
 					Resource: metav1.GroupVersionResource{
 						Group:    cdicorev1.SchemeGroupVersion.Group,
 						Version:  cdicorev1.SchemeGroupVersion.Version,
@@ -245,12 +302,41 @@ var _ = Describe("Mutating DataVolume Webhook", func() {
 			Expect(resp.Patch).To(BeNil())
 		})
 
+		DescribeTable("should accept a clone if the source PVC's namespace doesn't exist", func(anno string) {
+			dataVolume := newPVCDataVolume("testDV", "noNamespace", "test")
+			Expect(dataVolume.Annotations).To(BeNil())
+			dataVolume.Annotations = map[string]string{anno: "whatever"}
+			dvBytes, _ := json.Marshal(&dataVolume)
+
+			ar := &admissionv1.AdmissionReview{
+				Request: &admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
+					Resource: metav1.GroupVersionResource{
+						Group:    cdicorev1.SchemeGroupVersion.Group,
+						Version:  cdicorev1.SchemeGroupVersion.Version,
+						Resource: "datavolumes",
+					},
+					Object: runtime.RawExtension{
+						Raw: dvBytes,
+					},
+				},
+			}
+
+			resp := mutateDVs(key, ar, false)
+			Expect(resp.Allowed).To(BeTrue())
+			Expect(resp.Patch).ToNot(BeNil())
+		},
+			Entry("with static volume annotation", cc.AnnCheckStaticVolume),
+			Entry("with prePopulated volume annotation", cc.AnnPrePopulated),
+		)
+
 		DescribeTable("should", func(srcNamespace string) {
 			dataVolume := newPVCDataVolume("testDV", srcNamespace, "test")
 			dvBytes, _ := json.Marshal(&dataVolume)
 
 			ar := &admissionv1.AdmissionReview{
 				Request: &admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
 					Resource: metav1.GroupVersionResource{
 						Group:    cdicorev1.SchemeGroupVersion.Group,
 						Version:  cdicorev1.SchemeGroupVersion.Version,

pkg/apiserver/webhooks/datavolume-validate.go

@@ -592,6 +592,8 @@ func validateExternalPopulation(spec *cdiv1.DataVolumeSpec, field *k8sfield.Path
 }
 
 func (wh *dataVolumeValidatingWebhook) Admit(ar admissionv1.AdmissionReview) *admissionv1.AdmissionResponse {
+	klog.V(3).Infof("Got AdmissionReview %+v", ar)
+
 	if err := validateDataVolumeResource(ar); err != nil {
 		return toAdmissionResponseError(err)
 	}

pkg/apiserver/webhooks/handler.go

@@ -25,6 +25,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"reflect"
 	"time"
 
 	"github.com/appscode/jsonpatch"
@@ -197,6 +198,10 @@ func validateDataVolumeResource(ar admissionv1.AdmissionReview) error {
 }
 
 func toPatchResponse(original, current interface{}) *admissionv1.AdmissionResponse {
+	if reflect.DeepEqual(original, current) {
+		return allowedAdmissionResponse()
+	}
+
 	patchType := admissionv1.PatchTypeJSONPatch
 
 	ob, err := json.Marshal(original)
@@ -219,7 +224,7 @@ func toPatchResponse(original, current interface{}) *admissionv1.AdmissionRespon
 		return toAdmissionResponseError(err)
 	}
 
-	klog.V(3).Infof("sending patches\n%s", pb)
+	klog.V(1).Infof("sending patches\n%s", pb)
 
 	return &admissionv1.AdmissionResponse{
 		Allowed:   true,

pkg/controller/common/util.go

@@ -177,6 +177,13 @@ const (
 	// AnnUploadRequest marks that a PVC should be made available for upload
 	AnnUploadRequest = AnnAPIGroup + "/storage.upload.target"
 
+	// AnnCheckStaticVolume checks if a statically allocated PV exists before creating the target PVC.
+	// If so, PVC is still created but population is skipped
+	AnnCheckStaticVolume = AnnAPIGroup + "/storage.checkStaticVolume"
+
+	// AnnPersistentVolumeList is an annotation storing a list of PV names
+	AnnPersistentVolumeList = AnnAPIGroup + "/storage.persistentVolumeList"
+
 	//AnnDefaultStorageClass is the annotation indicating that a storage class is the default one.
 	AnnDefaultStorageClass = "storageclass.kubernetes.io/is-default-class"
 

pkg/controller/datavolume/BUILD.bazel

@@ -42,6 +42,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/k8s.io/client-go/tools/record:go_default_library",
+        "//vendor/k8s.io/component-helpers/storage/volume:go_default_library",
         "//vendor/sigs.k8s.io/controller-runtime/pkg/client:go_default_library",
         "//vendor/sigs.k8s.io/controller-runtime/pkg/controller:go_default_library",
         "//vendor/sigs.k8s.io/controller-runtime/pkg/controller/controllerutil:go_default_library",
@@ -64,6 +65,7 @@ go_test(
         "pvc-clone-controller_test.go",
         "smart-clone-controller_test.go",
         "snapshot-clone-controller_test.go",
+        "static-volume_test.go",
         "upload-controller_test.go",
         "util_test.go",
     ],

pkg/controller/datavolume/clone-controller-base.go

@@ -545,7 +545,7 @@ func (r *CloneReconcilerBase) syncCloneStatusPhase(syncRes *dataVolumeCloneSyncR
 	return r.syncDataVolumeStatusPhaseWithEvent(syncRes, phase, pvc, event)
 }
 
-func (r CloneReconcilerBase) updateStatusPhase(pvc *corev1.PersistentVolumeClaim, dataVolumeCopy *cdiv1.DataVolume, event *Event) error {
+func (r *CloneReconcilerBase) updateStatusPhase(pvc *corev1.PersistentVolumeClaim, dataVolumeCopy *cdiv1.DataVolume, event *Event) error {
 	phase, ok := pvc.Annotations[cc.AnnPodPhase]
 	if phase != string(corev1.PodSucceeded) {
 		_, ok = pvc.Annotations[cc.AnnCloneRequest]