Skip to content

Conversation

xing-yang
Copy link
Contributor

@xing-yang xing-yang commented Jul 28, 2025

What this PR does / why we need it:
This is to support PVCs in captured blueprints use case.

  • In the existing code, it allows an existing PVC to be used by CnsRegiserVolume. However, there isn't any validation for the DataSourceRef field. This means any DataSourceRef would be allowed to be used.
  • This PR tightens the validation. It checks if there is an existing PVC for CnsRegisterVolume and whether there is a valid DataSourceRef. If a DataSourceRef exists but not supported, fail the operation.
  • TODO: In a followup PR, we will add validation for topology annotation on PVC. If the topology annotation is added on an existing PVC, the topology of the volume must be compatible with that.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Testing done:
WCP pre-check pipeline: https://jenkins-vcf-csifvt.devops.broadcom.net/job/wcp-instapp-e2e-pre-checkin/140/ (passed)

Build #140 (Aug 13, 2025, 7:46:03 AM)
xy036828 <br> PR 3449<br>
Ran 10 of 1080 Specs in 1286.027 seconds
SUCCESS! -- 10 Passed | 0 Failed | 0 Pending | 1070 Skipped
PASS

Ginkgo ran 1 suite in 21m47.168354942s
Test Suite Passed

VKS pre-check pipeline: https://jenkins-vcf-csifvt.devops.broadcom.net/view/instapp/job/vks-instapp-e2e-pre-checkin/286 (passed)

Build #286 (Aug 14, 2025, 6:38:34 AM)
xy036828 <br> PR 3449<br>
Ran 6 of 1080 Specs in 506.852 seconds
SUCCESS! -- 6 Passed | 0 Failed | 0 Pending | 1074 Skipped
PASS

Ginkgo ran 1 suite in 8m50.12847442s
Test Suite Passed

Manual testing:

root@422447ee696fb43deecf644dff4325ea [ ~ ]# cat pop-pvc.yaml
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: pop-pvc
  namespace: test-ns
spec:
  storageClassName: wcpglobal-storage-profile
  dataSourceRef:
    apiGroup: vmoperator.vmware.com
    kind: VirtualMachine
    name: my-vm-1
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 1Gi


root@422447ee696fb43deecf644dff4325ea [ ~ ]# kubectl create -f pop-pvc.yaml
persistentvolumeclaim/pop-pvc created
root@422447ee696fb43deecf644dff4325ea [ ~ ]# kubectl get pvc -n test-ns
NAME      STATUS    VOLUME   CAPACITY   ACCESS MODES   STORAGECLASS                VOLUMEATTRIBUTESCLASS   AGE
pop-pvc   Pending                                      wcpglobal-storage-profile   <unset>                 6s


root@422447ee696fb43deecf644dff4325ea [ ~ ]# cat register.yaml
apiVersion: cns.vmware.com/v1alpha1
kind: CnsRegisterVolume
metadata:
  name: static-volume
  namespace: test-ns
spec:
  volumeID: 1a27144d-8e66-4816-aea4-51bb6388d0f0
  accessMode: ReadWriteOnce
  pvcName: pop-pvc

root@422447ee696fb43deecf644dff4325ea [ ~ ]# kubectl create -f register.yaml
cnsregistervolume.cns.vmware.com/static-volume created

root@422447ee696fb43deecf644dff4325ea [ ~ ]# kubectl get cnsregistervolume -n test-ns -o yaml
apiVersion: v1
items:
- apiVersion: cns.vmware.com/v1alpha1
  kind: CnsRegisterVolume
  metadata:
    creationTimestamp: "2025-08-13T14:29:57Z"
    generation: 2
    name: static-volume
    namespace: test-ns
    ownerReferences:
    - apiVersion: v1
      blockOwnerDeletion: true
      controller: true
      kind: PersistentVolumeClaim
      name: pop-pvc
      uid: fabe749c-88e8-43af-a078-6d8103c8b661
    resourceVersion: "10293876"
    uid: f31db484-aa58-4e34-ad84-e73b991a389d
  spec:
    accessMode: ReadWriteOnce
    pvcName: pop-pvc
    volumeID: 1a27144d-8e66-4816-aea4-51bb6388d0f0
  status:
    registered: true
kind: List
metadata:
  resourceVersion: ""

