Skip to content

Commit

Permalink
Changes to pick largest Restore Size (#1177)
Browse files Browse the repository at this point in the history
* changes to pick largest Restore Size

* check args restoreSize first

* lint issue

* additional lint fix

* final lint change

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
bathina2 and mergify[bot] committed Jan 11, 2022
1 parent 403dfd4 commit 83a76a3
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 21 deletions.
62 changes: 41 additions & 21 deletions pkg/kube/volume/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,25 +119,9 @@ type CreatePVCFromSnapshotArgs struct {
// CreatePVCFromSnapshot will restore a volume and returns the resulting
// PersistentVolumeClaim and any error that happened in the process.
func CreatePVCFromSnapshot(ctx context.Context, args *CreatePVCFromSnapshotArgs) (string, error) {
var size *resource.Quantity
if args.RestoreSize == "" {
sns, err := snapshot.NewSnapshotter(args.KubeCli, args.DynCli)
if err != nil {
return "", err
}
snap, err := sns.Get(ctx, args.SnapshotName, args.Namespace)
if err != nil {
return "", err
}

size = snap.Status.RestoreSize
} else {
s := resource.MustParse(args.RestoreSize)
size = &s
}

if size == nil {
return "", fmt.Errorf("Restore size is empty and no restore size argument given, Volumesnapshot: %s", args.SnapshotName)
storageSize, err := getPVCRestoreSize(ctx, args)
if err != nil {
return "", errors.Wrap(err, "Failed to get PVC restore size")
}

if len(args.AccessModes) == 0 {
Expand All @@ -159,7 +143,7 @@ func CreatePVCFromSnapshot(ctx context.Context, args *CreatePVCFromSnapshotArgs)
},
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceStorage: *size,
v1.ResourceStorage: *storageSize,
},
},
},
Expand All @@ -173,7 +157,7 @@ func CreatePVCFromSnapshot(ctx context.Context, args *CreatePVCFromSnapshotArgs)
pvc.Spec.StorageClassName = &args.StorageClassName
}

pvc, err := args.KubeCli.CoreV1().PersistentVolumeClaims(args.Namespace).Create(ctx, pvc, metav1.CreateOptions{})
pvc, err = args.KubeCli.CoreV1().PersistentVolumeClaims(args.Namespace).Create(ctx, pvc, metav1.CreateOptions{})
if err != nil {
if args.VolumeName != "" && apierrors.IsAlreadyExists(err) {
return args.VolumeName, nil
Expand All @@ -183,6 +167,42 @@ func CreatePVCFromSnapshot(ctx context.Context, args *CreatePVCFromSnapshotArgs)
return pvc.Name, err
}

func getPVCRestoreSize(ctx context.Context, args *CreatePVCFromSnapshotArgs) (*resource.Quantity, error) {
quantities := []*resource.Quantity{}

if args.RestoreSize != "" {
s, err := resource.ParseQuantity(args.RestoreSize)
if err != nil {
return nil, fmt.Errorf("Failed to parse quantity (%s)", args.RestoreSize)
}
quantities = append(quantities, &s)
}

sns, err := snapshot.NewSnapshotter(args.KubeCli, args.DynCli)
if err != nil {
return nil, errors.Wrap(err, "Failed to get snapshotter")
}
snap, err := sns.Get(ctx, args.SnapshotName, args.Namespace)
if err != nil {
return nil, errors.Wrap(err, "Failed to get snapshot")
}
if snap.Status != nil && snap.Status.RestoreSize != nil {
quantities = append(quantities, snap.Status.RestoreSize)
}

if len(quantities) == 0 {
return nil, fmt.Errorf("Restore size is empty and no restore size argument given, Volumesnapshot: %s", args.SnapshotName)
}

quantity := quantities[0]
for _, q := range quantities {
if q.Value() > quantity.Value() {
quantity = q
}
}
return quantity, nil
}

// CreatePV creates a PersistentVolume and returns its name
// For retry idempotency, checks whether PV associated with volume already exists
func CreatePV(ctx context.Context, kubeCli kubernetes.Interface, vol *blockstorage.Volume, volType blockstorage.Type, annotations map[string]string, accessmodes []v1.PersistentVolumeAccessMode, volumemode *v1.PersistentVolumeMode) (string, error) {
Expand Down
134 changes: 134 additions & 0 deletions pkg/kube/volume/volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,19 @@ package volume

import (
"context"
"fmt"
"path/filepath"
"reflect"
"testing"

. "gopkg.in/check.v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
dynfake "k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes/fake"
)

Expand Down Expand Up @@ -65,3 +71,131 @@ func (s *TestVolSuite) TestCreatePVC(c *C) {
c.Assert(len(pvc2.Spec.AccessModes) >= 1, Equals, true)
c.Assert(*pvc2.Spec.VolumeMode, Equals, v1.PersistentVolumeBlock)
}

func (s *TestVolSuite) TestGetPVCRestoreSize(c *C) {
ctx := context.Background()
scheme := runtime.NewScheme()
scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "snapshot.storage.k8s.io", Version: "v1", Kind: "VolumeSnapshotList"}, &unstructured.UnstructuredList{})
fakeCli := fake.NewSimpleClientset()
fakeCli.Resources = []*metav1.APIResourceList{{
TypeMeta: metav1.TypeMeta{
Kind: "VolumeSnapshot",
APIVersion: "v1",
},
GroupVersion: "snapshot.storage.k8s.io/v1",
}}
for _, tc := range []struct {
args *CreatePVCFromSnapshotArgs
sizeValue int64
errChecker Checker
}{
{ // only snapshot restore size
args: &CreatePVCFromSnapshotArgs{
KubeCli: fakeCli,
DynCli: dynfake.NewSimpleDynamicClient(scheme,
s.fakeUnstructuredSnasphotWSize("vsName", "vsNamespace", "10Gi")),
SnapshotName: "vsName",
Namespace: "vsNamespace",
},
sizeValue: 10737418240,
errChecker: IsNil,
},
{ // only args restore size
args: &CreatePVCFromSnapshotArgs{
KubeCli: fakeCli,
DynCli: dynfake.NewSimpleDynamicClient(scheme,
s.fakeUnstructuredSnasphotWSize("vsName", "vsNamespace", "")),
SnapshotName: "vsName",
Namespace: "vsNamespace",
RestoreSize: "10Gi",
},
sizeValue: 10737418240,
errChecker: IsNil,
},
{ // neither
args: &CreatePVCFromSnapshotArgs{
KubeCli: fakeCli,
DynCli: dynfake.NewSimpleDynamicClient(scheme,
s.fakeUnstructuredSnasphotWSize("vsName", "vsNamespace", "")),
SnapshotName: "vsName",
Namespace: "vsNamespace",
},
errChecker: NotNil,
},
{ // both, snapshot size is bigger
args: &CreatePVCFromSnapshotArgs{
KubeCli: fakeCli,
DynCli: dynfake.NewSimpleDynamicClient(scheme,
s.fakeUnstructuredSnasphotWSize("vsName", "vsNamespace", "10Gi")),
SnapshotName: "vsName",
Namespace: "vsNamespace",
RestoreSize: "9Gi",
},
sizeValue: 10737418240,
errChecker: IsNil,
},
{ // both, args size is bigger
args: &CreatePVCFromSnapshotArgs{
KubeCli: fakeCli,
DynCli: dynfake.NewSimpleDynamicClient(scheme,
s.fakeUnstructuredSnasphotWSize("vsName1", "vsNamespace1", "9Gi")),
SnapshotName: "vsName1",
Namespace: "vsNamespace1",
RestoreSize: "10Gi",
},
sizeValue: 10737418240,
errChecker: IsNil,
},
{ // Failed to find snapshot
args: &CreatePVCFromSnapshotArgs{
KubeCli: fakeCli,
DynCli: dynfake.NewSimpleDynamicClient(scheme),
SnapshotName: "vsName",
Namespace: "vsNamespace",
},
errChecker: NotNil,
},
{ // Failed to create snapshotter
args: &CreatePVCFromSnapshotArgs{
KubeCli: fake.NewSimpleClientset(), // fails to find dynamic api
DynCli: dynfake.NewSimpleDynamicClient(scheme),
SnapshotName: "vsName",
Namespace: "vsNamespace",
},
errChecker: NotNil,
},
{ // bad args restore size
args: &CreatePVCFromSnapshotArgs{
SnapshotName: "vsName",
Namespace: "vsNamespace",
RestoreSize: "10wut",
},
errChecker: NotNil,
},
} {
q, err := getPVCRestoreSize(ctx, tc.args)
c.Assert(err, tc.errChecker)
if tc.errChecker == IsNil {
c.Assert(q.Value(), Equals, tc.sizeValue)
}
}
}

func (s *TestVolSuite) fakeUnstructuredSnasphotWSize(vsName, namespace, size string) *unstructured.Unstructured {
gvr := schema.GroupVersionResource{Group: "snapshot.storage.k8s.io", Version: "v1", Resource: "volumesnapshots"}
Object := map[string]interface{}{
"apiVersion": fmt.Sprintf("%s/%s", gvr.Group, gvr.Version),
"kind": "VolumeSnapshot",
"metadata": map[string]interface{}{
"name": vsName,
"namespace": namespace,
},
}
if size != "" {
q := resource.MustParse(size)
Object["status"] = map[string]interface{}{
"restoreSize": q.ToUnstructured(),
}
}
return &unstructured.Unstructured{Object: Object}
}

0 comments on commit 83a76a3

Please sign in to comment.