adding support for plugins #238
Conversation
Pull Request Test Coverage Report for Build 12640406476Details
💛 - Coveralls |
containerz/containerz.proto
Outdated
|
|
||
| // StartPlugin starts the specified plugin identified by the name field | ||
| // of the request. It should be started using the instance_name of the | ||
| // request. If the plugin file named `name.tar` does not exist, this |
There was a problem hiding this comment.
I presume the plugin needs to be a packaged OCI image right? Does this necessarily mean it must be name.tar?
There was a problem hiding this comment.
Yes - fair point. Basically we need to make sure that the file uploaded during deploy is referenceable when PluginStart is called. So there is no need to mandate an extension.
| // of the request. It should be started using the instance_name of the | ||
| // request. If the plugin file named `name.tar` does not exist, this | ||
| // should return an error. | ||
| rpc StartPlugin(StartPluginRequest) returns (StartPluginResponse) {} |
There was a problem hiding this comment.
Given we have separate plugin-specific commands for start/stop/list -- did you consider having a different one for Deploy to make the API consistent? (Not sure of the pros/cons.)
Is there a link to how volume plugins look for podman? Best I could find is containers/podman#23762.
There was a problem hiding this comment.
AFAIK Docker plugins can be used in podman so podman plugins are the same as docker plugins.
I thought about a separate Deploy API but it seems like a lot of duplication since Deploy already uploads a file as we need.
There was a problem hiding this comment.
Sounds reasonable to me -- I was forgetting that Deploy didn't actually start the container, so this seems reasonable to me.
alshabib
force-pushed
the
plugins
branch
2 times, most recently
from
0e9e060 to
c2f51a1
Compare
| // of the request. It should be started using the instance_name of the | ||
| // request. If the plugin file named `name.tar` does not exist, this | ||
| // should return an error. | ||
| rpc StartPlugin(StartPluginRequest) returns (StartPluginResponse) {} |
There was a problem hiding this comment.
Sounds reasonable to me -- I was forgetting that Deploy didn't actually start the container, so this seems reasonable to me.
| @@ -144,6 +162,9 @@ message ImageTransfer { | |||
| // Optional. Instructs the target to fetch the image from a remote location. | |||
| // The above name and tag must be applied to the container once downloaded. | |||
| common.RemoteDownload remote_download = 4; | |||
|
|
|||
| // Indicates that the file to be transferred is a plugin. | |||
| bool is_plugin = 5; | |||
There was a problem hiding this comment.
Should this be within a oneof or alternatively an enum? It seems like we might have some other types that are mutually exclusive that might be transferred via Deploy?
There was a problem hiding this comment.
That's not clear to me - I don't think there are other common types images. Technically, this only tells the server not to attempt to load the received image so perhaps it should be no_load instead of is_plugin
No description provided.