-
Notifications
You must be signed in to change notification settings - Fork 2k
display port name field in 'service ls' #3724
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
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks, looks good to me!
The linter check is failing though, please give gofmt
a run :)
Looks like a good improvement over a previous behaviour. Not sure if this is something that shouldn't change due to backwards compatibility though. But since this is CLI and not Engine API - I think it's fine.
@ndeloof @thaJeztah WDYT?
I'm not sure if the information shown is accurate; the The
|
Signed-off-by: Arvid E. Picciani <arvid@kraud.cloud>
Codecov Report
@@ Coverage Diff @@
## master #3724 +/- ##
==========================================
- Coverage 59.12% 59.11% -0.01%
==========================================
Files 289 289
Lines 24669 24678 +9
==========================================
+ Hits 14585 14589 +4
- Misses 9212 9216 +4
- Partials 872 873 +1 |
yes. this change would let the cluster indicate that it's not all IP addresses, but a specific one (or name)
not sure i understand what you mean by that. The existing behavior will not change. At least i haven't seen Name actually be used. Of course if you would prefer the alternative option of adding a new field, i could do that. how does |
- What I did
in
docker service ls
we durrently format the port as "*:8080->9000" with the asterisk being hardcoded,it would be useful if the api server could return a dns name on the port, for api servers that can expose ports directly as ingress.
what we're currently doing instead is abusing the protocol field, but that breaks some assumptions of other api consumers,
and is on the wrong side of the arrow
- How I did it
the least invasive way to implement that appears to be to use the existing Name field.
We could add an additional field, if that's easy to upstream.
That would have the benefit of not changing anything unless the new field is set, but the downside is that it touches the api
- How to verify it
- Description for the changelog
display port name field in 'service ls'
- A picture of a cute animal (not mandatory but encouraged)