feat: supports db plugin grpc framework #7324
Conversation
xuriwuyun
requested review from
nayutah,
ldming,
heng4fun,
free6om,
wangyelei,
Y-Rookie and
weicao
as code owners
xuriwuyun
changed the title
xuriwuyun
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7324 +/- ##
==========================================
- Coverage 65.55% 64.81% -0.75%
==========================================
Files 343 345 +2
Lines 41771 42175 +404
==========================================
- Hits 27385 27334 -51
- Misses 12010 12454 +444
- Partials 2376 2387 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| } | ||
|
|
||
| // DBPlugin service provides APIs to kubeblocks operator to exec component operation. | ||
| service DBPlugin { |
There was a problem hiding this comment.
"DBPlugin" may not be the most appropriate name, as we have addons like Minio that are not necessarily database-related.
There was a problem hiding this comment.
update to ServicePlugin
There was a problem hiding this comment.
Although it's much longer, ComponentLifecyclePlugin is better.
There was a problem hiding this comment.
Actions may exist at different levels, such as component level, replica level, or even cluster level. Considering this, would it be more appropriate to use "lifecycleplugin" as the name?
| // alphanumerics between. This field is REQUIRED. | ||
| string name = 1; | ||
|
|
||
| string db_type = 2; |
There was a problem hiding this comment.
what's this field used for? is there any difference with name? is there any relationship with servicetype?
There was a problem hiding this comment.
This field is used to indicate which service the plugin is handling, primarily for logging and debugging purposes.
| string port = 2; | ||
|
|
||
| // The admin username used to access the DB service. | ||
| string admin_user = 3; |
There was a problem hiding this comment.
Is an admin account necessary? Can it be an account with lower privileges, such as a regular user account?
There was a problem hiding this comment.
Certainly, any account with appropriate privileges is allowed. In this context, "admin" refers to an administrator who possesses the necessary privileges to execute the action.
| DBInfo db_info = 1; | ||
|
|
||
| // The pod FQDN of the primary replica. | ||
| string primary_fqdn = 2; |
There was a problem hiding this comment.
Yes, this field is removed as it is duplicated with serviceinfo.fqdn.
| } | ||
|
|
||
| // DBPlugin service provides APIs to kubeblocks operator to exec component operation. | ||
| service DBPlugin { |
There was a problem hiding this comment.
What is the relationship with the original http protocol?
There was a problem hiding this comment.
The gRPC plugin is one of the implementations of the action handler, similar to the exec handler. The HTTP protocol is used for API communication between the controller and Lorry. An HTTP API call can invoke one or more gRPC plugin APIs.
There was a problem hiding this comment.
If the protocol is used for the communication with gRPC handers which are implemented by the provider, so:
- why there is a gRPC server in lorry? And where is the protocol conversion from HTTP to gRPC, or the calling to the gRPC client?
- the some problem in another PR(@#7343), how does the provider know to implement this protocol?
There was a problem hiding this comment.
The gRPC server in Lorry was initially provided in early versions solely for RSM role probing purposes. It is currently maintained for compatibility reasons and it is planned to be removed in future versions.
As the same problem with other action handlers, it is necessary to document how to implement different types of action handlers and provide examples at action level.
There was a problem hiding this comment.
So where is the calling to the customized gRPC services?
There was a problem hiding this comment.
The gRPC server in Lorry was initially provided in early versions solely for RSM role probing purposes. It is currently maintained for compatibility reasons and it is planned to be removed in future versions. As the same problem with other action handlers, it is necessary to document how to implement different types of action handlers and provide examples at action level.
And seems that the gRPC server and probe protocol are irrelevant to gRPC handlers, it's better not to involve their changes in this PR.
No description provided.