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 5, 2023
1 parent a36f258 commit b0e2ff2
Show file tree
Hide file tree
Showing 4 changed files with 240 additions and 20 deletions.
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
140 changes: 136 additions & 4 deletions pkg/controller/storageprofile-controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package controller
import (
"context"
"fmt"
"reflect"

snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
. "github.com/onsi/ginkgo"
"github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
Expand All @@ -30,6 +32,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

v1 "k8s.io/api/core/v1"
extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -311,9 +314,14 @@ var _ = Describe("Storage profile controller reconcile loop", func() {
table.Entry("Clone", cdiv1.CloneStrategyCsiClone),
)

table.DescribeTable("should set advised source format for dataimportcrons", func(provisioner string, expectedFormat cdiv1.DataImportCronSourceFormat) {
table.DescribeTable("should set advised source format for dataimportcrons", func(provisioner string, expectedFormat cdiv1.DataImportCronSourceFormat, deploySnapClass bool) {
storageClass := CreateStorageClassWithProvisioner(storageClassName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, provisioner)
reconciler := createStorageProfileReconciler(storageClass)
reconciler := createStorageProfileReconciler(storageClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
if deploySnapClass {
snapClass := createSnapshotClass(storageClassName+"-snapclass", nil, provisioner)
err := reconciler.client.Create(context.TODO(), snapClass)
Expect(err).ToNot(HaveOccurred())
}
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: storageClassName}})
Expect(err).ToNot(HaveOccurred())

Expand All @@ -327,8 +335,35 @@ var _ = Describe("Storage profile controller reconcile loop", func() {
Expect(*sp.Status.StorageClass).To(Equal(storageClassName))
Expect(*sp.Status.DataImportCronSourceFormat).To(Equal(expectedFormat))
},
table.Entry("provisioners where snapshot source is more appropriate", "rook-ceph.rbd.csi.ceph.com", cdiv1.DataImportCronSourceFormatSnapshot),
table.Entry("provisioners where there is no known preferred format", "format.unknown.provisioner.csi.com", cdiv1.DataImportCronSourceFormatPvc),
table.Entry("provisioners where snapshot source is more appropriate", "rook-ceph.rbd.csi.ceph.com", cdiv1.DataImportCronSourceFormatSnapshot, true),
table.Entry("provisioners where snapshot source is more appropriate but lack volumesnapclass", "rook-ceph.rbd.csi.ceph.com", cdiv1.DataImportCronSourceFormatPvc, false),
table.Entry("provisioners where there is no known preferred format", "format.unknown.provisioner.csi.com", cdiv1.DataImportCronSourceFormatPvc, false),
)

table.DescribeTable("should set cloneStrategy", func(provisioner string, expectedCloneStrategy cdiv1.CDICloneStrategy, deploySnapClass bool) {
storageClass := CreateStorageClassWithProvisioner(storageClassName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, provisioner)
reconciler := createStorageProfileReconciler(storageClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
if deploySnapClass {
snapClass := createSnapshotClass(storageClassName+"-snapclass", nil, provisioner)
err := reconciler.client.Create(context.TODO(), snapClass)
Expect(err).ToNot(HaveOccurred())
}
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: storageClassName}})
Expect(err).ToNot(HaveOccurred())

storageProfileList := &cdiv1.StorageProfileList{}
err = reconciler.client.List(context.TODO(), storageProfileList, &client.ListOptions{})

Expect(err).ToNot(HaveOccurred())
Expect(storageProfileList.Items).To(HaveLen(1))

sp := storageProfileList.Items[0]
Expect(*sp.Status.StorageClass).To(Equal(storageClassName))
Expect(*sp.Status.CloneStrategy).To(Equal(expectedCloneStrategy))
},
table.Entry("provisioner with volumesnapshotclass and no known advised strategy", "strategy.unknown.provisioner.csi.com", cdiv1.CloneStrategySnapshot, true),
table.Entry("provisioner without volumesnapshotclass and no known advised strategy", "strategy.unknown.provisioner.csi.com", cdiv1.CloneStrategyHostAssisted, false),
table.Entry("provisioner that is known to prefer csi clone", "csi-powermax.dellemc.com", cdiv1.CloneStrategyCsiClone, false),
)

table.DescribeTable("Should set the IncompleteProfileGauge correctly", func(provisioner string, count int) {
Expand Down Expand Up @@ -364,6 +399,8 @@ func createStorageProfileReconciler(objects ...runtime.Object) *StorageProfileRe
// Register operator types with the runtime scheme.
s := scheme.Scheme
_ = cdiv1.AddToScheme(s)
_ = snapshotv1.AddToScheme(s)
_ = extv1.AddToScheme(s)

// Create a fake client to mock API calls.
cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(objs...).Build()
Expand Down Expand Up @@ -400,3 +437,98 @@ func CreatePv(name string, storageClassName string) *v1.PersistentVolume {
}
return pv
}

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{
TypeMeta: metav1.TypeMeta{
Kind: "CustomResourceDefinition",
APIVersion: extv1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: pluralName + "." + snapshotv1.GroupName,
},
Spec: extv1.CustomResourceDefinitionSpec{
Group: snapshotv1.GroupName,
Scope: extv1.ClusterScoped,
Names: extv1.CustomResourceDefinitionNames{
Plural: pluralName,
Kind: reflect.TypeOf(snapshotv1.VolumeSnapshotContent{}).Name(),
},
Versions: []extv1.CustomResourceDefinitionVersion{
{
Name: snapshotv1.SchemeGroupVersion.Version,
Served: true,
},
},
},
}
}

func createVolumeSnapshotClassCrd() *extv1.CustomResourceDefinition {
pluralName := "volumesnapshotclasses"
return &extv1.CustomResourceDefinition{
TypeMeta: metav1.TypeMeta{
Kind: "CustomResourceDefinition",
APIVersion: extv1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: pluralName + "." + snapshotv1.GroupName,
},
Spec: extv1.CustomResourceDefinitionSpec{
Group: snapshotv1.GroupName,
Scope: extv1.ClusterScoped,
Names: extv1.CustomResourceDefinitionNames{
Plural: pluralName,
Kind: reflect.TypeOf(snapshotv1.VolumeSnapshotClass{}).Name(),
},
Versions: []extv1.CustomResourceDefinitionVersion{
{
Name: snapshotv1.SchemeGroupVersion.Version,
Served: true,
},
},
},
}
}

func createVolumeSnapshotCrd() *extv1.CustomResourceDefinition {
pluralName := "volumesnapshots"
return &extv1.CustomResourceDefinition{
TypeMeta: metav1.TypeMeta{
Kind: "CustomResourceDefinition",
APIVersion: extv1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: pluralName + "." + snapshotv1.GroupName,
},
Spec: extv1.CustomResourceDefinitionSpec{
Group: snapshotv1.GroupName,
Scope: extv1.NamespaceScoped,
Names: extv1.CustomResourceDefinitionNames{
Plural: pluralName,
Kind: reflect.TypeOf(snapshotv1.VolumeSnapshot{}).Name(),
},
Versions: []extv1.CustomResourceDefinitionVersion{
{
Name: snapshotv1.SchemeGroupVersion.Version,
Served: true,
},
},
},
}
}
17 changes: 17 additions & 0 deletions pkg/storagecapabilities/storagecapabilities.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ var SourceFormatsByProvisionerKey = map[string]cdiv1.DataImportCronSourceFormat{
"openshift-storage.rbd.csi.ceph.com": cdiv1.DataImportCronSourceFormatSnapshot,
}

// CloneStrategyByProvisionerKey defines the advised clone strategy for a provisioner
var CloneStrategyByProvisionerKey = map[string]cdiv1.CDICloneStrategy{
"csi-vxflexos.dellemc.com": cdiv1.CloneStrategyCsiClone,
"csi-isilon.dellemc.com": cdiv1.CloneStrategyCsiClone,
"csi-powermax.dellemc.com": cdiv1.CloneStrategyCsiClone,
"csi-powerstore.dellemc.com": cdiv1.CloneStrategyCsiClone,
"hspc.csi.hitachi.com": cdiv1.CloneStrategyCsiClone,
"csi.hpe.com": cdiv1.CloneStrategyCsiClone,
}

// ProvisionerNoobaa is the provisioner string for the Noobaa object bucket provisioner which does not work with CDI
const ProvisionerNoobaa = "openshift-storage.noobaa.io/obc"

Expand Down Expand Up @@ -133,6 +143,13 @@ func GetAdvisedSourceFormat(sc *storagev1.StorageClass) (cdiv1.DataImportCronSou
return format, found
}

// GetAdvisedCloneStrategy finds and returns the advised clone strategy
func GetAdvisedCloneStrategy(sc *storagev1.StorageClass) (cdiv1.CDICloneStrategy, bool) {
provisionerKey := storageProvisionerKey(sc)
strategy, found := CloneStrategyByProvisionerKey[provisionerKey]
return strategy, found
}

func isLocalStorageOperator(sc *storagev1.StorageClass) bool {
_, found := sc.Labels["local.storage.openshift.io/owner-name"]
return found
Expand Down

0 comments on commit b0e2ff2

Please sign in to comment.