-
Notifications
You must be signed in to change notification settings - Fork 225
Add documentation to deploy and test SnapshotMetadata support #575
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 documentation to deploy and test SnapshotMetadata support #575
Conversation
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
|
/check-cla |
Signed-off-by: Prasad Ghangal <[email protected]>
Signed-off-by: Prasad Ghangal <[email protected]>
Signed-off-by: Prasad Ghangal <[email protected]>
319ce44 to
0c52080
Compare
Signed-off-by: Prasad Ghangal <[email protected]>
Signed-off-by: Prasad Ghangal <[email protected]>
Signed-off-by: Prasad Ghangal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost LGTM, just few nits
@PrasadG193
tested with complete setup here https://github.com/Rakshith-R/external-snapshot-metadata/actions/runs/12480399908/job/34831167597?pr=8
deploy/kubernetes-1.27/hostpath/csi-snapshot-metadata-sidecar.patch
Outdated
Show resolved
Hide resolved
Signed-off-by: Prasad Ghangal <[email protected]>
| # TODO: Set release tag after new release is made with SnapshotMetadata support | ||
| image: gcr.io/k8s-staging-sig-storage/hostpathplugin:canary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rakshith-R @xing-yang We cannot use released tag here until new release is made having support for SnapshotMetada service. I don't think we can use canary image tag here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rakshith-R @xing-yang We cannot use released tag here until new release is made having support for SnapshotMetada service. I don't think we can use
canaryimage tag here.
I think we can override it to canary while testing it at ext-snapshot-metadata repo + SnapshotMetadata service is guarded by a flag too in deploy script.
Let's leave it at a released tag here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is one option. Its just until the release is made, if someone want to try the feature (using steps documented at docs/example-snapshot-metadata.md), they won't be able to setup things.
@xing-yang any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the DPWG meeting, we decided to move ahead with canary image tag for now. This will be overridden at release time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a second thought, we can't merge a canary image as that will affect CI for all the sidecars. We need to hold off this PR until we cut a release for K8s 1.32. We are still waiting for a few other sidecars to be released before doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rakshith-R @xing-yang We cannot use released tag here until new release is made having support for SnapshotMetada service. I don't think we can use
canaryimage tag here.I think we can override it to canary while testing it at ext-snapshot-metadata repo + SnapshotMetadata service is guarded by a flag too in deploy script.
Let's leave it at a released tag here ?
I don't think we need to hold the entire pr.
Let's go with this approach of leaving driver image at a released tag and overriding the image to canary and turn on snap-metadata flag to test the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything else other than csi-hosthpath-driver image looks good to me.
Signed-off-by: Prasad Ghangal <[email protected]>
Signed-off-by: Prasad Ghangal <[email protected]>
|
/hold |
|
I followed the instructions successfully - worked as described. |
Signed-off-by: Prasad Ghangal <[email protected]>
| b. Execute deploy script to setup hostpath plugin driver with external-snapshot-metadata change | ||
|
|
||
| ``` | ||
| $ SNAPSHOT_METADATA_TESTS=true HOSTPATHPLUGIN_REGISTRY=gcr.io/k8s-staging-sig-storage HOSTPATHPLUGIN_TAG=canary ./deploy/kubernetes-1.27/deploy.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xing-yang @Rakshith-R turned out the deploy.sh script already has a support to override container images. So we can use these flags to deploy the hostpathplugin with canary image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks,
Looks good to me!
|
/assign @xing-yang |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: PrasadG193, Rakshith-R, 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 |
|
/hold cancel |
What this PR does / why we need it:
This PR related to the SnapshotMetadata service support added with - #569
This PR:
SnapshotMetadatasupport.Steps to deploy csi hostpath driver with
snapshot-metadataservice usingdeploy.shscript:SnapshotMetadataServiceCRD using the following command$ VERSION=main $ kubectl apply -f https://raw.githubusercontent.com/kubernetes-csi/external-snapshotmetadata/${VERSION}/client/config/crd/cbt.storage.k8s.io_snapshotmetadataservices.yamlsnapshot-metadataservice using the following command: