failover revisit #1253
Merged
failover revisit #1253
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fredi-raspall
requested review from
daniel-noland
and removed request for
a team
Contributor
Author
|
The CI tests fail in this pipeline because I made it mandatory to know the BGP communities to advertise a route. Before, I made dataplane more forgiving and let it advertise anyway routes, while the fabric part was in the making. However, for a correct behavior of the failover strategy, no gateway should advertise routes without them IMO. No community seems to be set according to the table dumped by dataplane. And, then the config is rejected: This is affected by githedgehog/gateway#305 |
Fredi-raspall
force-pushed
the
pr/fredi/cleanup-and-validation
branch
from
5c4ea1f to
8196637
Compare
Fredi-raspall
force-pushed
the
pr/fredi/failover-revisit
branch
from
45d5c74 to
ebf1a2e
Compare
Member
|
We do now, you'll have to wait for the gateway bump in the fabricator |
Forbid peerings that are not mapped to a gateway group. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
When analyzing the priorities of a gateway for a VPC peering, if a gateway is part of the group responsible for that peering, fail if no community is available for its position in the ranked list of gateways to service the peering. This can only happen if the number of members in a group exceeds the numer of communities available. Also, simplify the logic, avoid unnecessary conversions between integer types and improve the tests. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Let the internal config have a community table and gateway group, and copy them from the external config. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Set the BGP communities of prefixes advertised in the context of a VPC peering from the internal config gateway group table and BGP community table, instead of the Peering object. Also, do not advertise any prefix if we can't determine the corresponding community. This change provides a better separation between what constitutes input configuration (external config) and its processing (internal config). Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
We no longer store communities in the Peering object, as we compute them from the community and gateway tables stored in the internal configuration. The validation will still be made on the External config, which will be significantly simplified by not storing the communities in the Peering. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
A VPC peering has to be mapped to a gateway group to be valid. In the initial implementation, internally, a peeering kept an optional reference to a group. The validation now requires a peering to have it. So, fix the tests that failed because of this stricter validatio by adding a constructor that sets the reference to a "default" group. We do not add this in the implementation of Peering::default(), as we do not want to default a value that is not provided by the configuration in this case, and we may make it non-optional in the future, once the tests are cleaned up. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Within a group, we prefer (order as first) gateways with highest priority value. In case of ties, we prefer gateways in alphabe- tical order. We do this by sorting the members of each group and defining Ord for members. The impl Ord compares also the ip address of members if their name is Equal. This will never happen within a gateway group as we require all members to have a distinct name. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Fredi-raspall
force-pushed
the
pr/fredi/failover-revisit
branch
from
bdce861 to
56793a4
Compare
Fredi-raspall
requested review from
qmonnet
and removed request for
daniel-noland
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This goes on top of #1245Summary: