Cache driver name #215
Cache driver name #215
Conversation
Provisioner should not ask for it every time, it should not change during driver container lifetime.
k8s-ci-robot
added
the
size/M
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
| } | ||
| 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) { |
There was a problem hiding this comment.
Why are we representing capabilities with sets.Int, is there no named type from the protobuf we can use? if not, can we define one
type PluginCapabilitiesType sets.Int
There was a problem hiding this comment.
I tried, but then I loose all sets.Int methods (no PluginCapabilitiesType.Has). I could restore driverState struct (with a better name) with sets.Int as a field, but that looks ugly.
| @@ -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) | |||
There was a problem hiding this comment.
provisionerName == driverName? why do we even have these two concepts then. Can we just call it driverName everywhere?
There was a problem hiding this comment.
I think it's mandatory for volume topology, where kubelet knows just driver name from driver registrar and scheduler knows only provider name from StorageClass. @msau42 knows details.
There was a problem hiding this comment.
Yes, they're the same. external-provisioner originally was implemented allowing them to be different but there's no reason they need to be so we removed it in #186
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Provisioner should not ask for it every time, it should not change during driver container lifetime.