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

Use group specific annotation for the group secrets #1048

Merged

Conversation

Madhu-1
Copy link
Contributor

@Madhu-1 Madhu-1 commented Mar 28, 2024

This PR fixes the bug in the secret handling of the volumegroupsnapshot and also uses two new annotations to store the group secret in volumegroupsnapshotcontent object "groupsnapshot.storage.kubernetes.io/deletion-secret-name" and "groupsnapshot.storage.kubernetes.io/deletion-secret-namespace" are the new annotation.

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:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Fix bug in retrieving the group secret from the volumegroupsnapshotcontent object

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 28, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @Madhu-1. 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.

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 28, 2024
@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Mar 28, 2024

/assign @xing-yang

@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Mar 28, 2024

Below is the log from the testing

[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl createtl create -f volumegroupsnapshot.yaml 
volumegroupsnapshot.groupsnapshot.storage.k8s.io/my-group-snapshot created
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl get volumegroupsnapshot
NAME                READYTOUSE   VOLUMEGROUPSNAPSHOTCLASS          VOLUMEGROUPSNAPSHOTCONTENT                              CREATIONTIME   AGE
my-group-snapshot   true         csi-cephfsplugin-groupsnapclass   groupsnapcontent-6356f378-6347-4ec5-8f18-deecb4961dcc   45s            46s
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl get volumegroupsnapshotclass -oyaml
apiVersion: v1
items:
- apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
  deletionPolicy: Delete
  driver: rook-ceph.cephfs.csi.ceph.com
  kind: VolumeGroupSnapshotClass
  metadata:
    creationTimestamp: "2024-03-28T12:05:00Z"
    generation: 1
    name: csi-cephfsplugin-groupsnapclass
    resourceVersion: "24592"
    uid: da89c0ef-39fb-4322-b695-e4bf40f89a2e
  parameters:
    clusterID: rook-ceph
    csi.storage.k8s.io/group-snapshotter-secret-name: rook-csi-cephfs-provisioner
    csi.storage.k8s.io/group-snapshotter-secret-namespace: rook-ceph
    fsName: myfs
kind: List
metadata:
  resourceVersion: ""
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl get volumesnapshot
NAME                                                                                           READYTOUSE   SOURCEPVC   SOURCESNAPSHOTCONTENT                                                                             RESTORESIZE   SNAPSHOTCLASS   SNAPSHOTCONTENT                                                                                   CREATIONTIME   AGE
snapshot-ddb89173044c5ccedc1909c90d804f9c40b36b1c99d149ea98222f1e03d2ad36-2024-03-28-1.40.24   true                     snapcontent-ddb89173044c5ccedc1909c90d804f9c40b36b1c99d149ea98222f1e03d2ad36-2024-03-28-1.40.24   1Gi                           snapcontent-ddb89173044c5ccedc1909c90d804f9c40b36b1c99d149ea98222f1e03d2ad36-2024-03-28-1.40.24   58s            58s
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl get volumegroupsnapshotcontent
NAME                                                    READYTOUSE   DELETIONPOLICY   DRIVER                          VOLUMEGROUPSNAPSHOTCLASS          VOLUMEGROUPSNAPSHOTNAMESPACE   VOLUMEGROUPSNAPSHOT   AGE
groupsnapcontent-6356f378-6347-4ec5-8f18-deecb4961dcc   true         Delete           rook-ceph.cephfs.csi.ceph.com   csi-cephfsplugin-groupsnapclass   default                        my-group-snapshot     65s
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl get volumegroupsnapshotcontent
NAME                                                    READYTOUSE   DELETIONPOLICY   DRIVER                          VOLUMEGROUPSNAPSHOTCLASS          VOLUMEGROUPSNAPSHOTNAMESPACE   VOLUMEGROUPSNAPSHOT   AGE
groupsnapcontent-6356f378-6347-4ec5-8f18-deecb4961dcc   true         Delete           rook-ceph.cephfs.csi.ceph.com   csi-cephfsplugin-groupsnapclass   default                        my-group-snapshot     81s
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl get volumegroupsnapshotcontent -oyaml
apiVersion: v1
items:
- apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
  kind: VolumeGroupSnapshotContent
  metadata:
    annotations:
      groupsnapshot.storage.kubernetes.io/deletion-secret-name: rook-csi-cephfs-provisioner
      groupsnapshot.storage.kubernetes.io/deletion-secret-namespace: rook-ceph
    creationTimestamp: "2024-03-28T13:40:23Z"
    finalizers:
    - groupsnapshot.storage.kubernetes.io/volumegroupsnapshotcontent-bound-protection
    generation: 1
    name: groupsnapcontent-6356f378-6347-4ec5-8f18-deecb4961dcc
    resourceVersion: "33924"
    uid: dd36873b-7d52-41a2-a175-5d2659fcc2bc
  spec:
    deletionPolicy: Delete
    driver: rook-ceph.cephfs.csi.ceph.com
    source:
      volumeHandles:
      - 0001-0009-rook-ceph-0000000000000001-e869764e-d890-407b-8ad8-4b6e4ed91373
    volumeGroupSnapshotClassName: csi-cephfsplugin-groupsnapclass
    volumeGroupSnapshotRef:
      apiVersion: groupsnapshot.storage.k8s.io/v1alpha1
      kind: VolumeGroupSnapshot
      name: my-group-snapshot
      namespace: default
      resourceVersion: "33901"
      uid: 6356f378-6347-4ec5-8f18-deecb4961dcc
  status:
    creationTime: 1711633224180366514
    readyToUse: true
    volumeGroupSnapshotHandle: 0001-0009-rook-ceph-0000000000000001-5b10f444-8e0e-4e37-93ae-796f3f66b0be
    volumeSnapshotContentRefList:
    - kind: VolumeSnapshotContent
      name: snapcontent-ddb89173044c5ccedc1909c90d804f9c40b36b1c99d149ea98222f1e03d2ad36-2024-03-28-1.40.24
kind: List
metadata:
  resourceVersion: ""
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl delete -f volumegroupsnapshot.yaml 
volumegroupsnapshot.groupsnapshot.storage.k8s.io "my-group-snapshot" deleted
[🎩︎]mrajanna@li-2cfbef4c-22d9-11b2-a85c-a3e4a93c405f cephfs $]kubectl get volumegroupsnapshot,volumegroupsnapshotcontent,volumesnapshot,volumesnapshotcontent
No resources found

@xing-yang
Copy link
Collaborator

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 28, 2024
@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Apr 2, 2024

@xing-yang Can you please review this PR?

@nixpanic
Copy link
Contributor

nixpanic commented Apr 2, 2024

/lgtm

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

Can you squash the commits?

Use the right groupsnapshot secret for the group
operations.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@Madhu-1 Madhu-1 force-pushed the fix-group-snapshot-secret-bug branch from 15cdf25 to 30b24e6 Compare April 3, 2024 16:31
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 3, 2024
@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Apr 3, 2024

Can you squash the commits?

@xing-yang squashed the commits, PTAL

@xing-yang
Copy link
Collaborator

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Madhu-1, 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 Apr 4, 2024
@k8s-ci-robot k8s-ci-robot merged commit a2ab92d into kubernetes-csi:master Apr 4, 2024
6 checks passed
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. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants