Skip to content

Commit

Permalink
Merge pull request #215 from jsafrane/cache-driver-capabilities
Browse files Browse the repository at this point in the history
Cache driver name
  • Loading branch information
k8s-ci-robot authored Jan 18, 2019
2 parents f752762 + f1687f6 commit 64844fb
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 44 deletions.
9 changes: 4 additions & 5 deletions cmd/csi-provisioner/csi-provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,25 @@ package main
import (
goflag "flag"
"fmt"
flag "github.com/spf13/pflag"
"math/rand"
"os"
"strconv"
"strings"
"time"

"github.com/golang/glog"
flag "github.com/spf13/pflag"
"google.golang.org/grpc"

ctrl "github.com/kubernetes-csi/external-provisioner/pkg/controller"
snapclientset "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned"
"github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/controller"
csiclientset "k8s.io/csi-api/pkg/client/clientset/versioned"

"google.golang.org/grpc"

"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"strings"

utilfeature "k8s.io/apiserver/pkg/util/feature"
utilflag "k8s.io/apiserver/pkg/util/flag"
Expand Down Expand Up @@ -145,7 +144,7 @@ func init() {

// Create the provisioner: it implements the Provisioner interface expected by
// the controller
csiProvisioner := ctrl.NewCSIProvisioner(clientset, csiAPIClient, *csiEndpoint, *connectionTimeout, identity, *volumeNamePrefix, *volumeNameUUIDLength, grpcClient, snapClient)
csiProvisioner := ctrl.NewCSIProvisioner(clientset, csiAPIClient, *csiEndpoint, *connectionTimeout, identity, *volumeNamePrefix, *volumeNameUUIDLength, grpcClient, snapClient, provisionerName)
provisionController = controller.NewProvisionController(
clientset,
provisionerName,
Expand Down
33 changes: 12 additions & 21 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,7 @@ type csiProvisioner struct {
volumeNamePrefix string
volumeNameUUIDLength int
config *rest.Config
}

type driverState struct {
driverName string
capabilities sets.Int
driverName string
}

const (
Expand Down Expand Up @@ -326,7 +322,8 @@ func NewCSIProvisioner(client kubernetes.Interface,
volumeNamePrefix string,
volumeNameUUIDLength int,
grpcClient *grpc.ClientConn,
snapshotClient snapclientset.Interface) controller.Provisioner {
snapshotClient snapclientset.Interface,
driverName string) controller.Provisioner {

csiClient := csi.NewControllerClient(grpcClient)
provisioner := &csiProvisioner{
Expand All @@ -339,14 +336,15 @@ func NewCSIProvisioner(client kubernetes.Interface,
identity: identity,
volumeNamePrefix: volumeNamePrefix,
volumeNameUUIDLength: volumeNameUUIDLength,
driverName: driverName,
}
return provisioner
}

// This function get called before any attempt to communicate with the driver.
// Before initiating Create/Delete API calls provisioner checks if Capabilities:
// PluginControllerService, ControllerCreateVolume sre supported and gets the driver name.
func checkDriverState(grpcClient *grpc.ClientConn, timeout time.Duration, needSnapshotSupport bool) (*driverState, error) {
func checkDriverCapabilities(grpcClient *grpc.ClientConn, timeout time.Duration, needSnapshotSupport bool) (sets.Int, error) {
capabilities, err := getDriverCapabilities(grpcClient, timeout)
if err != nil {
return nil, fmt.Errorf("failed to get capabilities: %v", err)
Expand All @@ -371,14 +369,7 @@ func checkDriverState(grpcClient *grpc.ClientConn, timeout time.Duration, needSn
}
}

driverName, err := GetDriverName(grpcClient, timeout)
if err != nil {
return nil, fmt.Errorf("failed to get driver info: %v", err)
}
return &driverState{
driverName: driverName,
capabilities: capabilities,
}, nil
return capabilities, nil
}

func makeVolumeName(prefix, pvcUID string, volumeNameUUIDLength int) (string, error) {
Expand Down Expand Up @@ -470,7 +461,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
}
needSnapshotSupport = true
}
driverState, err := checkDriverState(p.grpcClient, p.timeout, needSnapshotSupport)
capabilities, err := checkDriverCapabilities(p.grpcClient, p.timeout, needSnapshotSupport)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -526,12 +517,12 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
req.VolumeContentSource = volumeContentSource
}

if driverState.capabilities.Has(PluginCapability_ACCESSIBILITY_CONSTRAINTS) &&
if capabilities.Has(PluginCapability_ACCESSIBILITY_CONSTRAINTS) &&
utilfeature.DefaultFeatureGate.Enabled(features.Topology) {
requirements, err := GenerateAccessibilityRequirements(
p.client,
p.csiAPIClient,
driverState.driverName,
p.driverName,
options.PVC.Name,
options.AllowedTopologies,
options.SelectedNode)
Expand Down Expand Up @@ -638,7 +629,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
// TODO wait for CSI VolumeSource API
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{
Driver: driverState.driverName,
Driver: p.driverName,
VolumeHandle: p.volumeIdToHandle(rep.Volume.VolumeId),
VolumeAttributes: volumeAttributes,
ControllerPublishSecretRef: controllerPublishSecretRef,
Expand All @@ -649,7 +640,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
},
}

if driverState.capabilities.Has(PluginCapability_ACCESSIBILITY_CONSTRAINTS) &&
if capabilities.Has(PluginCapability_ACCESSIBILITY_CONSTRAINTS) &&
utilfeature.DefaultFeatureGate.Enabled(features.Topology) {
pv.Spec.NodeAffinity = GenerateVolumeNodeAffinity(rep.Volume.AccessibleTopology)
}
Expand Down Expand Up @@ -756,7 +747,7 @@ func (p *csiProvisioner) Delete(volume *v1.PersistentVolume) error {
}
volumeId := p.volumeHandleToId(volume.Spec.CSI.VolumeHandle)

_, err := checkDriverState(p.grpcClient, p.timeout, false)
_, err := checkDriverCapabilities(p.grpcClient, p.timeout, false)
if err != nil {
return err
}
Expand Down
25 changes: 7 additions & 18 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ import (
)

const (
timeout = 10 * time.Second
timeout = 10 * time.Second
driverName = "test-driver"
)

var (
Expand Down Expand Up @@ -533,7 +534,7 @@ func TestCreateDriverReturnsInvalidCapacityDuringProvision(t *testing.T) {
defer mockController.Finish()
defer driver.Stop()

csiProvisioner := NewCSIProvisioner(nil, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil)
csiProvisioner := NewCSIProvisioner(nil, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName)

// Requested PVC with requestedBytes storage
opts := controller.VolumeOptions{
Expand Down Expand Up @@ -591,10 +592,6 @@ func provisionMockServerSetupExpectations(identityServer *driver.MockIdentitySer
},
},
}, nil).Times(1)
identityServer.EXPECT().GetPluginInfo(gomock.Any(), gomock.Any()).Return(&csi.GetPluginInfoResponse{
Name: "test-driver",
VendorVersion: "test-vendor",
}, nil).Times(1)
}

// provisionFromSnapshotMockServerSetupExpectations mocks plugin and controller capabilities reported
Expand Down Expand Up @@ -629,10 +626,6 @@ func provisionFromSnapshotMockServerSetupExpectations(identityServer *driver.Moc
},
},
}, nil).Times(1)
identityServer.EXPECT().GetPluginInfo(gomock.Any(), gomock.Any()).Return(&csi.GetPluginInfoResponse{
Name: "test-driver",
VendorVersion: "test-vendor",
}, nil).Times(1)
}

