Skip to content

Commit

Permalink
add pv name to createreq and feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
@zetsub0u
zetsub0u committed Jan 28, 2020
1 parent 9d9c8e9 commit e18ad2a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 36 deletions.
13 changes: 8 additions & 5 deletions cmd/csi-provisioner/csi-provisioner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ var (
operationTimeout = flag.Duration("timeout", 10*time.Second, "Timeout for waiting for creation or deletion of a volume")
_ = deprecatedflags.Add("provisioner")

enableLeaderElection = flag.Bool("enable-leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")
leaderElectionType = flag.String("leader-election-type", "endpoints", "the type of leader election, options are 'endpoints' (default) or 'leases' (strongly recommended). The 'endpoints' option is deprecated in favor of 'leases'.")
leaderElectionNamespace = flag.String("leader-election-namespace", "", "Namespace where the leader election resource lives. Defaults to the pod namespace if not set.")
strictTopology = flag.Bool("strict-topology", false, "Passes only selected node topology to CreateVolume Request, unlike default behavior of passing aggregated cluster topologies that match with topology keys of the selected node.")
enableLeaderElection = flag.Bool("enable-leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules.")
leaderElectionType = flag.String("leader-election-type", "endpoints", "the type of leader election, options are 'endpoints' (default) or 'leases' (strongly recommended). The 'endpoints' option is deprecated in favor of 'leases'.")
leaderElectionNamespace = flag.String("leader-election-namespace", "", "Namespace where the leader election resource lives. Defaults to the pod namespace if not set.")
strictTopology = flag.Bool("strict-topology", false, "Passes only selected node topology to CreateVolume Request, unlike default behavior of passing aggregated cluster topologies that match with topology keys of the selected node.")
enableExtraMetadataForCreateReq = flag.Bool("enable-extra-metadata-for-create-req", false, "If set, add pv/pvc metadata to plugin create requests as parameters.")

metricsAddress = flag.String("metrics-address", "", "The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled.")
metricsPath = flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.")
Expand Down Expand Up @@ -221,7 +222,9 @@ func main() {
translator,
scLister,
csiNodeLister,
nodeLister)
nodeLister,
*enableExtraMetadataForCreateReq,
)

