deprecate: driver.spec.EnableMetadata field#454
deprecate: driver.spec.EnableMetadata field#454iPraveenParihar wants to merge 2 commits intoceph:mainfrom
Conversation
iPraveenParihar
changed the title
| // Not all users might be interested in getting volume/snapshot details as metadata on CephFS subvolume and RBD images. | ||
| // Hence enable metadata is false by default. | ||
| //+kubebuilder:validation:Optional | ||
| EnableMetadata *bool `json:"enableMetadata,omitempty"` |
There was a problem hiding this comment.
Removing CRD settings is a breaking change. Consider leaving this setting here, just deprecate it and only remove the internal code that calls the --setmetadata flag.
There was a problem hiding this comment.
I was thinking it is okay to remove since we are still at pre v1.0.0 release?
There was a problem hiding this comment.
Lets not remove the CRD setting, we can mark this as deprecated and add a notice that the feature is enabled by default in csi now
There was a problem hiding this comment.
I was thinking it is okay to remove since we are still at pre v1.0.0 release?
Since we are shipping already in stable upstream and downstream releases, we need to anyway treat it as v1.
There was a problem hiding this comment.
understood, will add deprecation notice and keep the field.
iPraveenParihar
force-pushed
the
deprecate/setmetadata-flag
branch
4 times, most recently
from
be23753 to
06806ae
Compare
| # -- Cluster name identifier (default: "") | ||
| clusterName: "" | ||
| # -- Flag to enable metadata (default: false) | ||
| # -- [Deprecated] This field is no longer used and will be ignored (default: false) |
There was a problem hiding this comment.
Removing a setting from a helm chart is not a breaking change. Any extra values in the values.yaml would be ignored. So you could go ahead and remove the setting from values.yaml
| } | ||
| if dest.EnableMetadata == nil { | ||
| dest.EnableMetadata = src.EnableMetadata | ||
| if dest.EnableMetadata == nil { //nolint:staticcheck // backward compatibility for deprecated field |
There was a problem hiding this comment.
I would think this is not necessary. Backward compatibility is to avoid breaking API changes, but we don't need to keep code that preserves the setting.
The EnableMetadata field and --setmetadata flag are no longer used by the ceph-csi driver. Mark the field as deprecated in the v1 DriverSpec API, generated CRD manifests, and Helm chart values instead of removing it, to avoid a breaking CRD schema change. Signed-off-by: Praveen M <m.praveen@ibm.com>
Remove the SetMetadataContainerArg utility function and its usage in the driver controller. The deprecated EnableMetadata API field is retained for backward compatibility but no longer results in the --setmetadata flag being passed to ceph-csi containers. Signed-off-by: Praveen M <m.praveen@ibm.com>
iPraveenParihar
force-pushed
the
deprecate/setmetadata-flag
branch
from
06806ae to
dc6c4e3
Compare
3 participants
Describe what this PR does
See: ceph/ceph-csi#6225
Checklist: