Skip to content

Commit

Permalink
feat(DataVolume): Add default instance type labels from VolumeSnapsho…
Browse files Browse the repository at this point in the history 
…ts and from Registry imports (#3381)

* feat(DataVolume): Add default instance type labels from VolumeSnapshots

de5ba85 started updating DataVolumes when reconciled, adding any labels
found on PVC or DataSource sources. This commit is a follow up that
starts adding labels also from VolumeSnapshots.

Signed-off-by: Felix Matouschek <[email protected]>

* feat(DataVolume): Add instance type labels from Registry imports

de5ba85 started updating DataVolumes when reconciled, adding any labels
found on PVC or DataSource sources. This commit is a follow up that
starts adding labels also from Registry imported PVCs.

Signed-off-by: Felix Matouschek <[email protected]>

---------

Signed-off-by: Felix Matouschek <[email protected]>
  • Loading branch information
@0xFelix
0xFelix authored Aug 14, 2024
1 parent 10e1ba8 commit e1a553a
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 1 deletion.
30 changes: 30 additions & 0 deletions pkg/controller/datavolume/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,36 @@ func updateDataVolumeDefaultInstancetypeLabels(client client.Client, syncState *
copyDefaultInstancetypeLabels(dv, pvc.Labels)
return nil
}
if dv.Spec.Source != nil && dv.Spec.Source.Snapshot != nil {
snapshot := &snapshotv1.VolumeSnapshot{}
key := types.NamespacedName{
Name: dv.Spec.Source.Snapshot.Name,
Namespace: dv.Spec.Source.Snapshot.Namespace,
}
if err := client.Get(context.TODO(), key, snapshot); err != nil {
if k8serrors.IsNotFound(err) {
return nil
}
return err
}
copyDefaultInstancetypeLabels(dv, snapshot.Labels)
return nil
}
if dv.Spec.Source != nil && dv.Spec.Source.Registry != nil {
pvc := &v1.PersistentVolumeClaim{}
key := types.NamespacedName{
Name: dv.Name,
Namespace: dv.Namespace,
}
if err := client.Get(context.TODO(), key, pvc); err != nil {
if k8serrors.IsNotFound(err) {
return nil
}
return err
}
copyDefaultInstancetypeLabels(dv, pvc.Labels)
return nil
}
if dv.Spec.SourceRef != nil && dv.Spec.SourceRef.Namespace != nil && dv.Spec.SourceRef.Kind == cdiv1.DataVolumeDataSource {
ds := &cdiv1.DataSource{}
key := types.NamespacedName{
Expand Down
82 changes: 82 additions & 0 deletions pkg/controller/datavolume/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
ocpconfigv1 "github.com/openshift/api/config/v1"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -115,12 +116,16 @@ var _ = Describe("updateDataVolumeDefaultInstancetypeLabels", func() {
const (
namespace = "namespace"
sourcePVCName = "sourcePVC"
sourceSnapshotName = "sourceSnapshot"
sourceRegistryName = "sourceRegistry"
sourceDataSourceName = "sourceDataSource"
)

var (
fakeClient client.Client
dataVolumeWithSourcePVC cdiv1.DataVolume
dataVolumeWithSourceSnapshot cdiv1.DataVolume
dataVolumeWithSourceRegistry cdiv1.DataVolume
dataVolumeWithSourceDataSource cdiv1.DataVolume
)

Expand All @@ -147,6 +152,35 @@ var _ = Describe("updateDataVolumeDefaultInstancetypeLabels", func() {
},
}

dataVolumeWithSourceSnapshot = cdiv1.DataVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "snapshot-datavolume",
Namespace: namespace,
},
Spec: cdiv1.DataVolumeSpec{
Source: &cdiv1.DataVolumeSource{
Snapshot: &cdiv1.DataVolumeSourceSnapshot{
Name: sourceSnapshotName,
Namespace: namespace,
},
},
},
}

dataVolumeWithSourceRegistry = cdiv1.DataVolume{
ObjectMeta: metav1.ObjectMeta{
Name: sourceRegistryName,
Namespace: namespace,
},
Spec: cdiv1.DataVolumeSpec{
Source: &cdiv1.DataVolumeSource{
Registry: &cdiv1.DataVolumeSourceRegistry{
URL: ptr.To("docker://testurl"),
},
},
},
}

dataVolumeWithSourceDataSource = cdiv1.DataVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "datasource-datavolume",
Expand All @@ -168,6 +202,20 @@ var _ = Describe("updateDataVolumeDefaultInstancetypeLabels", func() {
Labels: defaultInstancetypeLabelMap,
},
},
&snapshotv1.VolumeSnapshot{
ObjectMeta: metav1.ObjectMeta{
Name: sourceSnapshotName,
Namespace: namespace,
Labels: defaultInstancetypeLabelMap,
},
},
&corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: sourceRegistryName,
Namespace: namespace,
Labels: defaultInstancetypeLabelMap,
},
},
&cdiv1.DataSource{
ObjectMeta: metav1.ObjectMeta{
Name: sourceDataSourceName,
Expand Down Expand Up @@ -197,6 +245,8 @@ var _ = Describe("updateDataVolumeDefaultInstancetypeLabels", func() {
}
},
Entry("PVC", &dataVolumeWithSourcePVC),
Entry("Snapshot", &dataVolumeWithSourceSnapshot),
Entry("Registry", &dataVolumeWithSourceRegistry),
Entry("dataSource", &dataVolumeWithSourceDataSource),
)

Expand All @@ -215,6 +265,8 @@ var _ = Describe("updateDataVolumeDefaultInstancetypeLabels", func() {
Expect(syncState.dvMutated.Labels).To(HaveKeyWithValue(LabelDefaultInstancetype, customDefaultInstancetype))
},
Entry("PVC", &dataVolumeWithSourcePVC),
Entry("Snapshot", &dataVolumeWithSourceSnapshot),
Entry("Registry", &dataVolumeWithSourceRegistry),
Entry("DataSource", &dataVolumeWithSourceDataSource),
)

Expand All @@ -238,6 +290,33 @@ var _ = Describe("updateDataVolumeDefaultInstancetypeLabels", func() {
},
},
}),
Entry("Snapshot", &cdiv1.DataVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "snapshot-datavolume",
Namespace: namespace,
},
Spec: cdiv1.DataVolumeSpec{
Source: &cdiv1.DataVolumeSource{
Snapshot: &cdiv1.DataVolumeSourceSnapshot{
Name: "unknown",
Namespace: namespace,
},
},
},
}),
Entry("Registry", &cdiv1.DataVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "registry-datavolume-unknown",
Namespace: namespace,
},
Spec: cdiv1.DataVolumeSpec{
Source: &cdiv1.DataVolumeSource{
Registry: &cdiv1.DataVolumeSourceRegistry{
URL: ptr.To("docker://uknown"),
},
},
},
}),
Entry("DataSource", &cdiv1.DataVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "datasource-datavolume",
Expand Down Expand Up @@ -265,6 +344,8 @@ var _ = Describe("updateDataVolumeDefaultInstancetypeLabels", func() {
Expect(errors.IsServiceUnavailable(err)).To(BeTrue())
},
Entry("PVC", &dataVolumeWithSourcePVC),
Entry("Snapshot", &dataVolumeWithSourceSnapshot),
Entry("Registry", &dataVolumeWithSourceRegistry),
Entry("DataSource", &dataVolumeWithSourceDataSource),
)
})
Expand All @@ -288,6 +369,7 @@ func createClient(objs ...client.Object) client.Client {
_ = cdiv1.AddToScheme(s)
// Register other types with the runtime scheme.
_ = ocpconfigv1.Install(s)
_ = snapshotv1.AddToScheme(s)
// Create a fake client to mock API calls.
return fake.NewClientBuilder().WithScheme(s).WithObjects(objs...).Build()
}
Expand Down
4 changes: 4 additions & 0 deletions tests/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ container_image(
env = {
"TEST_KUBEVIRT_IO_TEST": "testvalue",
"TEST_KUBEVIRT_IO_EXISTING": "somethingelse",
"INSTANCETYPE_KUBEVIRT_IO_DEFAULT_INSTANCETYPE": "test-instancetype",
"INSTANCETYPE_KUBEVIRT_IO_DEFAULT_INSTANCETYPE_KIND": "VirtualMachineClusterInstancetype",
"INSTANCETYPE_KUBEVIRT_IO_DEFAULT_PREFERENCE": "test-preference",
"INSTANCETYPE_KUBEVIRT_IO_DEFAULT_PREFERENCE_KIND": "VirtualMachineClusterPreference",
},
tars = [":tinycore-tar"],
visibility = ["//visibility:public"],
Expand Down
42 changes: 41 additions & 1 deletion tests/datavolume_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3412,12 +3412,52 @@ var _ = Describe("[vendor:[email protected]][level:component]DataVolume tests",
}, 60*time.Second, 1*time.Second).Should(BeTrue())
},
Entry("PVC", func() *cdiv1.DataVolume {
By("createing a DataVolume pointing to a labelled PVC")
By("creating a DataVolume pointing to a labelled PVC")
dv := utils.NewDataVolumeForImageCloning("datavolume-from-pvc", "1Gi", sourceDataVolume.Namespace, sourceDataVolume.Name, nil, nil)
dv, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dv)
Expect(err).ToNot(HaveOccurred())
return dv
}),
Entry("Snapshot", func() *cdiv1.DataVolume {
if !f.IsSnapshotStorageClassAvailable() {
Skip("Clone from volumesnapshot does not work without snapshot capable storage")
}
By("creating a labelled VolumeSnapshot")
snapClass := f.GetSnapshotClass()
snapshot := utils.NewVolumeSnapshot(sourceDataVolume.Name, sourceDataVolume.Namespace, sourceDataVolume.Name, &snapClass.Name)
snapshot.Labels = make(map[string]string)
for _, defaultInstancetypeLabel := range controller.DefaultInstanceTypeLabels {
snapshot.Labels[defaultInstancetypeLabel] = "defined"
}
err = f.CrClient.Create(context.TODO(), snapshot)
Expect(err).ToNot(HaveOccurred())

By("creating a DataVolume pointing to a labelled VolumeSnapshot")
dv := utils.NewDataVolumeForSnapshotCloning("datavolume-from-snapshot", "1Gi", sourceDataVolume.Namespace, sourceDataVolume.Name, nil, nil)
dv, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dv)
Expect(err).ToNot(HaveOccurred())
return dv
}),
Entry("Registry", func() *cdiv1.DataVolume {
By("creating a DataVolume pointing to a Containerdisk")
dv := utils.NewDataVolumeWithRegistryImport("datavolume-from-registry", "1Gi", tinyCoreIsoRegistryURL())
cm, err := utils.CopyRegistryCertConfigMap(f.K8sClient, f.Namespace.Name, f.CdiInstallNs)
Expect(err).ToNot(HaveOccurred())
dv.Spec.Source.Registry.CertConfigMap = &cm
dv, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dv)
Expect(err).ToNot(HaveOccurred())

By("verifying PVC was created")
pvc, err := utils.WaitForPVC(f.K8sClient, dv.Namespace, dv.Name)
Expect(err).ToNot(HaveOccurred())
f.ForceBindIfWaitForFirstConsumer(pvc)

By("Wait for DV to succeed")
err = utils.WaitForDataVolumePhaseWithTimeout(f, dv.Namespace, cdiv1.Succeeded, dv.Name, 10*time.Minute)
Expect(err).ToNot(HaveOccurred())

return dv
}),
Entry("DataSource", func() *cdiv1.DataVolume {
By("createing a labelled DataSource")
ds := utils.NewPvcDataSource("datasource-from-pvc", f.Namespace.Name, sourceDataVolume.Name, sourceDataVolume.Namespace)
Expand Down
4 changes: 4 additions & 0 deletions tools/cdi-func-test-registry-init/populate-registry.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ function prepareImages {
COPY $FILENAME $IMAGE_LOCATION
ENV TEST_KUBEVIRT_IO_TEST=testvalue
ENV TEST_KUBEVIRT_IO_EXISTING=somethingelse
ENV INSTANCETYPE_KUBEVIRT_IO_DEFAULT_INSTANCETYPE=test-instancetype
ENV INSTANCETYPE_KUBEVIRT_IO_DEFAULT_INSTANCETYPE_KIND=VirtualMachineClusterInstancetype
ENV INSTANCETYPE_KUBEVIRT_IO_DEFAULT_PREFERENCE=test-preference
ENV INSTANCETYPE_KUBEVIRT_IO_DEFAULT_PREFERENCE_KIND=VirtualMachineClusterPreference
EOF

rm $FILENAME
Expand Down

0 comments on commit e1a553a

Please sign in to comment.