provisionController = controller.NewProvisionController(
clientset,
Expand Down
19 changes: 13 additions & 6 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ const (
nodePublishSecretNameKey = "csiNodePublishSecretName"
nodePublishSecretNamespaceKey = "csiNodePublishSecretNamespace"

// PVC's name and namespace passed on create requests, useful for the plugin to query for extra data
// prior to performing operations
// PV and PVC metadata, used for sending to drivers in the create requests, added as parameters, optional.
pvcNameKey = "kubernetes.io/created-for/pvc/name"
pvcNamespaceKey = "kubernetes.io/created-for/pvc/namespace"
pvNameKey = "kubernetes.io/created-for/pv/name"

// Defines parameters for ExponentialBackoff used for executing
// CSI CreateVolume API call, it gives approx 4 minutes for the CSI
Expand Down Expand Up @@ -210,6 +210,7 @@ type csiProvisioner struct {
scLister storagelistersv1.StorageClassLister
csiNodeLister storagelistersv1beta1.CSINodeLister
nodeLister corelisters.NodeLister
enableExtraMetadataForCreateReq bool
}

var _ controller.Provisioner = &csiProvisioner{}
Expand Down Expand Up @@ -272,7 +273,9 @@ func NewCSIProvisioner(client kubernetes.Interface,
translator ProvisionerCSITranslator,
scLister storagelistersv1.StorageClassLister,
csiNodeLister storagelistersv1beta1.CSINodeLister,
nodeLister corelisters.NodeLister) controller.Provisioner {
nodeLister corelisters.NodeLister,
enableExtraMetadataForCreateReq bool,
) controller.Provisioner {

csiClient := csi.NewControllerClient(grpcClient)
provisioner := &csiProvisioner{
Expand All @@ -293,6 +296,7 @@ func NewCSIProvisioner(client kubernetes.Interface,
scLister: scLister,
csiNodeLister: csiNodeLister,
nodeLister: nodeLister,
enableExtraMetadataForCreateReq: enableExtraMetadataForCreateReq,
}
return provisioner
}
Expand Down Expand Up @@ -572,9 +576,12 @@ func (p *csiProvisioner) ProvisionExt(options controller.ProvisionOptions) (*v1.
return nil, controller.ProvisioningFinished, fmt.Errorf("failed to strip CSI Parameters of prefixed keys: %v", err)
}

// add pvc metadata to request for use by the plugin
req.Parameters[pvcNameKey] = options.PVC.GetName()
req.Parameters[pvcNamespaceKey] = options.PVC.GetNamespace()
if p.enableExtraMetadataForCreateReq {
// add pvc and pv metadata to request for use by the plugin
req.Parameters[pvcNameKey] = options.PVC.GetName()
req.Parameters[pvcNamespaceKey] = options.PVC.GetNamespace()
req.Parameters[pvNameKey] = pvName
}

ctx, cancel := context.WithTimeout(context.Background(), p.timeout)
defer cancel()
Expand Down
80 changes: 55 additions & 25 deletions pkg/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ func TestCreateDriverReturnsInvalidCapacityDuringProvision(t *testing.T) {

pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(nil, 5*time.Second, "test-provisioner", "test",
5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil)
5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false)

// Requested PVC with requestedBytes storage
deletePolicy := v1.PersistentVolumeReclaimDelete
Expand Down Expand Up @@ -784,6 +784,7 @@ type provisioningTestcase struct {
expectErr bool
expectState controller.ProvisioningState
expectCreateVolDo interface{}
withExtraMetadata bool
}

type pvSpec struct {
Expand Down Expand Up @@ -828,6 +829,47 @@ func TestProvision(t *testing.T) {
},
expectState: controller.ProvisioningFinished,
},
"normal provision with extra metadata": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{
"fstype": "ext3",
},
},
PVName: "test-name",
PVC: createFakePVC(requestedBytes),
},
withExtraMetadata: true,
expectedPVSpec: &pvSpec{
Name: "test-testi",
ReclaimPolicy: v1.PersistentVolumeReclaimDelete,
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): bytesToGiQuantity(requestedBytes),
},
CSIPVS: &v1.CSIPersistentVolumeSource{
Driver: "test-driver",
VolumeHandle: "test-volume-id",
FSType: "ext3",
VolumeAttributes: map[string]string{
"storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner",
},
},
},
expectCreateVolDo: func(ctx context.Context, req *csi.CreateVolumeRequest) {
pvc := createFakePVC(requestedBytes)
expectedParams := map[string]string{
pvcNameKey: pvc.GetName(),
pvcNamespaceKey: pvc.GetNamespace(),
pvNameKey: "test-testi",
"fstype": "ext3",
}
if fmt.Sprintf("%v", req.Parameters) != fmt.Sprintf("%v", expectedParams) { // only pvc name/namespace left
t.Errorf("Unexpected parameters: %v", req.Parameters)
}
},
expectState: controller.ProvisioningFinished,
},
"multiple fsType provision": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
Expand Down Expand Up @@ -870,13 +912,8 @@ func TestProvision(t *testing.T) {
},
},
expectCreateVolDo: func(ctx context.Context, req *csi.CreateVolumeRequest) {
pvc := createFakePVC(requestedBytes)
expectedParams := map[string]string{
pvcNameKey: pvc.GetName(),
pvcNamespaceKey: pvc.GetNamespace(),
}
if fmt.Sprintf("%v", req.Parameters) != fmt.Sprintf("%v", expectedParams) { // only pvc name/namespace left
t.Errorf("Unexpected parameters: %v", req.Parameters)
if len(req.Parameters) != 0 {
t.Errorf("Parameters should have been stripped")
}
},
expectState: controller.ProvisioningFinished,
Expand Down Expand Up @@ -1471,7 +1508,7 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested

pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn,
nil, provisionDriverName, pluginCaps, controllerCaps, supportsMigrationFromInTreePluginName, false, csitrans.New(), nil, nil, nil)
nil, provisionDriverName, pluginCaps, controllerCaps, supportsMigrationFromInTreePluginName, false, csitrans.New(), nil, nil, nil, tc.withExtraMetadata)

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

pluginCaps, controllerCaps := provisionFromSnapshotCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn,
client, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil)
client, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -2392,7 +2429,7 @@ func TestProvisionWithTopologyEnabled(t *testing.T) {
defer close(stopChan)

csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5,
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, csiNodeLister, nodeLister)
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, csiNodeLister, nodeLister, false)

pv, err := csiProvisioner.Provision(controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{},
Expand Down Expand Up @@ -2447,7 +2484,7 @@ func TestProvisionWithTopologyDisabled(t *testing.T) {
clientSet := fakeclientset.NewSimpleClientset()
pluginCaps, controllerCaps := provisionWithTopologyCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5,
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil)
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -2619,7 +2656,7 @@ func runDeleteTest(t *testing.T, k string, tc deleteTestcase) {
pluginCaps, controllerCaps := provisionCapabilities()
scLister, _, _, _ := listers(clientSet)
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5,
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, nil, nil)
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, nil, nil, false)

err = csiProvisioner.Delete(tc.persistentVolume)
if tc.expectErr && err == nil {
Expand Down Expand Up @@ -3321,7 +3358,7 @@ func TestProvisionFromPVC(t *testing.T) {
}

csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn,
nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil)
nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false)

pv, err := csiProvisioner.Provision(tc.volOpts)
if tc.expectErr && err == nil {
Expand Down Expand Up @@ -3400,7 +3437,7 @@ func TestProvisionWithMigration(t *testing.T) {
pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner",
"test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps,
inTreePluginName, false, mockTranslator, nil, nil, nil)
inTreePluginName, false, mockTranslator, nil, nil, nil, false)

// Set up return values (AnyTimes to avoid overfitting on implementation)

Expand All @@ -3427,15 +3464,9 @@ func TestProvisionWithMigration(t *testing.T) {
},
).AnyTimes()

pvc := createPVCWithAnnotation(tc.annotation, requestBytes)

if !tc.expectErr {
// Set an expectation that the Create should be called
expectParams := map[string]string{ // Default
"fstype": "ext3",
pvcNameKey: pvc.GetName(),
pvcNamespaceKey: pvc.GetNamespace(),
}
expectParams := map[string]string{"fstype": "ext3"} // Default
if tc.expectTranslation {
// If translation is expected we check that the CreateVolume
// is called on the expected volume with a translated param
Expand Down Expand Up @@ -3466,7 +3497,7 @@ func TestProvisionWithMigration(t *testing.T) {
ReclaimPolicy: &deletePolicy,
},
PVName: "test-name",
PVC: pvc,
PVC: createPVCWithAnnotation(tc.annotation, requestBytes),
}

pv, state, err := csiProvisioner.(controller.ProvisionerExt).ProvisionExt(volOpts)
Expand Down Expand Up @@ -3503,7 +3534,6 @@ func createPVCWithAnnotation(ann map[string]string, requestBytes int64) *v1.Pers
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Name: "fake-pvc",
Namespace: "fake-ns",
Annotations: ann,
},
Spec: v1.PersistentVolumeClaimSpec{
Expand Down Expand Up @@ -3567,7 +3597,7 @@ func TestDeleteMigration(t *testing.T) {
pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner",
"test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "",
false, mockTranslator, nil, nil, nil)
false, mockTranslator, nil, nil, nil, false)

// Set mock return values (AnyTimes to avoid overfitting on implementation details)
mockTranslator.EXPECT().IsPVMigratable(gomock.Any()).Return(tc.expectTranslation).AnyTimes()
Expand Down

0 comments on commit e18ad2a

Please sign in to comment.