diff --git a/cmd/cdi-controller/controller.go b/cmd/cdi-controller/controller.go index 4c8dd99d52..b1eade2f8a 100644 --- a/cmd/cdi-controller/controller.go +++ b/cmd/cdi-controller/controller.go @@ -262,6 +262,11 @@ func start(ctx context.Context, cfg *rest.Config) { klog.Errorf("Unable to setup datasource controller: %v", err) os.Exit(1) } + // Populator controllers and indexes + if err := populators.CreateCommonPopulatorIndexes(mgr); err != nil { + klog.Errorf("Unable to create common populator indexes: %v", err) + os.Exit(1) + } if _, err := populators.NewImportPopulator(ctx, mgr, log, installerLabels); err != nil { klog.Errorf("Unable to setup import populator: %v", err) os.Exit(1) diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index e22fd9a756..95de3dcf78 100644 --- a/pkg/controller/common/util.go +++ b/pkg/controller/common/util.go @@ -87,9 +87,6 @@ const ( AnnExternalPopulation = AnnAPIGroup + "/externalPopulation" // AnnOwnedByDataVolume annotation has the owner DataVolume name AnnOwnedByDataVolume = AnnAPIGroup + "/ownedByDataVolume" - // AnnSelectedNodeName annotation is added to the PVC' to specify the node used when creating the populator pod for - // dynamic provisioning - AnnSelectedNodeName = AnnAPIGroup + "/storage.selected.node" // AnnDeleteAfterCompletion is PVC annotation for deleting DV after completion AnnDeleteAfterCompletion = AnnAPIGroup + "/storage.deleteAfterCompletion" @@ -222,6 +219,10 @@ const ( // AnnImmediateBinding provides a const to indicate whether immediate binding should be performed on the PV (overrides global config) AnnImmediateBinding = AnnAPIGroup + "/storage.bind.immediate.requested" + // AnnSelectedNode annotation is added to a PVC that has been triggered by scheduler to + // be dynamically provisioned. Its value is the name of the selected node. + AnnSelectedNode = "volume.kubernetes.io/selected-node" + // CloneUniqueID is used as a special label to be used when we search for the pod CloneUniqueID = "cdi.kubevirt.io/storage.clone.cloneUniqeId" @@ -571,10 +572,10 @@ func GetPriorityClass(pvc *v1.PersistentVolumeClaim) string { return anno[AnnPriorityClassName] } -// GetSelectedNodename gets the node name specified in a PVC for dynamic provisioning -func GetSelectedNodename(pvc *v1.PersistentVolumeClaim) string { +// GetSelectedNodeName gets the node name specified in a PVC for dynamic provisioning +func GetSelectedNodeName(pvc *v1.PersistentVolumeClaim) string { anno := pvc.GetAnnotations() - return anno[AnnSelectedNodeName] + return anno[AnnSelectedNode] } // ShouldDeletePod returns whether the PVC workload pod should be deleted diff --git a/pkg/controller/datavolume/pvc-clone-controller_test.go b/pkg/controller/datavolume/pvc-clone-controller_test.go index 504d9e41d2..f3b1823807 100644 --- a/pkg/controller/datavolume/pvc-clone-controller_test.go +++ b/pkg/controller/datavolume/pvc-clone-controller_test.go @@ -855,12 +855,8 @@ var _ = Describe("All DataVolume Tests", func() { // Get the expected value pvcSpec, err := renderPvcSpec(reconciler.client, reconciler.recorder, reconciler.log, dv) Expect(err).ToNot(HaveOccurred()) -<<<<<<< HEAD - expectedSize, err := inflateSizeWithOverhead(reconciler.client, int64(100), pvcSpec) - Expect(err).ToNot(HaveOccurred()) -======= expectedSize, err := cc.InflateSizeWithOverhead(reconciler.client, int64(100), pvcSpec) ->>>>>>> 9ca365d5b (Create populators package to be used for all populators) + Expect(err).ToNot(HaveOccurred()) expectedSizeInt64, _ := expectedSize.AsInt64() // Checks diff --git a/pkg/controller/import-controller.go b/pkg/controller/import-controller.go index ecc34cfddb..f304f1e9d4 100644 --- a/pkg/controller/import-controller.go +++ b/pkg/controller/import-controller.go @@ -523,7 +523,7 @@ func (r *ImportReconciler) createImporterPod(pvc *corev1.PersistentVolumeClaim) scratchPvcName: scratchPvcName, vddkImageName: vddkImageName, priorityClassName: cc.GetPriorityClass(pvc), - selectedNodeName: cc.GetSelectedNodename(pvc), + selectedNodeName: cc.GetSelectedNodeName(pvc), } pod, err := createImporterPod(r.log, r.client, podArgs, r.installerLabels) diff --git a/pkg/controller/populators/import-populator.go b/pkg/controller/populators/import-populator.go index 08dc2a2888..5f9883fe0b 100644 --- a/pkg/controller/populators/import-populator.go +++ b/pkg/controller/populators/import-populator.go @@ -1,3 +1,19 @@ +/* +Copyright 2023 The CDI 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 populators import ( @@ -131,8 +147,8 @@ func (r *ImportPopulatorReconciler) reconcileTargetPVC(pvc, pvcPrime *corev1.Per // Import-specific implementation of updateAnnotations func (r *ImportPopulatorReconciler) updateAnnotations(pvc *corev1.PersistentVolumeClaim, source client.Object) { importSource := source.(*cdiv1.ImportSource) - annotations := pvc.Annotations + annotations[AnnPopulatorKind] = importKind annotations[cc.AnnContentType] = cc.GetContentType(string(importSource.Spec.ContentType)) annotations[cc.AnnPreallocationRequested] = strconv.FormatBool(cc.GetPreallocation(r.client, importSource.Spec.Preallocation)) diff --git a/pkg/controller/populators/populator-base.go b/pkg/controller/populators/populator-base.go index 35e29f5027..78ae7325f5 100644 --- a/pkg/controller/populators/populator-base.go +++ b/pkg/controller/populators/populator-base.go @@ -1,8 +1,23 @@ +/* +Copyright 2023 The CDI 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 populators import ( "context" - "time" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -39,6 +54,10 @@ type ReconcilerBase struct { installerLabels map[string]string } +const ( + dataSourceRefField = "spec.dataSourceRef" +) + // Interface to store populator-specific methods type populatorController interface { // Returns the specific populator CR @@ -69,25 +88,9 @@ func addCommonPopulatorsWatches(mgr manager.Manager, c controller.Controller, lo return err } - const dataSourceRefField = "spec.dataSourceRef" - - indexKeyFunc := func(namespace, name string) string { - return namespace + "/" + name - } - - if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &corev1.PersistentVolumeClaim{}, dataSourceRefField, func(obj client.Object) []string { - pvc := obj.(*corev1.PersistentVolumeClaim) - if IsPVCDataSourceRefKind(pvc, sourceKind) && pvc.Spec.DataSourceRef.Name != "" { - return []string{indexKeyFunc(obj.GetNamespace(), pvc.Spec.DataSourceRef.Name)} - } - return nil - }); err != nil { - return err - } - mapDataSourceRefToPVC := func(obj client.Object) (reqs []reconcile.Request) { var pvcs corev1.PersistentVolumeClaimList - matchingFields := client.MatchingFields{dataSourceRefField: indexKeyFunc(obj.GetNamespace(), obj.GetName())} + matchingFields := client.MatchingFields{dataSourceRefField: getPopulatorIndexKey(obj.GetNamespace(), sourceKind, obj.GetName())} if err := mgr.GetClient().List(context.TODO(), &pvcs, matchingFields); err != nil { log.Error(err, "Unable to list PVCs", "matchingFields", matchingFields) return reqs @@ -107,6 +110,22 @@ func addCommonPopulatorsWatches(mgr manager.Manager, c controller.Controller, lo return nil } +// CreateCommonPopulatorIndexes creates indexes used by all populators +func CreateCommonPopulatorIndexes(mgr manager.Manager) error { + if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &corev1.PersistentVolumeClaim{}, dataSourceRefField, func(obj client.Object) []string { + pvc := obj.(*corev1.PersistentVolumeClaim) + dataSourceRef := pvc.Spec.DataSourceRef + if dataSourceRef != nil && dataSourceRef.APIGroup != nil && + *dataSourceRef.APIGroup == cc.AnnAPIGroup && dataSourceRef.Name != "" { + return []string{getPopulatorIndexKey(obj.GetNamespace(), pvc.Spec.DataSourceRef.Kind, pvc.Spec.DataSourceRef.Name)} + } + return nil + }); err != nil { + return err + } + return nil +} + func (r *ReconcilerBase) getPVCPrime(pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) { pvcPrime := &corev1.PersistentVolumeClaim{} pvcPrimeKey := types.NamespacedName{Namespace: pvc.Namespace, Name: PVCPrimeName(pvc)} @@ -148,7 +167,7 @@ func (r *ReconcilerBase) handleStorageClass(pvc *corev1.PersistentVolumeClaim) ( if storageClass.VolumeBindingMode != nil && *storageClass.VolumeBindingMode == storagev1.VolumeBindingWaitForFirstConsumer { waitForFirstConsumer = true - nodeName := pvc.Annotations[cc.AnnSelectedNodeName] + nodeName := pvc.Annotations[cc.AnnSelectedNode] if nodeName == "" { // Wait for the PVC to get a node name before continuing return false, waitForFirstConsumer, nil @@ -165,7 +184,7 @@ func (r *ReconcilerBase) createPVCPrime(pvc *corev1.PersistentVolumeClaim, sourc annotations := make(map[string]string) annotations[cc.AnnImmediateBinding] = "" if waitForFirstConsumer { - annotations[cc.AnnSelectedNodeName] = pvc.Annotations[cc.AnnSelectedNodeName] + annotations[cc.AnnSelectedNode] = pvc.Annotations[cc.AnnSelectedNode] } pvcPrimeName := PVCPrimeName(pvc) pvcPrime := &corev1.PersistentVolumeClaim{ @@ -308,11 +327,6 @@ func (r *ReconcilerBase) reconcileCommon(pvc *corev1.PersistentVolumeClaim, popu func (r *ReconcilerBase) reconcileCleanup(pvcPrime *corev1.PersistentVolumeClaim) (reconcile.Result, error) { if pvcPrime != nil { - // Wait for the bind controller to rebind the PV - if pvcPrime.Status.Phase != corev1.ClaimLost { - return reconcile.Result{RequeueAfter: 2 * time.Second}, nil - } - if err := r.client.Delete(context.TODO(), pvcPrime); err != nil { return reconcile.Result{}, err } diff --git a/pkg/controller/populators/util.go b/pkg/controller/populators/util.go index 41c02a996a..8a366c08b8 100644 --- a/pkg/controller/populators/util.go +++ b/pkg/controller/populators/util.go @@ -1,3 +1,19 @@ +/* +Copyright 2023 The CDI 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 populators import ( @@ -20,9 +36,9 @@ const ( // createdPVCPrimeSuccessfully provides a const to indicate we created PVC prime for population createdPVCPrimeSuccessfully = "CreatedPVCPrimeSuccessfully" - // AnnSelectedNode annotation is added to a PVC that has been triggered by scheduler to - // be dynamically provisioned. Its value is the name of the selected node. - AnnSelectedNode = "volume.kubernetes.io/selected-node" + // AnnPopulatorKind annotation is added to the PVC' to specify the population kind, so it's later + // checked by the common populator watches. + AnnPopulatorKind = cc.AnnAPIGroup + "/storage.populator.kind" // annMigratedTo annotation is added to a PVC and PV that is supposed to be // dynamically provisioned/deleted by by its corresponding CSI driver @@ -30,6 +46,11 @@ const ( // Kubernetes components will "stand-down" and the external-provisioner will // act on the objects annMigratedTo = "pv.kubernetes.io/migrated-to" + + // importKind is used to specify that the PVC' is meant to be populated by the import flow + importKind = "import" + // uploadKind is used to specify that the PVC' is meant to be populated by the upload flow + uploadKind = "upload" ) // IsPVCDataSourceRefKind returns if the PVC has DataSourceRef that @@ -44,15 +65,14 @@ func isPVCPrimeDataSourceRefKind(pvc *corev1.PersistentVolumeClaim, kind string) if owner == nil || owner.Kind != "PersistentVolumeClaim" { return false } - _, ok := pvc.Annotations[cc.AnnUploadRequest] - if ok { - return kind == cdiv1.UploadSourceRef - } - // TODO: Maybe use a more descriptive annotation? - _, ok = pvc.Annotations[cc.AnnSource] - if ok { + populatorKind, _ := pvc.Annotations[AnnPopulatorKind] + switch populatorKind { + case importKind: return kind == cdiv1.ImportSourceRef + case uploadKind: + return kind == cdiv1.UploadSourceRef } + return false } @@ -61,6 +81,10 @@ func PVCPrimeName(targetPVC *corev1.PersistentVolumeClaim) string { return fmt.Sprintf("%s-%s", primePvcPrefix, targetPVC.UID) } +func getPopulatorIndexKey(namespace, kind, name string) string { + return namespace + "/" + kind + "/" + name +} + func isPVCOwnedByDataVolume(pvc *corev1.PersistentVolumeClaim) bool { owner := metav1.GetControllerOf(pvc) return (owner != nil && owner.Kind == "DataVolume") || cc.HasAnnOwnedByDataVolume(pvc) diff --git a/tests/framework/pvc.go b/tests/framework/pvc.go index 41044c032a..42ffdad31a 100644 --- a/tests/framework/pvc.go +++ b/tests/framework/pvc.go @@ -133,7 +133,7 @@ func createConsumerPodForPopulationPVC(targetPvc *k8sv1.PersistentVolumeClaim, f executorPod, err := f.CreateNoopPodWithPVC(podName, namespace, targetPvc) gomega.Expect(err).ToNot(gomega.HaveOccurred()) - selectedNode, status, err := utils.WaitForPVCAnnotation(f.K8sClient, namespace, targetPvc, populators.AnnSelectedNode) + selectedNode, status, err := utils.WaitForPVCAnnotation(f.K8sClient, namespace, targetPvc, cc.AnnSelectedNode) gomega.Expect(err).ToNot(gomega.HaveOccurred()) gomega.Expect(status).To(gomega.BeTrue()) gomega.Expect(selectedNode).ToNot(gomega.BeEmpty())