Skip to content

fix empty volume group creation through cli tool#767

Merged
mergify[bot] merged 1 commit into
csi-addons:mainfrom
Nikhil-Ladha:fix-empty-group-creation
Feb 13, 2025
Merged

fix empty volume group creation through cli tool#767
mergify[bot] merged 1 commit into
csi-addons:mainfrom
Nikhil-Ladha:fix-empty-group-creation

Conversation

@Nikhil-Ladha

@Nikhil-Ladha Nikhil-Ladha commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

when creating empty volume group through cli, the strings.Split() function returns a [""] value, i.e, a slice of length 1 having the value as 1 ([""]) empty string. As a result, the csi panics when it tries to find the volumeID with the respective value.

@Nikhil-Ladha Nikhil-Ladha force-pushed the fix-empty-group-creation branch from 088819c to 4409e17 Compare February 13, 2025 12:56
nixpanic
nixpanic approved these changes Feb 13, 2025
View reviewed changes

@nixpanic nixpanic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Nikhil-Ladha, the change looks good to me.

Can you explain a little more about the csi panics? Is there a particular CSI driver that has a problem with an empty volume-id? If so, please report that as a bug against the driver too.

@Nikhil-Ladha

Nikhil-Ladha commented Feb 13, 2025

Copy link
Copy Markdown
Contributor Author

Can you explain a little more about the csi panics? Is there a particular CSI driver that has a problem with an empty volume-id? If so, please report that as a bug against the driver too.

The rbd ceph-csi driver panics when I try to create the VolumeGroup though CLI with an empty volumeid list.

The following is the error on CLI:

[nladha@dhcp53-204 ~]$ kubectl exec -it deployments/csi-rbdplugin-provisioner -c csi-addons -n rook-ceph -- csi-addons  --endpoint unix:///csi/csi-addons.sock --operation CreateVolumeGroup  -volumegroupname grp3 -parameters clusterID=rook-ceph,pool=replicapool -secret rook-ceph/rook-csi-rbd-provisioner
E0213 15:24:06.668477      13 volumegroup.go:75] Failed to create volume group: rpc error: code = Internal desc = panic runtime error: invalid memory address or nil pointer dereference
ERROR: failed to execute "CreateVolumeGroup": rpc error: code = Internal desc = panic runtime error: invalid memory address or nil pointer dereference
command terminated with exit code 1

And, the below is the csi-rbdplugin container logs:

I0213 15:24:06.656330       1 utils.go:266] ID: 24 GRPC call: /volumegroup.Controller/CreateVolumeGroup
I0213 15:24:06.659070       1 utils.go:267] ID: 24 GRPC request: {"name":"grp3","parameters":{"clusterID":"rook-ceph","pool":"replicapool"},"secrets":"***stripped***","volume_ids":[""]}
I0213 15:24:06.663248       1 volumegroup.go:98] ID: 24 Volume not found
E0213 15:24:06.663939       1 utils.go:335] panic occurred: runtime error: invalid memory address or nil pointer dereference
goroutine 157 [running]:
runtime/debug.Stack()
	/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.5.linux-amd64/src/runtime/debug/stack.go:26 +0x5e
runtime/debug.PrintStack()
	/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.5.linux-amd64/src/runtime/debug/stack.go:18 +0x13
github.com/ceph/ceph-csi/internal/csi-common.panicHandler.func1()
	/go/src/github.com/ceph/ceph-csi/internal/csi-common/utils.go:336 +0xca
panic({0x22ecac0?, 0x40086a0?})
	/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.5.linux-amd64/src/runtime/panic.go:785 +0x132
github.com/ceph/ceph-csi/internal/csi-addons/rbd.destoryVolumes(...)
	/go/src/github.com/ceph/ceph-csi/internal/csi-addons/rbd/replication.go:1006
github.com/ceph/ceph-csi/internal/csi-addons/rbd.(*VolumeGroupServer).CreateVolumeGroup(0x4017ff0?, {0x2b858d0, 0xc000461200}, 0xc00066c7e0)
	/go/src/github.com/ceph/ceph-csi/internal/csi-addons/rbd/volumegroup.go:99 +0x4b6
