[cinder-csi-plugin] Tag (metadata) from req.parameter behavior different for snapshot and volume

[jichenjc]

Is this a BUG REPORT or FEATURE REQUEST?:

Uncomment only one, leave it on its own line:

/kind bug

/kind feature

What happened:
discussion here
#1544 (comment)

What you expected to happen:

How to reproduce it:

Anything else we need to know?:

Environment:

• openstack-cloud-controller-manager(or other related binary) version:
• OpenStack version:
• Others:

[ramineni]

@jichenjc Could you explain what exactly is the problem , I have gone through the comment but its not clear

[jichenjc]

@ramineni every req.parameter will be used to create tag in current implementation

but from spec, the req parameter should not all be set to tags (check createvolume function, the tag only honor very specific annotations and other param will NOT become a tag )

// Plugin specific parameters passed in as opaque key-value pairs.
  // This field is OPTIONAL. The Plugin is responsible for parsing and
  // validating these parameters. COs will treat these as opaque.
  // Use cases for opaque parameters:
  // - Specify a policy to automatically clean up the snapshot.
  // - Specify an expiration date for the snapshot.
  // - Specify whether the snapshot is readonly or read/write.
  // - Specify if the snapshot should be replicated to some place.
  // - Specify primary or secondary for replication systems that
  //   support snapshotting only on primary.

[ramineni]

Use cases for opaque parameters

@jichenjc If you are talking about the above, they are only example usecases . It depends on provider if we can add the params as metadata or not. In the current scenario , I dont see why we cant add as metadata. what all params you want to avoid/not suitable for adding as metadata or how do we decide?

[jichenjc]

what all params you want to avoid/not suitable for adding as metadata or how do we decide

The createvolume and createsnapshot currently is inconsistent, createvolume doesn't create tag wihle createsnapshot do ; so the primary purpose is to make them sync

so I think it's reasonable to avoid create tag for every param, instead, if we knew some param we really need
, then let's honor it later on ; if we think we need honor every param, then I can work on createvolume part..

[lingxiankong]

I can see we are using some known keys from req.Parameters for creating volumes, @jichenjc do we know if there are such keys as well passed for creating snapshot?

[ramineni]

@lingxiankong No, the known keys are passed from external-provisioner for CreateVolume call only . Not passed for CreateSnapshot.
Other than those , we allow parameters from users as well which we append as metatdata of the snapshot. We need to decide if we should append the user metadata to snapshot or not.

[ramineni]

@lingxiankong From snapshotter-version 4.0.0 it supports --extra-create-metadata tag where we can add the metadata info for CreateSnapshot as well (ref: #1578)

But the question remains , if we want to support only metadata that is being passed from sidecars or user metadata as well?

[lingxiankong]

Passing all the parameters specified by the users to the Cinder snapshot doesn't sound like a good idea to me, I personally would prefer to only pass the sidecar params.

[ramineni]

@lingxiankong @jichenjc ok, I guess we can make it consistent to add the metadata from the sidecars. Later we can support specific metadata keys if used commonly as per requirement.

@jichenjc could you update the PR reflecting that if sounds good to you?

[jichenjc]

@lingxiankong @jichenjc ok, I guess we can make it consistent to add the metadata from the sidecars. Later we can support specific metadata keys if used commonly as per requirement.

@jichenjc could you update the PR reflecting that if sounds good to you?

ok, I will update the PR~