Prevent users from deploying fly tomls with duplicate service definitions #3522
Conversation
|
@gwuah do you know if this validation also checks if a http_service and a regular service use the same port? I feel like this could be a common source of misconception. |
|
@lubien yes it does. The thing is, http services are actually tcp services. |
gwuah
force-pushed
the
fix-duplicate-services
branch
from
67cc37a
to
3a0b951
Compare
gwuah
force-pushed
the
fix-duplicate-services
branch
from
3a0b951
to
f0539a1
Compare
| processes = ["web"] | ||
|
|
||
| [[services.ports]] | ||
| port = 80 |
There was a problem hiding this comment.
this is the kind of invalid config example for duplicated external ports I was referring to.
In this case, the app service and the web service both will listen on port 80. To which group should fly-proxy send the request?
| [[services]] | ||
| internal_port = 1738 | ||
| protocol = "tcp" | ||
| processes = ["web"] |
There was a problem hiding this comment.
I would add a more complete case, for example a service that belongs to two process groups, and then another service that only belong to one of them. Both services listen on tcp-1738, and the validation must fail only for the process group that is common to both services.
| func (cfg *Config) getServiceDeduplicationKey(s Service) string { | ||
| processes := s.Processes | ||
| if len(processes) == 0 { | ||
| processes = []string{fly.MachineProcessGroupApp} |
There was a problem hiding this comment.
in the context of appConfig, the default process group is cfg.defaultGroupName, this is important to avoid bugs when the configuration is flattened.
Change Summary
Users sometimes end up defining multiple services for the same protocol & port.
This harmless mistake would cause health-checks to fail because of only one of these services get picked by flaps.
To fix this, I'm introducing some client-side validation to flag these issues.
They would now see this warning/help text 馃憞
References
https://community.fly.io/t/health-checks-failing-with-failed-to-get-vm/18077
https://community.fly.io/t/unable-to-perform-health-checks/19604
https://community.fly.io/t/health-checks-always-failing/19641/2