Description
API: types/plugins/logdriver must be removed from API module
The api module currently has a dependency on github.com/gogo/protobuf;
|
github.com/gogo/protobuf v1.3.2 |
The github.com/gogo/protobuf module is deprecated, and we don't want it to be a dependency of the API; it looks like the dependency comes from the types/plugins/logdriver package;
|
proto "github.com/gogo/protobuf/proto" |
That package is not used as part of the API and not used by the CLI; it does not belong in the API module
Looking at history, this was actually brought up during review of the original PR; it was put inside api to indicate "this must be a stable interface", and at the time "go modules" were not yet in sight, so putting it in the API package did not have a big effect #28403 (comment)
As a general comment, everything in api/types is strictly the Request/Response types of the engine-api. The logger API doesnt belong in here. I think it should be moved out, maybe to a pkg/logger package?
Stuff in pkg/ tends to be a bit more open to change. The LogEntry type in particular should not be changed, or rather changed with extreme caution, which is why I put it in api/types... but honestly wherever people want to put it is fine.
We must move this package elsewhere, but TBD where it should be put;
Inside our own code, it's used in daaemon/logger, which uses it to handle log-driver plugins;
git grep '"github.com/moby/moby/api/types/plugins/logdriver"'
daemon/logger/adapter.go: "github.com/moby/moby/api/types/plugins/logdriver"
daemon/logger/adapter_test.go: "github.com/moby/moby/api/types/plugins/logdriver"
daemon/logger/local/local.go: "github.com/moby/moby/api/types/plugins/logdriver"
daemon/logger/local/local_test.go: "github.com/moby/moby/api/types/plugins/logdriver"
daemon/logger/local/read.go: "github.com/moby/moby/api/types/plugins/logdriver"
daemon/logger/plugin.go: "github.com/moby/moby/api/types/plugins/logdriver"
However those are uses as "consumer" of the specification, and the daemon/ module will not be meant for external consumers, so we can't put it there (at least not permanently).
What options do we have?
- It could be part of https://github.com/docker/go-plugins-helpers, which has a
sdk package
- ☝️ ⚠️ however, that repository should be moved to the moby org, as it's related to the engine
- ☝️ the repository may also need some cleaning up to distinguish between SDK and "sample implementations" (they look boilerplate, and are documented "to be used in your implementation" so perhaps they're all considered SDK?
- ❓ is there anything in
daemon/logge that's related and should ALSO be moved to go-plugin-helpers ?
- ❓ other options?
Description
API:
types/plugins/logdrivermust be removed from API moduleThe api module currently has a dependency on
github.com/gogo/protobuf;moby/api/go.mod
Line 9 in 596e088
The
github.com/gogo/protobufmodule is deprecated, and we don't want it to be a dependency of the API; it looks like the dependency comes from thetypes/plugins/logdriverpackage;moby/api/types/plugins/logdriver/entry.pb.go
Line 8 in 596e088
That package is not used as part of the API and not used by the CLI; it does not belong in the API module
Looking at history, this was actually brought up during review of the original PR; it was put inside
apito indicate "this must be a stable interface", and at the time "go modules" were not yet in sight, so putting it in the API package did not have a big effect #28403 (comment)We must move this package elsewhere, but TBD where it should be put;
Inside our own code, it's used in
daaemon/logger, which uses it to handle log-driver plugins;However those are uses as "consumer" of the specification, and the
daemon/module will not be meant for external consumers, so we can't put it there (at least not permanently).What options do we have?
sdkpackagedaemon/loggethat's related and should ALSO be moved togo-plugin-helpers?