Skip to content
New issue

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

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

Already on GitHub? # to your account

Add documentation for Secrets in the VolumeGroupSnapshotClass #609

Merged

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Nov 15, 2024

The Secrets that can be configured in the VolumeGroupSnapshotClass were not documented yet.

Note that the external-snapshotter v8.2.0 is not released yet, the secret for GetGroupSnapshotRequest operations depends on that.

No release-note for this, versions are included in the text itself.

NONE

Depends-on: kubernetes-csi/external-snapshotter#1122

@k8s-ci-robot k8s-ci-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Nov 15, 2024
@k8s-ci-robot
Copy link
Contributor

@nixpanic: GitHub didn't allow me to request PR reviews from the following users: yati1998.

Note that only kubernetes-csi members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

The Secrets that can be configured in the VolumeGroupSnapshotClass were not documented yet.

Note that the external-snapshotter v8.2.0 is not released yet, the secret for GetGroupSnapshotRequest operations depends on that.

Depends-on: kubernetes-csi/external-snapshotter#1122

/cc yati1998 xing-yang

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 15, 2024
@nixpanic
Copy link
Member Author

v8.2.0 has been released. Once this PR is reviewed it can be merged too.


Note, however, that the delete operation will continue even if the secret is not found (because, for example, the entire namespace containing the secret was deleted). In this case, if the driver requires a secret for deletion, then the volume and PV may need to be manually cleaned up.

The values of these parameters may be "templates". The `external-snapshotter` will automatically resolve templates at snapshot create time, as detailed below:
Copy link
Contributor

Choose a reason for hiding this comment

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

snapshot create time to volume group snapshot create time


If no such secret exists in the Kubernetes API, or the snapshotter is unable to fetch it, the create operation will fail.

Note, however, that the delete operation will continue even if the secret is not found (because, for example, the entire namespace containing the secret was deleted). In this case, if the driver requires a secret for deletion, then the volume and PV may need to be manually cleaned up.
Copy link
Contributor

Choose a reason for hiding this comment

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

then the volume and PV may need to be manually cleaned up. need to be updated to reflect volume group snapshot

Copy link
Member Author

Choose a reason for hiding this comment

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

Modified it to then the volume group snapshot and related snapshots need to be manually cleaned up.


If no such secret exists in the Kubernetes API, or the snapshotter is unable to fetch it, the create operation will fail.

The values of these parameters may be "templates". The `external-snapshotter` will automatically resolve templates at snapshot create time, as detailed below:
Copy link
Contributor

Choose a reason for hiding this comment

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

snapshot create time to group snapshot creation

@nixpanic nixpanic force-pushed the volume-group-snapshot/secrets branch from 15a5ad7 to aa2bf35 Compare December 11, 2024 11:42
@nixpanic nixpanic requested a review from Madhu-1 December 11, 2024 11:42
@Madhu-1
Copy link
Contributor

Madhu-1 commented Dec 11, 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 Dec 11, 2024
@Madhu-1
Copy link
Contributor

Madhu-1 commented Jan 7, 2025

@xing-yang PTAL

@nixpanic
Copy link
Member Author

nixpanic commented Jan 7, 2025

/unassign
/assign xing-yang

@xing-yang
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nixpanic, 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 Jan 7, 2025
@k8s-ci-robot k8s-ci-robot merged commit a0d9df3 into kubernetes-csi:master Jan 7, 2025
3 checks passed
# for free to join this conversation on GitHub. Already have an account? # 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. release-note-none Denotes a PR that doesn't merit a release note. 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.

4 participants