github.com/csi-addons/spec/lib/go/volumegroup._Controller_CreateVolumeGroup_Handler.func1({0x2b858d0?, 0xc000461200?}, {0x2509780?, 0xc00066c7e0?})
	/go/src/github.com/ceph/ceph-csi/vendor/github.com/csi-addons/spec/lib/go/volumegroup/volumegroup_grpc.pb.go:162 +0xcb
github.com/ceph/ceph-csi/internal/csi-common.panicHandler({0x2b858d0?, 0xc000461200?}, {0x2509780?, 0xc00066c7e0?}, 0xc0005048c0?, 0x1e778ab?)
	/go/src/github.com/ceph/ceph-csi/internal/csi-common/utils.go:341 +0x71
github.com/ceph/ceph-csi/internal/csi-common.NewMiddlewareServerOption.ChainUnaryServer.func3.1({0x2b858d0?, 0xc000461200?}, {0x2509780?, 0xc00066c7e0?})
	/go/src/github.com/ceph/ceph-csi/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:48 +0x45
github.com/ceph/ceph-csi/internal/csi-common.logSlowGRPC(0x6fc23ac00, {0x2b858d0, 0xc000461200}, {0x2509780, 0xc00066c7e0}, 0xc0006202c0, 0xc000888880)
	/go/src/github.com/ceph/ceph-csi/internal/csi-common/utils.go:320 +0x1db
github.com/ceph/ceph-csi/internal/csi-common.NewMiddlewareServerOption.func1({0x2b858d0?, 0xc000461200?}, {0x2509780?, 0xc00066c7e0?}, 0xc000132cf0?, 0xc00084a6f0?)
	/go/src/github.com/ceph/ceph-csi/internal/csi-common/utils.go:134 +0x45
github.com/ceph/ceph-csi/internal/csi-common.NewMiddlewareServerOption.ChainUnaryServer.func3.1({0x2b858d0?, 0xc000461200?}, {0x2509780?, 0xc00066c7e0?})
	/go/src/github.com/ceph/ceph-csi/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:48 +0x45
github.com/ceph/ceph-csi/internal/csi-common.logGRPC({0x2b858d0, 0xc000461200}, {0x2509780, 0xc00066c7e0}, 0x2188e00?, 0xc0008888c0)
	/go/src/github.com/ceph/ceph-csi/internal/csi-common/utils.go:269 +0x165
github.com/ceph/ceph-csi/internal/csi-common.NewMiddlewareServerOption.ChainUnaryServer.func3.1({0x2b858d0?, 0xc000461200?}, {0x2509780?, 0xc00066c7e0?})
	/go/src/github.com/ceph/ceph-csi/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:48 +0x45
github.com/ceph/ceph-csi/internal/csi-common.contextIDInjector({0x2b858d0, 0xc0004607e0}, {0x2509780, 0xc00066c7e0}, 0x0?, 0xc000888900)
	/go/src/github.com/ceph/ceph-csi/internal/csi-common/utils.go:257 +0x132
github.com/ceph/ceph-csi/internal/csi-common.NewMiddlewareServerOption.ChainUnaryServer.func3({0x2b858d0, 0xc0004607e0}, {0x2509780, 0xc00066c7e0}, 0xc0006202c0, 0x80?)
	/go/src/github.com/ceph/ceph-csi/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:53 +0x123
github.com/csi-addons/spec/lib/go/volumegroup._Controller_CreateVolumeGroup_Handler({0x249c580, 0xc0000134a0}, {0x2b858d0, 0xc0004607e0}, 0xc000a28400, 0xc00082c990)
	/go/src/github.com/ceph/ceph-csi/vendor/github.com/csi-addons/spec/lib/go/volumegroup/volumegroup_grpc.pb.go:164 +0x143
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0000bb200, {0x2b858d0, 0xc000460750}, 0xc00066c780, 0xc00082cd20, 0x402e9a0, 0x0)
	/go/src/github.com/ceph/ceph-csi/vendor/google.golang.org/grpc/server.go:1400 +0x103b
