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

PWX-39364 - Add support for Cloud snapshots in CSI #2492

Merged

Conversation

sradhakrishnan-px
Copy link
Collaborator

@sradhakrishnan-px sradhakrishnan-px commented Sep 30, 2024

What this PR does / why we need it:
Explain the PR and why it is needed.

This PR cherrypicks commits present in the PRs #2417 and #2271 to release-9.9 branch.

The original change introduced cloud snapshot capability via CSI. The APIs which have been changed are DeleteSnapshot/CreateSnapshot/CreateVolume and ListSnapshots.

Some modifications have been made to the original PR to make the delete snapshot logic better and fix unit tests. The PR also added facility to list snapshot API for cloud snapshots. Please note that Kubernetes only calls ListSnapshots for a single snapshot ID, during predefined snapshot(please see the section For pre-provisioned snapshots in the doc https://kubernetes.io/docs/concepts/storage/volume-snapshots/#volume-snapshot-contents)

Which issue(s) this PR fixes (optional)
Closes #
or
PWX-39364

Testing Notes
Add testing output or passing unit test output here.

Special notes for your reviewer:
Add any notes for the reviewer here.

@sradhakrishnan-px sradhakrishnan-px changed the title Sradhakrishnan/pwx 39364 Sradhakrishnan/pwx 39364 - Do not merge Sep 30, 2024
@sradhakrishnan-px sradhakrishnan-px marked this pull request as ready for review October 3, 2024 09:15
@sradhakrishnan-px sradhakrishnan-px changed the title Sradhakrishnan/pwx 39364 - Do not merge PWX-39364 - Add support for Cloud snapshots in CSI - Do not merge Oct 3, 2024
@sradhakrishnan-px sradhakrishnan-px changed the base branch from release-9.9 to feature-CSI-Cloud-Snapshots October 7, 2024 02:58
@sradhakrishnan-px sradhakrishnan-px force-pushed the sradhakrishnan/PWX-39364 branch from fbb6a2f to ec1827e Compare October 9, 2024 10:48
Signed-off-by: Shyam Radhakrishnan <[email protected]>
@sradhakrishnan-px sradhakrishnan-px force-pushed the sradhakrishnan/PWX-39364 branch from ec1827e to 03da896 Compare October 9, 2024 10:50
Copy link
Contributor

@pnookala-px pnookala-px left a comment

Choose a reason for hiding this comment

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

The change looks good to me.

@twang-ps twang-ps self-requested a review October 11, 2024 02:07
Copy link
Contributor

@twang-ps twang-ps left a comment

Choose a reason for hiding this comment

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

LGTM

@sradhakrishnan-px
Copy link
Collaborator Author

pr verify output

root@ip-10-13-186-26:~/workspace/git/go/src/github.com/libopenstorage/openstorage# make pr-verify
go fmt  | grep -v "api.pb.go" | wc -l | grep "^0";
package github.com/libopenstorage/openstorage: no Go files in /root/workspace/git/go/src/github.com/libopenstorage/openstorage
0
go run tools/sdkver/sdkver.go --check-major=0 --check-minor=101
0.101.54
git-validation -run DCO,short-subject
INFO[0000] using commit range: d41ec0f733cf838da680295aeeecc93a3a9844a6..03da8965d14f927942004bd4bcdc809f02d4bc0c 
 * 03da8965 "Add support for Cloud snapshots via CSI" ... PASS
make docker-proto
make[1]: Entering directory '/root/workspace/git/go/src/github.com/libopenstorage/openstorage'
docker pull quay.io/openstorage/osd-proto:pre-gomodules
pre-gomodules: Pulling from openstorage/osd-proto
Digest: sha256:76b09a1a0231f9cf78f22eecd243d7ddb2bcc464e0cea9493a2d08dfe534ef14
Status: Image is up to date for quay.io/openstorage/osd-proto:pre-gomodules
quay.io/openstorage/osd-proto:pre-gomodules
docker run \
	--privileged --rm \
	-v /root/workspace/git/go/src/github.com/libopenstorage/openstorage:/go/src/github.com/libopenstorage/openstorage \
	-e "GOPATH=/go" \
	-e "DOCKER_PROTO=yes" \
	-e "PATH=/bin:/usr/bin:/usr/local/bin:/go/bin:/usr/local/go/bin" \
	quay.io/openstorage/osd-proto:pre-gomodules \
		make proto mockgen
>>> Generating protobuf definitions from api/api.proto
protoc -I /go/src/github.com/libopenstorage/openstorage \
	-I /usr/local/include \
	-I /go/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
	--go_out=plugins=grpc:. \
	/go/src/github.com/libopenstorage/openstorage/api/api.proto
protoc -I /go/src/github.com/libopenstorage/openstorage \
	-I /usr/local/include \
	-I /go/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
	--grpc-gateway_out=logtostderr=true:. \
	/go/src/github.com/libopenstorage/openstorage/api/api.proto
protoc -I /go/src/github.com/libopenstorage/openstorage \
	-I /usr/local/include \
	-I /go/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
	--swagger_out=logtostderr=true:/go/src/github.com/libopenstorage/openstorage/api/server/sdk \
	/go/src/github.com/libopenstorage/openstorage/api/api.proto
>>> Upgrading swagger 2.0 to openapi 3.0
mv api/server/sdk/api/api.swagger.json api/server/sdk/api/20api.swagger.json
swagger2openapi api/server/sdk/api/20api.swagger.json -o api/server/sdk/api/api.swagger.json
rm -f api/server/sdk/api/20api.swagger.json
>>> Generating grpc protobuf definitions from pkg/flexvolume/flexvolume.proto
protoc -I/usr/local/include -I/go/src/github.com/libopenstorage/openstorage -I/go/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --go_out=plugins=grpc:. /go/src/github.com/libopenstorage/openstorage/pkg/flexvolume/flexvolume.proto
protoc -I/usr/local/include -I/go/src/github.com/libopenstorage/openstorage -I/go/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --grpc-gateway_out=logtostderr=true:. /go/src/github.com/libopenstorage/openstorage/pkg/flexvolume/flexvolume.proto
>>> Generating protobuf definitions from pkg/jsonpb/testing/testing.proto
protoc -I /go/src/github.com/libopenstorage/openstorage /go/src/github.com/libopenstorage/openstorage/pkg/jsonpb/testing/testing.proto --go_out=plugins=grpc:.
>>> Updating SDK versions
go run tools/sdkver/sdkver.go --swagger api/server/sdk/api/api.swagger.json
0.101.54
go get github.com/golang/mock/gomock
go get github.com/golang/mock/mockgen || echo "ignoring go get build error"
package io/fs: unrecognized import path "io/fs" (import path does not begin with hostname)
ignoring go get build error
cd /go/src/github.com/golang/mock/mockgen && git checkout v1.2.0 && go install github.com/golang/mock/mockgen
Note: checking out 'v1.2.0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at 51421b9 mockgen: use Controller.Helper() in generated mocks
mockgen -destination=api/mock/mock_storagepool.go -package=mock github.com/libopenstorage/openstorage/api OpenStoragePoolServer,OpenStoragePoolClient
mockgen -destination=api/mock/mock_cluster.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageClusterServer,OpenStorageClusterClient
mockgen -destination=api/mock/mock_node.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageNodeServer,OpenStorageNodeClient
mockgen -destination=api/mock/mock_diags.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageDiagsServer,OpenStorageDiagsClient
mockgen -destination=api/mock/mock_volume.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageVolumeServer,OpenStorageVolumeClient
mockgen -destination=api/mock/mock_watch.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageWatchServer,OpenStorageWatchClient,OpenStorageWatch_WatchClient,OpenStorageWatch_WatchServer
mockgen -destination=api/mock/mock_bucket.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageBucketServer,OpenStorageBucketClient
mockgen -destination=api/mock/mock_cloud_backup.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageCloudBackupServer,OpenStorageCloudBackupClient
mockgen -destination=cluster/mock/cluster.mock.go -package=mock github.com/libopenstorage/openstorage/cluster Cluster
mockgen -destination=cluster/mock/cluster_listener.mock.go -package=mock github.com/libopenstorage/openstorage/cluster ClusterListener
mockgen -destination=api/mock/mock_fstrim.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageFilesystemTrimServer,OpenStorageFilesystemTrimClient
mockgen -destination=api/mock/mock_fscheck.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageFilesystemCheckServer,OpenStorageFilesystemCheckClient
mockgen -destination=api/mock/mock_defrag.go -package=mock github.com/libopenstorage/openstorage/api OpenStorageFilesystemDefragServer,OpenStorageFilesystemDefragClient
mockgen -destination=api/server/mock/mock_schedops_k8s.go -package=mock github.com/portworx/sched-ops/k8s/core Ops
mockgen -destination=volume/drivers/mock/driver.mock.go -package=mock github.com/libopenstorage/openstorage/volume VolumeDriver
mockgen -destination=bucket/drivers/mock/bucket_driver.mock.go -package=mock github.com/libopenstorage/openstorage/bucket BucketDriver
mockgen -destination=pkg/loadbalancer/mock/balancer.go -package=mock github.com/libopenstorage/openstorage/pkg/loadbalancer Balancer
make[1]: Leaving directory '/root/workspace/git/go/src/github.com/libopenstorage/openstorage'
git diff  | wc -l | grep "^0"
0
hack/check-api-version.sh
fatal: ambiguous argument 'release-9.0..HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
git grep -rw GPL vendor | grep LICENSE | egrep -v "yaml.v2" | wc -l | grep "^0"
0
hack/check-registered-rest.sh
+ cat api/api.pb.go
+ grep func Register.*
+ wc -l
+ registerFuncs=24
+ cat api/server/sdk/rest_gateway.go
+ wc -l
+ grep Register.*Handler,
+ registered=24
+ [ 24 != 24 ]
+ echo All REST handlers are registered in api/server/sdk/rest_gateway.go.
All REST handlers are registered in api/server/sdk/rest_gateway.go.

@sradhakrishnan-px
Copy link
Collaborator Author

Unit testa re passing except alerts_test and sdk_test which are failing in 9.9 branch as well

  	github.com/libopenstorage/openstorage/csi	74.883s
?   	github.com/libopenstorage/openstorage/csi/sched	[no test files]
?   	github.com/libopenstorage/openstorage/csi/sched/k8s	[no test files]
ok  	github.com/libopenstorage/openstorage/csi/v0.3	0.099s
?   	github.com/libopenstorage/openstorage/csi/v0.3/spec	[no test file

@sradhakrishnan-px sradhakrishnan-px changed the title PWX-39364 - Add support for Cloud snapshots in CSI - Do not merge PWX-39364 - Add support for Cloud snapshots in CSI Oct 11, 2024
@sradhakrishnan-px sradhakrishnan-px self-assigned this Oct 11, 2024
@sradhakrishnan-px sradhakrishnan-px merged commit 35456e4 into feature-CSI-Cloud-Snapshots Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants