PVC dataSource is immutable

[uhthomas]

What happened?

Given a PVC created with CSI Volume Cloning:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: grafana
  namespace: grafana
spec:
  accessModes:
    - ReadWriteOnce
  dataSource:
    apiGroup: null
    kind: PersistentVolumeClaim
    name: grafana-hdd
  dataSourceRef:
    apiGroup: null
    kind: PersistentVolumeClaim
    name: grafana-hdd
  resources:
    requests:
      storage: 10Gi
  storageClassName: rook-ceph-nvme-ec-delete-block
  volumeMode: Filesystem
  volumeName: pvc-0fcb6ae8-99e0-4062-b027-5606ae963c2e

It's not possible to remove the dataSource and dataSourceRef fields, which is aggravating as the documentation claims the new PVC should be considered completely independent:

Upon availability of the new PVC, the cloned PVC is consumed the same as other PVC. It's also expected at this point that the newly created PVC is an independent object. It can be consumed, cloned, snapshotted, or deleted independently and without consideration for it's original dataSource PVC. This also implies that the source is not linked in any way to the newly created clone, it may also be modified or deleted without affecting the newly created clone.

https://kubernetes.io/docs/concepts/storage/volume-pvc-datasource/#usage

As such, if the reference PVC were to be deleted, there would be no meaning to the dataSource and dataSourceRef fields at all. This goes against "gitops" management.

What did you expect to happen?

The fields dataSource and dataSourceRef should not be immutable.

How can we reproduce it (as minimally and precisely as possible)?

Create a PVC, and then create another PVC with CSI Volume Cloning, then attempt to delete the fields dataSource and dataSourceRef.

Anything else we need to know?

No response

Kubernetes version

$ kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.3", GitCommit:"25b4e43193bcda6c7328a6d147b1fb73a33f1598", GitTreeState:"archive", BuildDate:"2023-06-15T08:14:06Z", GoVersion:"go1.20.5", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v5.0.1
Server Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.2", GitCommit:"7f6f68fdabc4df88cfea2dcf9a19b2b830f1e647", GitTreeState:"clean", BuildDate:"2023-05-17T14:13:28Z", GoVersion:"go1.20.4", Compiler:"gc", Platform:"linux/amd64"}

Cloud provider

N/A

OS version

N/A

Install tools

Container runtime (CRI) and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

[k8s-ci-robot]

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[uhthomas]

/sig storage

[carlory]

I'll take a look at it if I have time.
/assign

[xing-yang]

@uhthomas Can you explain what you do with this PVC in the GitOps process?

[uhthomas]

@uhthomas Can you explain what you do with this PVC in the GitOps process?

I am pretty sure I was just trying to clone a pv. I then wanted to remove .spec.dataSource after because it was misleading and incorrect once the source pv was deleted.

uhthomas/automata@c2c3fc8

[xing-yang]

@uhthomas Can you explain what you do with this PVC in the GitOps process?

I am pretty sure I was just trying to clone a pv. I then wanted to remove .spec.dataSource after because it was misleading and incorrect once the source pv was deleted.

uhthomas/automata@c2c3fc8

I don't think .spec.dataSource should be removed after the PV is cloned. It is still correct as the data was from the source PV which makes it different from a blank volume.

After a PVC is dynamically provisioned, the StorageClass itself could be deleted. We are not supposed to remove the storageClassName from the spec.

[uhthomas]

@uhthomas Can you explain what you do with this PVC in the GitOps process?

I am pretty sure I was just trying to clone a pv. I then wanted to remove .spec.dataSource after because it was misleading and incorrect once the source pv was deleted.

uhthomas/automata@c2c3fc8

I don't think .spec.dataSource should be removed after the PV is cloned. It is still correct as the data was from the source PV which makes it different from a blank volume.

After a PVC is dynamically provisioned, the StorageClass itself could be deleted. We are not supposed to remove the storageClassName from the spec.

If I delete the original PVC from the gitops repository, then the reference in .spec.dataSource is incorrect. It points to an object which does not exist. gitops is supposed to be a source of truth, and one of the nice things about this is that it's easier for disaster recovery events. The PVC would not be created in this instance, because the reference PVC would not exist. I don't believe there is any real reason for .spec.dataSource to be immutable.

[xing-yang]

If I delete the original PVC from the gitops repository, then the reference in .spec.dataSource is incorrect. It points to an object which does not exist. gitops is supposed to be a source of truth, and one of the nice things about this is that it's easier for disaster recovery events. The PVC would not be created in this instance, because the reference PVC would not exist. I don't believe there is any real reason for .spec.dataSource to be immutable.

For disaster recovery, you can create a blank PVC first, then overwrite the volume from a source PVC. This way you don't have to constantly add and remove the dataSource. For example, you can use https://github.com/vmware-tanzu/velero to do that.

[uhthomas]

That seems tedious, and isn't always what is intended. In my case I was just trying to move data from one storage class to another, and so I was not interested in retaining the original PV or PVC. The dataSource information is useless once this operation is finished and I'd like to make it clear in the manifests that the original PVC is gone.

Is there anything blocking this issue? I would really like to see the dataSource become mutable so it be removed once it's no longer needed. It could be accompanied by some documentation which states that changes to the field are no-op to avoid confusion.

[xing-yang]

This is an API change request and therefore needs more discussions. We can't break existing users relying on this field to be immutable. I'm also not convinced that this has to be mutable. I'd like others to chime in as well.

cc @msau42 @jsafrane @gnufied @jingxu97 @bswartz

/kind feature

[bswartz]

Yeah I don't see any way we could make a backwards incompatible change like what's being requested here. I'm afraid that the problem is with your gitops workflow. Just change the representation of the volume in git to not have the data source and don't worry about the fact that it doesn't match.

As a really extreme workaround, you can unbind the PVC from the PV and create an new PVC that lacks a datasource, but pre-binds to the PV.

I'm interested to know if there is a general compatibility problem between gitops style workflows and PVCs with data sources, however. There are good reasons for data sources to be immutable, and part of the understanding about the API construct is that it's only meaningful at creation time. This is because PVCs themselves are stateful objects, i.e. they get filled up with data shortly after creation and the data is not disposable in most cases. There's no way to express the contents of a PVC in gitops, to my knowledge, so I'm not sure how people do this.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.