API change to improve VolumeGroupSnapshot restore #1068

leonardoce · 2024-04-18T08:59:03Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change

/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

This PR implements an API change proposed by @xing-yang (see https://docs.google.com/document/d/1NdNwFD5Z64K2heQLYOnojt6Ogulg750BuYIJ6W9cEiM/edit?usp=sharing and the relative conversation in #969).

To ease the review process, this PR has been split into two commits:

a commit changing the API, including the generated files
some minimal changes on the controller and the CSI sidecar to allow the code to compile with the new API.

The information we have in VolumeGroupSnapshot.status.volumeSnapshotRefList has been moved into VolumeGroupSnapshot.status.pvcVolumeSnapshotRefList.volumeSnapshotRef.

The information we have in VolumeGroupSnapshotContent.status.volumeSnapshotContentRefList has been moved into VolumeGroupSnapshotContent.status.volumeSnapshotContentRefList.volumeSnapshotContentName.

The code to set the new fields has already been implemented, and I'm going to open a new PR based on this one.
I hope that it will be helpful for the reviewer.

If you prefer a single PR on multiple commits, just tell me.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

This is my first API-change PR. I followed the process described in client/hack/README.md as carefully as possible, but I encourage you to use special care when reviewing this.

Testing

After this patch, a VolumeGroupSnapshot will be defined as:

apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
kind: VolumeGroupSnapshot
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"groupsnapshot.storage.k8s.io/v1alpha1","kind":"VolumeGroupSnapshot","metadata":{"annotations":{},"name":"new-groupsnapshot-demo","namespace":"default"},"spec":{"source":{"selector":{"matchLabels":{"cnpg.io/cluster":"cluster-example"}}},"volumeGroupSnapshotClassName":"csi-hostpath-groupsnapclass"}}
  creationTimestamp: "2024-04-24T21:09:22Z"
  finalizers:
  - groupsnapshot.storage.kubernetes.io/volumegroupsnapshot-bound-protection
  generation: 1
  name: new-groupsnapshot-demo
  namespace: default
  resourceVersion: "664981"
  uid: e647e38f-d73e-4061-9113-c999d6ebd173
spec:
  source:
    selector:
      matchLabels:
        cnpg.io/cluster: cluster-example
  volumeGroupSnapshotClassName: csi-hostpath-groupsnapclass
status:
  boundVolumeGroupSnapshotContentName: groupsnapcontent-e647e38f-d73e-4061-9113-c999d6ebd173
  creationTime: "2024-04-24T21:09:23Z"
  pvcVolumeSnapshotRefList:
  - persistentVolumeClaimRef: {}
    volumeSnapshotRef:
      name: snapshot-527c30405cb932a46be1207bec23311ded70d6b0ffbfbe534680b8ebd24faea1-2024-04-24-9.9.24
  - persistentVolumeClaimRef: {}
    volumeSnapshotRef:
      name: snapshot-d7734103b40865afaa62b7aa69165c751ae07e9d3a153c62da33bdfe997e1170-2024-04-24-9.9.24
  readyToUse: true

And the corresponding VolumeGroupSnapshotContent would be like:

apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
kind: VolumeGroupSnapshotContent
metadata:
  creationTimestamp: "2024-04-24T21:09:22Z"
  finalizers:
  - groupsnapshot.storage.kubernetes.io/volumegroupsnapshotcontent-bound-protection
  generation: 1
  name: groupsnapcontent-e647e38f-d73e-4061-9113-c999d6ebd173
  resourceVersion: "664972"
  uid: f7eab355-63c6-4f9d-82a7-ef6768e74a17
spec:
  deletionPolicy: Delete
  driver: hostpath.csi.k8s.io
  source:
    volumeHandles:
    - d9c0e0b9-020d-11ef-962a-0abf9c1ed13e
    - d9c87ebe-020d-11ef-962a-0abf9c1ed13e
  volumeGroupSnapshotClassName: csi-hostpath-groupsnapclass
  volumeGroupSnapshotRef:
    apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
    kind: VolumeGroupSnapshot
    name: new-groupsnapshot-demo
    namespace: default
    resourceVersion: "664940"
    uid: e647e38f-d73e-4061-9113-c999d6ebd173
status:
  creationTime: 1713992963010846995
  pvVolumeSnapshotContentList:
  - persistentVolumeRef: {}
    volumeSnapshotContentRef:
      name: snapcontent-527c30405cb932a46be1207bec23311ded70d6b0ffbfbe534680b8ebd24faea1-2024-04-24-9.9.24
  - persistentVolumeRef: {}
    volumeSnapshotContentRef:
      name: snapcontent-d7734103b40865afaa62b7aa69165c751ae07e9d3a153c62da33bdfe997e1170-2024-04-24-9.9.24
  readyToUse: true
  volumeGroupSnapshotHandle: ec95172d-027e-11ef-9db6-961b7658d99d

Does this PR introduce a user-facing change?:

Update API for group snapshots, easing the restore process.

k8s-ci-robot · 2024-04-18T08:59:13Z

Hi @leonardoce. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Change `VolumeSnapshotRefList` in `VolumeGroupSnapshotStatus` to `PVCVolumeSnapshotRefList`, allowing users to map a `VolumeSnapshot` to a specific `PersistentVolumeClaim`. Change `VolumeSnapshotContentRefList` in `VolumeGroupSnapshotContentStatus` to `PVVolumeSnapshotContentRefList`, allowing users to map a `VolumeSnapshotContent` to a specific `PersistentVolume`. This two changes allow users to map each `VolumeSnapshot` back to the original PVC it was taken from.

This commit won't set any value in the new fields, but only change the logic to reproduce the behavior we already had.

xing-yang · 2024-04-18T13:14:22Z

/assign

xing-yang · 2024-04-18T13:14:34Z

/ok-to-test

xing-yang · 2024-04-19T17:50:27Z