Skip to content

Add --pvc-annotation-mappings arg to pass pvc annotations to parameters #173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions cmd/csi-provisioner/csi-provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,16 @@ import (
)

var (
master = flag.String("master", "", "Master URL to build a client config from. Either this or kubeconfig needs to be set if the provisioner is being run out of cluster.")
kubeconfig = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Either this or master needs to be set if the provisioner is being run out of cluster.")
csiEndpoint = flag.String("csi-address", "/run/csi/socket", "The gRPC endpoint for Target CSI Volume.")
connectionTimeout = flag.Duration("connection-timeout", 10*time.Second, "Timeout for waiting for CSI driver socket.")
volumeNamePrefix = flag.String("volume-name-prefix", "pvc", "Prefix to apply to the name of a created volume.")
volumeNameUUIDLength = flag.Int("volume-name-uuid-length", -1, "Truncates generated UUID of a created volume to this length. Defaults behavior is to NOT truncate.")
showVersion = flag.Bool("version", false, "Show version.")
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.")
featureGates map[string]bool
master = flag.String("master", "", "Master URL to build a client config from. Either this or kubeconfig needs to be set if the provisioner is being run out of cluster.")
kubeconfig = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Either this or master needs to be set if the provisioner is being run out of cluster.")
csiEndpoint = flag.String("csi-address", "/run/csi/socket", "The gRPC endpoint for Target CSI Volume.")
connectionTimeout = flag.Duration("connection-timeout", 10*time.Second, "Timeout for waiting for CSI driver socket.")
volumeNamePrefix = flag.String("volume-name-prefix", "pvc", "Prefix to apply to the name of a created volume.")
volumeNameUUIDLength = flag.Int("volume-name-uuid-length", -1, "Truncates generated UUID of a created volume to this length. Defaults behavior is to NOT truncate.")
showVersion = flag.Bool("version", false, "Show version.")
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.")
featureGates map[string]bool
pvcAnnotationMappings map[string]string

provisionController *controller.ProvisionController
version = "unknown"
Expand All @@ -65,6 +66,7 @@ func init() {

flag.Var(utilflag.NewMapStringBool(&featureGates), "feature-gates", "A set of key=value pairs that describe feature gates for alpha/experimental features. "+
"Options are:\n"+strings.Join(utilfeature.DefaultFeatureGate.KnownFeatures(), "\n"))
flag.Var(utilflag.NewMapStringString(&pvcAnnotationMappings), "pvc-annotation-mappings", "A set of key=value pairs that describe how pvc annotation should be mapped to parameters that are passed to csi drivers.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this description is a little confusing since its not really key=value, it's actually more like pvcannotationkey=csidriverparameterkey (<- this isn't actually good I urge you to reword so that this concept is more clear though)


flag.CommandLine.AddGoFlagSet(goflag.CommandLine)
flag.Parse()
Expand Down Expand Up @@ -145,7 +147,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, pvcAnnotationMappings, grpcClient, snapClient)
provisionController = controller.NewProvisionController(
clientset,
provisionerName,
Expand Down
60 changes: 41 additions & 19 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,17 @@ var (

// CSIProvisioner struct
type csiProvisioner struct {
client kubernetes.Interface
csiClient csi.ControllerClient
csiAPIClient csiclientset.Interface
grpcClient *grpc.ClientConn
snapshotClient snapclientset.Interface
timeout time.Duration
identity string
volumeNamePrefix string
volumeNameUUIDLength int
config *rest.Config
client kubernetes.Interface
csiClient csi.ControllerClient
csiAPIClient csiclientset.Interface
grpcClient *grpc.ClientConn
snapshotClient snapclientset.Interface
timeout time.Duration
identity string
volumeNamePrefix string
volumeNameUUIDLength int
pvcAnnotationMappings map[string]string
config *rest.Config
}

type driverState struct {
Expand Down Expand Up @@ -325,20 +326,22 @@ func NewCSIProvisioner(client kubernetes.Interface,
identity string,
volumeNamePrefix string,
volumeNameUUIDLength int,
pvcAnnotationMappings map[string]string,
grpcClient *grpc.ClientConn,
snapshotClient snapclientset.Interface) controller.Provisioner {

csiClient := csi.NewControllerClient(grpcClient)
provisioner := &csiProvisioner{
client: client,
grpcClient: grpcClient,
csiClient: csiClient,
csiAPIClient: csiAPIClient,
snapshotClient: snapshotClient,
timeout: connectionTimeout,
identity: identity,
volumeNamePrefix: volumeNamePrefix,
volumeNameUUIDLength: volumeNameUUIDLength,
client: client,
grpcClient: grpcClient,
csiClient: csiClient,
csiAPIClient: csiAPIClient,
snapshotClient: snapshotClient,
timeout: connectionTimeout,
identity: identity,
volumeNamePrefix: volumeNamePrefix,
volumeNameUUIDLength: volumeNameUUIDLength,
pvcAnnotationMappings: pvcAnnotationMappings,
}
return provisioner
}
Expand Down Expand Up @@ -575,6 +578,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
if err != nil {
return nil, fmt.Errorf("failed to strip CSI Parameters of prefixed keys: %v", err)
}
req.Parameters = addSpecifiedAnnotationsToParameters(req.Parameters, options.PVC.Annotations, p.pvcAnnotationMappings)

opts := wait.Backoff{Duration: backoffDuration, Factor: backoffFactor, Steps: backoffSteps}
err = wait.ExponentialBackoff(opts, func() (bool, error) {
Expand Down Expand Up @@ -695,6 +699,24 @@ func removePrefixedParameters(param map[string]string) (map[string]string, error
return newParam, nil
}

func addSpecifiedAnnotationsToParameters(param map[string]string, annotation map[string]string, mapping map[string]string) map[string]string {
newParam := map[string]string{}
// Copy all existing parameters
for k, v := range param {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add to existing param map instead of creating a new one?

newParam[k] = v
}

// Copy a parameter from annotation, if a mapping for the parameter exists.
// Key is replaced with the value that is specified as value of the mapping.
for k, v := range annotation {
if newKey, ok := mapping[k]; ok {
newParam[newKey] = v
}
}

return newParam
}

func (p *csiProvisioner) getVolumeContentSource(options controller.VolumeOptions) (*csi.VolumeContentSource, error) {
snapshotObj, err := p.snapshotClient.VolumesnapshotV1alpha1().VolumeSnapshots(options.PVC.Namespace).Get(options.PVC.Spec.DataSource.Name, metav1.GetOptions{})
if err != nil {
Expand Down
122 changes: 106 additions & 16 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,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, nil, csiConn.conn, nil)

// Requested PVC with requestedBytes storage
opts := controller.VolumeOptions{
Expand Down Expand Up @@ -671,6 +671,14 @@ func provisionWithTopologyMockServerSetupExpectations(identityServer *driver.Moc
}, nil).Times(1)
}

func generateCheckParameterFunc(t *testing.T, params map[string]string) func(ctx context.Context, req *csi.CreateVolumeRequest) {
return func(ctx context.Context, req *csi.CreateVolumeRequest) {
if !reflect.DeepEqual(req.Parameters, params) {
t.Errorf("Parameters passed are different from expected ones. actual: %v, expected:%v", req.Parameters, params)
}
}
}

// Minimal PVC required for tests to function
func createFakePVC(requestBytes int64) *v1.PersistentVolumeClaim {
return &v1.PersistentVolumeClaim{
Expand All @@ -695,6 +703,13 @@ func createFakePVCWithVolumeMode(requestBytes int64, volumeMode v1.PersistentVol
return claim
}

// createFakePVCWithAnnotation returns PVC with Annotation
func createFakePVCWithAnnotation(requestBytes int64, annotations map[string]string) *v1.PersistentVolumeClaim {
claim := createFakePVC(requestBytes)
claim.Annotations = annotations
return claim
}

func TestGetSecretReference(t *testing.T) {
testcases := map[string]struct {
secretParams deprecatedSecretParamsMap
Expand Down Expand Up @@ -848,17 +863,18 @@ func TestGetSecretReference(t *testing.T) {
}

type provisioningTestcase struct {
volOpts controller.VolumeOptions
notNilSelector bool
driverNotReady bool
makeVolumeNameErr bool
getSecretRefErr bool
getCredentialsErr bool
volWithLessCap bool
expectedPVSpec *pvSpec
withSecretRefs bool
expectErr bool
expectCreateVolDo interface{}
volOpts controller.VolumeOptions
notNilSelector bool
driverNotReady bool
makeVolumeNameErr bool
getSecretRefErr bool
getCredentialsErr bool
volWithLessCap bool
pvcAnnotationMappings map[string]string
expectedPVSpec *pvSpec
withSecretRefs bool
expectErr bool
expectCreateVolDo interface{}
}

type pvSpec struct {
Expand Down Expand Up @@ -1322,6 +1338,80 @@ func TestProvision(t *testing.T) {
}
},
},
"provision pvc with annotations by not specifying pvcAnnotationMappings": {
volOpts: controller.VolumeOptions{
PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete,
PVName: "test-name",
PVC: createFakePVCWithAnnotation(requestedBytes, map[string]string{"annotation1": "a1", "annotation2": "a2"}),
Parameters: map[string]string{"param1": "p1"},
},
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: "ext4",
VolumeAttributes: map[string]string{
"storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner",
},
},
},
expectCreateVolDo: generateCheckParameterFunc(t, map[string]string{"param1": "p1"}),
},
"provision pvc with annotations by specifying pvcAnnotationMappings that map annotation1 to annotation1": {
volOpts: controller.VolumeOptions{
PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete,
PVName: "test-name",
PVC: createFakePVCWithAnnotation(requestedBytes, map[string]string{"annotation1": "a1", "annotation2": "a2"}),
Parameters: map[string]string{"param1": "p1"},
},
pvcAnnotationMappings: map[string]string{"annotation1": "annotation1"},
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: "ext4",
VolumeAttributes: map[string]string{
"storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner",
},
},
},
expectCreateVolDo: generateCheckParameterFunc(t, map[string]string{"param1": "p1", "annotation1": "a1"}),
},
"provision pvc with annotations by specifying pvcAnnotationMappings that map annotation1 to param2": {
volOpts: controller.VolumeOptions{
PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete,
PVName: "test-name",
PVC: createFakePVCWithAnnotation(requestedBytes, map[string]string{"annotation1": "a1", "annotation2": "a2"}),
Parameters: map[string]string{"param1": "p1"},
},
pvcAnnotationMappings: map[string]string{"annotation1": "param2"},
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: "ext4",
VolumeAttributes: map[string]string{
"storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner",
},
},
},
expectCreateVolDo: generateCheckParameterFunc(t, map[string]string{"param1": "p1", "param2": "a1"}),
},
}

for k, tc := range testcases {
Expand Down Expand Up @@ -1391,7 +1481,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, tc.pvcAnnotationMappings, csiConn.conn, nil)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1746,7 +1836,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, nil, csiConn.conn, client)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1841,7 +1931,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, nil, csiConn.conn, nil)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1879,7 +1969,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, nil, csiConn.conn, nil)

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