Store published ports as a sorted Vec instead of a BTreeSet#139
Conversation
ec72178 to
a11c577
Compare
75e9c3f to
ceeb547
Compare
| // `PortMap` is a hash map, so iteration order is unstable. Sort by | ||
| // container port then protocol to match the canonical order the target | ||
| // state uses, so a reordering never triggers reconfiguration. | ||
| ports.sort_by_key(|p| (p.target, p.protocol)); |
There was a problem hiding this comment.
Would it be safer to include the other port fields in the sort to ensure stable ordering?
There was a problem hiding this comment.
This is already stable no, a composition defining the same port/protocol twice should perhaps be invalid?
Disregard that, I think you are right
| /// for a container create request. Multiple mappings of the same container | ||
| /// port/protocol are grouped into a single binding list. | ||
| pub(crate) fn to_oci_port_maps(ports: BTreeSet<PortMapping>) -> (Vec<String>, PortMap) { | ||
| pub(crate) fn to_oci_port_maps(ports: Vec<PortMapping>) -> (Vec<String>, PortMap) { |
There was a problem hiding this comment.
By changing the data structure to a Vec, isn't dedupe capability lost? So a repeated port entry in a call to helios (doable from local endpoint only) would result in a target state apply loop no?
There was a problem hiding this comment.
The previous dedupe was not correct either, a composition defining
- 8080:80
- target: 80
published: 8080
host_ip: 127.0.0.1
would still be allowed even if ambiguous. I'm unsure if to reject this case or go with the last definition in the given order
There was a problem hiding this comment.
Docker compose doesn't validate the scenario above, it just terminates with failed to bind host port 127.0.0.1:8080/tcp: address already in use
There was a problem hiding this comment.
I've added a dedup() step on target state processing to remove exact duplicate configurations but keep the docker behavior wrt ambiguous definitions.
This simplifies the internal storage of ports to avoid the need for additional data structures. Ports are now sorted during target state validation and stored internally in the state as a Vec. They are also sorted when reading them from the engine. This ensures round-trip consistency, avoiding container recreation due to a port re-ordering on input. Change-type: patch
ceeb547 to
fece8e9
Compare
2 participants
This simplifies the internal storage of ports to avoid the need for additional data structures. Ports are now sorted during target state validation and stored internally in the state as a Vec. They are also sorted when reading them from the engine. This ensures round-trip consistency, avoiding container recreation due to a port re-ordering on input.
Change-type: patch