Repository: kubernetes-csi/external-attacher (go)

Issue:
Stop using node annotation
Since the external-attacher uses CSINode object that has reached beta in all supported Kubernetes releases, the attacher should stop using `csi.volume.kubernetes.io/nodeid` annotation on Node objects.

 

Pull request:
Removed dependency on node annotation in favor of CSINode resource
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**

/kind cleanup

**What this PR does / why we need it**:

CSINode is in all supported Kubernetes releases, so no more Node annotation is required.

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #203 

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
Removed usage of annotation csi.volume.kubernetes.io/nodeid on Node objects. The external-attacher now requires Kubernetes 1.14 with feature gate CSINodeInfo enabled.
```


Patch:
diff --git a/cmd/csi-attacher/main.go b/cmd/csi-attacher/main.go
index 1a5c69c3..44c22e6f 100644
--- a/cmd/csi-attacher/main.go
+++ b/cmd/csi-attacher/main.go
@@ -154,12 +154,11 @@ func main() {
 		}
 		if supportsAttach {
 			pvLister := factory.Core().V1().PersistentVolumes().Lister()
-			nodeLister := factory.Core().V1().Nodes().Lister()
 			vaLister := factory.Storage().V1beta1().VolumeAttachments().Lister()
 			csiNodeLister := factory.Storage().V1beta1().CSINodes().Lister()
 			volAttacher := attacher.NewAttacher(csiConn)
 			CSIVolumeLister := attacher.NewVolumeLister(csiConn)
-			handler = controller.NewCSIHandler(clientset, csiAttacher, volAttacher, CSIVolumeLister, pvLister, nodeLister, csiNodeLister, vaLister, timeout, supportsReadOnly, csitrans.New())
+			handler = controller.NewCSIHandler(clientset, csiAttacher, volAttacher, CSIVolumeLister, pvLister, csiNodeLister, vaLister, timeout, supportsReadOnly, csitrans.New())
 			klog.V(2).Infof("CSI driver supports ControllerPublishUnpublish, using real CSI handler")
 		} else {
 			handler = controller.NewTrivialHandler(clientset)
diff --git a/deploy/kubernetes/rbac.yaml b/deploy/kubernetes/rbac.yaml
index f5d97a11..93ce3196 100644
--- a/deploy/kubernetes/rbac.yaml
+++ b/deploy/kubernetes/rbac.yaml
@@ -16,7 +16,7 @@ metadata:
   namespace: default
 
 ---
-# Attacher must be able to work with PVs, nodes and VolumeAttachments
+# Attacher must be able to work with PVs, CSINodes and VolumeAttachments
 kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
@@ -25,9 +25,6 @@ rules:
   - apiGroups: [""]
     resources: ["persistentvolumes"]
     verbs: ["get", "list", "watch", "update", "patch"]
-  - apiGroups: [""]
-    resources: ["nodes"]
-    verbs: ["get", "list", "watch"]
   - apiGroups: ["storage.k8s.io"]
     resources: ["csinodes"]
     verbs: ["get", "list", "watch"]
diff --git a/pkg/controller/csi_handler.go b/pkg/controller/csi_handler.go
index 2fc88d5d..c6c3a259 100644
--- a/pkg/controller/csi_handler.go
+++ b/pkg/controller/csi_handler.go
@@ -62,7 +62,6 @@ type csiHandler struct {
 	attacher                attacher.Attacher
 	CSIVolumeLister         VolumeLister
 	pvLister                corelisters.PersistentVolumeLister
-	nodeLister              corelisters.NodeLister
 	csiNodeLister           storagelisters.CSINodeLister
 	vaLister                storagelisters.VolumeAttachmentLister
 	vaQueue, pvQueue        workqueue.RateLimitingInterface
@@ -82,7 +81,6 @@ func NewCSIHandler(
 	attacher attacher.Attacher,
 	CSIVolumeLister VolumeLister,
 	pvLister corelisters.PersistentVolumeLister,
-	nodeLister corelisters.NodeLister,
 	csiNodeLister storagelisters.CSINodeLister,
 	vaLister storagelisters.VolumeAttachmentLister,
 	timeout *time.Duration,
@@ -95,7 +93,6 @@ func NewCSIHandler(
 		attacher:                attacher,
 		CSIVolumeLister:         CSIVolumeLister,
 		pvLister:                pvLister,
-		nodeLister:              nodeLister,
 		csiNodeLister:           csiNodeLister,
 		vaLister:                vaLister,
 		timeout:                 *timeout,
@@ -682,7 +679,7 @@ func (h *csiHandler) getCredentialsFromPV(csiSource *v1.CSIPersistentVolumeSourc
 	return credentials, nil
 }
 
-// getNodeID finds node ID from Node API object. If caller wants, it can find
+// getNodeID finds node ID from CSINode API object. If caller wants, it can find
 // node ID stored in VolumeAttachment annotation.
 func (h *csiHandler) getNodeID(driver string, nodeName string, va *storage.VolumeAttachment) (string, error) {
 	// Try to find CSINode first.
@@ -692,19 +689,14 @@ func (h *csiHandler) getNodeID(driver string, nodeName string, va *storage.Volum
 			klog.V(4).Infof("Found NodeID %s in CSINode %s", nodeID, nodeName)
 			return nodeID, nil
 		}
-		klog.V(4).Infof("CSINode %s does not contain driver %s", nodeName, driver)
 		// CSINode exists, but does not have the requested driver.
-		// Fall through to Node annotation.
-	} else {
-		// Can't get CSINode, fall through to Node annotation.
-		klog.V(4).Infof("Can't get CSINode %s: %s", nodeName, err)
+		errMessage := fmt.Sprintf("CSINode %s does not contain driver %s", nodeName, driver)
+		klog.V(4).Info(errMessage)
+		return "", errors.New(errMessage)
 	}
 
-	// Check Node annotation.
-	node, err := h.nodeLister.Get(nodeName)
-	if err == nil {
-		return GetNodeIDFromNode(driver, node)
-	}
+	// Can't get CSINode, check Volume Attachment.
+	klog.V(4).Infof("Can't get CSINode %s: %s", nodeName, err)
 
 	// Check VolumeAttachment annotation as the last resort if caller wants so (i.e. has provided one).
 	if va == nil {
@@ -714,7 +706,7 @@ func (h *csiHandler) getNodeID(driver string, nodeName string, va *storage.Volum
 		return nodeID, nil
 	}
 
-	// return nodeLister.Get error
+	// return csiNodeLister.Get error
 	return "", err
 }
 
diff --git a/pkg/controller/util.go b/pkg/controller/util.go
index e1e86f42..f65420d9 100644
--- a/pkg/controller/util.go
+++ b/pkg/controller/util.go
@@ -23,8 +23,8 @@ import (
 	"regexp"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
-	"github.com/evanphx/json-patch"
-	"k8s.io/api/core/v1"
+	jsonpatch "github.com/evanphx/json-patch"
+	v1 "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1beta1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
@@ -93,7 +93,6 @@ func markAsDetached(client kubernetes.Interface, va *storage.VolumeAttachment) (
 
 const (
 	defaultFSType              = "ext4"
-	nodeIDAnnotation           = "csi.volume.kubernetes.io/nodeid"
 	csiVolAttribsAnnotationKey = "csi.volume.kubernetes.io/volume-attributes"
 	vaNodeIDAnnotation         = "csi.alpha.kubernetes.io/node-id"
 )
@@ -114,25 +113,6 @@ func GetFinalizerName(driver string) string {
 	return "external-attacher/" + SanitizeDriverName(driver)
 }
 
-// GetNodeIDFromNode returns nodeID string from node annotations.
-func GetNodeIDFromNode(driver string, node *v1.Node) (string, error) {
-	nodeIDJSON, ok := node.Annotations[nodeIDAnnotation]
-	if !ok {
-		return "", fmt.Errorf("node %q has no NodeID annotation", node.Name)
-	}
-
-	var nodeIDs map[string]string
-	if err := json.Unmarshal([]byte(nodeIDJSON), &nodeIDs); err != nil {
-		return "", fmt.Errorf("cannot parse NodeID annotation on node %q: %s", node.Name, err)
-	}
-	nodeID, ok := nodeIDs[driver]
-	if !ok {
-		return "", fmt.Errorf("cannot find NodeID for driver %q for node %q", driver, node.Name)
-	}
-
-	return nodeID, nil
-}
-
 // GetNodeIDFromCSINode returns nodeID from CSIDriverInfoSpec
 func GetNodeIDFromCSINode(driver string, csiNode *storage.CSINode) (string, bool) {
 	for _, d := range csiNode.Spec.Drivers {


Test patch:
diff --git a/pkg/controller/csi_handler_test.go b/pkg/controller/csi_handler_test.go
index 8f2d06ee..d60f9262 100644
--- a/pkg/controller/csi_handler_test.go
+++ b/pkg/controller/csi_handler_test.go
@@ -57,7 +57,6 @@ func csiHandlerFactory(client kubernetes.Interface, informerFactory informers.Sh
 		csi,
 		lister,
 		informerFactory.Core().V1().PersistentVolumes().Lister(),
-		informerFactory.Core().V1().Nodes().Lister(),
 		informerFactory.Storage().V1beta1().CSINodes().Lister(),
 		informerFactory.Storage().V1beta1().VolumeAttachments().Lister(),
 		&timeout,
@@ -73,7 +72,6 @@ func csiHandlerFactoryNoReadOnly(client kubernetes.Interface, informerFactory in
 		csi,
 		lister,
 		informerFactory.Core().V1().PersistentVolumes().Lister(),
-		informerFactory.Core().V1().Nodes().Lister(),
 		informerFactory.Storage().V1beta1().CSINodes().Lister(),
 		informerFactory.Storage().V1beta1().VolumeAttachments().Lister(),
 		&timeout,
@@ -194,17 +192,16 @@ func node() *v1.Node {
 	return &v1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: testNodeName,
-			Annotations: map[string]string{
-				"csi.volume.kubernetes.io/nodeid": fmt.Sprintf("{ %q: %q }", testAttacherName, testNodeID),
-			},
 		},
 	}
 }
 
-func nodeWithoutAnnotations() *v1.Node {
-	n := node()
-	n.Annotations = nil
-	return n
+func nodeWithAnnotations() *v1.Node {
+	node := node()
+	node.Annotations = map[string]string{
+		"csi.volume.kubernetes.io/nodeid": fmt.Sprintf("{ %q: %q }", testAttacherName, testNodeID),
+	}
+	return node
 }
 
 func csiNode() *storage.CSINode {
@@ -295,7 +292,7 @@ func TestCSIHandler(t *testing.T) {
 		//
 		{
 			name:           "VolumeAttachment added -> successful attachment",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false /*attached*/, "" /*finalizer*/, nil /* annotations */),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -312,7 +309,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with InlineVolumeSpec -> successful attachment",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			addedVA:        vaWithInlineSpec(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */)),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -328,7 +325,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "readOnly VolumeAttachment added -> successful attachment",
-			initialObjects: []runtime.Object{pvReadOnly(pvWithFinalizer()), node()},
+			initialObjects: []runtime.Object{pvReadOnly(pvWithFinalizer()), csiNode()},
 			addedVA:        va(false /*attached*/, "" /*finalizer*/, nil /* annotations */),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -345,7 +342,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "readOnly VolumeAttachment with InlineVolumeSpec -> successful attachment",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			addedVA:        vaInlineSpecReadOnly(vaWithInlineSpec(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */))),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -361,7 +358,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment updated -> successful attachment",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			updatedVA:      va(false, "", nil),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -378,7 +375,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with InlineVolumeSpec updated -> successful attachment",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			updatedVA:      vaWithInlineSpec(va(false /*attached*/, "" /*finalizer*/, nil /* annotations */)),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -395,7 +392,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with attributes -> successful attachment",
-			initialObjects: []runtime.Object{pvWithAttributes(pvWithFinalizer(), map[string]string{"foo": "bar"}), node()},
+			initialObjects: []runtime.Object{pvWithAttributes(pvWithFinalizer(), map[string]string{"foo": "bar"}), csiNode()},
 			updatedVA:      va(false, "", nil),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -412,7 +409,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with InlineVolumeSpec and attributes -> successful attachment",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			updatedVA:      vaInlineSpecWithAttributes(vaWithInlineSpec(va(false, "", nil)) /*va*/, map[string]string{"foo": "bar"} /*attributes*/),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -429,7 +426,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with secrets -> successful attachment",
-			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "secret"), node(), secret()},
+			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "secret"), secret(), csiNode()},
 			updatedVA:      va(false, "", nil),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "secret"),
@@ -447,7 +444,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with InlineVolumeSpec and secrets -> successful attachment",
-			initialObjects: []runtime.Object{node(), secret()},
+			initialObjects: []runtime.Object{secret(), csiNode()},
 			updatedVA:      vaInlineSpecWithSecret(vaWithInlineSpec(va(false, "", nil)) /*va*/, "secret" /*secret*/),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "secret"),
@@ -465,7 +462,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with empty secrets -> successful attachment",
-			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "emptySecret"), node(), emptySecret()},
+			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "emptySecret"), emptySecret(), csiNode()},
 			updatedVA:      va(false, "", nil),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"),
@@ -483,7 +480,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with InlineVolumeSpec and empty secrets -> successful attachment",
-			initialObjects: []runtime.Object{node(), emptySecret()},
+			initialObjects: []runtime.Object{emptySecret(), csiNode()},
 			updatedVA:      vaInlineSpecWithSecret(vaWithInlineSpec(va(false, "", nil)) /*va*/, "emptySecret" /*secret*/),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"),
@@ -501,7 +498,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with missing secrets -> error",
-			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "unknownSecret"), node()},
+			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "unknownSecret"), csiNode()},
 			updatedVA:      va(false, "", nil),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "unknownSecret"),
@@ -513,7 +510,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment updated -> PV finalizer is added",
-			initialObjects: []runtime.Object{pv(), node()},
+			initialObjects: []runtime.Object{pv(), csiNode()},
 			updatedVA:      va(false, "", nil),
 			expectedActions: []core.Action{
 				// PV Finalizer after VA
@@ -534,7 +531,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "error saving PV finalizer -> controller retries",
-			initialObjects: []runtime.Object{pv(), node()},
+			initialObjects: []runtime.Object{pv(), csiNode()},
 			updatedVA:      va(false, "", nil),
 			reactors: []reaction{
 				{
@@ -586,14 +583,14 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:             "already attached volume -> ignored",
-			initialObjects:   []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects:   []runtime.Object{pvWithFinalizer(), csiNode()},
 			updatedVA:        va(true, fin, ann),
 			expectedActions:  []core.Action{},
 			expectedCSICalls: []csiCall{},
 		},
 		{
 			name:           "PV with deletion timestamp -> ignored with error",
-			initialObjects: []runtime.Object{pvDeleted(pv()), node()},
+			initialObjects: []runtime.Object{pvDeleted(pv()), csiNode()},
 			updatedVA:      va(false, fin, ann),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -604,7 +601,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment added -> successful attachment incl. metadata",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false, "", nil),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -620,13 +617,13 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:            "unknown driver -> ignored",
-			initialObjects:  []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects:  []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:         vaWithInvalidDriver(va(false, fin, ann)),
 			expectedActions: []core.Action{},
 		},
 		{
 			name:           "unknown PV -> error",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			addedVA:        va(false, fin, ann),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -636,7 +633,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "unknown PV -> error + error saving the error",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			addedVA:        va(false, fin, ann),
 			reactors: []reaction{
 				{
@@ -670,7 +667,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "neither PV nor InlineVolumeSpec reference-> error",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        vaWithNoPVReferenceNorInlineVolumeSpec(va(false, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -680,7 +677,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "both PV and InlineVolumeSpec reference-> error",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        vaAddInlineSpec(va(false, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -695,12 +692,12 @@ 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),
-						"node \"node1\" not found"))),
+						"csinode.storage.k8s.io \"node1\" not found"))),
 			},
 		},
 		{
 			name:           "failed write with VA finializers -> controller retries",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false, "", nil),
 			reactors: []reaction{
 				{
@@ -744,7 +741,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "failed write with attached=true -> controller retries",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false, "", nil),
 			reactors: []reaction{
 				{
@@ -788,7 +785,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "CSI attach fails -> controller retries",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false, "", nil),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -815,7 +812,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "CSI attach times out -> controller retries",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false, "", nil),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -832,28 +829,38 @@ func TestCSIHandler(t *testing.T) {
 			},
 		},
 		{
-			name:           "Node without annotations -> error",
-			initialObjects: []runtime.Object{pvWithFinalizer(), nodeWithoutAnnotations()},
+			name:           "Node without CSINode -> error",
+			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			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), "csinode.storage.k8s.io \"node1\" not found"))),
+			},
+		},
+		{
+			name:           "Node with annotations, CSINode is absent -> error",
+			initialObjects: []runtime.Object{pvWithFinalizer(), nodeWithAnnotations()},
 			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"))),
+						vaWithAttachError(va(false, fin, ann), "csinode.storage.k8s.io \"node1\" not found"))),
 			},
 		},
 		{
-			name:           "CSINode exists without the driver, Node without annotations -> error",
-			initialObjects: []runtime.Object{pvWithFinalizer(), nodeWithoutAnnotations(), csiNodeEmpty()},
+			name:           "CSINode exists without the driver -> error",
+			initialObjects: []runtime.Object{pvWithFinalizer(), 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"))),
+						vaWithAttachError(va(false, fin, ann), "CSINode node1 does not contain driver csi/test"))),
 			},
 		},
 		{
 			name:           "CSINode exists with the driver, Node without annotations -> success",
-			initialObjects: []runtime.Object{pvWithFinalizer(), nodeWithoutAnnotations(), csiNode()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false /*attached*/, "" /*finalizer*/, nil),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -870,7 +877,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with GCEPersistentDiskVolumeSource -> successful attachment",
-			initialObjects: []runtime.Object{gcePDPVWithFinalizer(), node()},
+			initialObjects: []runtime.Object{gcePDPVWithFinalizer(), csiNode()},
 			addedVA:        va(false /*attached*/, "" /*finalizer*/, nil),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -891,7 +898,7 @@ func TestCSIHandler(t *testing.T) {
 
 		{
 			name:           "VolumeAttachment marked for deletion -> successful detach",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        deleted(va(true, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -904,7 +911,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with InlineVolumeSpec marked for deletion -> successful detach",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			addedVA:        deleted(vaWithInlineSpec(va(true, fin, ann))),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -917,7 +924,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "volume with secrets -> successful detach",
-			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "secret"), node(), secret()},
+			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "secret"), secret()},
 			addedVA:        deleted(va(true, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "secret"),
@@ -931,7 +938,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "volume attachment with InlineVolumeSpec and secrets -> successful detach",
-			initialObjects: []runtime.Object{node(), secret()},
+			initialObjects: []runtime.Object{secret()},
 			addedVA:        deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(true, fin, ann)), "secret")),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "secret"),
@@ -945,7 +952,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "volume with empty secrets -> successful detach",
-			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "emptySecret"), node(), emptySecret()},
+			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "emptySecret"), emptySecret()},
 			addedVA:        deleted(va(true, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"),
@@ -959,7 +966,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "volume attachment with InlineVolumeSpec and empty secrets -> successful detach",
-			initialObjects: []runtime.Object{node(), emptySecret()},
+			initialObjects: []runtime.Object{emptySecret()},
 			addedVA:        deleted(vaInlineSpecWithSecret(vaWithInlineSpec(va(true, fin, ann)), "emptySecret")),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "emptySecret"),
@@ -973,7 +980,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "volume with missing secrets -> error",
-			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "unknownSecret"), node()},
+			initialObjects: []runtime.Object{pvWithSecret(pvWithFinalizer(), "unknownSecret"), csiNode()},
 			addedVA:        deleted(va(true, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewGetAction(secretGroupResourceVersion, "default", "unknownSecret"),
@@ -986,7 +993,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "CSI detach fails with an error -> controller retries",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        deleted(va(true, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -1003,7 +1010,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "CSI detach times out -> controller retries",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        deleted(va(true, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -1017,14 +1024,14 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:             "already detached volume -> ignored",
-			initialObjects:   []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects:   []runtime.Object{pvWithFinalizer(), csiNode()},
 			updatedVA:        deleted(va(false, "", nil)),
 			expectedActions:  []core.Action{},
 			expectedCSICalls: []csiCall{},
 		},
 		{
 			name:           "detach unknown PV -> error",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			addedVA:        deleted(va(true, fin, ann)),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -1034,7 +1041,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "detach unknown PV -> error + error saving the error",
-			initialObjects: []runtime.Object{node()},
+			initialObjects: []runtime.Object{csiNode()},
 			addedVA:        deleted(va(true, fin, ann)),
 			reactors: []reaction{
 				{
@@ -1068,7 +1075,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "detach VA with neither PV nor InlineCSIVolumeSource reference-> error",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        deleted(vaWithNoPVReferenceNorInlineVolumeSpec(va(true, fin, ann))),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -1079,7 +1086,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "detach VA with both PV and InlineCSIVolumeSource reference-> error",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        deleted(vaAddInlineSpec(va(true, fin, ann))),
 			expectedActions: []core.Action{
 				core.NewPatchAction(vaGroupResourceVersion, metav1.NamespaceNone, testPVName+"-"+testNodeName,
@@ -1095,7 +1102,7 @@ func TestCSIHandler(t *testing.T) {
 			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")))),
+						deleted(vaWithDetachError(va(true, fin, nil), "csinode.storage.k8s.io \"node1\" not found")))),
 			},
 		},
 		{
@@ -1112,22 +1119,9 @@ func TestCSIHandler(t *testing.T) {
 				{"detach", testVolumeHandle, "annotatedNodeID", noAttrs, noSecrets, readWrite, success, detached, noMetadata, 0},
 			},
 		},
-		{
-			name:           "VolumeAttachment marked for deletion -> node is preferred over VA annotation for NodeID",
-			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"})))),
-			},
-			expectedCSICalls: []csiCall{
-				{"detach", testVolumeHandle, testNodeID, noAttrs, noSecrets, readWrite, success, detached, noMetadata, 0},
-			},
-		},
 		{
 			name:           "failed write with attached=false -> controller retries",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        deleted(va(false, fin, ann)),
 			reactors: []reaction{
 				{
@@ -1170,7 +1164,7 @@ func TestCSIHandler(t *testing.T) {
 		},
 		{
 			name:           "VolumeAttachment with GCEPersistentDiskVolumeSource marked for deletion -> successful detach",
-			initialObjects: []runtime.Object{gcePDPVWithFinalizer(), node()},
+			initialObjects: []runtime.Object{gcePDPVWithFinalizer(), csiNode()},
 			addedVA:        deleted(va(true /*attached*/, fin /*finalizer*/, ann)),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -1354,7 +1348,7 @@ func TestCSIHandlerReadOnly(t *testing.T) {
 		//
 		{
 			name:           "read-write PV -> attached as read-write",
-			initialObjects: []runtime.Object{pvWithFinalizer(), node()},
+			initialObjects: []runtime.Object{pvWithFinalizer(), csiNode()},
 			addedVA:        va(false /*attached*/, "" /*finalizer*/, nil /* annotations */),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
@@ -1371,7 +1365,7 @@ func TestCSIHandlerReadOnly(t *testing.T) {
 		},
 		{
 			name:           "read-only PV -> attached as read-write",
-			initialObjects: []runtime.Object{pvReadOnly(pvWithFinalizer()), node()},
+			initialObjects: []runtime.Object{pvReadOnly(pvWithFinalizer()), csiNode()},
 			addedVA:        va(false /*attached*/, "" /*finalizer*/, nil /* annotations */),
 			expectedActions: []core.Action{
 				// Finalizer is saved first
diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go
index f2b6a9bb..321c836f 100644
--- a/pkg/controller/util_test.go
+++ b/pkg/controller/util_test.go
@@ -20,78 +20,13 @@ import (
 	"testing"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
-	"k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 const (
 	driverName = "foo/bar"
 )
 
-func TestGetNodeIDFromNode(t *testing.T) {
-	tests := []struct {
-		name        string
-		annotations map[string]string
-		expectedID  string
-		expectError bool
-	}{
-		{
-			name:        "single key",
-			annotations: map[string]string{"csi.volume.kubernetes.io/nodeid": "{\"foo/bar\": \"MyNodeID\"}"},
-			expectedID:  "MyNodeID",
-			expectError: false,
-		},
-		{
-			name: "multiple keys",
-			annotations: map[string]string{
-				"csi.volume.kubernetes.io/nodeid": "{\"foo/bar\": \"MyNodeID\", \"-foo/bar\": \"MyNodeID2\", \"foo/bar-\": \"MyNodeID3\"}",
-			},
-			expectedID:  "MyNodeID",
-			expectError: false,
-		},
-		{
-			name:        "no annotations",
-			annotations: nil,
-			expectedID:  "",
-			expectError: true,
-		},
-		{
-			name:        "invalid JSON",
-			annotations: map[string]string{"csi.volume.kubernetes.io/nodeid": "\"foo/bar\": \"MyNodeID\""},
-			expectedID:  "",
-			expectError: true,
-		},
-		{
-			name: "annotations for another driver",
-			annotations: map[string]string{
-				"csi.volume.kubernetes.io/nodeid": "{\"-foo/bar\": \"MyNodeID2\", \"foo/bar-\": \"MyNodeID3\"}",
-			},
-			expectedID:  "",
-			expectError: true,
-		},
-	}
-
-	for _, test := range tests {
-		node := &v1.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:        "abc",
-				Annotations: test.annotations,
-			},
-		}
-		nodeID, err := GetNodeIDFromNode(driverName, node)
-
-		if err == nil && test.expectError {
-			t.Errorf("test %s: expected error, got none", test.name)
-		}
-		if err != nil && !test.expectError {
-			t.Errorf("test %s: got error: %s", test.name, err)
-		}
-		if !test.expectError && nodeID != test.expectedID {
-			t.Errorf("test %s: unexpected NodeID: %s", test.name, nodeID)
-		}
-	}
-}
-
 func createBlockCapability(mode csi.VolumeCapability_AccessMode_Mode) *csi.VolumeCapability {
 	return &csi.VolumeCapability{
 		AccessType: &csi.VolumeCapability_Block{