root@422447ee696fb43deecf644dff4325ea [ ~ ]# kubectl get pvc -n test-ns
NAME      STATUS   VOLUME                                           CAPACITY   ACCESS MODES   STORAGECLASS                VOLUMEATTRIBUTESCLASS   AGE
pop-pvc   Bound    static-pv-1a27144d-8e66-4816-aea4-51bb6388d0f0   1Gi        RWO            wcpglobal-storage-profile   <unset>                 9m4s

Syncer Logs:

2025-08-13T14:29:57.698Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:248   Reconciling CnsRegisterVolume with instance: "static-volume" from namespace: "test-ns". timeout "1s" seconds
2025-08-13T14:29:58.380Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:310   Created CNS volume with volumeID: 1a27144d-8e66-4816-aea4-51bb6388d0f0
2025-08-13T14:29:58.699Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:527   PV: static-pv-1a27144d-8e66-4816-aea4-51bb6388d0f0 not found. Creating a new PV
2025-08-13T14:29:58.747Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:546   PV: static-pv-1a27144d-8e66-4816-aea4-51bb6388d0f0 is created successfully

2025-08-13T14:29:58.755Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:789   Existing PVC pop-pvc in namespace test-ns has valid DataSourceRef (apiGroup: vmoperator.vmware.com, kind: VirtualMachine), can reuse
2025-08-13T14:29:58.755Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:573   PVC: pop-pvc already exists
2025-08-13T14:29:58.755Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:602   PVC pop-pvc in namespace test-ns has valid DataSourceRef with apiGroup: vmoperator.vmware.com, kind: VirtualMachine, name: my-vm-1
2025-08-13T14:30:07.624Z        INFO    cnsregistervolume/cnsregistervolume_controller.go:638   PVC: pop-pvc is bound

Special notes for your reviewer:

Release note:

Add validation for DataSourceRef in an existing PVC for CnsRegisterVolume.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 28, 2025
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 28, 2025
@k8s-ci-robot k8s-ci-robot requested a review from chethanv28 July 28, 2025 22:02
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 28, 2025
@xing-yang xing-yang force-pushed the captured_blueprints branch from 91f9a7a to 6546bbf Compare July 29, 2025 15:23
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 29, 2025
@xing-yang xing-yang force-pushed the captured_blueprints branch from 6546bbf to c5aaeeb Compare July 29, 2025 16:45
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2025
@xing-yang xing-yang force-pushed the captured_blueprints branch from c5aaeeb to 94ef27d Compare August 12, 2025 19:56
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2025
@xing-yang xing-yang force-pushed the captured_blueprints branch 2 times, most recently from ffa512b to d614299 Compare August 12, 2025 20:02
@deepakkinni
Copy link
Collaborator

Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #140

@xing-yang xing-yang force-pushed the captured_blueprints branch from d614299 to c5d9e99 Compare August 13, 2025 14:59
@xing-yang xing-yang changed the title WIP: Add a message to show DataSourceRef Validate DataSourceRef Aug 13, 2025
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 13, 2025
@xing-yang xing-yang changed the title Validate DataSourceRef Validate DataSourceRef for CnsRegisterVolume Aug 13, 2025
@deepakkinni
Copy link
Collaborator

Triggering CSI-TKG Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #279

@deepakkinni
Copy link
Collaborator

FAILED --- Jenkins Build #280

@divyenpatel
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Aug 13, 2025
@deepakkinni
Copy link
Collaborator

Triggering CSI-TKG Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #281

@deepakkinni
Copy link
Collaborator

FAILED --- Jenkins Build #281

@xing-yang xing-yang force-pushed the captured_blueprints branch from c5d9e99 to 7e99dc3 Compare August 14, 2025 13:37
@deepakkinni
Copy link
Collaborator

Triggering CSI-TKG Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #286

@deepakkinni
Copy link
Collaborator

SUCCESS --- Jenkins Build #286

Copy link
Member

@divyenpatel divyenpatel left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyenpatel, xing-yang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [divyenpatel,xing-yang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 7878e6f into kubernetes-sigs:master Aug 15, 2025
12 checks passed
@deepakkinni
Copy link
Collaborator

Triggering CSI-TKG Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete
Jenkins Build #296

@deepakkinni
Copy link
Collaborator

SUCCESS --- Jenkins Build #296

@deepakkinni
Copy link
Collaborator

SUCCESS --- Jenkins Build #299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants