Skip to content

Updated external-snapshot-metadata sidecar support to its alpha level#595

Merged
k8s-ci-robot merged 4 commits intokubernetes-csi:masterfrom
carlbraganza:update-example-snapshot-metadata
Jun 23, 2025
Merged

Updated external-snapshot-metadata sidecar support to its alpha level#595
k8s-ci-robot merged 4 commits intokubernetes-csi:masterfrom
carlbraganza:update-example-snapshot-metadata

Conversation

@carlbraganza
Copy link
Contributor

@carlbraganza carlbraganza commented Mar 27, 2025 •
edited by xing-yang
Loading

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 #594

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Update to csi-snapshot-metadata alpha image.

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 27, 2025
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 27, 2025
@carlbraganza
Updated instructions in docs/example-snapshot-metadata.md for the
3963162 
alpha release of the external snapshot metadata sidecar.
@carlbraganza carlbraganza force-pushed the update-example-snapshot-metadata branch from 7097bcd to 3963162 Compare March 28, 2025 00:48
PrasadG193
PrasadG193 reviewed Apr 1, 2025
View reviewed changes
docs/example-snapshot-metadata.md
kubectl wait -n csi-client --for=jsonpath='{.status.phase}'=Running --timeout=5m pod/csi-client
```

This client performs following actions:
Copy link
Contributor

@PrasadG193 PrasadG193 Apr 1, 2025

Choose a reason for hiding this comment

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

Do you think its a good idea to summarize actions performed by the client behind the scene to give more clarity?

Copy link
Contributor Author

@carlbraganza carlbraganza Apr 2, 2025

Choose a reason for hiding this comment

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

It made a lot of sense before but now it repeats what is present in the external-snapshot-metadata repo and the documentation.

@carlbraganza @PrasadG193
Apply suggestions from code review
5b40a28 
Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
@carlbraganza carlbraganza requested a review from PrasadG193 April 2, 2025 12:52
Rakshith-R
Rakshith-R reviewed Apr 3, 2025
View reviewed changes
Copy link

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

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

Looks good to me,
Thanks !

@Rakshith-R
Copy link

Rakshith-R commented Apr 8, 2025

/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 8, 2025
@Rakshith-R
Copy link

Rakshith-R commented Apr 8, 2025

/cc @xing-yang

@k8s-ci-robot k8s-ci-robot requested a review from xing-yang April 8, 2025 06:50
iPraveenParihar
iPraveenParihar suggested changes Apr 8, 2025
View reviewed changes
@carlbraganza @iPraveenParihar
Update docs/example-snapshot-metadata.md
b3034e0 
Co-authored-by: Praveen M <30765265+iPraveenParihar@users.noreply.github.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 8, 2025
Rakshith-R
Rakshith-R approved these changes Apr 9, 2025
View reviewed changes
Copy link

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

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

/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 9, 2025
@carlbraganza
Copy link
Contributor Author

carlbraganza commented Apr 9, 2025 •
edited
Loading

/assign @msau42
/unassign @Rakshith-R

iPraveenParihar
iPraveenParihar approved these changes Apr 10, 2025
View reviewed changes
Copy link
Contributor

@iPraveenParihar iPraveenParihar left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Apr 10, 2025

@iPraveenParihar: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

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.

PrasadG193
PrasadG193 reviewed Apr 10, 2025
View reviewed changes
docs/example-snapshot-metadata.md
```
2. Execute the deploy script to setup the hostpath plugin driver with the Kubernetes SnapshotMetadata service:
```
SNAPSHOT_METADATA_TESTS=true HOSTPATHPLUGIN_REGISTRY=registry.k8s.io/sig-storage HOSTPATHPLUGIN_TAG=v1.16.1 ./deploy/kubernetes-latest/deploy.sh
Copy link
Contributor

@PrasadG193 PrasadG193 Apr 10, 2025

Choose a reason for hiding this comment

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

Suggested change
SNAPSHOT_METADATA_TESTS=true HOSTPATHPLUGIN_REGISTRY=registry.k8s.io/sig-storage HOSTPATHPLUGIN_TAG=v1.16.1 ./deploy/kubernetes-latest/deploy.sh
SNAPSHOT_METADATA_TESTS=true ./deploy/kubernetes-latest/deploy.sh

We don't need to override the registry and tag as the latest released hostpath driver has SnapshotMetadata API support

Copy link
Contributor Author

@carlbraganza carlbraganza Apr 16, 2025

Choose a reason for hiding this comment

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

Are you sure about that? When I last tried it out (just before KubeCon) it failed without explicitly specifying the tag.

Copy link
Contributor

@PrasadG193 PrasadG193 May 28, 2025

Choose a reason for hiding this comment

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

@carlbraganza looks like we didn't update the image tag for hostpath plugin deployment to latest release.
This will work once the tag is updated to the latest. We can do that in the same PR as well
https://github.com/kubernetes-csi/csi-driver-host-path/pull/607/files

docs/example-snapshot-metadata.md
```
Specifying the `SNAPSHOT_METADATA_TESTS=true` environment variable causes it to configure the `external-snapshot-metadata` sidecar
in the CSI hostpath driver Pod.
The `HOSTPATHPLUGIN_REGISTRY` and `HOSTPATHPLUGIN_TAG` environment variables are used to override defaults for the CSI hostpath driver image.
Copy link
Contributor

@PrasadG193 PrasadG193 Apr 10, 2025

Choose a reason for hiding this comment

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

Suggested change
The `HOSTPATHPLUGIN_REGISTRY` and `HOSTPATHPLUGIN_TAG` environment variables are used to override defaults for the CSI hostpath driver image.

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jun 23, 2025
@k8s-ci-robot k8s-ci-robot removed the release-note-none Denotes a PR that doesn't merit a release note. label Jun 23, 2025
@xing-yang
Copy link
Contributor

xing-yang commented Jun 23, 2025

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jun 23, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carlbraganza, iPraveenParihar, Rakshith-R, xing-yang

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

The pull request process is described here

Details 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 Jun 23, 2025
@k8s-ci-robot k8s-ci-robot merged commit d26d6aa into kubernetes-csi:master Jun 23, 2025
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andyzhangx andyzhangx Awaiting requested review from andyzhangx

@pohly pohly Awaiting requested review from pohly

@xing-yang xing-yang Awaiting requested review from xing-yang

3 more reviewers

@PrasadG193 PrasadG193 PrasadG193 left review comments

@iPraveenParihar iPraveenParihar iPraveenParihar approved these changes

@Rakshith-R Rakshith-R Rakshith-R approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@xing-yang xing-yang

@msau42 msau42

@Rakshith-R Rakshith-R

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/feature Categorizes issue or PR as related to a new feature. 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update support for the external-snapshot-metadata sidecar to its alpha level

7 participants

@carlbraganza @Rakshith-R @k8s-ci-robot @xing-yang @PrasadG193 @iPraveenParihar @msau42