alexlebens/external-snapshotter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix unit tests and don't patch with empty finalizers

Browse Source 
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
This commit is contained in:
Grant Griffiths
2021-09-27 15:15:38 -07:00
parent 2039ad9101
commit 3b9f0e2c72
7 changed files with 162 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
go.mod
View File

@@ -4,6 +4,7 @@ go 1.16
require (
github.com/container-storage-interface/spec v1.5.0
github.com/evanphx/json-patch v4.11.0+incompatible
github.com/fsnotify/fsnotify v1.4.9
github.com/golang/mock v1.4.4
github.com/golang/protobuf v1.5.2

94
pkg/common-controller/framework_test.go
View File

@@ -17,6 +17,7 @@ limitations under the License.
package common_controller
import (
"encoding/json"
"errors"
"fmt"
"net/http"
@@ -29,8 +30,7 @@ import (
"testing"
"time"
"k8s.io/client-go/util/workqueue"
jsonpatch "github.com/evanphx/json-patch"
crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
clientset "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
"github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/fake"
@@ -55,6 +55,7 @@ import (
core "k8s.io/client-go/testing"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/workqueue"
klog "k8s.io/klog/v2"
)
@@ -258,6 +259,46 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
klog.V(4).Infof("saved updated content %s", content.Name)
return true, content, nil
case action.Matches("patch", "volumesnapshotcontents"):
content := &crdv1.VolumeSnapshotContent{}
action := action.(core.PatchAction)
// Check and bump object version
storedSnapshotContent, found := r.contents[action.GetName()]
if found {
// Apply patch
storedSnapshotBytes, err := json.Marshal(storedSnapshotContent)
if err != nil {
return true, nil, err
}
contentPatch, err := jsonpatch.DecodePatch(action.GetPatch())
if err != nil {
return true, nil, err
}
modified, err := contentPatch.Apply(storedSnapshotBytes)
if err != nil {
return true, nil, err
}
err = json.Unmarshal(modified, content)
if err != nil {
return true, nil, err
}
storedVer, _ := strconv.Atoi(content.ResourceVersion)
content.ResourceVersion = strconv.Itoa(storedVer + 1)
} else {
return true, nil, fmt.Errorf("cannot update snapshot content %s: snapshot content not found", action.GetName())
}
// Store the updated object to appropriate places.
r.contents[content.Name] = content
r.changedObjects = append(r.changedObjects, content)
r.changedSinceLastSync++
klog.V(4).Infof("saved updated content %s", content.Name)
return true, content, nil
case action.Matches("update", "volumesnapshots"):
obj := action.(core.UpdateAction).GetObject()
snapshot := obj.(*crdv1.VolumeSnapshot)
@@ -284,6 +325,52 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
klog.V(4).Infof("saved updated snapshot %s", snapshot.Name)
return true, snapshot, nil
case action.Matches("patch", "volumesnapshots"):
snapshot := &crdv1.VolumeSnapshot{}
action := action.(core.PatchAction)
// Check and bump object version
storedSnapshot, found := r.snapshots[action.GetName()]
if found {
// Apply patch
storedSnapshotBytes, err := json.Marshal(storedSnapshot)
if err != nil {
return true, nil, err
}
snapPatch, err := jsonpatch.DecodePatch(action.GetPatch())
if err != nil {
return true, nil, err
}
modified, err := snapPatch.Apply(storedSnapshotBytes)
if err != nil {
return true, nil, err
}
err = json.Unmarshal(modified, storedSnapshot)
if err != nil {
return true, nil, err
}
storedVer, _ := strconv.Atoi(storedSnapshot.ResourceVersion)
storedSnapshot.ResourceVersion = strconv.Itoa(storedVer + 1)
// // If we were updating annotations and the new annotations are nil, leave as empty.
// // This seems to be the behavior for merge-patching nil & empty annotations
// if !reflect.DeepEqual(storedSnapshotContent.Annotations, content.Annotations) && content.Annotations == nil {
// content.Annotations = make(map[string]string)
// }
} else {
return true, nil, fmt.Errorf("cannot update snapshot %s: snapshot not found", action.GetName())
}
// Store the updated object to appropriate places.
r.snapshots[snapshot.Name] = storedSnapshot
r.changedObjects = append(r.changedObjects, storedSnapshot)
r.changedSinceLastSync++
klog.V(4).Infof("saved updated snapshot %s", storedSnapshot.Name)
return true, storedSnapshot, nil
case action.Matches("get", "volumesnapshotcontents"):
name := action.(core.GetAction).GetName()
content, found := r.contents[name]
@@ -437,6 +524,7 @@ func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshot
}
gotMap[v.Name] = v
}
if !reflect.DeepEqual(expectedMap, gotMap) {
// Print ugly but useful diff of expected and received objects for
// easier debugging.
@@ -714,6 +802,8 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset,
client.AddReactor("create", "volumesnapshotcontents", reactor.React)
client.AddReactor("update", "volumesnapshotcontents", reactor.React)
client.AddReactor("update", "volumesnapshots", reactor.React)
client.AddReactor("patch", "volumesnapshotcontents", reactor.React)
client.AddReactor("patch", "volumesnapshots", reactor.React)
client.AddReactor("update", "volumesnapshotclasses", reactor.React)
client.AddReactor("get", "volumesnapshotcontents", reactor.React)
client.AddReactor("get", "volumesnapshots", reactor.React)

34
pkg/common-controller/snapshot_controller.go
View File

@@ -809,13 +809,24 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotErrorStatusWithEvent(snap
// addContentFinalizer adds a Finalizer for VolumeSnapshotContent.
func (ctrl *csiSnapshotCommonController) addContentFinalizer(content *crdv1.VolumeSnapshotContent) error {
var patches []utils.PatchOp
patches = append(patches, utils.PatchOp{
Op: "add",
Path: "/metadata/finalizers/-",
Value: utils.VolumeSnapshotContentFinalizer,
})
var patches []utils.PatchOp
if len(content.Finalizers) > 0 {
// Add to the end of the finalizers if we have any other finalizers
patches = append(patches, utils.PatchOp{
Op: "add",
Path: "/metadata/finalizers/-",
Value: utils.VolumeSnapshotContentFinalizer,
})
} else {
// Replace finalizers with new array if there are no other finalizers
patches = append(patches, utils.PatchOp{
Op: "add",
Path: "/metadata/finalizers",
Value: []string{utils.VolumeSnapshotContentFinalizer},
})
}
newContent, err := utils.PatchVolumeSnapshotContent(content, patches, ctrl.clientset)
if err != nil {
return newControllerUpdateError(content.Name, err.Error())
@@ -987,17 +998,6 @@ func (ctrl *csiSnapshotCommonController) checkandBindSnapshotContent(snapshot *c
} else if content.Spec.VolumeSnapshotRef.UID != "" && content.Spec.VolumeSnapshotClassName != nil {
return content, nil
}
// contentClone := content.DeepCopy()
// contentClone.Spec.VolumeSnapshotRef.UID = snapshot.UID
// if snapshot.Spec.VolumeSnapshotClassName != nil {
// className := *(snapshot.Spec.VolumeSnapshotClassName)
// contentClone.Spec.VolumeSnapshotClassName = &className
// }
// newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{})
// if err != nil {
// klog.V(4).Infof("updating VolumeSnapshotContent[%s] error status failed %v", contentClone.Name, err)
// return content, err
// }
patches := []utils.PatchOp{
{

4
pkg/common-controller/snapshot_update_test.go
View File

@@ -162,7 +162,7 @@ func TestSync(t *testing.T) {
expectedSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, mock update error"), false, true, nil),
errors: []reactorError{
// Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call.
{"update", "volumesnapshotcontents", errors.New("mock update error")},
{"patch", "volumesnapshotcontents", errors.New("mock update error")},
},
test: testSyncSnapshot,
},
@@ -312,7 +312,7 @@ func TestSync(t *testing.T) {
initialSecrets: []*v1.Secret{secret()},
errors: []reactorError{
// Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call.
{"update", "volumesnapshotcontents", errors.New("mock update error")},
{"patch", "volumesnapshotcontents", errors.New("mock update error")},
},
expectSuccess: false,
test: testSyncContentError,

12
pkg/sidecar-controller/content_create_test.go
View File

@@ -85,7 +85,7 @@ func TestSyncContent(t *testing.T) {
{
name: "1-3: Basic sync content create snapshot with non-existent secret",
initialContents: withContentAnnotations(withContentStatus(newContentArray("content1-3", "snapuid1-3", "snap1-3", "sid1-3", invalidSecretClass, "", "volume-handle-1-3", retainPolicy, nil, &defaultSize, true),
nil), map[string]string{
&crdv1.VolumeSnapshotContentStatus{}), map[string]string{
utils.AnnDeletionSecretRefName: "",
utils.AnnDeletionSecretRefNamespace: "",
}),
@@ -94,12 +94,12 @@ func TestSyncContent(t *testing.T) {
SnapshotHandle: nil,
RestoreSize: nil,
ReadyToUse: &False,
Error: newSnapshotError("Failed to create snapshot: failed to get input parameters to create snapshot for content content1-3: \"cannot retrieve secrets for snapshot content \\\"content1-3\\\", err: secret name or namespace not specified\""),
Error: newSnapshotError("Failed to check and update snapshot content: failed to get input parameters to create snapshot for content content1-3: \"cannot retrieve secrets for snapshot content \\\"content1-3\\\", err: secret name or namespace not specified\""),
}), map[string]string{
utils.AnnDeletionSecretRefName: "",
utils.AnnDeletionSecretRefNamespace: "",
}), initialSecrets: []*v1.Secret{}, // no initial secret created
expectedEvents: []string{"Warning SnapshotCreationFailed"},
expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed"},
errors: noerrors,
test: testSyncContent,
},
@@ -149,7 +149,7 @@ func TestSyncContent(t *testing.T) {
{
name: "1-5: Basic sync content create snapshot with failed secret call",
initialContents: withContentAnnotations(withContentStatus(newContentArray("content1-5", "snapuid1-5", "snap1-5", "sid1-5", invalidSecretClass, "", "volume-handle-1-5", retainPolicy, nil, &defaultSize, true),
nil), map[string]string{
&crdv1.VolumeSnapshotContentStatus{}), map[string]string{
utils.AnnDeletionSecretRefName: "secret",
utils.AnnDeletionSecretRefNamespace: "default",
}),
@@ -158,12 +158,12 @@ func TestSyncContent(t *testing.T) {
SnapshotHandle: nil,
RestoreSize: nil,
ReadyToUse: &False,
Error: newSnapshotError("Failed to create snapshot: failed to get input parameters to create snapshot for content content1-5: \"cannot get credentials for snapshot content \\\"content1-5\\\"\""),
Error: newSnapshotError(`Failed to check and update snapshot content: failed to get input parameters to create snapshot for content content1-5: "cannot get credentials for snapshot content \"content1-5\""`),
}), map[string]string{
utils.AnnDeletionSecretRefName: "secret",
utils.AnnDeletionSecretRefNamespace: "default",
}), initialSecrets: []*v1.Secret{}, // no initial secret created
expectedEvents: []string{"Warning SnapshotCreationFailed"},
expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed"},
errors: []reactorError{
// Inject error to the first client.VolumesnapshotV1().VolumeSnapshots().Update call.
// All other calls will succeed.

43
pkg/sidecar-controller/framework_test.go
View File

@@ -15,6 +15,7 @@ package sidecar_controller
import (
"context"
"encoding/json"
"errors"
"fmt"
"reflect"
@@ -25,6 +26,7 @@ import (
"testing"
"time"
jsonpatch "github.com/evanphx/json-patch"
crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
clientset "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
"github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/fake"
@@ -215,6 +217,46 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
klog.V(4).Infof("saved updated content %s", content.Name)
return true, content, nil
case action.Matches("patch", "volumesnapshotcontents"):
content := &crdv1.VolumeSnapshotContent{}
action := action.(core.PatchAction)
// Check and bump object version
storedSnapshotContent, found := r.contents[action.GetName()]
if found {
// Apply patch
storedSnapshotBytes, err := json.Marshal(storedSnapshotContent)
if err != nil {
return true, nil, err
}
contentPatch, err := jsonpatch.DecodePatch(action.GetPatch())
if err != nil {
return true, nil, err
}
modified, err := contentPatch.Apply(storedSnapshotBytes)
if err != nil {
return true, nil, err
}
err = json.Unmarshal(modified, content)
if err != nil {
return true, nil, err
}
storedVer, _ := strconv.Atoi(content.ResourceVersion)
content.ResourceVersion = strconv.Itoa(storedVer + 1)
} else {
return true, nil, fmt.Errorf("cannot update snapshot content %s: snapshot content not found", action.GetName())
}
// Store the updated object to appropriate places.
r.contents[content.Name] = content
r.changedObjects = append(r.changedObjects, content)
r.changedSinceLastSync++
klog.V(4).Infof("saved updated content %s", content.Name)
return true, content, nil
case action.Matches("get", "volumesnapshotcontents"):
name := action.(core.GetAction).GetName()
content, found := r.contents[name]
@@ -488,6 +530,7 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset,
client.AddReactor("create", "volumesnapshotcontents", reactor.React)
client.AddReactor("update", "volumesnapshotcontents", reactor.React)
client.AddReactor("patch", "volumesnapshotcontents", reactor.React)
client.AddReactor("get", "volumesnapshotcontents", reactor.React)
client.AddReactor("delete", "volumesnapshotcontents", reactor.React)
kubeClient.AddReactor("get", "secrets", reactor.React)

1
vendor/modules.txt vendored
View File

@@ -10,6 +10,7 @@ github.com/container-storage-interface/spec/lib/go/csi
# github.com/davecgh/go-spew v1.1.1
github.com/davecgh/go-spew/spew
# github.com/evanphx/json-patch v4.11.0+incompatible
## explicit
github.com/evanphx/json-patch
# github.com/fsnotify/fsnotify v1.4.9
## explicit