Merge pull request kubernetes-csi#20 from jsafrane/rebase-v2.2.0

Rebase v2.2.0

CHANGELOG-2.2.md

@@ -0,0 +1,13 @@
+# Changelog since v2.1.0
+
+### Bug Fixes
+
+- A bug that prevented the external-attacher from releasing its finalizer from a `PersistentVolume` object that was created using a legacy storage class provisioner and migrated to CSI has been fixed. ([#218](https://github.com/kubernetes-csi/external-attacher/pull/218), [@rfranzke](https://github.com/rfranzke))
+- Update package path to v2. Vendoring with dep depends on https://github.com/golang/dep/pull/1963 or the workaround described in v2/README.md. ([#209](https://github.com/kubernetes-csi/external-attacher/pull/209), [@alex1989hu](https://github.com/alex1989hu))
+
+
+### Other Notable Changes
+
+- 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. ([#213](https://github.com/kubernetes-csi/external-attacher/pull/213), [@Danil-Grigorev](https://github.com/Danil-Grigorev))
+
+

README.md

@@ -21,7 +21,7 @@ This information reflects the head of this branch.
 
 | Compatible with CSI Version                                                                | Container Image             | Min K8s Version | Recommended K8s Version |
 | ------------------------------------------------------------------------------------------ | ----------------------------| --------------- | ----------------------- |
-| [CSI Spec v1.2.0](https://github.com/container-storage-interface/spec/releases/tag/v1.2.0) | quay.io/k8scsi/csi-attacher | 1.15            | 1.17                    |
+| [CSI Spec v1.2.0](https://github.com/container-storage-interface/spec/releases/tag/v1.2.0) | quay.io/k8scsi/csi-attacher | 1.14            | 1.18                    |
 
 ## Feature Status
 
@@ -31,10 +31,9 @@ The following table reflects the head of this branch.
 
 | Feature       | Status  | Default | Description                                                                                   |
 | ------------- | ------- | ------- | --------------------------------------------------------------------------------------------- |
-| CSINode*      | Beta    | On      | external-attacher uses the CSINode object to get the driver's node name instead of the Node annotation. |
 | CSIMigration* | Beta    | On      | [Migrating in-tree volume plugins to CSI](https://kubernetes.io/docs/concepts/storage/volumes/#csi-migration). |
 
-*) There are no special feature gates for these features. They are enabled by turning on the corresponding features in Kubernetes.
+*) There is no special feature gate for this feature. It is enabled by turning on the corresponding features in Kubernetes.
 
 All other external-attacher features and the external-attacher itself is considered GA and fully supported.
 

cmd/csi-attacher/main.go

@@ -36,8 +36,8 @@ import (
 	"github.com/kubernetes-csi/csi-lib-utils/leaderelection"
 	"github.com/kubernetes-csi/csi-lib-utils/metrics"
 	"github.com/kubernetes-csi/csi-lib-utils/rpc"
-	"github.com/kubernetes-csi/external-attacher/pkg/attacher"
-	"github.com/kubernetes-csi/external-attacher/pkg/controller"
+	"github.com/kubernetes-csi/external-attacher/v2/pkg/attacher"
+	"github.com/kubernetes-csi/external-attacher/v2/pkg/controller"
 	"google.golang.org/grpc"
 )
 
@@ -46,8 +46,7 @@ const (
 	// Default timeout of short CSI calls like GetPluginInfo
 	csiTimeout = time.Second
 
-	leaderElectionTypeLeases     = "leases"
-	leaderElectionTypeConfigMaps = "configmaps"
+	leaderElectionTypeLeases = "leases"
 )
 
 // Command line flags
@@ -155,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)

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"]

doc/development.md

@@ -8,7 +8,7 @@ csi-attacher -kubeconfig ~/.kube/config -v 5 -csi-address /run/csi/socket
 
 ## Implementation details
 
-The external-attacher follows [controller](https://github.com/kubernetes/community/blob/master/contributors/devel/controllers.md) pattern and uses informers to watch for `VolumeAttachment` and `PersistentVolume` create/update/delete events. It filters out `VolumeAttachment` instances with `Attacher==<CSI driver name>` and processes these events in workqueues with exponential backoff. Real handling is deferred to `Handler` interface.
+The external-attacher follows [controller](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-api-machinery/controllers.md) pattern and uses informers to watch for `VolumeAttachment` and `PersistentVolume` create/update/delete events. It filters out `VolumeAttachment` instances with `Attacher==<CSI driver name>` and processes these events in workqueues with exponential backoff. Real handling is deferred to `Handler` interface.
 
 `Handler` interface has two implementations, trivial and real one.
 

go.mod

@@ -1,6 +1,6 @@
-module github.com/kubernetes-csi/external-attacher
+module github.com/kubernetes-csi/external-attacher/v2
 
-go 1.12
+go 1.13
 
 require (
 	github.com/container-storage-interface/spec v1.2.0

pkg/controller/controller.go

@@ -20,12 +20,11 @@ import (
 	"fmt"
 	"time"
 
-	"k8s.io/klog"
-
 	v1 "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1beta1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	coreinformers "k8s.io/client-go/informers/core/v1"
 	storageinformers "k8s.io/client-go/informers/storage/v1beta1"
@@ -37,6 +36,7 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
+	"k8s.io/klog"
 )
 
 // CSIAttachController is a controller that attaches / detaches CSI volumes using provided Handler interface
@@ -217,10 +217,7 @@ func (ctrl *CSIAttachController) syncVA() {
 }
 
 func (ctrl *CSIAttachController) processFinalizers(pv *v1.PersistentVolume) bool {
-	if pv.Spec.CSI != nil && pv.Spec.CSI.Driver == ctrl.attacherName {
-		return true
-	}
-	return false
+	return pv.DeletionTimestamp != nil && sets.NewString(pv.Finalizers...).Has(GetFinalizerName(ctrl.attacherName))
 }
 
 // syncPV deals with one key off the queue.  It returns false when it's time to quit.

pkg/controller/csi_handler.go

@@ -25,7 +25,7 @@ import (
 
 	"k8s.io/klog"
 
-	"github.com/kubernetes-csi/external-attacher/pkg/attacher"
+	"github.com/kubernetes-csi/external-attacher/v2/pkg/attacher"
 	v1 "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -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,
@@ -188,7 +185,7 @@ func (h *csiHandler) ReconcileVA() error {
 
 // setForceSync sets the intention that next time the VolumeAttachment
 // referenced by vaName is processed on the VA queue that attach or detach will
-// proceed even when the VA.Status.Attached mayalready show the desired state
+// proceed even when the VA.Status.Attached may already show the desired state
 func (h *csiHandler) setForceSync(vaName string) {
 	h.forceSyncMux.Lock()
 	defer h.forceSyncMux.Unlock()
@@ -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
 }