Skip to content

Commit

Permalink
Add a Preallocation field in the Import populator API
Browse files Browse the repository at this point in the history
This commit adds a new field in the ImportSource Spec so we can request preallocation for specific PVC imports, just as we do with DataVolumes.

Signed-off-by: Alvaro Romero <alromero@redhat.com>
  • Loading branch information
alromeros committed Apr 17, 2023
1 parent 62f41f0 commit 3db6b8a
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 32 deletions.
15 changes: 11 additions & 4 deletions pkg/apis/core/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions pkg/controller/common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,11 +549,11 @@ func IsPopulated(pvc *v1.PersistentVolumeClaim, c client.Client) (bool, error) {
})
}

// GetPreallocation retuns the preallocation setting for DV, falling back to StorageClass and global setting (in this order)
func GetPreallocation(client client.Client, dataVolume *cdiv1.DataVolume) bool {
// GetPreallocation retuns the preallocation setting for the specified object (DV or ImportSource), falling back to StorageClass and global setting (in this order)
func GetPreallocation(client client.Client, preallocation *bool) bool {
// First, the DV's preallocation
if dataVolume.Spec.Preallocation != nil {
return *dataVolume.Spec.Preallocation
if preallocation != nil {
return *preallocation
}

cdiconfig := &cdiv1.CDIConfig{}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/datavolume/controller-base.go
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ func (r *ReconcilerBase) newPersistentVolumeClaim(dataVolume *cdiv1.DataVolume,
if dataVolume.Spec.PriorityClassName != "" {
annotations[cc.AnnPriorityClassName] = dataVolume.Spec.PriorityClassName
}
annotations[cc.AnnPreallocationRequested] = strconv.FormatBool(cc.GetPreallocation(r.client, dataVolume))
annotations[cc.AnnPreallocationRequested] = strconv.FormatBool(cc.GetPreallocation(r.client, dataVolume.Spec.Preallocation))

pvc := &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/populators/import-populator.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,13 @@ 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 := pvc.Annotations
annotations[cc.AnnContentType] = cc.GetContentType(string(importSource.Spec.ContentType))
annotations[cc.AnnPreallocationRequested] = strconv.FormatBool(cc.GetPreallocation(r.client, importSource.Spec.Preallocation))

// TODO: Avoid reusing that much code. Non trivial because DataVolumes and ImportSources store these fields in diferent ways.
// TODO: Should implement some kind of webhook to avoid allowing different sources.
// TODO: Should implement some kind of webhook to avoid allowing more than one source in the populator spec.
if importSource.Spec.HTTP != nil {
annotations[cc.AnnEndpoint] = importSource.Spec.HTTP.URL
annotations[cc.AnnSource] = cc.SourceHTTP
Expand Down Expand Up @@ -196,6 +197,7 @@ func (r *ImportPopulatorReconciler) updateAnnotations(pvc *corev1.PersistentVolu
annotations[cc.AnnSource] = cc.SourceNone
return
}
// Should deprecate ImageIO and VDDK?
if importSource.Spec.Imageio != nil {
annotations[cc.AnnEndpoint] = importSource.Spec.Imageio.URL
annotations[cc.AnnSource] = cc.SourceImageio
Expand Down
12 changes: 6 additions & 6 deletions pkg/controller/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,35 +218,35 @@ var _ = Describe("GetPreallocation", func() {
It("Should return preallocation for DataVolume if specified", func() {
client := CreateClient()
dv := createDataVolumeWithPreallocation("test-dv", "test-ns", true)
preallocation := GetPreallocation(client, dv)
preallocation := GetPreallocation(client, dv.Spec.Preallocation)
Expect(preallocation).To(BeTrue())

dv = createDataVolumeWithPreallocation("test-dv", "test-ns", false)
preallocation = GetPreallocation(client, dv)
preallocation = GetPreallocation(client, dv.Spec.Preallocation)
Expect(preallocation).To(BeFalse())

// global: true, data volume overrides to false
client = CreateClient(createCDIConfigWithGlobalPreallocation(true))
dv = createDataVolumeWithStorageClassPreallocation("test-dv", "test-ns", "test-class", false)
preallocation = GetPreallocation(client, dv)
preallocation = GetPreallocation(client, dv.Spec.Preallocation)
Expect(preallocation).To(BeFalse())
})

It("Should return global preallocation setting if not defined in DV or SC", func() {
client := CreateClient(createCDIConfigWithGlobalPreallocation(true))
dv := createDataVolumeWithStorageClass("test-dv", "test-ns", "test-class")
preallocation := GetPreallocation(client, dv)
preallocation := GetPreallocation(client, dv.Spec.Preallocation)
Expect(preallocation).To(BeTrue())

client = CreateClient(createCDIConfigWithGlobalPreallocation(false))
preallocation = GetPreallocation(client, dv)
preallocation = GetPreallocation(client, dv.Spec.Preallocation)
Expect(preallocation).To(BeFalse())
})

It("Should be false when niether DV nor Config defines preallocation", func() {
client := CreateClient(createCDIConfig("test"))
dv := createDataVolumeWithStorageClass("test-dv", "test-ns", "test-class")
preallocation := GetPreallocation(client, dv)
preallocation := GetPreallocation(client, dv.Spec.Preallocation)
Expect(preallocation).To(BeFalse())
})
})
Expand Down
14 changes: 9 additions & 5 deletions pkg/operator/resources/crds_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -6513,8 +6513,8 @@ spec:
- name: v1beta1
schema:
openAPIV3Schema:
description: ImportSource is a specification to populate PersistentVolumeClaims
with import data
description: ImportSource works as a specification to populate PersistentVolumeClaims
with data imported from an HTTP/S3/Registry/Blank/ImageIO/VDDK source
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
Expand All @@ -6529,7 +6529,7 @@ spec:
metadata:
type: object
spec:
description: ImportSourceSpec defines specification for ImportSource
description: ImportSourceSpec defines the Spec field for ImportSource
properties:
blank:
description: DataVolumeBlankImage provides the parameters to create
Expand Down Expand Up @@ -6572,8 +6572,8 @@ spec:
- url
type: object
imageio:
description: DataVolumeSourceImageIO provides the parameters to create
a Data Volume from an imageio source
description: 'TODO before merging this PR: Consider deprecating ImageIO
and VDDK'
properties:
certConfigMap:
description: CertConfigMap provides a reference to the CA cert
Expand All @@ -6592,6 +6592,10 @@ spec:
- diskId
- url
type: object
preallocation:
description: Preallocation controls whether storage for the target
PVC should be allocated in advance.
type: boolean
registry:
description: DataVolumeSourceRegistry provides the parameters to create
a Data Volume from an registry source
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,8 @@ type DataImportCronList struct {
Items []DataImportCron `json:"items"`
}

// ImportSource is a specification to populate PersistentVolumeClaims with import data
// ImportSource works as a specification to populate PersistentVolumeClaims with data
// imported from an HTTP/S3/Registry/Blank/ImageIO/VDDK source
// +genclient
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +kubebuilder:object:root=true
Expand All @@ -632,16 +633,19 @@ type ImportSource struct {
Status ImportSourceStatus `json:"status,omitempty"`
}

// ImportSourceSpec defines specification for ImportSource
// ImportSourceSpec defines the Spec field for ImportSource
type ImportSourceSpec struct {
ContentType DataVolumeContentType `json:"contentType,omitempty"`
// Import sources
HTTP *DataVolumeSourceHTTP `json:"http,omitempty"`
S3 *DataVolumeSourceS3 `json:"s3,omitempty"`
Registry *DataVolumeSourceRegistry `json:"registry,omitempty"`
Imageio *DataVolumeSourceImageIO `json:"imageio,omitempty"`
VDDK *DataVolumeSourceVDDK `json:"vddk,omitempty"`
Blank *DataVolumeBlankImage `json:"blank,omitempty"`
// TODO before merging this PR: Consider deprecating ImageIO and VDDK
Imageio *DataVolumeSourceImageIO `json:"imageio,omitempty"`
VDDK *DataVolumeSourceVDDK `json:"vddk,omitempty"`
// Preallocation controls whether storage for the target PVC should be allocated in advance.
Preallocation *bool `json:"preallocation,omitempty"`
}

// ImportSourceStatus provides the most recently observed status of the ImportSource
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3db6b8a

Please sign in to comment.