Skip to content

Commit

Permalink
more unit tests
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
  • Loading branch information
mhenriks committed May 11, 2023
1 parent 7c403ff commit b2c15d8
Show file tree
Hide file tree
Showing 16 changed files with 949 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cmd/cdi-controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func start() {
os.Exit(1)
}
if _, err := populators.NewClonePopulator(ctx, mgr, log, clonerImage, pullPolicy, installerLabels); err != nil {
klog.Errorf("Unable to setup import populator: %v", err)
klog.Errorf("Unable to setup clone populator: %v", err)
os.Exit(1)
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/clone/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ go_test(
"clone_suite_test.go",
"csi-clone_test.go",
"host-clone_test.go",
"planner_test.go",
"prep-claim_test.go",
"rebind_test.go",
"snap-clone_test.go",
"snapshot_test.go",
],
embed = [":go_default_library"],
deps = [
Expand All @@ -61,6 +64,7 @@ go_test(
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
"//vendor/k8s.io/client-go/kubernetes/scheme:go_default_library",
"//vendor/k8s.io/client-go/tools/record:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
Expand Down
14 changes: 14 additions & 0 deletions pkg/controller/clone/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,20 @@ func GetStorageClassForClaim(ctx context.Context, c client.Client, pvc *corev1.P
return nil, nil
}

// MustGetStorageClassForClaim returns the storageclass for a PVC
func MustGetStorageClassForClaim(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (*storagev1.StorageClass, error) {
sc, err := GetStorageClassForClaim(ctx, c, pvc)
if err != nil {
return nil, err
}

if sc == nil {
return nil, fmt.Errorf("no storageclass for pvc")
}

return sc, nil
}

// GetCompatibleVolumeSnapshotClass returns a VolumeSNapshotClass name that works for all PVCs
func GetCompatibleVolumeSnapshotClass(ctx context.Context, c client.Client, pvcs ...*corev1.PersistentVolumeClaim) (*string, error) {
var storageClasses []*storagev1.StorageClass
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/clone/csi-clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (p *CSIClonePhase) Name() string {
return "CSIClone"
}

// Reconcile ensures a snapshot is created correctly
// Reconcile ensures a csi cloned pvc is created correctly
func (p *CSIClonePhase) Reconcile(ctx context.Context) (*reconcile.Result, error) {
pvc := &corev1.PersistentVolumeClaim{}
exists, err := getResource(ctx, p.Client, p.Namespace, p.DesiredClaim.Name, pvc)
Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/clone/host-clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ type HostClonePhase struct {
DesiredClaim *corev1.PersistentVolumeClaim
ImmediateBind bool
OwnershipLabel string
Preallocation bool
ContentType string
Client client.Client
Log logr.Logger
Recorder record.EventRecorder
Expand Down Expand Up @@ -101,6 +103,8 @@ func (p *HostClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVol
claim := p.DesiredClaim.DeepCopy()

claim.Namespace = p.Namespace
cc.AddAnnotation(claim, cc.AnnContentType, p.ContentType)
cc.AddAnnotation(claim, cc.AnnPreallocationRequested, fmt.Sprintf("%t", p.Preallocation))
cc.AddAnnotation(claim, cc.AnnOwnerUID, string(p.Owner.GetUID()))
cc.AddAnnotation(claim, cc.AnnPodRestarts, "0")
cc.AddAnnotation(claim, cc.AnnCloneRequest, fmt.Sprintf("%s/%s", p.Namespace, p.SourceName))
Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/clone/host-clone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ var _ = Describe("HostClonePhase test", func() {
SourceName: "source",
DesiredClaim: desired,
ImmediateBind: true,
ContentType: "kubevirt",
Preallocation: false,
Client: cl,
Recorder: rec,
Log: log,
Expand All @@ -99,6 +101,8 @@ var _ = Describe("HostClonePhase test", func() {

pvc := getDesiredClaim(p)
Expect(pvc.Spec.DataSourceRef).To(BeNil())
Expect(pvc.Annotations[cc.AnnContentType]).To(Equal("kubevirt"))
Expect(pvc.Annotations[cc.AnnPreallocationRequested]).To(Equal("false"))
Expect(pvc.Annotations[cc.AnnOwnerUID]).To(Equal(string(p.Owner.GetUID())))
Expect(pvc.Annotations[cc.AnnPodRestarts]).To(Equal("0"))
Expect(pvc.Annotations[cc.AnnCloneRequest]).To(Equal("ns/source"))
Expand Down
24 changes: 16 additions & 8 deletions pkg/controller/clone/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (p *Planner) watchOwned(log logr.Logger, obj client.Object) error {
}

func (p *Planner) strategyForSourcePVC(ctx context.Context, args *ChooseStrategyArgs) (*cdiv1.CDICloneStrategy, error) {
if ok, err := p.validateTargetStorageClassAssignment(args); !ok || err != nil {
if ok, err := p.validateTargetStorageClassAssignment(ctx, args); !ok || err != nil {
return nil, err
}

Expand Down Expand Up @@ -239,7 +239,7 @@ func (p *Planner) strategyForSourcePVC(ctx context.Context, args *ChooseStrategy
}

if !exists {
args.Log.V(1).Info("missing storageprofile for", "name", *args.TargetClaim.Spec.StorageClassName)
args.Log.V(3).Info("missing storageprofile for", "name", *args.TargetClaim.Spec.StorageClassName)
}

if exists && sp.Status.CloneStrategy != nil {
Expand Down Expand Up @@ -273,7 +273,7 @@ func (p *Planner) strategyForSourcePVC(ctx context.Context, args *ChooseStrategy
return &strategy, nil
}

func (p *Planner) validateTargetStorageClassAssignment(args *ChooseStrategyArgs) (bool, error) {
func (p *Planner) validateTargetStorageClassAssignment(ctx context.Context, args *ChooseStrategyArgs) (bool, error) {
if args.TargetClaim.Spec.StorageClassName == nil {
args.Log.V(3).Info("Target PVC has nil storage class, cannot compute strategy")
return false, nil
Expand All @@ -284,6 +284,11 @@ func (p *Planner) validateTargetStorageClassAssignment(args *ChooseStrategyArgs)
return false, fmt.Errorf("claim has emptystring storageclass, will not work")
}

_, err := MustGetStorageClassForClaim(ctx, p.Client, args.TargetClaim)
if err != nil {
return false, err
}

return true, nil
}

Expand All @@ -305,15 +310,11 @@ func (p *Planner) validateAdvancedClonePVC(ctx context.Context, args *ChooseStra
return false, nil
}

sc, err := GetStorageClassForClaim(ctx, p.Client, args.TargetClaim)
sc, err := MustGetStorageClassForClaim(ctx, p.Client, args.TargetClaim)
if err != nil {
return false, err
}

if sc == nil {
return false, fmt.Errorf("target storageclass should exist")
}

srcCapacity, hasSrcCapacity := sourceClaim.Status.Capacity[corev1.ResourceStorage]
targetRequest, hasTargetRequest := args.TargetClaim.Spec.Resources.Requests[corev1.ResourceStorage]
allowExpansion := sc.AllowVolumeExpansion != nil && *sc.AllowVolumeExpansion
Expand Down Expand Up @@ -344,13 +345,20 @@ func (p *Planner) planHostAssistedFromPVC(ctx context.Context, args *PlanArgs) (
desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] = is
}

ct := cdiv1.DataVolumeKubeVirt
if args.DataSource.Spec.ContentType != "" {
ct = args.DataSource.Spec.ContentType
}

hcp := &HostClonePhase{
Owner: args.TargetClaim,
Namespace: args.DataSource.Namespace,
SourceName: args.DataSource.Spec.Source.Name,
DesiredClaim: desiredClaim,
ImmediateBind: true,
OwnershipLabel: p.OwnershipLabel,
ContentType: string(ct),
Preallocation: cc.GetPreallocation(ctx, p.Client, args.DataSource.Spec.Preallocation),
Client: p.Client,
Log: args.Log,
Recorder: p.Recorder,
Expand Down
Loading

0 comments on commit b2c15d8

Please sign in to comment.