func provisionWithTopologyMockServerSetupExpectations(identityServer *driver.MockIdentityServer, controllerServer *driver.MockControllerServer) {
Expand Down Expand Up @@ -665,10 +658,6 @@ func provisionWithTopologyMockServerSetupExpectations(identityServer *driver.Moc
},
},
}, nil).Times(1)
identityServer.EXPECT().GetPluginInfo(gomock.Any(), gomock.Any()).Return(&csi.GetPluginInfoResponse{
Name: "test-driver",
VendorVersion: "test-vendor",
}, nil).Times(1)
}

// Minimal PVC required for tests to function
Expand Down Expand Up @@ -1390,7 +1379,7 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested
clientSet = fakeclientset.NewSimpleClientset()
}

csiProvisioner := NewCSIProvisioner(clientSet, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil)
csiProvisioner := NewCSIProvisioner(clientSet, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1745,7 +1734,7 @@ func TestProvisionFromSnapshot(t *testing.T) {
return true, content, nil
})

csiProvisioner := NewCSIProvisioner(clientSet, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, client)
csiProvisioner := NewCSIProvisioner(clientSet, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, client, driverName)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1840,7 +1829,7 @@ func TestProvisionWithTopology(t *testing.T) {

clientSet := fakeclientset.NewSimpleClientset()
csiClientSet := fakecsiclientset.NewSimpleClientset()
csiProvisioner := NewCSIProvisioner(clientSet, csiClientSet, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil)
csiProvisioner := NewCSIProvisioner(clientSet, csiClientSet, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1878,7 +1867,7 @@ func TestProvisionWithMountOptions(t *testing.T) {

clientSet := fakeclientset.NewSimpleClientset()
csiClientSet := fakecsiclientset.NewSimpleClientset()
csiProvisioner := NewCSIProvisioner(clientSet, csiClientSet, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil)
csiProvisioner := NewCSIProvisioner(clientSet, csiClientSet, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down

0 comments on commit 64844fb

Please sign in to comment.