google.golang.org/grpc.(*Server).handleStream(0xc0000bb200, {0x2b86568, 0xc00084c000}, 0xc00066c780)
	/go/src/github.com/ceph/ceph-csi/vendor/google.golang.org/grpc/server.go:1810 +0xbaa
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	/go/src/github.com/ceph/ceph-csi/vendor/google.golang.org/grpc/server.go:1030 +0x7f
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 156
	/go/src/github.com/ceph/ceph-csi/vendor/google.golang.org/grpc/server.go:1041 +0x125
E0213 15:24:06.665498       1 utils.go:271] ID: 24 GRPC error: rpc error: code = Internal desc = panic runtime error: invalid memory address or nil pointer dereference

Notice the volume_ids: [""] in the GRPC request. I don't think we need a fix in the csi driver per se, even if we do not sure how exactly we could safeguard a situation like this 🤔
The error is happening here

@Nikhil-Ladha
fix empty volume group creation through cli tool
db71f1b 
when creating empty volume group through cli, the strings.Split()
function returns a [""] value, i.e, a slice of length 1 having
the value as 1 `[""]` empty string. As a result, csi panics
when it tries to find the volumeID with the respective value.

Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>
@Nikhil-Ladha Nikhil-Ladha force-pushed the fix-empty-group-creation branch from 4409e17 to db71f1b Compare February 13, 2025 17:04
@mergify mergify Bot merged commit f8e615a into csi-addons:main Feb 13, 2025
@nixpanic

nixpanic commented Feb 17, 2025

Copy link
Copy Markdown
Member

github.com/ceph/ceph-csi/internal/csi-addons/rbd.(*VolumeGroupServer).CreateVolumeGroup(0x4017ff0?, {0x2b858d0, 0xc000461200}, 0xc00066c7e0)
/go/src/github.com/ceph/ceph-csi/internal/csi-addons/rbd/volumegroup.go:99 +0x4b6

This code seems to return an error when the volume-id can not be found. I do not think there is a bug in the current Ceph-CSI implementation for this. You seem to have a different/modified version (line numbers do not match or exist (/go/src/github.com/ceph/ceph-csi/internal/csi-addons/rbd/replication.go:1006).

It should be safe to pass an empty volume-id to any Ceph-CSI gRPC procedures, and Ceph-CSI should definitely not crash. If you hit such a problem, please create an issue against Ceph-CSI for that. Thank!

@Nikhil-Ladha

Nikhil-Ladha commented Feb 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

github.com/ceph/ceph-csi/internal/csi-addons/rbd.(*VolumeGroupServer).CreateVolumeGroup(0x4017ff0?, {0x2b858d0, 0xc000461200}, 0xc00066c7e0)
/go/src/github.com/ceph/ceph-csi/internal/csi-addons/rbd/volumegroup.go:99 +0x4b6

This code seems to return an error when the volume-id can not be found. I do not think there is a bug in the current Ceph-CSI implementation for this. You seem to have a different/modified version (line numbers do not match or exist (/go/src/github.com/ceph/ceph-csi/internal/csi-addons/rbd/replication.go:1006).

Yeah, I have few changes included for VGR testing.

It should be safe to pass an empty volume-id to any Ceph-CSI gRPC procedures, and Ceph-CSI should definitely not crash. If you hit such a problem, please create an issue against Ceph-CSI for that. Thank!

It is safe to pass an empty id, and with this change that is verified.

@Nikhil-Ladha Nikhil-Ladha deleted the fix-empty-group-creation branch June 26, 2025 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nixpanic nixpanic nixpanic approved these changes

@Madhu-1 Madhu-1 Madhu-1 approved these changes

@black-dragon74 black-dragon74 Awaiting requested review from black-dragon74

@iPraveenParihar iPraveenParihar Awaiting requested review from iPraveenParihar

@Rakshith-R Rakshith-R Awaiting requested review from Rakshith-R

@yati1998 yati1998 Awaiting requested review from yati1998

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Nikhil-Ladha @nixpanic @Madhu-1