Clarification on copying labels from source PVC to prime PVC

[pruthvitd]

What happened:
We noticed that CDI copies labels from the original PVC to the prime PVC, which is a temporary object created during the import process. Could you clarify the reasoning behind this behavior?

What you expected to happen:
Prime PVC should not inherit user-defined labels unless necessary for CDI operations.

How to reproduce it (as minimally and precisely as possible):
Steps to reproduce the behavior.
Create a PVC with custom labels (e.g., app=myvm).
Trigger an import using CDI (e.g., DataVolume or VolumeImportSource with source: http://quay.io/nirsof/cirros:0.6.2-1).
Observe that the prime PVC created during the process inherits the labels from the original PVC.

Additional context:
Prime PVC inherits all labels from the original PVC.

• Is this intentional and required for CDI functionality?
• Could this lead to unintended side effects if external controllers interpret these labels?
• Would it make sense to avoid copying user-defined labels?

Environment:

• CDI version (use kubectl get deployments cdi-deployment -o yaml): image: quay.io/kubevirt/cdi-controller:v1.63.0
• Kubernetes version (use kubectl version):

# kubectl version
Client Version: v1.34.1
Kustomize Version: v5.7.1
Server Version: v1.34.0

• DV specification: N/A
• Cloud provider or hardware configuration: N/A
• OS (e.g. from /etc/os-release): RHEL9.6
• Kernel (e.g. uname -a): N/A
• Install tools: N/A
• Others: N/A

CC: nirsof@gmail.com

[akalenyu]

So this is a recent-ish addition with the context in this issue - #3674 and the PR that closed it.

I can see arguments in both directions but generally we hoped labels would not alter functionality like annotations. If you could describe your particular case that might be helpful 🙏

[nirs]

So this is a recent-ish addition with the context in this issue - #3674 and the PR that closed it.

I can see arguments in both directions but generally we hoped labels would not alter functionality like annotations. If you could describe your particular case that might be helpful 🙏

@akalenyu Trying to explain this issue better.

The issue is this: RamenDR/ramen#2353

Ramen protects PVCs using a PCC label selector. For example you have to label the PVC in your workload with a label:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  labels:
    protected-by: ramen    
  name: root-disk
spec:
  dataSourceRef:
    apiGroup: cdi.kubevirt.io
    kind: VolumeImportSource
    name: cirros-source
  accessModes:
    - ReadWriteMany
  resources:
    requests:
      storage: 128Mi
  # Customize in your overlay.
  storageClassName: STORAGE_CLASS_NAME
  volumeMode: Block```

And you configure the DRPC with the label selector:

apiVersion: ramendr.openshift.io/v1alpha1
kind: DRPlacementControl
metadata:
  name: drpc
spec:
  preferredCluster: dr1
  drPolicyRef:
    name: dr-policy-1m
  placementRef:
    kind: Placement
    name: placement
  pvcSelector:
    matchLabels:
      protected-by: ramen

The label can be anything, "protected-by": "ramen" is not an example. When using multiple workloads in same name, you need a unique label for each workload.

This is the way ramen can protect any workload without knowing anything about the workload.

When CDI provision a PVC, it currently copy the PVC lables to the temporary PVCs used for provisioning. This basically tells ramen, "please protect the temporary PVC". This breaks DR in interesting ways, as you can in the ramen issue.

So basically it is not safe to blindly copy user labels from the original PVC to the temporary PVC.

If CDI must copy the labels for some reason, ramen can provide a special label like:

ramendr.openshift.io/do-not-protect: "true"

and CDI will label the temporary the PVCs to avoid this issue.

[akalenyu]

So this is a recent-ish addition with the context in this issue - #3674 and the PR that closed it.
I can see arguments in both directions but generally we hoped labels would not alter functionality like annotations. If you could describe your particular case that might be helpful 🙏

@akalenyu Trying to explain this issue better.

The issue is this: RamenDR/ramen#2353

Ramen protects PVCs using a PCC label selector. For example you have to label the PVC in your workload with a label:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
labels:
protected-by: ramen
name: root-disk
spec:
dataSourceRef:
apiGroup: cdi.kubevirt.io
kind: VolumeImportSource
name: cirros-source
accessModes:
- ReadWriteMany
resources:
requests:
storage: 128Mi

Customize in your overlay.

storageClassName: STORAGE_CLASS_NAME
volumeMode: Block```
And you configure the DRPC with the label selector:

apiVersion: ramendr.openshift.io/v1alpha1
kind: DRPlacementControl
metadata:
name: drpc
spec:
preferredCluster: dr1
drPolicyRef:
name: dr-policy-1m
placementRef:
kind: Placement
name: placement
pvcSelector:
matchLabels:
protected-by: ramen
The label can be anything, "protected-by": "ramen" is not an example. When using multiple workloads in same name, you need a unique label for each workload.

This is the way ramen can protect any workload without knowing anything about the workload.

When CDI provision a PVC, it currently copy the PVC lables to the temporary PVCs used for provisioning. This basically tells ramen, "please protect the temporary PVC". This breaks DR in interesting ways, as you can in the ramen issue.

So basically it is not safe to blindly copy user labels from the original PVC to the temporary PVC.

If CDI must copy the labels for some reason, ramen can provide a special label like:

ramendr.openshift.io/do-not-protect: "true"

and CDI will label the temporary the PVCs to avoid this issue.

I see. looping in @Dsanatar and @ShellyKa13 (velero) for their thoughts. I think we should be able to achieve the goal of #3744 (pass label to importer) without going through the prime PVC (everybody wins). So maybe a mechanism similar to AnnEventSource.

[ShellyKa13]

Well actually we had a similar issue with Velero and we added a do-not-backup label to the pvc prime: #3522

[akalenyu]

Just to unblock we could have an exclude list for certain labels, but long term I think we can achieve what #3744 was after without going through the target->prime. @pruthvitd or @nirs If you could submit the PR that would be awesome 🙏

[nirs]

Just to unblock we could have an exclude list for certain labels, but long term I think we can achieve what #3744 was after without going through the target->prime. @pruthvitd or @nirs If you could submit the PR that would be awesome 🙏

@akalenyu Exclude list will not help, since ramen does not have special able for marking a PVC for protection. It can use any label configured by the user.

@awels wrote in Slack that maybe the solution is coping only certain CNI labels form the PVC to the temporary PVCs. Any update on that?

[akalenyu]

Just to unblock we could have an exclude list for certain labels, but long term I think we can achieve what #3744 was after without going through the target->prime. @pruthvitd or @nirs If you could submit the PR that would be awesome 🙏

@akalenyu Exclude list will not help, since ramen does not have special able for marking a PVC for protection. It can use any label configured by the user.

@awels wrote in Slack that maybe the solution is coping only certain CNI labels form the PVC to the temporary PVCs. Any update on that?

Right in that case I'd just jump ahead and alter the label passing implementation. @Dsanatar wdyt?

So maybe a mechanism similar to AnnEventSource.

[Dsanatar]

Right in that case I'd just jump ahead and alter the label passing implementation. @Dsanatar wdyt?

So maybe a mechanism similar to AnnEventSource.

that seems reasonable to me!