Skip to content

Commit

Permalink
Take snapclass existence into consideration when populating cloneStra…
Browse files Browse the repository at this point in the history 
…tegy and sourceFormat

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
  • Loading branch information
@akalenyu
akalenyu committed Jun 4, 2023
1 parent c5f4d8e commit ed3de0a
Show file tree
Hide file tree
Showing 7 changed files with 251 additions and 44 deletions.
15 changes: 15 additions & 0 deletions pkg/controller/common/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,21 @@ func CreateStorageClass(name string, annotations map[string]string) *storagev1.S
}
}

// CreateSnapshotClass creates snapshot class
func CreateSnapshotClass(name string, annotations map[string]string, snapshotter string) *snapshotv1.VolumeSnapshotClass {
return &snapshotv1.VolumeSnapshotClass{
TypeMeta: metav1.TypeMeta{
Kind: "VolumeSnapshotClass",
APIVersion: snapshotv1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Annotations: annotations,
},
Driver: snapshotter,
}
}

// CreateImporterTestPod creates importer test pod CR
func CreateImporterTestPod(pvc *corev1.PersistentVolumeClaim, dvname string, scratchPvc *corev1.PersistentVolumeClaim) *corev1.Pod {
// importer pod name contains the pvc name
Expand Down
26 changes: 6 additions & 20 deletions pkg/controller/datavolume/pvc-clone-controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ var _ = Describe("All DataVolume Tests", func() {
dv.Spec.PVC.StorageClassName = &scName
pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
expectedSnapshotClass := "snap-class"
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand All @@ -105,7 +105,7 @@ var _ = Describe("All DataVolume Tests", func() {
dv.Spec.PVC.StorageClassName = &scName
pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
expectedSnapshotClass := "snap-class"
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -170,7 +170,7 @@ var _ = Describe("All DataVolume Tests", func() {
},
}
expectedSnapshotClass := "snap-class"
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, ot, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand All @@ -192,7 +192,7 @@ var _ = Describe("All DataVolume Tests", func() {
dv.Spec.PVC.StorageClassName = &scName
pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
expectedSnapshotClass := "snap-class"
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, podFunc(dv), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -399,7 +399,7 @@ var _ = Describe("All DataVolume Tests", func() {
dv.Spec.PVC.StorageClassName = &scName
pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
expectedSnapshotClass := "snap-class"
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createCloneReconciler(sc, dv, pvc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
snapclass, err := GetSnapshotClassForSmartClone(dv.Name, dv.Spec.PVC.StorageClassName, reconciler.log, reconciler.client)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -440,7 +440,7 @@ var _ = Describe("All DataVolume Tests", func() {
[]cdiv1.ClaimPropertySet{{AccessModes: accessMode, VolumeMode: &BlockMode}},
&strategy)
snapshotClassName := "snap-class"
snapClass := createSnapshotClass(snapshotClassName, nil, "csi-plugin")
snapClass := CreateSnapshotClass(snapshotClassName, nil, "csi-plugin")

srcPvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
targetPvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
Expand Down Expand Up @@ -1077,20 +1077,6 @@ func (f *fakeControllerStarter) Start(ctx context.Context) error {
func (f *fakeControllerStarter) StartController() {
}

func createSnapshotClass(name string, annotations map[string]string, snapshotter string) *snapshotv1.VolumeSnapshotClass {
return &snapshotv1.VolumeSnapshotClass{
TypeMeta: metav1.TypeMeta{
Kind: "VolumeSnapshotClass",
APIVersion: snapshotv1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Annotations: annotations,
},
Driver: snapshotter,
}
}

func createVolumeSnapshotContentCrd() *extv1.CustomResourceDefinition {
pluralName := "volumesnapshotcontents"
return &extv1.CustomResourceDefinition{
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/datavolume/snapshot-clone-controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ var _ = Describe("All DataVolume Tests", func() {

dv.Spec.PVC.StorageClassName = &scName
snapshot := createSnapshotInVolumeSnapshotClass("test-snap", metav1.NamespaceDefault, &expectedSnapshotClass, nil, nil, true)
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createSnapshotCloneReconciler(sc, sp, dv, snapshot, snapClass, createDefaultVolumeSnapshotContent(), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -141,7 +141,7 @@ var _ = Describe("All DataVolume Tests", func() {

dv.Spec.PVC.StorageClassName = &targetScName
snapshot := createSnapshotInVolumeSnapshotClass("test-snap", metav1.NamespaceDefault, &expectedSnapshotClass, nil, nil, true)
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createSnapshotCloneReconciler(sc, tsc, sp, sp2, dv, snapshot, snapClass, createDefaultVolumeSnapshotContent(), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -188,7 +188,7 @@ var _ = Describe("All DataVolume Tests", func() {

dv.Spec.PVC.StorageClassName = &targetScName
snapshot := createSnapshotInVolumeSnapshotClass("test-snap", metav1.NamespaceDefault, &expectedSnapshotClass, nil, nil, true)
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createSnapshotCloneReconciler(sc, scSameProvisioner, tsc, sp, sp2, sp3, dv, snapshot, snapClass, createDefaultVolumeSnapshotContent(), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -227,7 +227,7 @@ var _ = Describe("All DataVolume Tests", func() {
Name: "test-dv",
UID: dv.UID,
})
snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
snapClass := CreateSnapshotClass(expectedSnapshotClass, nil, "csi-plugin")
reconciler = createSnapshotCloneReconciler(sc, tsc, sp, sp2, dv, snapshot, tempHostAssistedPvc, targetPvc, snapClass, createDefaultVolumeSnapshotContent(), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
_, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand Down
95 changes: 82 additions & 13 deletions pkg/controller/storageprofile-controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"reflect"

"github.com/go-logr/logr"
snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
"github.com/prometheus/client_golang/prometheus"
v1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -84,8 +86,12 @@ func (r *StorageProfileReconciler) reconcileStorageProfile(sc *storagev1.Storage

storageProfile.Status.StorageClass = &sc.Name
storageProfile.Status.Provisioner = &sc.Provisioner
storageProfile.Status.CloneStrategy = r.reconcileCloneStrategy(sc, storageProfile.Spec.CloneStrategy)
storageProfile.Status.DataImportCronSourceFormat = r.reconcileDataImportCronSourceFormat(sc, storageProfile.Spec.DataImportCronSourceFormat)
snapClass, err := cc.GetSnapshotClassForSmartClone("", &sc.Name, r.log, r.client)
if err != nil {
return reconcile.Result{}, err
}
storageProfile.Status.CloneStrategy = r.reconcileCloneStrategy(sc, storageProfile.Spec.CloneStrategy, snapClass)
storageProfile.Status.DataImportCronSourceFormat = r.reconcileDataImportCronSourceFormat(sc, storageProfile.Spec.DataImportCronSourceFormat, snapClass)

var claimPropertySets []cdiv1.ClaimPropertySet

Expand Down Expand Up @@ -159,36 +165,67 @@ func (r *StorageProfileReconciler) reconcilePropertySets(sc *storagev1.StorageCl
return claimPropertySets
}

func (r *StorageProfileReconciler) reconcileCloneStrategy(sc *storagev1.StorageClass, desiredCloneStrategy *cdiv1.CDICloneStrategy) *cdiv1.CDICloneStrategy {
func (r *StorageProfileReconciler) reconcileCloneStrategy(sc *storagev1.StorageClass, desiredCloneStrategy *cdiv1.CDICloneStrategy, snapClass string) *cdiv1.CDICloneStrategy {
if desiredCloneStrategy != nil {
return desiredCloneStrategy
}

if annStrategyVal, ok := sc.Annotations["cdi.kubevirt.io/clone-strategy"]; ok {
return r.getCloneStrategyFromStorageClass(annStrategyVal)
}

// Default to trying snapshot clone unless volume snapshot class missing
hostAssistedStrategy := cdiv1.CloneStrategyHostAssisted
strategy := hostAssistedStrategy
if snapClass != "" {
strategy = cdiv1.CloneStrategySnapshot
}

if knownStrategy, ok := storagecapabilities.GetAdvisedCloneStrategy(sc); ok {
strategy = knownStrategy
}

if strategy == cdiv1.CloneStrategySnapshot && snapClass == "" {
r.log.Info("No VolumeSnapshotClass found for storage class, falling back to host assisted cloning", "StorageClass.Name", sc.Name)
return &hostAssistedStrategy
}

return &strategy
}

func (r *StorageProfileReconciler) getCloneStrategyFromStorageClass(annStrategyVal string) *cdiv1.CDICloneStrategy {
var strategy cdiv1.CDICloneStrategy
if sc.Annotations["cdi.kubevirt.io/clone-strategy"] == "copy" {

switch annStrategyVal {
case "copy":
strategy = cdiv1.CloneStrategyHostAssisted
} else if sc.Annotations["cdi.kubevirt.io/clone-strategy"] == "snapshot" {
case "snapshot":
strategy = cdiv1.CloneStrategySnapshot
} else if sc.Annotations["cdi.kubevirt.io/clone-strategy"] == "csi-clone" {
case "csi-clone":
strategy = cdiv1.CloneStrategyCsiClone
} else {
return nil
}

return &strategy
}

func (r *StorageProfileReconciler) reconcileDataImportCronSourceFormat(sc *storagev1.StorageClass, desiredFormat *cdiv1.DataImportCronSourceFormat) *cdiv1.DataImportCronSourceFormat {
func (r *StorageProfileReconciler) reconcileDataImportCronSourceFormat(sc *storagev1.StorageClass, desiredFormat *cdiv1.DataImportCronSourceFormat, snapClass string) *cdiv1.DataImportCronSourceFormat {
if desiredFormat != nil {
return desiredFormat
}

// This can be changed later on
// for example, if at some point we're confident snapshot sources should be the default
defaultFormat := cdiv1.DataImportCronSourceFormatPvc
format, ok := storagecapabilities.GetAdvisedSourceFormat(sc)
if !ok {
return &defaultFormat
pvcFormat := cdiv1.DataImportCronSourceFormatPvc
format := pvcFormat

if knownFormat, ok := storagecapabilities.GetAdvisedSourceFormat(sc); ok {
format = knownFormat
}

if format == cdiv1.DataImportCronSourceFormatSnapshot && snapClass == "" {
// No point using snapshots without a corresponding snapshot class
r.log.Info("No VolumeSnapshotClass found for storage class, falling back to pvc sources for DataImportCrons", "StorageClass.Name", sc.Name)
return &pvcFormat
}

return &format
Expand Down Expand Up @@ -303,9 +340,11 @@ func addStorageProfileControllerWatches(mgr manager.Manager, c controller.Contro
if err := c.Watch(&source.Kind{Type: &storagev1.StorageClass{}}, &handler.EnqueueRequestForObject{}); err != nil {
return err
}

if err := c.Watch(&source.Kind{Type: &cdiv1.StorageProfile{}}, &handler.EnqueueRequestForObject{}); err != nil {
return err
}

if err := c.Watch(&source.Kind{Type: &v1.PersistentVolume{}}, handler.EnqueueRequestsFromMapFunc(
func(obj client.Object) []reconcile.Request {
return []reconcile.Request{{
Expand All @@ -320,6 +359,36 @@ func addStorageProfileControllerWatches(mgr manager.Manager, c controller.Contro
}); err != nil {
return err
}

mapSnapshotClassToProfile := func(obj client.Object) (reqs []reconcile.Request) {
var scList storagev1.StorageClassList
if err := mgr.GetClient().List(context.TODO(), &scList); err != nil {
c.GetLogger().Error(err, "Unable to list StorageClasses")
return
}
vsc := obj.(*snapshotv1.VolumeSnapshotClass)
for _, sc := range scList.Items {
if sc.Provisioner == vsc.Driver {
reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Name: sc.Name}})
}
}
return
}
if err := mgr.GetClient().List(context.TODO(), &snapshotv1.VolumeSnapshotClassList{}, &client.ListOptions{Limit: 1}); err != nil {
if meta.IsNoMatchError(err) {
// Back out if there's no point to attempt watch
return nil
}
if !cc.IsErrCacheNotStarted(err) {
return err
}
}
if err := c.Watch(&source.Kind{Type: &snapshotv1.VolumeSnapshotClass{}},
handler.EnqueueRequestsFromMapFunc(mapSnapshotClassToProfile),
); err != nil {
return err
}

return nil
}

Expand Down

0 comments on commit ed3de0a

Please sign in to comment.