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 `PORTS` field for `docker service ls` (`ingress`) #30813

Merged
merged 1 commit into from Apr 4, 2017

Conversation

@yongtang
Member

yongtang commented Feb 8, 2017

This fix is related to #30232 (comment) where docker service ls does not show PORTS information like docker service ps.

This fix adds PORTS fields for services that publish ports in ingress mode.

Additional unit tests cases have been updated.

This fix is related to #30232.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Feb 8, 2017

Member

The ports published in host mode is not shown in this PR. I am thinking it might make sense to have it in PORTS column as well. Though not sure what is the best way to represent host mode (and distinguish itself from ingress mode).

The ingress port is in *:8002->80/tcp format.

Member

yongtang commented Feb 8, 2017

The ports published in host mode is not shown in this PR. I am thinking it might make sense to have it in PORTS column as well. Though not sure what is the best way to represent host mode (and distinguish itself from ingress mode).

The ingress port is in *:8002->80/tcp format.

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Feb 8, 2017

Member

Also, not sure if the PORT columns should be based on service.Spec.EndpointSpec.Ports, or service.Endpoint.Ports.

Member

yongtang commented Feb 8, 2017

Also, not sure if the PORT columns should be based on service.Spec.EndpointSpec.Ports, or service.Endpoint.Ports.

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester
Member

vdemeester commented Mar 27, 2017

@vdemeester

LGTM 🐮

Add `PORTS` field for `docker service ls` (`ingress`)
This fix is related to 30232 wherw `docker service ls`
does not show `PORTS` information like `docker service ps`.

This fix adds `PORTS` fields for services that publish
ports in ingress mode.

Additional unit tests cases have been updated.

This fix is related to 30232.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Apr 4, 2017

Member

@vdemeester @thaJeztah Thanks for the review. The doc has been updated.

Member

yongtang commented Apr 4, 2017

@vdemeester @thaJeztah Thanks for the review. The doc has been updated.

@vdemeester

LGTM 🦁

@thaJeztah

LGTM, thanks!

@thaJeztah thaJeztah merged commit fe4d7db into moby:master Apr 4, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 32509 has succeeded
Details
janky Jenkins build Docker-PRs 41099 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 1271 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 12223 has succeeded
Details
z Jenkins build Docker-PRs-s390x 1105 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 17.05.0 milestone Apr 4, 2017

@yongtang yongtang deleted the yongtang:30232-service-ls-ports branch Apr 4, 2017

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#30813 from yongtang/30232-service-ls-ports
Add `PORTS` field for `docker service ls` (`ingress`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment