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

Add logdrivers to /info #32540

Merged
merged 1 commit into from Apr 26, 2017

Conversation

@cpuguy83
Contributor

cpuguy83 commented Apr 11, 2017

This is required for swarmkit to be able to filter based on log driver.

download-1

@cpuguy83 cpuguy83 added this to the 17.05.0 milestone Apr 11, 2017

@cpuguy83 cpuguy83 requested a review from anusha-ragunathan Apr 11, 2017

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Apr 11, 2017

Contributor
daemon/logger/factory.go:38:1: exported function ListDrivers should have comment or be unexported
Contributor

aaronlehmann commented Apr 11, 2017

daemon/logger/factory.go:38:1: exported function ListDrivers should have comment or be unexported
Add logdrivers to /info
This is required for swarmkit to be able to filter based on log driver.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@anusha-ragunathan

This comment has been minimized.

Show comment
Hide comment
@anusha-ragunathan

anusha-ragunathan Apr 11, 2017

Contributor

Adding pluginv2 drivers to docker info is not necessary. Swarmkit should query docker plugin ls and look for type LogDriver for scheduling decisions. This is how its done for volumes and networks. https://github.com/docker/swarmkit/blob/4dbc0a367de19b9f0dfb64302cd9fd28b5909b66/agent/exec/dockerapi/executor.go#L54

Contributor

anusha-ragunathan commented Apr 11, 2017

Adding pluginv2 drivers to docker info is not necessary. Swarmkit should query docker plugin ls and look for type LogDriver for scheduling decisions. This is how its done for volumes and networks. https://github.com/docker/swarmkit/blob/4dbc0a367de19b9f0dfb64302cd9fd28b5909b66/agent/exec/dockerapi/executor.go#L54

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Apr 12, 2017

Contributor

@anusha-ragunathan V2 drivers are not registered in the logger package, only with the plugingetter. So /info only ends up returning built-ins and v1.

EDIT:
Actually we only do v2 plugins for logs, so only built-ins end up on /info.

Contributor

cpuguy83 commented Apr 12, 2017

@anusha-ragunathan V2 drivers are not registered in the logger package, only with the plugingetter. So /info only ends up returning built-ins and v1.

EDIT:
Actually we only do v2 plugins for logs, so only built-ins end up on /info.

@anusha-ragunathan

This comment has been minimized.

Show comment
Hide comment
@anusha-ragunathan

anusha-ragunathan Apr 17, 2017

Contributor

LGTM

Contributor

anusha-ragunathan commented Apr 17, 2017

LGTM

@dnephin dnephin modified the milestones: 17.05.0, 17.06 Apr 25, 2017

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Apr 25, 2017

Member

I moved this to 17.06. I assume it's not making 17.05, right ?

Member

dnephin commented Apr 25, 2017

I moved this to 17.06. I assume it's not making 17.05, right ?

@dnephin

LGTM

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Apr 25, 2017

Contributor

Yep

Contributor

cpuguy83 commented Apr 25, 2017

Yep

@mlaventure mlaventure merged commit e8abe0a into moby:master Apr 26, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 32863 has succeeded
Details
janky Jenkins build Docker-PRs 41472 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 1667 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 12614 has succeeded
Details
z Jenkins build Docker-PRs-s390x 1504 has succeeded
Details
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Apr 30, 2017

Member

@cpuguy83 looks like this needs an update to the swagger file?

Member

thaJeztah commented Apr 30, 2017

@cpuguy83 looks like this needs an update to the swagger file?

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Apr 30, 2017

Member

and API changelog

Member

thaJeztah commented Apr 30, 2017

and API changelog

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure May 1, 2017

Contributor

Oups. Should probably teach Gordon how to detect those 😅 👼

Contributor

mlaventure commented May 1, 2017

Oups. Should probably teach Gordon how to detect those 😅 👼

@cpuguy83 cpuguy83 deleted the cpuguy83:add_logdrivers_to_info branch May 1, 2017

@cpuguy83 cpuguy83 restored the cpuguy83:add_logdrivers_to_info branch May 1, 2017

@cpuguy83 cpuguy83 deleted the cpuguy83:add_logdrivers_to_info branch Sep 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment