Skip to content

Commit

Permalink
Merge pull request #399 from zetsub0u/master
Browse files Browse the repository at this point in the history
add pvc metadata to createvolume req
  • Loading branch information
k8s-ci-robot authored Feb 7, 2020
2 parents 8444597 + 53f0949 commit 4aa560e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 11 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ Note that the external-provisioner does not scale with more replicas. Only one e

* `--metrics-path`: The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.

* `--extraCreateMetadata`: Enables the injection of extra PVC and PV metadata as parameters when calling `CreateVolume` on the driver (keys: "csi.storage.k8s.io/pvc/name", "csi.storage.k8s.io/pvc/namespace", "csi.storage.k8s.io/pv/name")

#### Other recognized arguments
* `--feature-gates <gates>`: A set of comma separated `<feature-name>=<true|false>` pairs that describe feature gates for alpha/experimental features. See [list of features](#feature-status) or `--help` output for list of recognized features. Example: `--feature-gates Topology=true` to enable Topology feature that's disabled by default.

Expand Down
5 changes: 4 additions & 1 deletion cmd/csi-provisioner/csi-provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ var (
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.")
extraCreateMetadata = flag.Bool("extra-create-metadata", 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,
*extraCreateMetadata,
)

provisionController = controller.NewProvisionController(
clientset,
Expand Down
18 changes: 17 additions & 1 deletion pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ const (
nodePublishSecretNameKey = "csiNodePublishSecretName"
nodePublishSecretNamespaceKey = "csiNodePublishSecretNamespace"

// PV and PVC metadata, used for sending to drivers in the create requests, added as parameters, optional.
pvcNameKey = "csi.storage.k8s.io/pvc/name"
pvcNamespaceKey = "csi.storage.k8s.io/pvc/namespace"
pvNameKey = "csi.storage.k8s.io/pv/name"

// Defines parameters for ExponentialBackoff used for executing
// CSI CreateVolume API call, it gives approx 4 minutes for the CSI
// driver to complete a volume creation.
Expand Down Expand Up @@ -214,6 +219,7 @@ type csiProvisioner struct {
scLister storagelistersv1.StorageClassLister
csiNodeLister storagelistersv1beta1.CSINodeLister
nodeLister corelisters.NodeLister
extraCreateMetadata bool
}

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

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

if p.extraCreateMetadata {
// 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()
rep, err = p.csiClient.CreateVolume(ctx, &req)
Expand Down
61 changes: 52 additions & 9 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,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 @@ -487,6 +487,7 @@ func createFakeNamedPVC(requestBytes int64, name string, userAnnotations map[str
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Name: name,
Namespace: "fake-ns",
Annotations: annotations,
},
Spec: v1.PersistentVolumeClaimSpec{
Expand Down Expand Up @@ -794,6 +795,7 @@ type provisioningTestcase struct {
expectErr bool
expectState controller.ProvisioningState
expectCreateVolDo interface{}
withExtraMetadata bool
}

type pvSpec struct {
Expand Down Expand Up @@ -879,6 +881,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 @@ -1619,7 +1662,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 @@ -2355,7 +2398,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 @@ -2529,7 +2572,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 @@ -2584,7 +2627,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 @@ -2756,7 +2799,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 @@ -3458,7 +3501,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 @@ -3537,7 +3580,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 Down Expand Up @@ -3697,7 +3740,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 4aa560e

Please sign in to comment.