Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and set the new VolumeSnapshotHandlePairList field #1169

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

leonardoce
Copy link
Contributor

@leonardoce leonardoce commented Oct 23, 2024

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:

Adds and sets the new field VolumeSnapshotHandlePairList of the VolumeGroupSnapshotContent resource, as described in the latest iteration of the relative KEP.

This is a prerequisite to promote the volume group snapshot API to v1beta1.

Which issue(s) this PR fixes:

Special notes for your reviewer:

This has been extracted from #1150

This is an example of how a VolumeGroupSnapshotContent looks:

❯ kubectl get volumegroupsnapshotcontent groupsnapcontent-cea7182b-4b04-4614-a3e8-a5b09c669e5c  -o yaml
apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
kind: VolumeGroupSnapshotContent
metadata:
  creationTimestamp: "2024-10-23T15:24:11Z"
  finalizers:
  - groupsnapshot.storage.kubernetes.io/volumegroupsnapshotcontent-bound-protection
  generation: 1
  name: groupsnapcontent-cea7182b-4b04-4614-a3e8-a5b09c669e5c
  resourceVersion: "458156"
  uid: 740d4c07-9efc-4c6b-b46a-3c4733a391d3
spec:
  deletionPolicy: Delete
  driver: hostpath.csi.k8s.io
  source:
    volumeHandles:
    - 75b1b2e9-8cc4-11ef-b606-c20bfb388315
    - 75b196d5-8cc4-11ef-b606-c20bfb388315
  volumeGroupSnapshotClassName: csi-hostpath-groupsnapclass
  volumeGroupSnapshotRef:
    apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
    kind: VolumeGroupSnapshot
    name: new-groupsnapshot-demo
    namespace: default
    resourceVersion: "458124"
    uid: cea7182b-4b04-4614-a3e8-a5b09c669e5c
status:
  creationTime: 1729697051387803433
  pvVolumeSnapshotContentList:
  - persistentVolumeRef:
      name: pvc-4469b2c3-c5ca-415e-a601-427de7ebe333
    volumeSnapshotContentRef:
      name: snapcontent-5943c2815c1b999992c542a608e005e211cbedaf8165b06b4a33a96876a25752-2024-10-23-3.24.13
  - persistentVolumeRef:
      name: pvc-7cd5e901-3c7e-464f-9bff-2adb860487e6
    volumeSnapshotContentRef:
      name: snapcontent-d12702e75e2f70c6437ff30683d8e76fb6d2b0f8e8d08b950d30ca919242e063-2024-10-23-3.24.13
  readyToUse: true
  volumeGroupSnapshotHandle: daacc21b-9152-11ef-b66d-2e29031d8e65
  volumeSnapshotHandlePairList:
  - snapshotHandle: daaccc26-9152-11ef-b66d-2e29031d8e65
    volumeHandle: 75b1b2e9-8cc4-11ef-b606-c20bfb388315
  - snapshotHandle: db2e0c02-9152-11ef-b66d-2e29031d8e65
    volumeHandle: 75b196d5-8cc4-11ef-b606-c20bfb388315

Does this PR introduce a user-facing change?:

Add a field called `volumegroupsnapshotcontent.status.volumeSnapshotHandlePairList` that allows the consumer to quickly map volume handles with snapshot handles.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 23, 2024
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 23, 2024
@xing-yang
Copy link
Collaborator

Can you please fix the PR title? It is VolumeSnapshotHandlePairList, not pvVolumeSnapshotContentList.

@xing-yang
Copy link
Collaborator

/assign @jsafrane

@leonardoce leonardoce changed the title Add and set the new pvVolumeSnapshotContentList field Add and set the new VolumeSnapshotHandlePairList field Oct 25, 2024
@leonardoce
Copy link
Contributor Author

@xing-yang I just applied your suggestions. Thanks!

@jsafrane
Copy link
Contributor

I take it as a first step towards API rework. I would expect some pairs to be removed in the end, like VolumeGroupSnapshotContentStatus.PVVolumeSnapshotContentList is actually replaced by the new field and perhaps VolumeGroupSnapshotStatus.PVCVolumeSnapshotRefList is useless too.

@jsafrane
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 25, 2024
@xing-yang
Copy link
Collaborator

xing-yang commented Oct 25, 2024

I take it as a first step towards API rework. I would expect some pairs to be removed in the end, like VolumeGroupSnapshotContentStatus.PVVolumeSnapshotContentList is actually replaced by the new field and perhaps VolumeGroupSnapshotStatus.PVCVolumeSnapshotRefList is useless too.

@jsafrane VolumeGroupSnapshotStatus.PVCVolumeSnapshotRefList is still required for restore as we also need to support static provisioning.
We will try to remove VolumeGroupSnapshotContentStatus.PVVolumeSnapshotContentList at a later phase. We have the plan listed in this doc: https://docs.google.com/document/d/1jjliw0tFhCPnT9ixlYpj61k5u8_-awf1qSCWrkunshQ/edit?tab=t.0#heading=h.fizenae5evi0

@xing-yang
Copy link
Collaborator

@andyzhangx Can you help take a look at the Trivy scanner CI error? Thanks.

Running Trivy with options: trivy image test/snapshot-validation-webhook:latest
2024-10-25T07:59:09Z INFO [vulndb] Need to update DB
2024-10-25T07:59:09Z INFO [vulndb] Downloading vulnerability DB...
2024-10-25T07:59:09Z INFO [vulndb] Downloading artifact... repo="ghcr.io/aquasecurity/trivy-db:2"
2024-10-25T07:59:09Z ERROR [vulndb] Failed to download artifact repo="ghcr.io/aquasecurity/trivy-db:2" err="OCI repository error: 1 error occurred:\n\t* GET https://ghcr.io/v2/aquasecurity/trivy-db/manifests/2: TOOMANYREQUESTS: retry-after: 295.49µs, allowed: 44000/minute\n\n"
2024-10-25T07:59:09Z FATAL Fatal error init error: DB error: failed to download vulnerability DB: OCI artifact error: failed to download vulnerability DB: failed to download artifact from any source
Error: Process completed with exit code 1.

@andyzhangx
Copy link
Member

that's a transient error, rerun should work

@xing-yang
Copy link
Collaborator

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leonardoce, 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:

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 28, 2024
@k8s-ci-robot k8s-ci-robot merged commit 33b4e61 into kubernetes-csi:master Oct 28, 2024
8 checks passed
@xing-yang
Copy link
Collaborator

/assign @msau42

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. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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.

6 participants