design-proposal: make VPC peering work end to end - #52
Draft
mattia-eleuteri wants to merge 6 commits into
Draft
Conversation
VirtualPrivateCloud.spec.peers shipped in v1.2.0 but declaring a peer produces no connectivity and leaves both Vpc objects in a kube-ovn CIDRInvalid reconcile loop. Peering is three layers (OVN routing, subnet ACLs, guest routes); the chart gets the first wrong by a missing netmask, does not address the second, and the third is unaddressed in Cozystack. Documents measurements from the first production rollout and proposes: a /30 on localConnectIP, allocated instead of hashed interconnect blocks, allowSubnets derived from the declaration (including sibling subnets of the same VPC), guest routes via the provider-scoped routes annotation kube-ovn already uses for its own VPC components, and per-peer status conditions so one-sided declarations stop reporting healthy. Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…eferring them Measured with ovn-trace on a live cluster, which changes the conclusion: - Egress is dropped in the source subnet's egress pipeline on the way to its own router patch port, so a staticRoutes entry for the destination is never evaluated. The earlier text blamed the return path. - allowSubnets renders pair rules anchored on the subnet's own CIDR, so a transit VM whose traffic has both endpoints foreign to its subnet can never be matched by any rule. Declaring the destination on both subnets is not enough. - With private: false on the subnet hosting the firewall, the trace is delivered to its port. The design is achievable; what is missing is a way to express transit without disabling isolation wholesale. Also identifies the second gap: the virtual-router UX is implemented by Cozystack's kubeovn-webhook, which only propagates the unprefixed ovn.kubernetes.io/routes and port_security, so it applies to the default pod network only. kube-ovn already defines the provider-scoped forms. Adds ovn-trace as the recommended regression gate for the dataplane. Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…nsit exception Disabling isolation on the subnet hosting a gateway VM reads like it exposes a public-facing VM, and that is the wrong axis. Verified from a node: VPC subnets are unreachable from the node and pod networks by routing, independently of ACLs, and a gateway VM's public exposure lives on its pod NIC behind a Service, not on the VPC subnet's ACL. What actually changes is the trust boundary: filtering moves into the gateway's own configuration, and port_security: false grants it source spoofing inside the VPC. The blast radius stays bounded by the peers' own privacy, since their pair rules anchor on their own CIDR. Hence the deployment shape (gateway alone in a dedicated transit subnet) matters more than the flag, and hence the ask for a transit allowance rather than settling for private: false. Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…ss mechanism exists Three exist for custom VPCs: VpcNatGateway, the OVN gateway with EIP/FIP/SNAT, and the newer VpcEgressGateway with ECMP and BFD, whose CRD is served in the version Cozystack ships. The earlier text asserted a gap that is not there. The real gap is narrower: none of them can put a tenant-owned appliance in the path. VpcEgressGateway is always kube-ovn's own pod pair with a macvlan leg, snat: true is mandatory, and DNAT/EIP are unsupported. So a tenant cannot interpose its own firewall to police egress. Also records what I could not determine rather than asserting it: whether an egress gateway's traffic passes a private subnet's ACL, and by which rule. That answer decides whether the transit allowance already half exists, so it is now an open question instead of a premise. Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…cardinality claim The docs line "only interconnection of two VPCs is supported" reads as a cardinality limit. It is not one: pkg/controller/vpc.go iterates spec.vpcPeerings and calls CreatePeerRouterPort once per entry, with no cap, reconciling removals against status.VpcPeerings. A VPC can hold many pairwise peerings, the same model as Azure VNet peering. What the sentence means is that a peering joins two VPCs and is non-transitive. Removes the recommendation to cap the peers array and the PeerLimitExceeded condition, which would have made the API strictly less useful, and replaces them with the two constraints that do matter at N peers: subnet CIDRs must be mutually non-overlapping across the whole peer set, not just against the local VPC, and allowSubnets derivation must open only to directly peered VPCs so the ACLs do not become a transitive mesh. Adds an upstream docs clarification to the open questions. Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
…ng dual-homing as the direction Adds an appendix with the hub-and-spoke-with-a-firewall architecture a tenant asked us to reproduce, mapped brick by brick onto Cozystack with the state of each. It establishes that the gaps in this proposal are not theoretical: every missing brick in that table is inside the VPC. Two things it settles. The internet edge is not missing: a dual-homed firewall VM already gets egress and a stable public identity through its pod NIC and a LoadBalancer Service, which is why this proposal asks for a transit allowance and not for a gateway object. But that same edge is what forces dual-homing, and dual-homing is the root cause of the guest-route problem in section 5, so moving the edge inside the VPC (external segment plus OvnEip/OvnSnatRule/OvnDnatRule, no per-VPC runtime) is recorded as the strategic direction that dissolves section 5 instead of working around it. Its prerequisite is stated too, because it is smaller than it looks and should not be mistaken for a datacenter project. Open question 5 becomes a choice of form rather than a question of whether. Also anonymises VM and namespace names in the ovn-trace excerpts. Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
VirtualPrivateCloud.spec.peersshipped in v1.2.0 (#2152) and the API reads like a finished feature. Taking it to production for the first time showed that declaring a peer produces no working connectivity at all, and leaves bothVpcobjects in a permanent kube-ovn reconcile-error loop:Peering turns out to be three independent layers that must all be correct: OVN routing, subnet ACLs, and routes inside the guest. The chart gets the first wrong by a missing netmask, does not address the second at all, and the third is unaddressed anywhere in Cozystack. Each layer fails silently and in a way that points the operator at the wrong layer.
The proposal is written from measurements on a production cluster (kube-ovn v1.15.10, KubeVirt v1.8.2), with commands and outputs quoted inline. What it proposes:
/30onlocalConnectIP— a two-character fix, and the whole reason the feature has never worked. Ships on its own, ahead of the rest./30s with no conflict check, so collision probability reaches 26 % at 100 peerings in a cluster.allowSubnetsderived from the declaration — today every subnet isprivate: truewith no allow list, so peered traffic is routed correctly and then dropped by the destination switch. This also fixes an arguably worse bug that needs no new API: two subnets of the same VPC cannot reach each other.<provider>.kubernetes.io/routes, cf.pkg/controller/vpc_dns.go,pkg/controller/vpc_nat_gateway.go). Validated end to end on a Windows guest. Gated on one KubeVirt question, documented in §5.1.Ready=Trueand does nothing.It also documents a dead end (
Subnet.enableDHCP/dhcpV4Optionsare irrelevant for KubeVirt VMs, because virt-launcher is the DHCP server, not OVN) so that nobody spends time re-testing it, and states precisely why firewall-mediated internet egress is structurally blocked today.Complementary to
tenant-site-connectivity(external sites via gateway VMs; this one is intra-cluster VPC-to-VPC) and related to #35 for the deferred egress item.Opened as a draft: I would like a read on the two design calls in Open questions before it goes to review, in particular whether a VPC should be internally open by default, and who should own interconnect allocation.