Cherry-pick fixes into 1.0

Gopkg.toml

@@ -20,18 +20,24 @@
   name = "google.golang.org/grpc"
   version = "1.9.2"
 
-[[constraint]]
+[[override]]
+  name = "github.com/json-iterator/go"
+  version = "1.1.4"
+
+// TODO: use [[constraint]] when kubernetes-csi/kubernetes-csi-migration-library uses 1.13 code
+[[override]]
   name = "k8s.io/api"
-  version = "kubernetes-1.12.1"
+  version = "kubernetes-1.13.0-beta.2"
 
-[[constraint]]
+[[override]]
   name = "k8s.io/apimachinery"
-  version = "kubernetes-1.12.0"
+  version = "kubernetes-1.13.0-beta.2"
 
-[[constraint]]
+[[override]]
   name = "k8s.io/client-go"
-  version = "kubernetes-1.12.0"
+  version = "kubernetes-1.13.0-beta.2"
+
 
 [[override]]
-  name = "github.com/json-iterator/go"
-  version = "1.1.4"
+  name = "k8s.io/csi-api"
+  version = "kubernetes-1.13.0-beta.2"

cmd/csi-attacher/main.go

@@ -137,7 +137,7 @@ func main() {
 			glog.V(2).Infof("CSI driver does not support Plugin Controller Service, using trivial handler")
 		} else {
 			// Find out if the driver supports attach/detach.
-			supportsAttach, err := csiConn.SupportsControllerPublish(ctx)
+			supportsAttach, supportsReadOnly, err := csiConn.SupportsControllerPublish(ctx)
 			if err != nil {
 				glog.Error(err.Error())
 				os.Exit(1)
@@ -148,7 +148,7 @@ func main() {
 				vaLister := factory.Storage().V1beta1().VolumeAttachments().Lister()
 				csiFactory := csiinformers.NewSharedInformerFactory(csiClientset, *resync)
 				nodeInfoLister := csiFactory.Csi().V1alpha1().CSINodeInfos().Lister()
-				handler = controller.NewCSIHandler(clientset, csiClientset, attacher, csiConn, pvLister, nodeLister, nodeInfoLister, vaLister, timeout)
+				handler = controller.NewCSIHandler(clientset, csiClientset, attacher, csiConn, pvLister, nodeLister, nodeInfoLister, vaLister, timeout, supportsReadOnly)
 				glog.V(2).Infof("CSI driver supports ControllerPublishUnpublish, using real CSI handler")
 			} else {
 				handler = controller.NewTrivialHandler(clientset)

pkg/connection/connection.go

@@ -40,7 +40,7 @@ type CSIConnection interface {
 
 	// SupportsControllerPublish returns true if the CSI driver reports
 	// PUBLISH_UNPUBLISH_VOLUME in ControllerGetCapabilities() gRPC call.
-	SupportsControllerPublish(ctx context.Context) (bool, error)
+	SupportsControllerPublish(ctx context.Context) (supportsControllerPublish bool, supportsPublishReadOnly bool, err error)
 
 	// SupportsPluginControllerService return true if the CSI driver reports
 	// CONTROLLER_SERVICE in GetPluginCapabilities() gRPC call.
@@ -144,13 +144,16 @@ func (c *csiConnection) Probe(ctx context.Context) error {
 	return nil
 }
 
-func (c *csiConnection) SupportsControllerPublish(ctx context.Context) (bool, error) {
+func (c *csiConnection) SupportsControllerPublish(ctx context.Context) (supportsControllerPublish bool, supportsPublishReadOnly bool, err error) {
+	supportsControllerPublish = false
+	supportsPublishReadOnly = false
+
 	client := csi.NewControllerClient(c.conn)
 	req := csi.ControllerGetCapabilitiesRequest{}
 
 	rsp, err := client.ControllerGetCapabilities(ctx, &req)
 	if err != nil {
-		return false, err
+		return false, false, err
 	}
 	caps := rsp.GetCapabilities()
 	for _, cap := range caps {
@@ -162,10 +165,13 @@ func (c *csiConnection) SupportsControllerPublish(ctx context.Context) (bool, er
 			continue
 		}
 		if rpc.GetType() == csi.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME {
-			return true, nil
+			supportsControllerPublish = true
+		}
+		if rpc.GetType() == csi.ControllerServiceCapability_RPC_PUBLISH_READONLY {
+			supportsPublishReadOnly = true
 		}
 	}
-	return false, nil
+	return supportsControllerPublish, supportsPublishReadOnly, nil
 }
 
 func (c *csiConnection) SupportsPluginControllerService(ctx context.Context) (bool, error) {

pkg/connection/connection_test.go

@@ -142,10 +142,12 @@ func TestGetPluginInfo(t *testing.T) {
 
 func TestSupportsControllerPublish(t *testing.T) {
 	tests := []struct {
-		name        string
-		output      *csi.ControllerGetCapabilitiesResponse
-		injectError bool
-		expectError bool
+		name                   string
+		output                 *csi.ControllerGetCapabilitiesResponse
+		injectError            bool
+		expectSupportsPublish  bool
+		expectSupportsReadOnly bool
+		expectError            bool
 	}{
 		{
 			name: "success",
@@ -167,7 +169,33 @@ func TestSupportsControllerPublish(t *testing.T) {
 					},
 				},
 			},
-			expectError: false,
+			expectSupportsPublish:  true,
+			expectSupportsReadOnly: false,
+			expectError:            false,
+		},
+		{
+			name: "supports read only",
+			output: &csi.ControllerGetCapabilitiesResponse{
+				Capabilities: []*csi.ControllerServiceCapability{
+					{
+						Type: &csi.ControllerServiceCapability_Rpc{
+							Rpc: &csi.ControllerServiceCapability_RPC{
+								Type: csi.ControllerServiceCapability_RPC_PUBLISH_READONLY,
+							},
+						},
+					},
+					{
+						Type: &csi.ControllerServiceCapability_Rpc{
+							Rpc: &csi.ControllerServiceCapability_RPC{
+								Type: csi.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME,
+							},
+						},
+					},
+				},
+			},
+			expectSupportsPublish:  true,
+			expectSupportsReadOnly: true,
+			expectError:            false,
 		},
 		{
 			name:        "gRPC error",
@@ -188,7 +216,9 @@ func TestSupportsControllerPublish(t *testing.T) {
 					},
 				},
 			},
-			expectError: false,
+			expectSupportsPublish:  false,
+			expectSupportsReadOnly: false,
+			expectError:            false,
 		},
 		{
 			name: "empty capability",
@@ -199,14 +229,18 @@ func TestSupportsControllerPublish(t *testing.T) {
 					},
 				},
 			},
-			expectError: false,
+			expectSupportsPublish:  false,
+			expectSupportsReadOnly: false,
+			expectError:            false,
 		},
 		{
 			name: "no capabilities",
 			output: &csi.ControllerGetCapabilitiesResponse{
 				Capabilities: []*csi.ControllerServiceCapability{},
 			},
-			expectError: false,
+			expectSupportsPublish:  false,
+			expectSupportsReadOnly: false,
+			expectError:            false,
 		},
 	}
 
@@ -231,13 +265,19 @@ func TestSupportsControllerPublish(t *testing.T) {
 		// Setup expectation
 		controllerServer.EXPECT().ControllerGetCapabilities(gomock.Any(), pbMatch(in)).Return(out, injectedErr).Times(1)
 
-		_, err = csiConn.SupportsControllerPublish(context.Background())
+		supportsPublish, supportsReadOnly, err := csiConn.SupportsControllerPublish(context.Background())
 		if test.expectError && err == nil {
 			t.Errorf("test %q: Expected error, got none", test.name)
 		}
 		if !test.expectError && err != nil {
 			t.Errorf("test %q: got error: %v", test.name, err)
 		}
+		if test.expectSupportsPublish != supportsPublish {
+			t.Errorf("test %q: expected PUBLISH_UNPUBLISH_VOLUME %t, got %t", test.name, test.expectSupportsPublish, supportsPublish)
+		}
+		if test.expectSupportsReadOnly != supportsReadOnly {
+			t.Errorf("test %q: expected PUBLISH_READONLY %t, got %t", test.name, test.expectSupportsReadOnly, supportsReadOnly)
+		}
 	}
 }
 

pkg/controller/csi_handler.go

@@ -23,6 +23,9 @@ import (
 
 	"github.com/golang/glog"
 
+	"github.com/kubernetes-csi/external-attacher/pkg/connection"
+
+	csiMigration "github.com/kubernetes-csi/kubernetes-csi-migration-library"
 	"k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -31,28 +34,25 @@ import (
 	corelisters "k8s.io/client-go/listers/core/v1"
 	storagelisters "k8s.io/client-go/listers/storage/v1beta1"
 	"k8s.io/client-go/util/workqueue"
-	csilisters "k8s.io/csi-api/pkg/client/listers/csi/v1alpha1"
-
-	"github.com/kubernetes-csi/external-attacher/pkg/connection"
-
-	csiMigration "github.com/kubernetes-csi/kubernetes-csi-migration-library"
 	csiclient "k8s.io/csi-api/pkg/client/clientset/versioned"
+	csilisters "k8s.io/csi-api/pkg/client/listers/csi/v1alpha1"
 )
 
 // csiHandler is a handler that calls CSI to attach/detach volume.
 // It adds finalizer to VolumeAttachment instance to make sure they're detached
 // before deletion.
 type csiHandler struct {
-	client           kubernetes.Interface
-	csiClientSet     csiclient.Interface
-	attacherName     string
-	csiConnection    connection.CSIConnection
-	pvLister         corelisters.PersistentVolumeLister
-	nodeLister       corelisters.NodeLister
-	nodeInfoLister   csilisters.CSINodeInfoLister
-	vaLister         storagelisters.VolumeAttachmentLister
-	vaQueue, pvQueue workqueue.RateLimitingInterface
-	timeout          time.Duration
+	client                  kubernetes.Interface
+	csiClientSet            csiclient.Interface
+	attacherName            string
+	csiConnection           connection.CSIConnection
+	pvLister                corelisters.PersistentVolumeLister
+	nodeLister              corelisters.NodeLister
+	nodeInfoLister          csilisters.CSINodeInfoLister
+	vaLister                storagelisters.VolumeAttachmentLister
+	vaQueue, pvQueue        workqueue.RateLimitingInterface
+	timeout                 time.Duration
+	supportsPublishReadOnly bool
 }
 
 var _ Handler = &csiHandler{}
@@ -67,18 +67,20 @@ func NewCSIHandler(
 	nodeLister corelisters.NodeLister,
 	nodeInfoLister csilisters.CSINodeInfoLister,
 	vaLister storagelisters.VolumeAttachmentLister,
-	timeout *time.Duration) Handler {
+	timeout *time.Duration,
+	supportsPublishReadOnly bool) Handler {
 
 	return &csiHandler{
-		client:         client,
-		csiClientSet:   csiClientSet,
-		attacherName:   attacherName,
-		csiConnection:  csiConnection,
-		pvLister:       pvLister,
-		nodeLister:     nodeLister,
-		nodeInfoLister: nodeInfoLister,
-		vaLister:       vaLister,
-		timeout:        *timeout,
+		client:                  client,
+		csiClientSet:            csiClientSet,
+		attacherName:            attacherName,
+		csiConnection:           csiConnection,
+		pvLister:                pvLister,
+		nodeLister:              nodeLister,
+		nodeInfoLister:          nodeInfoLister,
+		vaLister:                vaLister,
+		timeout:                 *timeout,
+		supportsPublishReadOnly: supportsPublishReadOnly,
 	}
 }
 
@@ -289,6 +291,12 @@ func (h *csiHandler) csiAttach(va *storage.VolumeAttachment) (*storage.VolumeAtt
 	if err != nil {
 		return va, nil, err
 	}
+	if !h.supportsPublishReadOnly {
+		// "CO MUST set this field to false if SP does not have the
+		// PUBLISH_READONLY controller capability"
+		readOnly = false
+	}
+
 	volumeCapabilities, err := GetVolumeCapabilities(pv, csiSource)
 	if err != nil {
 		return va, nil, err