diff --git a/Gopkg.lock b/Gopkg.lock index 9c790fce0..2da97b97d 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -699,7 +699,6 @@ input-imports = [ "github.com/container-storage-interface/spec/lib/go/csi", "github.com/davecgh/go-spew/spew", - "github.com/evanphx/json-patch", "github.com/golang/mock/gomock", "github.com/golang/protobuf/proto", "github.com/kubernetes-csi/csi-lib-utils/connection", @@ -720,7 +719,6 @@ "k8s.io/apimachinery/pkg/labels", "k8s.io/apimachinery/pkg/runtime", "k8s.io/apimachinery/pkg/runtime/schema", - "k8s.io/apimachinery/pkg/types", "k8s.io/apimachinery/pkg/util/wait", "k8s.io/client-go/informers", "k8s.io/client-go/informers/core/v1", diff --git a/deploy/kubernetes/rbac.yaml b/deploy/kubernetes/rbac.yaml index 97bca15e9..8b252ae8a 100644 --- a/deploy/kubernetes/rbac.yaml +++ b/deploy/kubernetes/rbac.yaml @@ -24,7 +24,7 @@ metadata: rules: - apiGroups: [""] resources: ["persistentvolumes"] - verbs: ["get", "list", "watch", "update", "patch"] + verbs: ["get", "list", "watch", "update"] - apiGroups: [""] resources: ["nodes"] verbs: ["get", "list", "watch"] @@ -33,7 +33,7 @@ rules: verbs: ["get", "list", "watch"] - apiGroups: ["storage.k8s.io"] resources: ["volumeattachments"] - verbs: ["get", "list", "watch", "update", "patch"] + verbs: ["get", "list", "watch", "update"] #Secret permission is optional. #Enable it if you need value from secret. #For example, you have key `csi.storage.k8s.io/controller-publish-secret-name` in StorageClass.parameters diff --git a/pkg/controller/csi_handler.go b/pkg/controller/csi_handler.go index d76b9a01e..21840f393 100644 --- a/pkg/controller/csi_handler.go +++ b/pkg/controller/csi_handler.go @@ -29,7 +29,6 @@ import ( storage "k8s.io/api/storage/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" corelisters "k8s.io/client-go/listers/core/v1" storagelisters "k8s.io/client-go/listers/storage/v1beta1" @@ -192,8 +191,9 @@ func (h *csiHandler) prepareVANodeID(va *storage.VolumeAttachment, nodeID string return clone, true } -func (h *csiHandler) saveVA(va *storage.VolumeAttachment, patch []byte) (*storage.VolumeAttachment, error) { - newVA, err := h.client.StorageV1beta1().VolumeAttachments().Patch(va.Name, types.MergePatchType, patch) +func (h *csiHandler) saveVA(va *storage.VolumeAttachment) (*storage.VolumeAttachment, error) { + // TODO: #158 #157: use patch to save us from VersionError + newVA, err := h.client.StorageV1beta1().VolumeAttachments().Update(va) if err != nil { return va, err } @@ -215,13 +215,11 @@ func (h *csiHandler) addPVFinalizer(pv *v1.PersistentVolume) (*v1.PersistentVolu klog.V(4).Infof("Adding finalizer to PV %q", pv.Name) clone := pv.DeepCopy() clone.Finalizers = append(clone.Finalizers, finalizerName) - - var newPV *v1.PersistentVolume - var err error - if newPV, err = h.patchPV(pv, clone); err != nil { + // TODO: #158 #157: use patch to save us from VersionError + newPV, err := h.client.CoreV1().PersistentVolumes().Update(clone) + if err != nil { return pv, err } - klog.V(4).Infof("PV finalizer added to %q", pv.Name) return newPV, nil } @@ -326,9 +324,10 @@ func (h *csiHandler) csiAttach(va *storage.VolumeAttachment) (*storage.VolumeAtt originalVA := va va, finalizerAdded := h.prepareVAFinalizer(va) va, nodeIDAdded := h.prepareVANodeID(va, nodeID) - if finalizerAdded || nodeIDAdded { - if va, err = h.patchVA(originalVA, va); err != nil { + va, err = h.saveVA(va) + if err != nil { + // va modification failed, return the original va that's still on API server return originalVA, nil, fmt.Errorf("could not save VolumeAttachment: %s", err) } } @@ -411,9 +410,8 @@ func (h *csiHandler) saveAttachError(va *storage.VolumeAttachment, err error) (* Message: err.Error(), Time: metav1.Now(), } - - var newVa *storage.VolumeAttachment - if newVa, err = h.patchVA(va, clone); err != nil { + newVa, err := h.client.StorageV1beta1().VolumeAttachments().Update(clone) + if err != nil { return va, err } klog.V(4).Infof("Saved attach error to %q", va.Name) @@ -427,9 +425,8 @@ func (h *csiHandler) saveDetachError(va *storage.VolumeAttachment, err error) (* Message: err.Error(), Time: metav1.Now(), } - - var newVa *storage.VolumeAttachment - if newVa, err = h.patchVA(va, clone); err != nil { + newVa, err := h.client.StorageV1beta1().VolumeAttachments().Update(clone) + if err != nil { return va, err } klog.V(4).Infof("Saved detach error to %q", va.Name) @@ -494,13 +491,12 @@ func (h *csiHandler) SyncNewOrUpdatedPersistentVolume(pv *v1.PersistentVolume) { newFinalizers = nil } clone.Finalizers = newFinalizers - - if _, err = h.patchPV(pv, clone); err != nil { + _, err = h.client.CoreV1().PersistentVolumes().Update(clone) + if err != nil { klog.Errorf("Failed to remove finalizer from PV %q: %s", pv.Name, err.Error()) h.pvQueue.AddRateLimited(pv.Name) return } - klog.V(2).Infof("Removed finalizer from PV %q", pv.Name) h.pvQueue.Forget(pv.Name) @@ -563,29 +559,3 @@ func (h *csiHandler) getNodeID(driver string, nodeName string, va *storage.Volum // return nodeLister.Get error return "", err } - -func (h *csiHandler) patchVA(va, clone *storage.VolumeAttachment) (*storage.VolumeAttachment, error) { - patch, err := createMergePatch(va, clone) - if err != nil { - return va, err - } - - newVa, err := h.client.StorageV1beta1().VolumeAttachments().Patch(va.Name, types.MergePatchType, patch) - if err != nil { - return va, err - } - return newVa, nil -} - -func (h *csiHandler) patchPV(pv, clone *v1.PersistentVolume) (*v1.PersistentVolume, error) { - patch, err := createMergePatch(pv, clone) - if err != nil { - return pv, err - } - - newPV, err := h.client.CoreV1().PersistentVolumes().Patch(pv.Name, types.MergePatchType, patch) - if err != nil { - return pv, err - } - return newPV, nil -} diff --git a/pkg/controller/csi_handler_test.go b/pkg/controller/csi_handler_test.go index 085aee746..98da47868 100644 --- a/pkg/controller/csi_handler_test.go +++ b/pkg/controller/csi_handler_test.go @@ -30,11 +30,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" core "k8s.io/client-go/testing" - "k8s.io/klog" ) const ( @@ -243,15 +241,6 @@ func secret() *v1.Secret { } } -func patch(original, new interface{}) []byte { - patch, err := createMergePatch(original, new) - if err != nil { - klog.Fatalf("Failed to create patch %+v", err) - return nil - } - return patch -} - func TestCSIHandler(t *testing.T) { vaGroupResourceVersion := schema.GroupVersionResource{ Group: storage.GroupName, @@ -288,12 +277,8 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -304,12 +289,8 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{node()}, addedVA: vaWithInlineSpec(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithInlineSpec(va(false /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithInlineSpec(va(true /*attached*/, fin, ann))), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -321,12 +302,8 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readOnly, success, notDetached, noMetadata, 0}, @@ -337,12 +314,8 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{node()}, addedVA: vaInlineSpecReadOnly(vaWithInlineSpec(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */))), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecReadOnly(vaWithInlineSpec(va(false /*attached*/, fin, ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecReadOnly(vaWithInlineSpec(va(true /*attached*/, fin, ann)))), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readOnly, success, notDetached, noMetadata, 0}, @@ -354,12 +327,8 @@ func TestCSIHandler(t *testing.T) { updatedVA: va(false, "", nil), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -371,12 +340,8 @@ func TestCSIHandler(t *testing.T) { updatedVA: vaWithInlineSpec(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */)), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithInlineSpec(va(false /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithInlineSpec(va(true /*attached*/, fin, ann))), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -388,12 +353,8 @@ func TestCSIHandler(t *testing.T) { updatedVA: va(false, "", nil), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, map[string]string{"foo": "bar"}, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -405,12 +366,8 @@ func TestCSIHandler(t *testing.T) { updatedVA: vaInlineSpecWithAttributes(vaWithInlineSpec(va(false, "", nil)) /*va*/, map[string]string{"foo": "bar"} /*attributes*/), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecWithAttributes(vaWithInlineSpec(va(false /*attached*/, fin, ann)), map[string]string{"foo": "bar"})), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecWithAttributes(vaWithInlineSpec(va(true /*attached*/, fin, ann)), map[string]string{"foo": "bar"})), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, map[string]string{"foo": "bar"}, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -423,12 +380,8 @@ func TestCSIHandler(t *testing.T) { expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "secret"), // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, map[string]string{"foo": "bar"}, readWrite, success, notDetached, noMetadata, 0}, @@ -441,12 +394,8 @@ func TestCSIHandler(t *testing.T) { expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "secret"), // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecWithSecret(vaWithInlineSpec(va(false /*attached*/, fin, ann)), "secret")), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecWithSecret(vaWithInlineSpec(va(true /*attached*/, fin, ann)), "secret")), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, map[string]string{"foo": "bar"}, readWrite, success, notDetached, noMetadata, 0}, @@ -459,12 +408,8 @@ func TestCSIHandler(t *testing.T) { expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"), // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, map[string]string{}, readWrite, success, notDetached, noMetadata, 0}, @@ -477,12 +422,8 @@ func TestCSIHandler(t *testing.T) { expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"), // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecWithSecret(vaWithInlineSpec(va(false /*attached*/, fin, ann)), "emptySecret")), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaInlineSpecWithSecret(vaWithInlineSpec(va(true /*attached*/, fin, ann)), "emptySecret")), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, map[string]string{}, readWrite, success, notDetached, noMetadata, 0}, @@ -494,9 +435,7 @@ func TestCSIHandler(t *testing.T) { updatedVA: va(false, "", nil), expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "unknownSecret"), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "", nil), - vaWithAttachError(va(false, "", nil), "failed to load secret \"default/unknownSecret\": secrets \"unknownSecret\" not found"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, "", nil), "failed to load secret \"default/unknownSecret\": secrets \"unknownSecret\" not found")), }, expectedCSICalls: []csiCall{}, }, @@ -506,16 +445,10 @@ func TestCSIHandler(t *testing.T) { updatedVA: va(false, "", nil), expectedActions: []core.Action{ // PV Finalizer after VA - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pv(), pvWithFinalizer())), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvWithFinalizer()), // VA Finalizer is saved last - // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -527,7 +460,7 @@ func TestCSIHandler(t *testing.T) { updatedVA: va(false, "", nil), reactors: []reaction{ { - verb: "patch", + verb: "update", resource: "persistentvolumes", reactor: func(t *testing.T) core.ReactionFunc { i := 0 @@ -545,29 +478,15 @@ func TestCSIHandler(t *testing.T) { }, expectedActions: []core.Action{ // PV Finalizer - fails - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pv(), pvWithFinalizer())), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvWithFinalizer()), // Error is saved - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - vaWithAttachError(va(false, "", nil), "could not add PersistentVolume finalizer: persistentvolume \"pv1\" is forbidden: Mock error"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, "", nil), "could not add PersistentVolume finalizer: persistentvolume \"pv1\" is forbidden: Mock error")), // Second PV Finalizer - succeeds - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pv(), pvWithFinalizer())), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvWithFinalizer()), // VA Finalizer is saved first, error remains - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - vaWithAttachError(va(false, "", nil), - "could not add PersistentVolume finalizer: persistentvolume \"pv1\" is forbidden: Mock error"), - vaWithAttachError(va(false, fin, ann), - "could not add PersistentVolume finalizer: persistentvolume \"pv1\" is forbidden: Mock error"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "could not add PersistentVolume finalizer: persistentvolume \"pv1\" is forbidden: Mock error")), // Attach succeeds, error is deleted - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - vaWithAttachError(va(false, fin, ann), - "could not add PersistentVolume finalizer: persistentvolume \"pv1\" is forbidden: Mock error"), - va(true, fin, ann)), - ), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -585,9 +504,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvDeleted(pv()), node()}, updatedVA: va(false, fin, ann), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin /*finalizer*/, ann /* annotations */), - vaWithAttachError(va(false, fin, ann), "PersistentVolume \"pv1\" is marked for deletion"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "PersistentVolume \"pv1\" is marked for deletion")), }, expectedCSICalls: []csiCall{}, }, @@ -597,11 +514,8 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false, "", nil), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "", nil), va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - vaWithMetadata(va(true, fin, ann), map[string]string{"foo": "bar"}))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithMetadata(va(true, fin, ann), map[string]string{"foo": "bar"})), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, map[string]string{"foo": "bar"}, 0}, @@ -618,9 +532,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{node()}, addedVA: va(false, fin, ann), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false, fin, ann), vaWithAttachError(va(false, fin, ann), - "persistentvolume \"pv1\" not found"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "persistentvolume \"pv1\" not found")), }, }, { @@ -629,7 +541,7 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false, fin, ann), reactors: []reaction{ { - verb: "patch", + verb: "update", resource: "volumeattachments", reactor: func(t *testing.T) core.ReactionFunc { i := 0 @@ -646,15 +558,9 @@ func TestCSIHandler(t *testing.T) { }, }, expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false, fin, ann), vaWithAttachError(va(false, fin, ann), - "persistentvolume \"pv1\" not found"))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false, fin, ann), vaWithAttachError(va(false, fin, ann), - "persistentvolume \"pv1\" not found"))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false, fin, ann), vaWithAttachError(va(false, fin, ann), - "persistentvolume \"pv1\" not found"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "persistentvolume \"pv1\" not found")), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "persistentvolume \"pv1\" not found")), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "persistentvolume \"pv1\" not found")), }, }, { @@ -662,9 +568,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: vaWithNoPVReferenceNorInlineVolumeSpec(va(false, fin, ann)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(vaWithNoPVReferenceNorInlineVolumeSpec(va(false, fin, ann)), - vaWithAttachError(vaWithNoPVReferenceNorInlineVolumeSpec(va(false, fin, ann)), "neither InlineCSIVolumeSource nor PersistentVolumeName specified in VA source"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(vaWithNoPVReferenceNorInlineVolumeSpec(va(false, fin, ann)), "neither InlineCSIVolumeSource nor PersistentVolumeName specified in VA source")), }, }, { @@ -672,9 +576,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: vaAddInlineSpec(va(false, fin, ann)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(vaWithNoPVReferenceNorInlineVolumeSpec(va(false, fin, ann)), - vaWithAttachError(vaWithNoPVReferenceNorInlineVolumeSpec(va(false, fin, ann)), "both InlineCSIVolumeSource and PersistentVolumeName specified in VA source"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(vaAddInlineSpec(va(false, fin, ann)), "both InlineCSIVolumeSource and PersistentVolumeName specified in VA source")), }, }, { @@ -682,9 +584,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer()}, addedVA: va(false, fin, ann), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false, fin, ann), vaWithAttachError(va(false, fin, ann), - "node \"node1\" not found"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "node \"node1\" not found")), }, }, { @@ -693,7 +593,7 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false, "", nil), reactors: []reaction{ { - verb: "patch", + verb: "update", resource: "volumeattachments", reactor: func(t *testing.T) core.ReactionFunc { i := 0 @@ -711,21 +611,12 @@ func TestCSIHandler(t *testing.T) { }, expectedActions: []core.Action{ // Controller tries to save VA finalizer, it fails - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false, "", nil), - va(false /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), // Controller tries to save error, it fails too - - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - vaWithAttachError(va(false /*attached*/, "", nil), "could not save VolumeAttachment: volumeattachments.storage.k8s.io \"pv1-node1\" is forbidden: Mock error"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false /*attached*/, "", nil), "could not save VolumeAttachment: volumeattachments.storage.k8s.io \"pv1-node1\" is forbidden: Mock error")), // Controller tries to save VA finalizer again, it succeeds - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -737,7 +628,7 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false, "", nil), reactors: []reaction{ { - verb: "patch", + verb: "update", resource: "volumeattachments", reactor: func(t *testing.T) core.ReactionFunc { i := 0 @@ -753,16 +644,10 @@ func TestCSIHandler(t *testing.T) { }, expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), // Second save with attached=true fails - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin /*finalizer*/, ann /* annotations */), - va(true /*attached*/, fin, ann))), - //core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - // types.MergePatchType, patch(va(false /*attached*/ , fin /*finalizer*/ , ann /* annotations */), - // va(true /*attached*/ , fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -775,15 +660,9 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false, "", nil), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin /*finalizer*/, ann /* annotations */), - vaWithAttachError(va(false, fin, ann), "mock error"))), - //core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - // types.MergePatchType, patch(vaWithAttachError(va(false, fin, ann), "mock error"), - // va(true /*attached*/ , fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "mock error")), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, fmt.Errorf("mock error"), notDetached, noMetadata, 0}, @@ -796,12 +675,8 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false, "", nil), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 500 * time.Millisecond}, @@ -813,9 +688,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), nodeWithoutAnnotations()}, addedVA: va(false, fin, ann), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - vaWithAttachError(va(false, fin, ann), "node \"node1\" has no NodeID annotation"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "node \"node1\" has no NodeID annotation")), }, }, { @@ -823,9 +696,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), nodeWithoutAnnotations(), csiNodeEmpty()}, addedVA: va(false, fin, ann), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - vaWithAttachError(va(false, fin, ann), "node \"node1\" has no NodeID annotation"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithAttachError(va(false, fin, ann), "node \"node1\" has no NodeID annotation")), }, }, { @@ -834,12 +705,8 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false /*attached*/, "" /*finalizer*/, nil), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -851,29 +718,23 @@ func TestCSIHandler(t *testing.T) { addedVA: va(false /*attached*/, "" /*finalizer*/, nil), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", "projects/UNSPECIFIED/zones/testZone/disks/testpd", testNodeID, map[string]string{"partition": ""}, noSecrets, readWrite, success, notDetached, noMetadata, 0}, }, }, - - //DETACH - + // + // DETACH + // { name: "VolumeAttachment marked for deletion -> successful detach", initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(va(false /*attached*/, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, detached, noMetadata, 0}, @@ -884,9 +745,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{node()}, addedVA: deleted(vaWithInlineSpec(va(true, fin, ann))), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(vaWithInlineSpec(va(true, fin, ann))), - deleted(vaWithInlineSpec(va(false /*attached*/, "", ann))))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithInlineSpec(va(false /*attached*/, "", ann)))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, detached, noMetadata, 0}, @@ -898,9 +757,7 @@ func TestCSIHandler(t *testing.T) { addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "secret"), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(va(false /*attached*/, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, map[string]string{"foo": "bar"}, readWrite, success, detached, noMetadata, 0}, @@ -912,9 +769,7 @@ func TestCSIHandler(t *testing.T) { addedVA: deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(true, fin, ann)), "secret")), expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "secret"), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(true, fin, ann)), "secret")), - deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(false /*attached*/, "", ann)), "secret")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(false /*attached*/, "", ann)), "secret"))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, map[string]string{"foo": "bar"}, readWrite, success, detached, noMetadata, 0}, @@ -926,9 +781,7 @@ func TestCSIHandler(t *testing.T) { addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(va(false /*attached*/, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, map[string]string{}, readWrite, success, detached, noMetadata, 0}, @@ -940,9 +793,7 @@ func TestCSIHandler(t *testing.T) { addedVA: deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(true, fin, ann)), "emptySecret")), expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(true, fin, ann)), "emptySecret")), - deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(false /*attached*/, "", ann)), "emptySecret")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(false /*attached*/, "", ann)), "emptySecret"))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, map[string]string{}, readWrite, success, detached, noMetadata, 0}, @@ -954,10 +805,7 @@ func TestCSIHandler(t *testing.T) { addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ core.NewGetAction(secretGroupResourceVersion, "default", "unknownSecret"), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(vaWithDetachError(va(true, fin, ann), - "failed to load secret \"default/unknownSecret\": secrets \"unknownSecret\" not found")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(va(true, fin, ann), "failed to load secret \"default/unknownSecret\": secrets \"unknownSecret\" not found"))), }, expectedCSICalls: []csiCall{}, }, @@ -966,12 +814,8 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(vaWithDetachError(va(true, fin, ann), "mock error")))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(va(false, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithDetachError(deleted(va(true /*attached*/, fin, ann)), "mock error")), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, fmt.Errorf("mock error"), notDetached, noMetadata, 0}, @@ -983,9 +827,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(va(false /*attached*/, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, detached, noMetadata, 500 * time.Millisecond}, @@ -997,9 +839,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(va(false /*attached*/, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, fmt.Errorf("mock error"), detached, noMetadata, 0}, @@ -1017,9 +857,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{node()}, addedVA: deleted(va(true, fin, ann)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found"))), }, }, { @@ -1045,15 +883,9 @@ func TestCSIHandler(t *testing.T) { }, }, expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found")))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found")))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, ann)), - deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(va(true, fin, ann), "persistentvolume \"pv1\" not found"))), }, }, { @@ -1061,10 +893,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: deleted(vaWithNoPVReferenceNorInlineVolumeSpec(va(true, fin, ann))), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(vaWithNoPVReferenceNorInlineVolumeSpec(va(true, fin, ann))), - deleted(vaWithDetachError(vaWithNoPVReferenceNorInlineVolumeSpec(va(true, fin, ann)), - "neither InlineCSIVolumeSource nor PersistentVolumeName specified in VA source")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(vaWithNoPVReferenceNorInlineVolumeSpec(va(true, fin, ann)), "neither InlineCSIVolumeSource nor PersistentVolumeName specified in VA source"))), }, }, { @@ -1072,10 +901,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: deleted(vaAddInlineSpec(va(true, fin, ann))), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(vaAddInlineSpec(va(true, fin, ann))), - deleted(vaWithDetachError(vaAddInlineSpec(va(true, fin, ann)), - "both InlineCSIVolumeSource and PersistentVolumeName specified in VA source")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(vaAddInlineSpec(va(true, fin, ann)), "both InlineCSIVolumeSource and PersistentVolumeName specified in VA source"))), }, }, { @@ -1083,9 +909,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer()}, addedVA: deleted(va(true, fin, nil)), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, nil)), - deleted(vaWithDetachError(va(true, fin, nil), "node \"node1\" not found")))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(vaWithDetachError(va(true, fin, nil), "node \"node1\" not found"))), }, }, { @@ -1093,10 +917,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer()}, addedVA: deleted(va(true, fin, map[string]string{vaNodeIDAnnotation: "annotatedNodeID"})), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - deleted(va(true, fin, map[string]string{vaNodeIDAnnotation: "annotatedNodeID"})), - deleted(va(false /*attached*/, "", map[string]string{vaNodeIDAnnotation: "annotatedNodeID"})))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", map[string]string{vaNodeIDAnnotation: "annotatedNodeID"}))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, "annotatedNodeID", noAttrs, noSecrets, readWrite, success, detached, noMetadata, 0}, @@ -1107,9 +928,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvWithFinalizer(), node()}, addedVA: deleted(va(true, fin, map[string]string{vaNodeIDAnnotation: "annotatedNodeID"})), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(deleted(va(true, fin, map[string]string{vaNodeIDAnnotation: "annotatedNodeID"})), - deleted(va(false /*attached*/, "", map[string]string{vaNodeIDAnnotation: "annotatedNodeID"})))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", map[string]string{vaNodeIDAnnotation: "annotatedNodeID"}))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, detached, noMetadata, 0}, @@ -1121,7 +940,7 @@ func TestCSIHandler(t *testing.T) { addedVA: deleted(va(false, fin, ann)), reactors: []reaction{ { - verb: "patch", + verb: "update", resource: "volumeattachments", reactor: func(t *testing.T) core.ReactionFunc { i := 0 @@ -1137,21 +956,11 @@ func TestCSIHandler(t *testing.T) { }, expectedActions: []core.Action{ // This fails - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - deleted(va(false, fin, ann)), - deleted(va(false, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false, "", ann))), // Saving error succeeds - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - deleted(va(false, fin, ann)), - vaWithDetachError(deleted(va(false, fin, ann)), - "could not mark as detached: volumeattachments.storage.k8s.io \"pv1-node1\" is forbidden: mock error"))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, vaWithDetachError(deleted(va(false, fin, ann)), "could not mark as detached: volumeattachments.storage.k8s.io \"pv1-node1\" is forbidden: mock error")), // Second save of attached=false succeeds - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - deleted(va(false, fin, ann)), - deleted(va(false, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, detached, noMetadata, 0}, @@ -1164,10 +973,7 @@ func TestCSIHandler(t *testing.T) { addedVA: deleted(va(true /*attached*/, fin /*finalizer*/, ann)), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - deleted(va(true, fin, ann)), - deleted(va(false /*attached*/, "", ann)))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, deleted(va(false /*attached*/, "", ann))), }, expectedCSICalls: []csiCall{ {"detach", "projects/UNSPECIFIED/zones/testZone/disks/testpd", testNodeID, @@ -1182,9 +988,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvDeleted(pvWithFinalizer())}, deletedVA: va(false, "", nil), expectedActions: []core.Action{ - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pvDeleted(pvWithFinalizer()), - pvDeleted(pv()))), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvDeleted(pv())), }, }, { @@ -1192,9 +996,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{}, updatedPV: pvDeleted(pvWithFinalizer()), expectedActions: []core.Action{ - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pvDeleted(pvWithFinalizer()), - pvDeleted(pv()))), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvDeleted(pv())), }, }, { @@ -1202,8 +1004,7 @@ func TestCSIHandler(t *testing.T) { initialObjects: []runtime.Object{pvDeleted(pvWithFinalizers(pvWithFinalizer(), "foo/bar", "bar/baz"))}, deletedVA: va(false, "", nil), expectedActions: []core.Action{ - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pvDeleted(pvWithFinalizers(pvWithFinalizer(), "foo/bar", "bar/baz")), pvDeleted(pvWithFinalizers(pv(), "foo/bar", "bar/baz")))), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvDeleted(pvWithFinalizers(pv(), "foo/bar", "bar/baz"))), }, }, { @@ -1212,7 +1013,7 @@ func TestCSIHandler(t *testing.T) { deletedVA: va(false, "", nil), reactors: []reaction{ { - verb: "patch", + verb: "update", resource: "persistentvolumes", reactor: func(t *testing.T) core.ReactionFunc { i := 0 @@ -1228,14 +1029,11 @@ func TestCSIHandler(t *testing.T) { }, expectedActions: []core.Action{ // This update fails - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pvDeleted(pvWithFinalizer()), pvDeleted(pv()))), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvDeleted(pv())), // This one fails too - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pvDeleted(pvWithFinalizer()), pvDeleted(pv()))), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvDeleted(pv())), // This one succeeds - core.NewPatchAction(pvGroupResourceVersion, metav1.NamespaceNone, testPVName, - types.MergePatchType, patch(pvDeleted(pvWithFinalizer()), pvDeleted(pv()))), + core.NewUpdateAction(pvGroupResourceVersion, metav1.NamespaceNone, pvDeleted(pv())), }, }, { @@ -1284,12 +1082,8 @@ func TestCSIHandlerReadOnly(t *testing.T) { addedVA: va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, @@ -1301,12 +1095,8 @@ func TestCSIHandlerReadOnly(t *testing.T) { addedVA: va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), expectedActions: []core.Action{ // Finalizer is saved first - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */), - va(false /*attached*/, fin, ann))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch(va(false /*attached*/, fin, ann), - va(true /*attached*/, fin, ann))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(false /*attached*/, fin, ann)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true /*attached*/, fin, ann)), }, expectedCSICalls: []csiCall{ {"attach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, notDetached, noMetadata, 0}, diff --git a/pkg/controller/framework_test.go b/pkg/controller/framework_test.go index f05ee3bec..ffba6c375 100644 --- a/pkg/controller/framework_test.go +++ b/pkg/controller/framework_test.go @@ -29,8 +29,7 @@ import ( "github.com/kubernetes-csi/external-attacher/pkg/attacher" "k8s.io/klog" - "encoding/json" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -245,30 +244,6 @@ func runTests(t *testing.T, handlerFactory handlerFactory, tests []testCase) { } } - if action.GetVerb() == "patch" && action.GetResource().Resource == "volumeattachments" { - patchAction := action.(core.PatchActionImpl) - patch := patchAction.GetPatch() - var va storage.VolumeAttachment - err := json.Unmarshal(patch, &va) - if va.Status.AttachError != nil { - va.Status.AttachError.Time = metav1.Time{} - } - if va.Status.DetachError != nil { - va.Status.DetachError.Time = metav1.Time{} - } - - if va.Status.AttachError != nil || va.Status.DetachError != nil { - - patch, err = createMergePatch(storage.VolumeAttachment{}, va) - if err != nil { - t.Errorf("Test %q create patch failed", t.Name()) - } - patchAction.Patch = patch - action = patchAction - } - - } - expectedAction := test.expectedActions[i] if !reflect.DeepEqual(expectedAction, action) { t.Errorf("Test %q: action %d\nExpected:\n%s\ngot:\n%s", test.name, i, spew.Sdump(expectedAction), spew.Sdump(action)) diff --git a/pkg/controller/trivial_handler_test.go b/pkg/controller/trivial_handler_test.go index eae7314fe..086790ce3 100644 --- a/pkg/controller/trivial_handler_test.go +++ b/pkg/controller/trivial_handler_test.go @@ -27,7 +27,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" core "k8s.io/client-go/testing" @@ -49,20 +48,14 @@ func TestTrivialHandler(t *testing.T) { name: "add -> successful write", addedVA: va(false, "", nil), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - va(false, "", nil), - va(true, "", nil))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true, "", nil)), }, }, { name: "update -> successful write", updatedVA: va(false, "", nil), expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - va(false, "", nil), - va(true, "", nil))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true, "", nil)), }, }, { @@ -75,7 +68,7 @@ func TestTrivialHandler(t *testing.T) { addedVA: va(false, "", nil), reactors: []reaction{ { - verb: "patch", + verb: "update", resource: "volumeattachments", reactor: func(t *testing.T) core.ReactionFunc { i := 0 @@ -92,18 +85,9 @@ func TestTrivialHandler(t *testing.T) { }, }, expectedActions: []core.Action{ - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - va(false, "", nil), - va(true, "", nil))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - va(false, "", nil), - va(true, "", nil))), - core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName, - types.MergePatchType, patch( - va(false, "", nil), - va(true, "", nil))), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true, "", nil)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true, "", nil)), + core.NewUpdateAction(vaGroupResourceVersion, metav1.NamespaceNone, va(true, "", nil)), }, }, } diff --git a/pkg/controller/util.go b/pkg/controller/util.go index a70e0d876..4cf7481ad 100644 --- a/pkg/controller/util.go +++ b/pkg/controller/util.go @@ -22,10 +22,8 @@ import ( "regexp" "github.com/container-storage-interface/spec/lib/go/csi" - "github.com/evanphx/json-patch" "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1beta1" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "k8s.io/klog" ) @@ -36,11 +34,8 @@ func markAsAttached(client kubernetes.Interface, va *storage.VolumeAttachment, m clone.Status.Attached = true clone.Status.AttachmentMetadata = metadata clone.Status.AttachError = nil - patch, err := createMergePatch(va, clone) - if err != nil { - return va, err - } - newVA, err := client.StorageV1beta1().VolumeAttachments().Patch(va.Name, types.MergePatchType, patch) + // TODO: #158 #157: use patch to save us from VersionError + newVA, err := client.StorageV1beta1().VolumeAttachments().Update(clone) if err != nil { return va, err } @@ -78,11 +73,8 @@ func markAsDetached(client kubernetes.Interface, va *storage.VolumeAttachment) ( clone.Status.Attached = false clone.Status.DetachError = nil clone.Status.AttachmentMetadata = nil - patch, err := createMergePatch(va, clone) - if err != nil { - return va, err - } - newVA, err := client.StorageV1beta1().VolumeAttachments().Patch(va.Name, types.MergePatchType, patch) + // TODO: #158 #157: use patch to save us from VersionError + newVA, err := client.StorageV1beta1().VolumeAttachments().Update(clone) if err != nil { return va, err } @@ -220,20 +212,3 @@ func GetVolumeAttributes(csiSource *v1.CSIPersistentVolumeSource) (map[string]st } return csiSource.VolumeAttributes, nil } - -// createMergePatch return patch generated from original and new interfaces -func createMergePatch(original, new interface{}) ([]byte, error) { - pvByte, err := json.Marshal(original) - if err != nil { - return nil, err - } - cloneByte, err := json.Marshal(new) - if err != nil { - return nil, err - } - patch, err := jsonpatch.CreateMergePatch(pvByte, cloneByte) - if err != nil { - return nil, err - } - return patch, nil -}