Skip to content

Commit

Permalink
Copy volumeSnapshotClass parameters when cloning (#1128)
Browse files Browse the repository at this point in the history
* Copy snapshot parameters

* nil check

* add comment describing new function

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
bathina2 and mergify[bot] committed Nov 3, 2021
1 parent 240b5a3 commit a5bb37d
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 30 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ require (
github.com/dnaeon/go-vcr v1.0.1 // indirect
github.com/efarrer/iothrottler v0.0.2 // indirect
github.com/elazarl/goproxy v0.0.0-20190711103511-473e67f1d7d2 // indirect
github.com/elazarl/goproxy/ext v0.0.0-20210801061803-8e322dfb79c4 // indirect
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
github.com/go-openapi/strfmt v0.19.3
github.com/go-sql-driver/mysql v1.6.0
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ github.com/efarrer/iothrottler v0.0.2/go.mod h1:zGWF5N0NKSCskcPFytDAFwI121DdU/Nf
github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
github.com/elazarl/goproxy v0.0.0-20190711103511-473e67f1d7d2 h1:aZtFdDNWY/yH86JPR2WX/PN63635VsE/f/nXNPAbYxY=
github.com/elazarl/goproxy v0.0.0-20190711103511-473e67f1d7d2/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
github.com/elazarl/goproxy/ext v0.0.0-20210801061803-8e322dfb79c4 h1:jLY5dBmGEgdHm34NvYB0jQC+M4D+C0cB1C8/5aRdPcw=
github.com/elazarl/goproxy/ext v0.0.0-20210801061803-8e322dfb79c4/go.mod h1:gNh8nYJoAm43RfaxurUnxr+N1PwuFV3ZMl/efxlIlY8=
github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
github.com/emicklei/go-restful v2.9.5+incompatible h1:spTtZBk5DYEvbxMVutUuTyh1Ao2r4iyvLdACqsl/Ljk=
github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
Expand Down Expand Up @@ -317,6 +319,7 @@ github.com/franela/goreq v0.0.0-20171204163338-bcd34c9993f8/go.mod h1:ZhphrRTfi2
github.com/frankban/quicktest v1.10.2 h1:19ARM85nVi4xH7xPXuc5eM/udya5ieh7b/Sv+d844Tk=
github.com/frankban/quicktest v1.10.2/go.mod h1:K+q6oSqb0W0Ininfk863uOk1lMy69l/P6txr3mVT54s=
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
github.com/fvbommel/sortorder v1.0.1/go.mod h1:uk88iVf1ovNn1iLfgUVU2F9o5eO30ui720w+kxuqRs0=
github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
Expand Down Expand Up @@ -934,6 +937,7 @@ github.com/renier/xmlrpc v0.0.0-20170708154548-ce4a1a486c03/go.mod h1:gRAiPF5C5N
github.com/rjeczalik/notify v0.9.2/go.mod h1:aErll2f0sUX9PXZnVNyeiObbmTlk5jnMoCa4QEjJeqM=
github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
github.com/rogpeppe/go-charset v0.0.0-20180617210344-2471d30d28b4/go.mod h1:qgYeAmZ5ZIpBWTGllZSQnw97Dj+woV0toclVaRGI8pc=
github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.2.2/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
Expand Down
5 changes: 3 additions & 2 deletions pkg/kube/snapshot/snapshot_alpha.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (sna *SnapshotAlpha) CloneVolumeSnapshotClass(ctx context.Context, sourceCl
for _, key := range excludeAnnotations {
delete(existingAnnotations, key)
}
usNew := UnstructuredVolumeSnapshotClassAlpha(targetClassName, sourceSnapClass.Snapshotter, newDeletionPolicy)
usNew := UnstructuredVolumeSnapshotClassAlpha(targetClassName, sourceSnapClass.Snapshotter, newDeletionPolicy, sourceSnapClass.Parameters)
// Set Annotations/Labels
usNew.SetAnnotations(existingAnnotations)
usNew.SetLabels(map[string]string{CloneVolumeSnapshotClassLabelName: sourceClassName})
Expand Down Expand Up @@ -421,7 +421,7 @@ func UnstructuredVolumeSnapshotContentAlpha(name, snapshotName, snapshotNs, dele
}
}

func UnstructuredVolumeSnapshotClassAlpha(name, driver, deletionPolicy string) *unstructured.Unstructured {
func UnstructuredVolumeSnapshotClassAlpha(name, driver, deletionPolicy string, params map[string]string) *unstructured.Unstructured {
return &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": fmt.Sprintf("%s/%s", v1alpha1.GroupName, v1alpha1.Version),
Expand All @@ -431,6 +431,7 @@ func UnstructuredVolumeSnapshotClassAlpha(name, driver, deletionPolicy string) *
},
VolSnapClassAlphaDriverKey: driver,
"deletionPolicy": deletionPolicy,
"parameters": Mss2msi(params),
},
}
}
Expand Down
18 changes: 16 additions & 2 deletions pkg/kube/snapshot/snapshot_beta.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func cloneSnapshotClass(ctx context.Context, dynCli dynamic.Interface, snapClass
for _, key := range excludeAnnotations {
delete(existingAnnotations, key)
}
usNew := UnstructuredVolumeSnapshotClass(snapClassGVR, targetClassName, sourceSnapClass.Driver, newDeletionPolicy)
usNew := UnstructuredVolumeSnapshotClass(snapClassGVR, targetClassName, sourceSnapClass.Driver, newDeletionPolicy, sourceSnapClass.Parameters)
// Set Annotations/Labels
usNew.SetAnnotations(existingAnnotations)
usNew.SetLabels(map[string]string{CloneVolumeSnapshotClassLabelName: sourceClassName})
Expand Down Expand Up @@ -401,7 +401,7 @@ func UnstructuredVolumeSnapshotContent(gvr schema.GroupVersionResource, name, sn
}
}

