Don't PublishAllPorts as we already explicitly do :8008/8448 #772
Conversation
This may break use cases which want extra ports to be exposed. This may fix issues where Complement sporadically decides that there are port conflicts, causing CI flakes.
| @@ -380,8 +380,7 @@ func deployImage( | |||
| "complement_hs_name": hsName, | |||
| }, | |||
| }, &container.HostConfig{ | |||
| CapAdd: []string{"NET_ADMIN"}, // TODO : this should be some sort of option | |||
| PublishAllPorts: true, | |||
There was a problem hiding this comment.
This may fix issues where Complement sporadically decides that there are port conflicts, causing CI flakes.
Where are you running into the CI flakes?
I'm curious because previously you said you didn't see them in vanilla Complement runs, #761
There was a problem hiding this comment.
Indeed, I don't. I'm referring to reports from others who say they have when using Complement on other projects.
| @@ -380,8 +380,7 @@ func deployImage( | |||
| "complement_hs_name": hsName, | |||
| }, | |||
| }, &container.HostConfig{ | |||
| CapAdd: []string{"NET_ADMIN"}, // TODO : this should be some sort of option | |||
| PublishAllPorts: true, | |||
There was a problem hiding this comment.
I think this will cause issues for some internal images we use at Element like testing the Secure Border Gateway where we export ports for a few more services and interact with them in the Complement tests.
Internal Element link: https://github.com/element-hq/sbg/blob/3871b51144c4eacf1ea0355da2ac625a0bfadce5/complement/Dockerfile#L83-L88
Any advice for how to work around this if this merges?
There was a problem hiding this comment.
Hmm I wonder if we keep PublishAllPorts in the default case and then only if cfg.HSPortBindingIP is not the default do we switch to PortBindings? That means it will publish all exposed ports by default, unless you bind to a specific IP (which afaik is only used by homerunner).
There was a problem hiding this comment.
🤷 Kinda seems fragile logic to gate the behavior change behind and would be hard to reason about for someone suddenly triggering this behavior.
This may break use cases which want extra ports to be exposed. This may fix issues where Complement sporadically decides that there are port conflicts, causing CI flakes.