-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Improve presentation of published port ranges #581
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
Conversation
Port mappings in `docker service ls` are quite verbose, and occupy a lot of
space when ranges of ports are published.
This patch improves the output by reconstructing ranges of ports.
Given the following service;
$ docker service create \
-p 60-61:60-61 \
-p 62:61 \
-p 80:80 \
-p 81:80 \
-p 90-95:90-95 \
-p 90-92:90-92/udp \
-p 93-96:93-96/udp \
--name foo \
nginx:alpine
Before this patch is applied:
$ docker service ls
ID NAME MODE REPLICAS IMAGE PORTS
u1kwguv841qg foo replicated 1/1 nginx:alpine *:60->60/tcp,*:61->61/tcp,*:62->61/tcp,*:80->80/tcp,*:81->80/tcp,*:90->90/tcp,*:91->91/tcp,*:92->92/tcp,*:93->93/tcp,*:94->94/tcp,*:95->95/tcp,*:90->90/udp,*:91->91/udp,*:92->92/udp,*:93->93/udp,*:94->94/udp,*:95->95/udp,*:96->96/udp
After this patch is applied:
$ docker service ls
ID NAME MODE REPLICAS IMAGE PORTS
u1kwguv841qg foo replicated 1/1 nginx:alpine *:60-62->60-61/tcp,*:80-81->80/tcp,*:90-95->90-95/tcp,*:90-96->90-96/udp
Additional enhancements can still be made, and marked as TODO in this change;
- combine non-consecutive ports mapped to a single port (`80->80`, `81->80`,
`84->80`, `86->80`, `87->80`); to be printed as `*:80-81,84,86-87->80`.
- combine `tcp` and `udp` mappings if their port-mapping is the same;
print `*:80-81->80-81/tcp+udp` instead of `*:80-81->80-81/tcp, *:80-81->80-81/udp`
Signed-off-by: Sebastiaan van Stijn <[email protected]>
|
ping @dnephin @vdemeester @jefflill PTAL While working on this, I noticed that various commands include the formatted values when using |
Codecov Report
@@ Coverage Diff @@
## master #581 +/- ##
==========================================
+ Coverage 49.37% 49.52% +0.14%
==========================================
Files 208 208
Lines 17178 17225 +47
==========================================
+ Hits 8482 8530 +48
+ Misses 8263 8262 -1
Partials 433 433 |
|
ping @dnephin @vdemeester 🤗 |
dnephin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
vdemeester
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
Improve presentation of published port ranges Upstream-commit: fab4762 Component: cli
fixes #178
Port mappings in
docker service lsare quite verbose, and occupy a lot ofspace when ranges of ports are published.
This patch improves the output by reconstructing ranges of ports.
Given the following service;
Before this patch is applied:
After this patch is applied:
Additional enhancements can still be made, and marked as TODO in this change;
80->80,81->80,84->80,86->80,87->80); to be printed as*:80-81,84,86-87->80.tcpandudpmappings if their port-mapping is the same;print
*:80-81->80-81/tcp+udpinstead of*:80-81->80-81/tcp, *:80-81->80-81/udp- Description for the changelog