func UnstructuredVolumeSnapshotClass(gvr schema.GroupVersionResource, name, driver, deletionPolicy string) *unstructured.Unstructured {
func UnstructuredVolumeSnapshotClass(gvr schema.GroupVersionResource, name, driver, deletionPolicy string, params map[string]string) *unstructured.Unstructured {
return &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": fmt.Sprintf("%s/%s", gvr.Group, gvr.Version),
Expand All @@ -411,6 +411,20 @@ func UnstructuredVolumeSnapshotClass(gvr schema.GroupVersionResource, name, driv
},
VolSnapClassBetaDriverKey: driver,
"deletionPolicy": deletionPolicy,
"parameters": Mss2msi(params),
},
}
}

// Mss2msi takes a map of string:string and returns a string:inteface map.
// This is useful since the unstructured type take map[string]interface{} as values.
func Mss2msi(in map[string]string) map[string]interface{} {
if in == nil {
return nil
}
paramsMap := map[string]interface{}{}
for k, v := range in {
paramsMap[k] = v
}
return paramsMap
}
60 changes: 34 additions & 26 deletions pkg/kube/snapshot/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,23 +195,28 @@ func (s *SnapshotTestSuite) TestVolumeSnapshotClassCloneFake(c *C) {
)
dynCli := dynfake.NewSimpleDynamicClient(scheme)

fakeParams := map[string]string{
"param1": "value1",
"param2": "value2",
}

for _, tc := range []struct {
sourceSnapClassSpec *unstructured.Unstructured
snapClassGVR schema.GroupVersionResource
snapshotter snapshot.Snapshotter
}{
{
sourceSnapClassSpec: snapshot.UnstructuredVolumeSnapshotClassAlpha(fakeClass, fakeDriver, snapshot.DeletionPolicyDelete),
sourceSnapClassSpec: snapshot.UnstructuredVolumeSnapshotClassAlpha(fakeClass, fakeDriver, snapshot.DeletionPolicyDelete, fakeParams),
snapClassGVR: v1alpha1.VolSnapClassGVR,
snapshotter: snapshot.NewSnapshotAlpha(fakeCli, dynCli),
},
{
sourceSnapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, fakeClass, fakeDriver, snapshot.DeletionPolicyDelete),
sourceSnapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, fakeClass, fakeDriver, snapshot.DeletionPolicyDelete, fakeParams),
snapClassGVR: v1beta1.VolSnapClassGVR,
snapshotter: snapshot.NewSnapshotBeta(fakeCli, dynCli),
},
{
sourceSnapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, fakeClass, fakeDriver, snapshot.DeletionPolicyDelete),
sourceSnapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, fakeClass, fakeDriver, snapshot.DeletionPolicyDelete, fakeParams),
snapClassGVR: snapshot.VolSnapClassGVR,
snapshotter: snapshot.NewSnapshotStable(fakeCli, dynCli),
},
Expand All @@ -235,6 +240,9 @@ func (s *SnapshotTestSuite) TestVolumeSnapshotClassCloneFake(c *C) {
c.Assert(createdVSC.GetAnnotations(), DeepEquals, map[string]string{annotationKeyToKeep: "true"})
c.Assert(createdVSC.GetLabels(), DeepEquals, map[string]string{snapshot.CloneVolumeSnapshotClassLabelName: tc.sourceSnapClassSpec.GetName()})

// Parameters are set correctly
c.Assert(createdVSC.Object["parameters"], DeepEquals, snapshot.Mss2msi(fakeParams))

// Lookup by old annotation correctly returns the source VSC
scWithOldAnnotation, err := tc.snapshotter.GetVolumeSnapshotClass(ctx, annotationKeyToRemove, "true", fakeSC)
c.Assert(err, IsNil)
Expand Down Expand Up @@ -268,7 +276,7 @@ func (s *SnapshotTestSuite) TestVolumeSnapshotCloneFake(c *C) {
fakeSs snapshot.Snapshotter
}{
{
snapClassSpec: snapshot.UnstructuredVolumeSnapshotClassAlpha(fakeClass, fakeDriver, deletionPolicy),
snapClassSpec: snapshot.UnstructuredVolumeSnapshotClassAlpha(fakeClass, fakeDriver, deletionPolicy, nil),
snapClassGVR: v1alpha1.VolSnapClassGVR,
contentSpec: snapshot.UnstructuredVolumeSnapshotContentAlpha(fakeContentName, fakeSnapshotName, defaultNamespace, deletionPolicy, fakeDriver, fakeSnapshotHandle, fakeClass),
contentGVR: v1alpha1.VolSnapContentGVR,
Expand All @@ -278,7 +286,7 @@ func (s *SnapshotTestSuite) TestVolumeSnapshotCloneFake(c *C) {
fakeSs: snapshot.NewSnapshotAlpha(nil, dynCli),
},
{
snapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, fakeClass, fakeDriver, deletionPolicy),
snapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, fakeClass, fakeDriver, deletionPolicy, nil),
snapClassGVR: v1beta1.VolSnapClassGVR,
contentSpec: snapshot.UnstructuredVolumeSnapshotContent(v1beta1.VolSnapContentGVR, fakeContentName, fakeSnapshotName, defaultNamespace, deletionPolicy, fakeDriver, fakeSnapshotHandle, fakeClass),
contentGVR: v1beta1.VolSnapContentGVR,
Expand All @@ -289,7 +297,7 @@ func (s *SnapshotTestSuite) TestVolumeSnapshotCloneFake(c *C) {
fakeSs: snapshot.NewSnapshotBeta(nil, dynCli),
},
{
snapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, fakeClass, fakeDriver, deletionPolicy),
snapClassSpec: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, fakeClass, fakeDriver, deletionPolicy, nil),
snapClassGVR: snapshot.VolSnapClassGVR,
contentSpec: snapshot.UnstructuredVolumeSnapshotContent(snapshot.VolSnapContentGVR, fakeContentName, fakeSnapshotName, defaultNamespace, deletionPolicy, fakeDriver, fakeSnapshotHandle, fakeClass),
contentGVR: snapshot.VolSnapContentGVR,
Expand Down Expand Up @@ -625,9 +633,9 @@ func (s *SnapshotTestSuite) TestGetVolumeSnapshotClassFake(c *C) {
annotationKey: "test-1",
annotationValue: "true",
storageClassName: fakeSC,
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-1", fakeDriver, "Delete"),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-1", fakeDriver, "Delete"),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-1", fakeDriver, "Delete"),
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-1", fakeDriver, "Delete", nil),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-1", fakeDriver, "Delete", nil),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-1", fakeDriver, "Delete", nil),
testKey: "test-1",
testValue: "true",
check: IsNil,
Expand All @@ -637,9 +645,9 @@ func (s *SnapshotTestSuite) TestGetVolumeSnapshotClassFake(c *C) {
annotationKey: "",
annotationValue: "",
storageClassName: fakeSC,
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-2", fakeDriver, "Delete"),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-2", fakeDriver, "Delete"),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-2", fakeDriver, "Delete"),
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-2", fakeDriver, "Delete", nil),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-2", fakeDriver, "Delete", nil),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-2", fakeDriver, "Delete", nil),
testKey: "",
testValue: "",
check: IsNil,
Expand All @@ -649,9 +657,9 @@ func (s *SnapshotTestSuite) TestGetVolumeSnapshotClassFake(c *C) {
annotationKey: "test-3",
annotationValue: "false",
storageClassName: fakeSC,
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-2", fakeDriver, "Delete"),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-2", fakeDriver, "Delete"),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-2", fakeDriver, "Delete"),
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-2", fakeDriver, "Delete", nil),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-2", fakeDriver, "Delete", nil),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-2", fakeDriver, "Delete", nil),
testKey: "invalid",
testValue: "false",
check: NotNil,
Expand All @@ -661,9 +669,9 @@ func (s *SnapshotTestSuite) TestGetVolumeSnapshotClassFake(c *C) {
annotationKey: "test-4",
annotationValue: "false",
storageClassName: fakeSC,
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-4", fakeDriver, "Delete"),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-4", fakeDriver, "Delete"),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-4", fakeDriver, "Delete"),
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-4", fakeDriver, "Delete", nil),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-4", fakeDriver, "Delete", nil),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-4", fakeDriver, "Delete", nil),
testKey: "test-4",
testValue: "true",
check: NotNil,
Expand All @@ -673,9 +681,9 @@ func (s *SnapshotTestSuite) TestGetVolumeSnapshotClassFake(c *C) {
annotationKey: "test-5",
annotationValue: "true",
storageClassName: "badStorageClass",
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-5", fakeDriver, "Delete"),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-5", fakeDriver, "Delete"),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-5", fakeDriver, "Delete"),
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-5", fakeDriver, "Delete", nil),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-5", fakeDriver, "Delete", nil),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-5", fakeDriver, "Delete", nil),
testKey: "test-5",
testValue: "true",
check: NotNil,
Expand All @@ -685,9 +693,9 @@ func (s *SnapshotTestSuite) TestGetVolumeSnapshotClassFake(c *C) {
annotationKey: "test-6",
annotationValue: "true",
storageClassName: fakeSC,
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-6", "driverMismatch", "Delete"),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-6", "driverMismatch", "Delete"),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-6", "driverMismatch", "Delete"),
snapClassAlpha: snapshot.UnstructuredVolumeSnapshotClassAlpha("test-6", "driverMismatch", "Delete", nil),
snapClassBeta: snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "test-6", "driverMismatch", "Delete", nil),
snapClassStable: snapshot.UnstructuredVolumeSnapshotClass(snapshot.VolSnapClassGVR, "test-6", "driverMismatch", "Delete", nil),
testKey: "test-6",
testValue: "true",
check: NotNil,
Expand Down Expand Up @@ -1024,11 +1032,11 @@ func (s *SnapshotTestSuite) TestCreateFromSourceStable(c *C) {

func (s *SnapshotTestSuite) TestGetSnapshotClassbyAnnotation(c *C) {
ctx := context.Background()
vsc1 := snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "vsc1", "driver", snapshot.DeletionPolicyDelete)
vsc1 := snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "vsc1", "driver", snapshot.DeletionPolicyDelete, nil)
vsc1.SetAnnotations(map[string]string{
"key": "value",
})
vsc2 := snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "vsc2", "driver", snapshot.DeletionPolicyDelete)
vsc2 := snapshot.UnstructuredVolumeSnapshotClass(v1beta1.VolSnapClassGVR, "vsc2", "driver", snapshot.DeletionPolicyDelete, nil)
sc1 := &scv1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "sc1",
Expand Down

0 comments on commit a5bb37d

Please sign in to comment.