diff --git a/cmd/hhfab/main.go b/cmd/hhfab/main.go index 22b0a3939..25589fa0d 100644 --- a/cmd/hhfab/main.go +++ b/cmd/hhfab/main.go @@ -1431,7 +1431,7 @@ Examples: return err } - if err := hhfab.DoVLABSetupVPCs(ctx, workDir, cacheDir, hhfab.SetupVPCsOpts{ + if _, err := hhfab.DoVLABSetupVPCs(ctx, workDir, cacheDir, hhfab.SetupVPCsOpts{ WaitSwitchesReady: c.Bool("wait-switches-ready"), ForceCleanup: c.Bool("force-cleanup"), VLANNamespace: c.String("vlanns"), diff --git a/pkg/hhfab/cmdvlab.go b/pkg/hhfab/cmdvlab.go index 3f3c261ad..65da31f20 100644 --- a/pkg/hhfab/cmdvlab.go +++ b/pkg/hhfab/cmdvlab.go @@ -183,10 +183,10 @@ func DoShowTech(ctx context.Context, workDir, cacheDir string) error { return c.VLABShowTech(ctx, vlab, ShowTechOpts{}) } -func DoVLABSetupVPCs(ctx context.Context, workDir, cacheDir string, opts SetupVPCsOpts) error { +func DoVLABSetupVPCs(ctx context.Context, workDir, cacheDir string, opts SetupVPCsOpts) ([]*Endpoint, error) { c, vlab, err := loadVLABForHelpers(ctx, workDir, cacheDir) if err != nil { - return err + return nil, err } return c.SetupVPCs(ctx, vlab, opts) diff --git a/pkg/hhfab/endpoints.go b/pkg/hhfab/endpoints.go new file mode 100644 index 000000000..07b2d1f58 --- /dev/null +++ b/pkg/hhfab/endpoints.go @@ -0,0 +1,376 @@ +// Copyright 2026 Hedgehog +// SPDX-License-Identifier: Apache-2.0 + +package hhfab + +import ( + "context" + "fmt" + "log/slog" + "net/netip" + "slices" + "strings" + "sync" + + vpcapi "go.githedgehog.com/fabric/api/vpc/v1beta1" + wiringapi "go.githedgehog.com/fabric/api/wiring/v1beta1" + "go.githedgehog.com/fabricator/pkg/util/sshutil" + "golang.org/x/sync/errgroup" + kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +// SSHResolver returns the SSH config for a server. The collector and per- +// server refresh helpers take one of these instead of a pre-built +// map[string]*sshutil.Config so callers can share whatever SSH plumbing +// they already have (SetupVPCs builds a map keyed by VM name; rt_* tests +// resolve lazily through testCtx.getSSH). +type SSHResolver func(server string) (*sshutil.Config, error) + +// SSHResolverFromMap adapts a pre-built map into an SSHResolver. +func SSHResolverFromMap(m map[string]*sshutil.Config) SSHResolver { + return func(server string) (*sshutil.Config, error) { + cfg, ok := m[server] + if !ok { + return nil, fmt.Errorf("no ssh config for server %q", server) //nolint:goerr113 + } + + return cfg, nil + } +} + +// discoveredIP pairs a server-side interface name with the address found +// on it. The interface tells us whether the address is a hostBGP /32 VIP +// (on `lo`) or a regular subnet address (on the bond/VLAN interface). +type discoveredIP struct { + iface string + prefix netip.Prefix +} + +// discoverServerIPs returns every eligible IPv4 address configured on the +// server, paired with the interface it lives on. The management interface +// (enp2s0), the docker bridge (docker0), and the loopback 127.0.0.1/8 entry +// are skipped; other lo addresses (hostBGP /32 VIPs) are kept. +func discoverServerIPs(ctx context.Context, sshCfg *sshutil.Config, server string) ([]discoveredIP, error) { + stdout, stderr, err := sshCfg.Run(ctx, "ip -o -4 addr show | awk '{print $2, $4}'") + if err != nil { + return nil, fmt.Errorf("running ip addr show on %s: %w: %s", server, err, stderr) + } + + lines := strings.Split(strings.TrimSpace(stdout), "\n") + out := make([]discoveredIP, 0, len(lines)) + for _, line := range lines { + fields := strings.Fields(line) + if len(fields) != 2 { + continue + } + if (fields[0] == "lo" && fields[1] == "127.0.0.1/8") || fields[0] == "enp2s0" || fields[0] == "docker0" { + continue + } + prefix, err := netip.ParsePrefix(fields[1]) + if err != nil { + return nil, fmt.Errorf("parsing %q on %s: %w", fields[1], server, err) + } + out = append(out, discoveredIP{iface: fields[0], prefix: prefix}) + } + + return out, nil +} + +// serverAttachment is the (vpc, subnet) information the collector resolves +// from a VPCAttachment + its referenced VPC CRD. +type serverAttachment struct { + vpcName string + subnetName string + subnetCIDR netip.Prefix + hostBGP bool + attachName string // for diagnostics +} + +// CollectServerEndpoints observes the live cluster and produces one +// *Endpoint per (server, vpc, subnet) attachment for each server in the +// `servers` filter (nil → all servers attached to at least one VPC). +// +// Algorithm: list VPCAttachments → group by server (resolved via each +// attachment's Connection); for each candidate server, SSH it and read all +// IPv4 addresses; match each address to one of its attachments by CIDR +// containment (narrowest prefix wins on ties). HostBGP is derived from +// VPCSubnet.HostBGP. An attachment with no matching configured address is +// dropped with a warning; a server with no configured addresses contributes +// nothing — this matches today's SetupVPCs behavior, where ESLAG servers +// in L3VNI mode are skipped before hhnet runs and therefore have no IPs. +// +// Returns an error only when SSH itself fails on a queried server or when a +// VPCAttachment cannot be resolved to a Connection. +func CollectServerEndpoints(ctx context.Context, kube kclient.Client, ssh SSHResolver, servers []string) ([]*Endpoint, error) { + attaches := &vpcapi.VPCAttachmentList{} + if err := kube.List(ctx, attaches); err != nil { + return nil, fmt.Errorf("listing VPCAttachments: %w", err) + } + + connCache := map[string]*wiringapi.Connection{} + getConn := func(name string) (*wiringapi.Connection, error) { + if c, ok := connCache[name]; ok { + return c, nil + } + c := &wiringapi.Connection{} + if err := kube.Get(ctx, kclient.ObjectKey{Name: name, Namespace: kmetav1.NamespaceDefault}, c); err != nil { + return nil, fmt.Errorf("getting connection %q: %w", name, err) + } + connCache[name] = c + + return c, nil + } + + vpcCache := map[string]*vpcapi.VPC{} + getVPC := func(name string) (*vpcapi.VPC, error) { + if v, ok := vpcCache[name]; ok { + return v, nil + } + v := &vpcapi.VPC{} + if err := kube.Get(ctx, kclient.ObjectKey{Name: name, Namespace: kmetav1.NamespaceDefault}, v); err != nil { + return nil, fmt.Errorf("getting VPC %q: %w", name, err) + } + vpcCache[name] = v + + return v, nil + } + + want := map[string]bool{} + for _, s := range servers { + want[s] = true + } + + serverAttachments := map[string][]serverAttachment{} + for _, attach := range attaches.Items { + conn, err := getConn(attach.Spec.Connection) + if err != nil { + return nil, fmt.Errorf("resolving attachment %q: %w", attach.Name, err) + } + _, srvs, _, _, err := conn.Spec.Endpoints() + if err != nil { + return nil, fmt.Errorf("getting endpoints of connection %q: %w", conn.Name, err) + } + if len(srvs) != 1 { + // VPCAttachments only reference server-facing connections; if a + // connection has no server endpoint we skip it as a malformed + // attachment rather than erroring out, since fabric webhooks + // already gate this. + continue + } + serverName := srvs[0] + if len(want) > 0 && !want[serverName] { + continue + } + + vpc, err := getVPC(attach.Spec.VPCName()) + if err != nil { + return nil, fmt.Errorf("resolving attachment %q: %w", attach.Name, err) + } + subnetName := attach.Spec.SubnetName() + subnet, ok := vpc.Spec.Subnets[subnetName] + if !ok { + return nil, fmt.Errorf("attachment %q references missing subnet %s/%s", attach.Name, vpc.Name, subnetName) //nolint:goerr113 + } + cidr, err := netip.ParsePrefix(subnet.Subnet) + if err != nil { + return nil, fmt.Errorf("parsing VPC %s/%s subnet CIDR %q: %w", vpc.Name, subnetName, subnet.Subnet, err) + } + + // A multihomed hostBGP server has several VPCAttachments pointing at the + // same (vpc, subnet) — one per connection — but a single /32 VIP. Collapse + // them to one candidate attachment so the server yields exactly one + // endpoint instead of dropping the duplicates with misleading warnings. + if slices.ContainsFunc(serverAttachments[serverName], func(a serverAttachment) bool { + return a.vpcName == vpc.Name && a.subnetName == subnetName + }) { + continue + } + + serverAttachments[serverName] = append(serverAttachments[serverName], serverAttachment{ + vpcName: vpc.Name, + subnetName: subnetName, + subnetCIDR: cidr, + hostBGP: subnet.HostBGP, + attachName: attach.Name, + }) + } + + // Probe every candidate server in parallel; errgroup mirrors what + // SetupVPCs does for the hhnet config loop. + type collected struct { + serverName string + ips []discoveredIP + } + var ( + mu sync.Mutex + probed []collected + eg, ectx = errgroup.WithContext(ctx) + ) + names := make([]string, 0, len(serverAttachments)) + for name := range serverAttachments { + names = append(names, name) + } + slices.Sort(names) + for _, name := range names { + eg.Go(func() error { + cfg, err := ssh(name) + if err != nil { + return fmt.Errorf("getting ssh config for %s: %w", name, err) + } + ips, err := discoverServerIPs(ectx, cfg, name) + if err != nil { + return fmt.Errorf("discovering IPs on %s: %w", name, err) + } + mu.Lock() + probed = append(probed, collected{serverName: name, ips: ips}) + mu.Unlock() + + return nil + }) + } + if err := eg.Wait(); err != nil { + return nil, fmt.Errorf("probing servers for IPs: %w", err) + } + + // Build endpoints by matching each discovered IP against the server's + // candidate attachments (already narrowed by VPCAttachment list). When + // multiple attachments contain the IP, the narrowest-prefix attachment + // wins (handles the P2P /31 case where a /31 sits inside the parent + // /24). + slices.SortFunc(probed, func(a, b collected) int { return strings.Compare(a.serverName, b.serverName) }) + out := []*Endpoint{} + for _, p := range probed { + atts := serverAttachments[p.serverName] + used := make([]bool, len(atts)) + if len(p.ips) == 0 { + slog.Warn("Server has no configured IPs, skipping endpoints", "server", p.serverName, "attachments", len(atts)) + + continue + } + for _, ip := range p.ips { + bestIdx := -1 + bestBits := -1 + for i, att := range atts { + if !att.subnetCIDR.Contains(ip.prefix.Addr()) { + continue + } + if att.subnetCIDR.Bits() > bestBits { + bestBits = att.subnetCIDR.Bits() + bestIdx = i + } + } + if bestIdx < 0 { + slog.Warn("Server IP does not match any attachment subnet", "server", p.serverName, "iface", ip.iface, "addr", ip.prefix.String()) + + continue + } + if used[bestIdx] { + slog.Warn("Multiple server IPs match the same attachment, keeping the first one", + "server", p.serverName, "iface", ip.iface, "addr", ip.prefix.String(), + "vpc", atts[bestIdx].vpcName, "subnet", atts[bestIdx].subnetName) + + continue + } + + att := atts[bestIdx] + used[bestIdx] = true + out = append(out, &Endpoint{ + Server: &ServerEndpoint{ + Name: p.serverName, + VPC: att.vpcName, + Subnet: att.subnetName, + HostBGP: att.hostBGP, + IP: ip.prefix.Addr(), + }, + }) + } + for i, att := range atts { + if !used[i] { + slog.Warn("Attachment has no matching IP on server, dropping endpoint", + "server", p.serverName, "vpc", att.vpcName, "subnet", att.subnetName, "attachment", att.attachName) + } + } + } + + return out, nil +} + +// ReplaceServerEndpoints reconciles the matrix's endpoints for one +// server against newEPs: +// +// - Existing endpoints whose (vpc, subnet) match an entry in newEPs +// are updated in place — the IP and HostBGP fields are copied +// across, but the *Endpoint pointer stays the same. Matrix entries +// keyed on that pointer remain valid, so suite setups that don't +// wipe between tests keep their verdicts. +// - Existing endpoints with no (vpc, subnet) match in newEPs are +// dropped from AllEndpoints; entries referencing them are deleted +// (the topology those verdicts assumed no longer holds). +// - newEPs that don't match any existing endpoint are appended; the +// matrix has no entries for them until the next Repopulate or +// overlay. +// +// Use after a runtime topology change (server moved to a different VPC, +// or DHCP lease refreshed) to bring the matrix back in sync with the +// cluster while preserving entries for attachments that didn't change. +// +// Entries in newEPs whose Server.Name != name are appended as-is, so +// mixing External endpoints into the slice is a no-op for the matching +// pass. +func (m *ConnectivityMatrix) ReplaceServerEndpoints(name string, newEPs []*Endpoint) { + if m == nil { + return + } + + type vsKey struct{ vpc, subnet string } + byVS := map[vsKey]*Endpoint{} + for _, ep := range newEPs { + if ep == nil || ep.Server == nil || ep.Server.Name != name { + continue + } + byVS[vsKey{ep.Server.VPC, ep.Server.Subnet}] = ep + } + + matched := map[*Endpoint]bool{} + stale := map[*Endpoint]struct{}{} + kept := make([]*Endpoint, 0, len(m.AllEndpoints)+len(newEPs)) + for _, ep := range m.AllEndpoints { + if ep == nil || ep.Server == nil || ep.Server.Name != name { + kept = append(kept, ep) + + continue + } + key := vsKey{ep.Server.VPC, ep.Server.Subnet} + if newEp, ok := byVS[key]; ok { + ep.Server.IP = newEp.Server.IP + ep.Server.HostBGP = newEp.Server.HostBGP + matched[newEp] = true + kept = append(kept, ep) + + continue + } + stale[ep] = struct{}{} + } + for _, ep := range newEPs { + if ep == nil || matched[ep] { + continue + } + kept = append(kept, ep) + } + m.AllEndpoints = kept + + if len(stale) == 0 || len(m.entries) == 0 { + return + } + for pair := range m.entries { + if _, ok := stale[pair.Source]; ok { + delete(m.entries, pair) + + continue + } + if _, ok := stale[pair.Destination]; ok { + delete(m.entries, pair) + } + } +} diff --git a/pkg/hhfab/endpoints_test.go b/pkg/hhfab/endpoints_test.go new file mode 100644 index 000000000..a31e8acaa --- /dev/null +++ b/pkg/hhfab/endpoints_test.go @@ -0,0 +1,186 @@ +// Copyright 2026 Hedgehog +// SPDX-License-Identifier: Apache-2.0 + +package hhfab + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/require" +) + +// serverEP is a small helper for building a server-only Endpoint. +func serverEP(name, vpc, subnet, addr string) *Endpoint { + return &Endpoint{ + Server: &ServerEndpoint{ + Name: name, + VPC: vpc, + Subnet: subnet, + IP: netip.MustParseAddr(addr), + }, + } +} + +func TestReplaceServerEndpoints_InPlaceUpdate(t *testing.T) { + // Same (vpc, subnet) → mutate IP in place; pointer identity preserved + // so matrix entries referencing it stay valid (the DHCP-test case). + m := NewConnectivityMatrix() + ep := serverEP("server-5", "vpc-1", "default", "10.0.1.5") + other := serverEP("server-3", "vpc-1", "default", "10.0.1.3") + m.AllEndpoints = []*Endpoint{ep, other} + + allow := ConnectivityExpectation{ + Pair: EndpointPair{Source: other, Destination: ep}, + Verdict: VerdictAllow, + Reason: ReachabilityReasonIntraVPC, + } + m.Add(allow) + + newEP := serverEP("server-5", "vpc-1", "default", "10.0.1.99") + m.ReplaceServerEndpoints("server-5", []*Endpoint{newEP}) + + require.Len(t, m.AllEndpoints, 2) + require.Same(t, ep, m.AllEndpoints[0], "existing pointer should be preserved") + require.Equal(t, netip.MustParseAddr("10.0.1.99"), ep.Server.IP, "IP should be updated in place") + + got := m.Lookup(other, ep, ProtoPort{}) + require.Equal(t, VerdictAllow, got.Verdict, "entry keyed on the preserved pointer should still resolve") +} + +func TestReplaceServerEndpoints_VPCMove(t *testing.T) { + // (vpc, subnet) changed → old pointer dropped, new appended; entries + // involving the old pointer are wiped (the overlap-NAT case). + m := NewConnectivityMatrix() + donor := serverEP("server-1", "vpc-donor", "default", "10.0.1.1") + other := serverEP("server-2", "vpc-x", "default", "10.0.2.2") + m.AllEndpoints = []*Endpoint{donor, other} + m.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: donor, Destination: other}, + Verdict: VerdictAllow, + }) + + overlap := serverEP("server-1", "vpc-overlap", "overlap-sub", "10.99.0.1") + m.ReplaceServerEndpoints("server-1", []*Endpoint{overlap}) + + require.Len(t, m.AllEndpoints, 2) + require.NotContains(t, m.AllEndpoints, donor, "donor pointer should be dropped") + require.Contains(t, m.AllEndpoints, overlap, "overlap pointer should be appended") + + // Entry referenced the dropped pointer → should be gone. + got := m.Lookup(donor, other, ProtoPort{}) + require.Equal(t, VerdictDeny, got.Verdict, "default-deny after stale entry pruned") +} + +func TestReplaceServerEndpoints_PreservesOtherServerEntries(t *testing.T) { + m := NewConnectivityMatrix() + moved := serverEP("server-1", "vpc-donor", "default", "10.0.1.1") + a := serverEP("server-2", "vpc-x", "default", "10.0.2.2") + b := serverEP("server-3", "vpc-x", "default", "10.0.2.3") + m.AllEndpoints = []*Endpoint{moved, a, b} + + m.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: a, Destination: b}, + Verdict: VerdictAllow, + Reason: ReachabilityReasonIntraVPC, + }) + + overlap := serverEP("server-1", "vpc-overlap", "overlap-sub", "10.99.0.1") + m.ReplaceServerEndpoints("server-1", []*Endpoint{overlap}) + + got := m.Lookup(a, b, ProtoPort{}) + require.Equal(t, VerdictAllow, got.Verdict, "unrelated entries should be untouched") +} + +func TestReplaceServerEndpoints_MultiAttachment(t *testing.T) { + // Server with two attachments: vpc-a unchanged → mutate in place; + // vpc-b dropped from cluster → prune; vpc-c new → append. + m := NewConnectivityMatrix() + epA := serverEP("server-1", "vpc-a", "default", "10.0.1.1") + epB := serverEP("server-1", "vpc-b", "default", "10.0.2.1") + peer := serverEP("server-2", "vpc-a", "default", "10.0.1.2") + m.AllEndpoints = []*Endpoint{epA, epB, peer} + + m.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: epA, Destination: peer}, Verdict: VerdictAllow, + }) + m.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: epB, Destination: peer}, Verdict: VerdictAllow, + }) + + newA := serverEP("server-1", "vpc-a", "default", "10.0.1.99") + newC := serverEP("server-1", "vpc-c", "default", "10.0.3.1") + m.ReplaceServerEndpoints("server-1", []*Endpoint{newA, newC}) + + require.Same(t, epA, m.AllEndpoints[0], "vpc-a pointer kept") + require.Equal(t, netip.MustParseAddr("10.0.1.99"), epA.Server.IP) + require.NotContains(t, m.AllEndpoints, epB, "vpc-b dropped") + require.Contains(t, m.AllEndpoints, newC, "vpc-c appended") + + require.Equal(t, VerdictAllow, m.Lookup(epA, peer, ProtoPort{}).Verdict, "epA's entry preserved") + require.Equal(t, VerdictDeny, m.Lookup(epB, peer, ProtoPort{}).Verdict, "epB's entry pruned") +} + +func TestReplaceServerEndpoints_EmptyNewEPsRemovesAll(t *testing.T) { + m := NewConnectivityMatrix() + a := serverEP("server-1", "vpc-1", "default", "10.0.1.1") + b := serverEP("server-2", "vpc-1", "default", "10.0.1.2") + m.AllEndpoints = []*Endpoint{a, b} + m.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: a, Destination: b}, Verdict: VerdictAllow, + }) + + m.ReplaceServerEndpoints("server-1", nil) + + require.Len(t, m.AllEndpoints, 1) + require.Same(t, b, m.AllEndpoints[0]) + require.Equal(t, VerdictDeny, m.Lookup(a, b, ProtoPort{}).Verdict) +} + +func TestReplaceServerEndpoints_AppendOnEmptyMatrix(t *testing.T) { + m := NewConnectivityMatrix() + newEP := serverEP("server-1", "vpc-1", "default", "10.0.1.1") + m.ReplaceServerEndpoints("server-1", []*Endpoint{newEP}) + require.Equal(t, []*Endpoint{newEP}, m.AllEndpoints) +} + +func TestReplaceServerEndpoints_NilMatrix(t *testing.T) { + var m *ConnectivityMatrix + require.NotPanics(t, func() { + m.ReplaceServerEndpoints("server-1", []*Endpoint{serverEP("server-1", "vpc-1", "default", "10.0.1.1")}) + }) +} + +func TestReplaceServerEndpoints_IgnoresOtherServerNamesInNewEPs(t *testing.T) { + // Defensive: if the caller mixes endpoints for a different server, + // they should not match against existing entries for `name`. + m := NewConnectivityMatrix() + a := serverEP("server-1", "vpc-1", "default", "10.0.1.1") + m.AllEndpoints = []*Endpoint{a} + + stray := serverEP("server-2", "vpc-1", "default", "10.0.1.2") + m.ReplaceServerEndpoints("server-1", []*Endpoint{stray}) + + // server-1's only endpoint had no match → dropped. stray is appended + // (the function's documented "no-op for the matching pass" behavior). + require.NotContains(t, m.AllEndpoints, a) + require.Contains(t, m.AllEndpoints, stray) +} + +func TestReplaceServerEndpoints_PreservesHostBGP(t *testing.T) { + m := NewConnectivityMatrix() + ep := &Endpoint{Server: &ServerEndpoint{ + Name: "server-1", VPC: "vpc-1", Subnet: "default", + HostBGP: false, IP: netip.MustParseAddr("10.0.1.1"), + }} + m.AllEndpoints = []*Endpoint{ep} + + newEP := &Endpoint{Server: &ServerEndpoint{ + Name: "server-1", VPC: "vpc-1", Subnet: "default", + HostBGP: true, IP: netip.MustParseAddr("10.0.1.99"), + }} + m.ReplaceServerEndpoints("server-1", []*Endpoint{newEP}) + + require.Same(t, ep, m.AllEndpoints[0]) + require.True(t, ep.Server.HostBGP, "HostBGP should be copied across on in-place update") +} diff --git a/pkg/hhfab/matrix.go b/pkg/hhfab/matrix.go new file mode 100644 index 000000000..97df430ae --- /dev/null +++ b/pkg/hhfab/matrix.go @@ -0,0 +1,773 @@ +// Copyright 2026 Hedgehog +// SPDX-License-Identifier: Apache-2.0 + +package hhfab + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/netip" + "slices" + "sync" + "time" + + vpcapi "go.githedgehog.com/fabric/api/vpc/v1beta1" + "go.githedgehog.com/fabricator/pkg/fab" + "go.githedgehog.com/fabricator/pkg/util/sshutil" + "golang.org/x/sync/semaphore" + kclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +// The connectivity matrix models the expected traffic behavior between every +// pair of test endpoints in a topology. It is populated by generators +// (typically setup-vpcs / setup-peerings) and consumed by a runner that +// exercises each pair. +// +// Design assumptions: +// - A single matrix represents one steady-state topology. Dynamic changes +// (overlap-NAT, gateway failover, peering churn) are modeled as a +// sequence of distinct matrices, not as mutations to one. +// - Endpoints are canonical: generators allocate one *Endpoint per +// (server, vpc, subnet) attachment and one per External CRD; the matrix +// references those pointers from AllEndpoints and as EndpointPair keys. +// - A server with multiple IPs (attached to several subnets or VPCs) is +// represented as multiple endpoints, one per (vpc, subnet). The verdict +// depends on which address is used, so collapsing them is not safe. +// - Absence of an entry for a pair means default DENY (isolation). +// Generators may emit explicit Verdict=Deny entries when a Reason aids +// diagnostics. + +// gwNATPortForwardProbeTimeout is the maximum time to wait for the gateway's +// port-forward NAT rule to become active in the dataplane after a peering is +// applied. Unlike fabric route propagation (which waitForNATPoolInLeaves gates +// on), the gateway's DNAT rule programming has its own latency that no +// Kubernetes condition signals. +const gwNATPortForwardProbeTimeout = 2 * time.Minute + +// gwNATPortForwardProbeInterval is the polling interval between TCP-reachability probes. +const gwNATPortForwardProbeInterval = 5 * time.Second + +// ConnectivityVerdict describes what should happen to traffic on a path. +type ConnectivityVerdict string + +const ( + VerdictAllow ConnectivityVerdict = "allow" + VerdictDeny ConnectivityVerdict = "deny" +) + +// TranslatedAddress describes NAT translation expected on a path. Each field +// is optional; an unset field means "no translation on that axis". +type TranslatedAddress struct { + // SourcePool: CIDR from which the destination may observe any source IP + // (masquerade SNAT — the runtime pool selection is not predictable, only + // the containing range is). + SourcePool netip.Prefix + + // DestinationIP: IP the source must target to reach the destination + // (DNAT). Unset means use the destination's real IP. + DestinationIP netip.Addr + + // DestinationPort: port the destination actually listens on, when + // different from the source-facing port in + // ConnectivityExpectation.ProtoPort (port-forward DNAT). Zero means no + // port translation. + DestinationPort uint16 +} + +// ProtoPort is a protocol + port tuple. The zero value (empty protocol, +// port 0) is the sentinel for "applies to the default connectivity check +// the runner performs" (today: ICMP + TCP/any). +type ProtoPort struct { + Protocol string // "tcp", "udp", "icmp" + Port uint16 +} + +// ConnectivityExpectation describes what should happen on a directional path. +// To express bidirectional behavior, emit two entries with swapped pairs. +type ConnectivityExpectation struct { + Pair EndpointPair + + // Verdict: should traffic be allowed or denied on this path? + Verdict ConnectivityVerdict + + // NAT: optional address translation expected on this path. + NAT *TranslatedAddress + + // Reason: why this expectation exists (diagnostic; the ReachabilityReason + // enum may be extended with NAT, isolation, and permit-list values as + // new generators land). + Reason ReachabilityReason + + // Peering: name of the CRD that produced this expectation (diagnostic). + Peering string + + // ProtoPort scopes this expectation to a specific protocol/port. The + // zero value applies to the runner's default check. + ProtoPort ProtoPort +} + +// ServerEndpoint identifies one (server, vpc, subnet) attachment. A server +// attached to multiple subnets is represented by multiple endpoints. +type ServerEndpoint struct { + Name string // e.g. "server-1" + VPC string // e.g. "vpc-01" + Subnet string // e.g. "default" + + // HostBGP: if true, this attachment uses BGP to advertise a /32 VIP on + // the loopback; IP holds that VIP (discovered at runtime). Otherwise IP + // is the DHCP/static address on the subnet interface. + HostBGP bool + IP netip.Addr +} + +// ExternalEndpoint identifies an External CRD. +type ExternalEndpoint struct { + ExternalName string + Prefixes []netip.Prefix + + // SourceIP: optional address used when this external originates traffic. + // Empty means the external is destination-only in the matrix. + SourceIP netip.Addr +} + +// Endpoint is a tagged union; exactly one of Server, External is non-nil. +type Endpoint struct { + Server *ServerEndpoint + External *ExternalEndpoint +} + +// EndpointPair is a directional (source → destination) key. Both fields must +// be non-nil and must point to endpoints listed in the owning matrix's +// AllEndpoints (the matrix uses pointer identity for lookups). +type EndpointPair struct { + Source *Endpoint + Destination *Endpoint +} + +// ConnectivityMatrix holds the complete set of expectations for a topology. +type ConnectivityMatrix struct { + // AllEndpoints: canonical, ordered list of all endpoints in the matrix. + AllEndpoints []*Endpoint + + // entries[pair][protoPort] = expectation. The zero ProtoPort{} key holds + // the default-check expectation for the pair. + entries map[EndpointPair]map[ProtoPort]ConnectivityExpectation +} + +// NewConnectivityMatrix returns an empty matrix. AllEndpoints should be set +// by the caller before adding expectations that reference them. +func NewConnectivityMatrix() *ConnectivityMatrix { + return &ConnectivityMatrix{ + entries: map[EndpointPair]map[ProtoPort]ConnectivityExpectation{}, + } +} + +// EndpointPredicate selects endpoints during matrix overlays. Composable +// with the helpers below (ServerInVPC, ExternalNamed). +type EndpointPredicate func(*Endpoint) bool + +// ServerInVPC matches server endpoints attached to the given VPC. +func ServerInVPC(vpc string) EndpointPredicate { + return func(ep *Endpoint) bool { + return ep.Server != nil && ep.Server.VPC == vpc + } +} + +// ExternalNamed matches external endpoints with the given External CRD name. +func ExternalNamed(name string) EndpointPredicate { + return func(ep *Endpoint) bool { + return ep.External != nil && ep.External.ExternalName == name + } +} + +// NATMutator updates a TranslatedAddress for a specific (src, dst) pair. +// The passed-in nat is seeded from any existing entry's NAT (or zero +// value if none) and is the value the helper writes back via matrix.Add. +type NATMutator func(src, dst *Endpoint, nat *TranslatedAddress) error + +// OverlayMatrixNAT marks every (src, dst) pair whose endpoints satisfy +// both predicates as Allow, then runs mutator to set NAT info on the +// resulting entry. +// +// Returns an error if no pair matched the predicates, which almost +// always indicates mismatched predicates relative to the matrix's +// AllEndpoints set. +func OverlayMatrixNAT( + matrix *ConnectivityMatrix, + srcPred, dstPred EndpointPredicate, + mutator NATMutator, +) error { + var touched int + for _, src := range matrix.AllEndpoints { + if !srcPred(src) { + continue + } + for _, dst := range matrix.AllEndpoints { + if !dstPred(dst) { + continue + } + existing := matrix.Lookup(src, dst, ProtoPort{}) + nat := TranslatedAddress{} + if existing.NAT != nil { + nat = *existing.NAT + } + if err := mutator(src, dst, &nat); err != nil { + return err + } + matrix.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: src, Destination: dst}, + Verdict: VerdictAllow, + Reason: ReachabilityReasonGatewayPeering, + Peering: existing.Peering, + NAT: &nat, + }) + touched++ + } + } + if touched == 0 { + return fmt.Errorf("matrix overlay applied to no entries (check predicates)") //nolint:goerr113 + } + + return nil +} + +// BuildConnectivityMatrix assembles a matrix from a pre-discovered set of +// server endpoints (typically returned by SetupVPCs), enumerates external +// endpoints from the live cluster, and populates Allow entries by querying +// IsServerReachable / IsExternalSubnetReachable for every endpoint pair. +// The gatewayEnabled flag is auto-derived from the current Fabricator +// config. NAT translations are not modeled here — callers overlay them on +// the returned matrix before running connectivity tests. +// +// hhfab's CLI / vlabrunner paths don't build a matrix at all; this is +// strictly a test-side opt-in. setupTest calls it once at suite startup; +// matrix-driven tests call Repopulate on the existing matrix after +// DoSetupPeerings to refresh verdicts to the post-peering state. +func BuildConnectivityMatrix(ctx context.Context, kube kclient.Client, serverEndpoints []*Endpoint) (*ConnectivityMatrix, error) { + matrix := NewConnectivityMatrix() + matrix.AllEndpoints = append(matrix.AllEndpoints, serverEndpoints...) + + externalList := vpcapi.ExternalList{} + if err := kube.List(ctx, &externalList); err != nil { + return nil, fmt.Errorf("listing externals for connectivity matrix: %w", err) + } + matrix.AllEndpoints = append(matrix.AllEndpoints, buildExternalEndpoints(externalList.Items)...) + + if err := matrix.Repopulate(ctx, kube); err != nil { + return nil, err + } + + return matrix, nil +} + +// BuildConnectivityMatrixFromCluster discovers server endpoints by +// observing the live cluster (via CollectServerEndpoints) rather than +// taking a pre-built slice, then assembles a matrix the same way as +// BuildConnectivityMatrix. Used when a caller needs to build a matrix +// against a pre-existing topology without re-running SetupVPCs. +func BuildConnectivityMatrixFromCluster(ctx context.Context, kube kclient.Client, ssh SSHResolver) (*ConnectivityMatrix, error) { + endpoints, err := CollectServerEndpoints(ctx, kube, ssh, nil) + if err != nil { + return nil, fmt.Errorf("collecting server endpoints for matrix: %w", err) + } + + return BuildConnectivityMatrix(ctx, kube, endpoints) +} + +// Repopulate clears the matrix's expectation entries and refills Allow +// entries by querying the live cluster for reachability between every +// (src, dst) endpoint pair in AllEndpoints. The gatewayEnabled flag is +// derived from the current Fabricator config. NAT translations are +// reset; callers re-apply any overlays after a Repopulate. +func (m *ConnectivityMatrix) Repopulate(ctx context.Context, kube kclient.Client) error { + f, _, _, err := fab.GetFabAndNodes(ctx, kube, fab.GetFabAndNodesOpts{AllowNotHydrated: true}) + if err != nil { + return fmt.Errorf("getting fab for matrix repopulate: %w", err) + } + if err := populateConnectivityMatrix(ctx, kube, m, f.Spec.Config.Gateway.Enable); err != nil { + return fmt.Errorf("populating connectivity matrix: %w", err) + } + + return nil +} + +// Add inserts or replaces the expectation for (Pair, ProtoPort). Generators +// call this to populate the matrix; tests may also call it to override +// individual entries for advanced scenarios. +func (m *ConnectivityMatrix) Add(e ConnectivityExpectation) { + if m.entries == nil { + m.entries = map[EndpointPair]map[ProtoPort]ConnectivityExpectation{} + } + byPP, ok := m.entries[e.Pair] + if !ok { + byPP = map[ProtoPort]ConnectivityExpectation{} + m.entries[e.Pair] = byPP + } + byPP[e.ProtoPort] = e +} + +// Lookup returns the expectation for (src, dst, pp). If pp is non-zero and +// no protocol-specific entry exists, falls back to the default ProtoPort{} +// entry. If no entry exists at all, returns a synthetic Verdict=Deny +// expectation (default isolation). +func (m *ConnectivityMatrix) Lookup(src, dst *Endpoint, pp ProtoPort) ConnectivityExpectation { + pair := EndpointPair{Source: src, Destination: dst} + if byPP, ok := m.entries[pair]; ok { + if e, ok := byPP[pp]; ok { + return e + } + if pp != (ProtoPort{}) { + if e, ok := byPP[ProtoPort{}]; ok { + return e + } + } + } + + return ConnectivityExpectation{ + Pair: pair, + Verdict: VerdictDeny, + ProtoPort: pp, + } +} + +// reachabilityFromExpectation projects a matrix expectation onto the +// Reachability struct used by the ping/iperf helpers. The matrix's +// Verdict, Reason, and Peering map directly. +func reachabilityFromExpectation(e ConnectivityExpectation) Reachability { + return Reachability{ + Reachable: e.Verdict == VerdictAllow, + Reason: e.Reason, + Peering: e.Peering, + } +} + +// check whether two endpoints belong to the same server / external, regardless +// of the specific IP being tested (in case of multi-homed servers) +func IsSameEndpointNode(a, b *Endpoint) bool { + if a == nil || b == nil { + return false + } + + return (a.External != nil && b.External != nil && a.External.ExternalName == b.External.ExternalName) || + (a.Server != nil && b.Server != nil && a.Server.Name == b.Server.Name) +} + +// TestConnectivityWithMatrix runs ping/iperf/curl against the topology, using +// the supplied ConnectivityMatrix as the authoritative source for both the +// addresses to target and the expected verdicts. No live reachability +// queries are made; matrix.Lookup is the only oracle. +// +// Server-server pairs run ping always (allow → expect success, deny → +// expect failure) and iperf only when the matrix allows them. Bidirectional +// iperf is detected by looking up the reverse pair in the matrix. +// +// Externals are treated as in the legacy test: one curl per source server +// to a hardcoded environment IP ("1.0.0.1"), with the expectation derived +// from the OR of all (src → *_external) matrix verdicts. External-as-source +// paths are not exercised — the matrix doesn't track them today. +// +// opts.Sources and opts.Destinations filter the matrix iteration by server +// name (matching the legacy TestConnectivity semantics): a non-empty +// Sources restricts the source side, a non-empty Destinations restricts the +// destination side, and bidir only triggers when the reverse pair also +// falls inside the filters. + +// matrixTestDeps bundles the shared scaffolding TestConnectivityWithMatrix +// hands to its per-phase helpers: SSH map, concurrency +// semaphores, source/destination filter predicates, the WaitGroup that +// drives goroutine completion, and the error channel that collects probe +// failures. +type matrixTestDeps struct { + sshByServer map[string]*sshutil.Config + pings *semaphore.Weighted + iperfs *semaphore.Weighted + curls *semaphore.Weighted + inSources func(string) bool + inDestinations func(string) bool + wg *sync.WaitGroup + errChan chan<- error +} + +// runMatrixServerServerPhase fans out ping (and iperf3 when allowed) +// goroutines for every (src, dst) server pair the matrix knows about, +// honoring NAT.DestinationIP when present. Pairs that carry a +// DestinationPort are skipped here and instead exercised by +// runMatrixPortForwardPhase, which uses the L4-aware iperf3 helper. +func runMatrixServerServerPhase(ctx context.Context, opts TestConnectivityOpts, matrix *ConnectivityMatrix, deps *matrixTestDeps) error { + for _, src := range matrix.AllEndpoints { + if src.Server == nil { + continue + } + if !deps.inSources(src.Server.Name) { + continue + } + for _, dst := range matrix.AllEndpoints { + if dst.Server == nil || src == dst { + continue + } + // Skip pairs that ultimately point at the same host: same-host + // traffic short-circuits via lo and does not exercise the fabric. + if IsSameEndpointNode(src, dst) { + continue + } + if !deps.inDestinations(dst.Server.Name) { + continue + } + + entry := matrix.Lookup(src, dst, ProtoPort{}) + // Port-forward destinations (DestinationPort set) are L4-only + // and handled by runMatrixPortForwardPhase below. + if entry.NAT != nil && entry.NAT.DestinationPort != 0 { + continue + } + + // Resolve the target IP: a static DNAT entry replaces the + // destination's real IP with the NAT pool address the source + // is expected to target. + toIP := dst.Server.IP + if entry.NAT != nil && entry.NAT.DestinationIP.IsValid() { + toIP = entry.NAT.DestinationIP + } + if !toIP.IsValid() { + return fmt.Errorf("matrix entry %s→%s (vpc %s/%s) has no valid target IP", src.Server.Name, dst.Server.Name, dst.Server.VPC, dst.Server.Subnet) //nolint:goerr113 + } + + expected := reachabilityFromExpectation(entry) + bidir := false + if opts.IPerfsSeconds > 0 && expected.Reachable && deps.inSources(dst.Server.Name) && deps.inDestinations(src.Server.Name) { + reverse := matrix.Lookup(dst, src, ProtoPort{}) + if reverse.Verdict == VerdictAllow { + // bidir iperf3 uses one TCP session; both halves share + // a target IP. Any DNAT on either side breaks that + // symmetry, so fall back to two separate sessions. + forwardDNAT := entry.NAT != nil && entry.NAT.DestinationIP.IsValid() + reverseDNAT := reverse.NAT != nil && reverse.NAT.DestinationIP.IsValid() + if !forwardDNAT && !reverseDNAT { + bidir = true + } + } + } + + args := pingIperfPairArgs{ + From: src.Server.Name, + To: dst.Server.Name, + FromSSH: deps.sshByServer[src.Server.Name], + ToIP: toIP, + Expected: expected, + Bidir: bidir, + Pings: deps.pings, + Iperfs: deps.iperfs, + } + deps.wg.Go(func() { + for _, e := range runPingIperfPair(ctx, opts, args) { + deps.errChan <- e + } + }) + } + } + + return nil +} + +// runMatrixCurlPhase launches one curl per source server in +// deps.inSources to the hardcoded outbound target ("1.0.0.1"). +// Each server's expected.Reachable is the OR of all (src → *_external) +// matrix Allow entries that have SNAT info or no NAT at all; DNAT-only +// (port-forward) entries don't generically route outbound and so don't +// raise the expectation. +func runMatrixCurlPhase(ctx context.Context, opts TestConnectivityOpts, matrix *ConnectivityMatrix, deps *matrixTestDeps) { + expectedByServer := map[string]Reachability{} + for _, src := range matrix.AllEndpoints { + if src.Server == nil { + continue + } + name := src.Server.Name + if !deps.inSources(name) { + continue + } + if _, seen := expectedByServer[name]; !seen { + expectedByServer[name] = Reachability{} + } + if expectedByServer[name].Reachable { + continue + } + for _, dst := range matrix.AllEndpoints { + if dst.External == nil { + continue + } + e := matrix.Lookup(src, dst, ProtoPort{}) + if e.Verdict != VerdictAllow { + continue + } + if e.NAT != nil && !e.NAT.SourcePool.IsValid() { + continue + } + expectedByServer[name] = reachabilityFromExpectation(e) + + break + } + } + + for name, ssh := range deps.sshByServer { + if !deps.inSources(name) { + continue + } + expected := expectedByServer[name] + deps.wg.Go(func() { + logArgs := []any{"from", name, "expected", expected.Reachable} + if expected.Reachable { + logArgs = append(logArgs, "reason", expected.Reason) + if expected.Peering != "" { + logArgs = append(logArgs, "peering", expected.Peering) + } + } + slog.Debug("Checking external connectivity", logArgs...) + + if ce := checkCurl(ctx, opts, deps.curls, name, ssh, "1.0.0.1", expected.Reachable); ce != nil { + deps.errChan <- ce + } + }) + } +} + +// runMatrixPortForwardPhase launches iperf3 against every port-forward +// NAT virtual endpoint encoded in the matrix. External destinations are +// deduped by (srcServer, IP, port) so the single external iperf3 server is hit once; +// server destinations exercise every (src, dst) pair for full cross-product +// coverage. +func runMatrixPortForwardPhase(ctx context.Context, opts TestConnectivityOpts, matrix *ConnectivityMatrix, deps *matrixTestDeps) { + type pfTargetKey struct { + from string + ip netip.Addr + port uint16 + } + extTargets := map[pfTargetKey]Reachability{} + for _, src := range matrix.AllEndpoints { + if src.Server == nil { + continue + } + if !deps.inSources(src.Server.Name) { + continue + } + for _, dst := range matrix.AllEndpoints { + if src == dst { + continue + } + e := matrix.Lookup(src, dst, ProtoPort{}) + if e.Verdict != VerdictAllow || e.NAT == nil { + continue + } + if !e.NAT.DestinationIP.IsValid() || e.NAT.DestinationPort == 0 { + continue + } + switch { + case dst.External != nil: + key := pfTargetKey{from: src.Server.Name, ip: e.NAT.DestinationIP, port: e.NAT.DestinationPort} + if _, seen := extTargets[key]; seen { + continue + } + extTargets[key] = reachabilityFromExpectation(e) + case dst.Server != nil: + if IsSameEndpointNode(src, dst) { + continue + } + if !deps.inDestinations(dst.Server.Name) { + continue + } + expected := reachabilityFromExpectation(e) + target := e.NAT.DestinationIP + port := e.NAT.DestinationPort + fromName := src.Server.Name + deps.wg.Go(func() { + if ie := runMatrixIperfPortForward(ctx, opts, deps.iperfs, fromName, deps.sshByServer[fromName], target, port, expected); ie != nil { + deps.errChan <- ie + } + }) + } + } + } + for key, val := range extTargets { + deps.wg.Go(func() { + if ie := runMatrixIperfPortForward(ctx, opts, deps.iperfs, key.from, deps.sshByServer[key.from], key.ip, key.port, val); ie != nil { + deps.errChan <- ie + } + }) + } +} + +func (c *Config) TestConnectivityWithMatrix(ctx context.Context, vlab *VLAB, opts TestConnectivityOpts, matrix *ConnectivityMatrix) error { + if matrix == nil { + return fmt.Errorf("connectivity matrix must be non-nil") //nolint:goerr113 + } + if opts.PingsCount == 0 && opts.IPerfsSeconds == 0 && opts.CurlsCount == 0 { + return fmt.Errorf("at least one of pings, iperfs or curls should be enabled") //nolint:goerr113 + } + start := time.Now() + + if opts.PingsParallel <= 0 { + opts.PingsParallel = 50 + } + if opts.IPerfsParallel <= 0 { + opts.IPerfsParallel = 1 + } + if opts.CurlsParallel <= 0 { + opts.CurlsParallel = 50 + } + + slog.Info("Testing connectivity from matrix", "endpoints", len(matrix.AllEndpoints)) + + sshConfigs, _, cacheCancel, err := c.prepareConnectivityTest(ctx, vlab, &opts) + if err != nil { + return err + } + defer cacheCancel() + + // Resolve the SSH config and a toolbox mutex for every unique server + // name referenced by the matrix. + sshByServer := map[string]*sshutil.Config{} + toolboxMutexes := map[string]*sync.Mutex{} + for _, ep := range matrix.AllEndpoints { + if ep.Server == nil { + continue + } + name := ep.Server.Name + if _, ok := toolboxMutexes[name]; ok { + continue + } + ssh, ok := sshConfigs[name] + if !ok { + return fmt.Errorf("no ssh config for server %q referenced by matrix", name) //nolint:goerr113 + } + sshByServer[name] = ssh + toolboxMutexes[name] = &sync.Mutex{} + } + + n := len(matrix.AllEndpoints) + errChan := make(chan error, 2*n*n+n) + deps := &matrixTestDeps{ + sshByServer: sshByServer, + pings: semaphore.NewWeighted(opts.PingsParallel), + iperfs: semaphore.NewWeighted(opts.IPerfsParallel), + curls: semaphore.NewWeighted(opts.CurlsParallel), + inSources: func(name string) bool { + return len(opts.Sources) == 0 || slices.Contains(opts.Sources, name) + }, + inDestinations: func(name string) bool { + return len(opts.Destinations) == 0 || slices.Contains(opts.Destinations, name) + }, + wg: &sync.WaitGroup{}, + errChan: errChan, + } + + if opts.PingsCount > 0 || opts.IPerfsSeconds > 0 { + if err := runMatrixServerServerPhase(ctx, opts, matrix, deps); err != nil { + return err + } + } + if opts.CurlsCount > 0 { + runMatrixCurlPhase(ctx, opts, matrix, deps) + } + if opts.IPerfsSeconds > 0 { + runMatrixPortForwardPhase(ctx, opts, matrix, deps) + } + + deps.wg.Wait() + close(errChan) + + var joined error + var numPingErrs, numIperfErrs, numCurlErrs int + for e := range errChan { + var ( + pingErr *PingError + iperfErr *IperfError + curlErr *CurlError + ) + switch { + case errors.As(e, &pingErr): + numPingErrs++ + case errors.As(e, &iperfErr): + numIperfErrs++ + case errors.As(e, &curlErr): + numCurlErrs++ + } + joined = errors.Join(joined, e) + } + + if joined != nil { + slog.Error("Test connectivity (matrix) failed", "ping", numPingErrs, "iperf", numIperfErrs, "curl", numCurlErrs, "took", time.Since(start), "errors", joined) + } else { + slog.Info("Test connectivity (matrix) passed", "took", time.Since(start)) + } + + return joined +} + +// runMatrixIperfPortForward exercises one port-forward NAT path encoded by +// the matrix: TCP-probe NAT.DestinationIP:Port until the gateway's DNAT +// rule is programmed, then run iperf3 once. Targets may be external NAT +// virtual IPs or other VPCs' NAT pool addresses — the function is agnostic +// to where the (ip, port) lives. +func runMatrixIperfPortForward(ctx context.Context, opts TestConnectivityOpts, iperfs *semaphore.Weighted, from string, ssh *sshutil.Config, toIP netip.Addr, toPort uint16, expected Reachability) *IperfError { + target := fmt.Sprintf("%s:%d", toIP.String(), toPort) + logArgs := []any{"from", from, "target", target, "expected", expected.Reachable} + if expected.Reason != "" { + logArgs = append(logArgs, "reason", expected.Reason) + } + if expected.Peering != "" { + logArgs = append(logArgs, "peering", expected.Peering) + } + slog.Debug("Checking iperf3 through port-forward NAT (matrix)", logArgs...) + + // Gate on TCP reachability: the gateway's port-forward DNAT rule has + // its own programming lag separate from fabric route propagation, and + // a successful TCP connect is the precise signal that both halves of + // the path (fabric route + gateway DNAT) are active. After the probe + // succeeds, iperf3 runs once and any failure is a real test failure. + probe := fmt.Sprintf("nc -zw2 %s %d", toIP.String(), toPort) + deadline := time.Now().Add(gwNATPortForwardProbeTimeout) + var lastErr error + for { + if _, _, err := retrySSHCmd(ctx, ssh, probe, from); err == nil { + break + } else { //nolint:revive + lastErr = err + } + if time.Now().After(deadline) { + return &IperfError{ + Source: from, + Destination: target, + ClientMsg: fmt.Sprintf("port-forward target not reachable after %s: %s", gwNATPortForwardProbeTimeout, lastErr), + } + } + select { + case <-ctx.Done(): + return &IperfError{Source: from, Destination: target, ClientMsg: ctx.Err().Error()} + case <-time.After(gwNATPortForwardProbeInterval): + } + } + + if err := iperfs.Acquire(ctx, 1); err != nil { + return &IperfError{Source: from, Destination: target, ClientMsg: fmt.Sprintf("acquiring iperf3 semaphore: %s", err)} + } + defer iperfs.Release(1) + + secs := opts.IPerfsSeconds + cmd := fmt.Sprintf("toolbox -E LD_PRELOAD=/lib/x86_64-linux-gnu/libgcc_s.so.1 -q timeout %d iperf3 -J -c %s -p %d -t %d", + secs+25, toIP.String(), toPort, secs) + if _, _, iperfErr := retrySSHCmd(ctx, ssh, cmd, from); iperfErr != nil { + return &IperfError{Source: from, Destination: target, ClientMsg: iperfErr.Error()} + } + + return nil +} + +func DoVLABTestConnectivityWithMatrix(ctx context.Context, workDir, cacheDir string, opts TestConnectivityOpts, matrix *ConnectivityMatrix) error { + c, vlab, err := loadVLABForHelpers(ctx, workDir, cacheDir) + if err != nil { + return err + } + + return c.TestConnectivityWithMatrix(ctx, vlab, opts, matrix) +} diff --git a/pkg/hhfab/rt_base.go b/pkg/hhfab/rt_base.go index 4d0097c02..28236d5de 100644 --- a/pkg/hhfab/rt_base.go +++ b/pkg/hhfab/rt_base.go @@ -75,11 +75,12 @@ type VPCPeeringTestCtx struct { // to be run after the test is done, regardless of whether it succeeded or failed. type RevertFunc func(context.Context) error -// A test function is a function that runs a test. It takes a go context and a test context, and returns -// a boolean indicating whether the test was skipped (e.g. due to missing resources), -// a list of revert functions to be run after the test, and an error if the test failed. +// A test function is a function that runs a test. It takes a go context, a test context, +// and a connectivity matrix, and returns a boolean indicating whether the test was skipped +// (e.g. due to missing resources), a list of revert functions to be run after the test, +// and an error if the test failed. // note that the error contains the reason for the skip if the test was skipped. -type TestFunc func(context.Context, *VPCPeeringTestCtx) (bool, []RevertFunc, error) +type TestFunc func(context.Context, *VPCPeeringTestCtx, *ConnectivityMatrix) (bool, []RevertFunc, error) // Utilities and suite runners @@ -414,28 +415,38 @@ func pauseOnFailure(ctx context.Context) error { } // prepare for a test: create the VPCs according to the options in the test context -func (testCtx *VPCPeeringTestCtx) setupTest(ctx context.Context, initialSuiteSetup bool) error { +func (testCtx *VPCPeeringTestCtx) setupTest(ctx context.Context, initialSuiteSetup bool) (*ConnectivityMatrix, error) { if testCtx.noSetup { // nothing to setup, but we still want to wait for the switches to be ready if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { - return fmt.Errorf("waiting for switches to be ready: %w", err) + return nil, fmt.Errorf("waiting for switches to be ready: %w", err) } - return nil + return NewConnectivityMatrix(), nil } // if it is the first setup of the suite, we also want to remove the old VPCs (might have different parameters) opts := testCtx.setupOpts opts.ForceCleanup = initialSuiteSetup // this will also remove all peerings - if err := DoVLABSetupVPCs(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, opts); err != nil { - return fmt.Errorf("setting up VPCs: %w", err) + endpoints, err := DoVLABSetupVPCs(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, opts) + if err != nil { + return nil, fmt.Errorf("setting up VPCs: %w", err) } // in case of L3 VPC mode, we need to give it time to switch to the longer lease time and switches to learn the routes if opts.VPCMode == vpcapi.VPCModeL3VNI || opts.VPCMode == vpcapi.VPCModeL3Flat { time.Sleep(10 * time.Second) } - return nil + // Wrap discovered endpoints in a matrix with an initial reachability + // sweep. Tests that don't use the matrix simply ignore it; + // matrix-driven tests call matrix.Repopulate after applying their + // peerings to refresh verdicts. + matrix, err := BuildConnectivityMatrix(ctx, testCtx.kube, endpoints) + if err != nil { + return nil, fmt.Errorf("building initial connectivity matrix: %w", err) + } + + return matrix, nil } func doRunSuite(ctx context.Context, testCtx *VPCPeeringTestCtx, ts *JUnitTestSuite) (*JUnitTestSuite, error) { @@ -444,7 +455,8 @@ func doRunSuite(ctx context.Context, testCtx *VPCPeeringTestCtx, ts *JUnitTestSu slog.Info("** Running test suite", "suite", ts.Name, "tests", len(ts.TestCases), "start-time", suiteStart.Format(time.RFC3339)) // initial setup - if err := testCtx.setupTest(ctx, true); err != nil { + matrix, err := testCtx.setupTest(ctx, true) + if err != nil { slog.Error("Initial test suite setup failed", "suite", ts.Name, "error", err.Error()) // Collect diagnostics for suite setup failure @@ -468,7 +480,8 @@ func doRunSuite(ctx context.Context, testCtx *VPCPeeringTestCtx, ts *JUnitTestSu } slog.Info("* Running test", "test", test.Name) if (ranSomeTests && testCtx.wipeBetweenTests) || prevRevertsFailed { - if err := testCtx.setupTest(ctx, false); err != nil { + matrix, err = testCtx.setupTest(ctx, false) + if err != nil { ts.TestCases[i].Failure = &Failure{ Message: fmt.Sprintf("Failed to run setupTest between tests: %s", err.Error()), } @@ -492,7 +505,7 @@ func doRunSuite(ctx context.Context, testCtx *VPCPeeringTestCtx, ts *JUnitTestSu } prevRevertsFailed = false testStart := time.Now() - skip, reverts, err := test.F(ctx, testCtx) + skip, reverts, err := test.F(ctx, testCtx, matrix) ts.TestCases[i].Time = time.Since(testStart).Seconds() ranSomeTests = true // logic is getting complex, so let's make a recap: diff --git a/pkg/hhfab/rt_eslag_fallback.go b/pkg/hhfab/rt_eslag_fallback.go index c363fbf72..69271b733 100644 --- a/pkg/hhfab/rt_eslag_fallback.go +++ b/pkg/hhfab/rt_eslag_fallback.go @@ -22,7 +22,7 @@ const ( eslagPxeAttemptsPerLeg = 3 ) -func eslagFallbackTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func eslagFallbackTest(ctx context.Context, testCtx *VPCPeeringTestCtx, _ *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.setupOpts.VPCMode == vpcapi.VPCModeL3VNI { return true, nil, fmt.Errorf("L3VNI mode is not compatible with ESLAG") //nolint:goerr113 } diff --git a/pkg/hhfab/rt_multi_vpc_multi_subnet_suite.go b/pkg/hhfab/rt_multi_vpc_multi_subnet_suite.go index 5f15eb7f2..6c0f45e36 100644 --- a/pkg/hhfab/rt_multi_vpc_multi_subnet_suite.go +++ b/pkg/hhfab/rt_multi_vpc_multi_subnet_suite.go @@ -74,7 +74,7 @@ func makeMultiVPCMultiSubnetSuite() *JUnitTestSuite { // 2. Override isolation with explicit permit list, test connectivity // 3. Set restricted flag in subnet-02 in vpc2, test connectivity // 4. Remove all restrictions and peerings -func multiSubnetsIsolationTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func multiSubnetsIsolationTest(ctx context.Context, testCtx *VPCPeeringTestCtx, _ *ConnectivityMatrix) (bool, []RevertFunc, error) { var returnErr error var vpc1, vpc2 *vpcapi.VPC @@ -210,7 +210,7 @@ func multiSubnetsIsolationTest(ctx context.Context, testCtx *VPCPeeringTestCtx) // Assumes the scenario has at least 2 VPCs with at least 2 subnets each. // It creates peering between them, but restricts the peering to only // one subnet each. It then tests connectivity. -func multiSubnetsSubnetFilteringTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func multiSubnetsSubnetFilteringTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcList := &vpcapi.VPCList{} if err := testCtx.kube.List(ctx, vpcList); err != nil { return false, nil, fmt.Errorf("listing VPCs: %w", err) @@ -261,7 +261,11 @@ func multiSubnetsSubnetFilteringTest(ctx context.Context, testCtx *VPCPeeringTes return nil }) - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, reverts, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, reverts, err } @@ -311,7 +315,7 @@ func (testCtx *VPCPeeringTestCtx) pingStaticExternal(ctx context.Context, source * 10a. repeat tests 7a and 7b from a switch that's not the one the static external is attached to (should succeed) * 11. cleanup everything and restore the original state */ -func staticExternalTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func staticExternalTest(ctx context.Context, testCtx *VPCPeeringTestCtx, _ *ConnectivityMatrix) (bool, []RevertFunc, error) { // find an unbundled connection not attached to an MCLAG switch (see https://github.com/githedgehog/fabricator/issues/673#issuecomment-3028423762) connList := &wiringapi.ConnectionList{} if err := testCtx.kube.List(ctx, connList, kclient.MatchingLabels{wiringapi.LabelConnectionType: wiringapi.ConnectionTypeUnbundled}); err != nil { diff --git a/pkg/hhfab/rt_multi_vpc_single_subnet_suite.go b/pkg/hhfab/rt_multi_vpc_single_subnet_suite.go index ff5e3ac88..156c4bb4c 100644 --- a/pkg/hhfab/rt_multi_vpc_single_subnet_suite.go +++ b/pkg/hhfab/rt_multi_vpc_single_subnet_suite.go @@ -130,7 +130,7 @@ func makeMultiVPCSingleSubnetSuite() *JUnitTestSuite { // It was presumably chosen because going from this to a full mesh configuration could trigger // the gNMI bug. Note that in order to reproduce it one should disable the forced cleanup between // tests. -func vpcPeeringsStarterTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func vpcPeeringsStarterTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // 1+2 1+3 3+5 2+4 4+6 5+6 6+7 7+8 8+9 5~default--5835:s=subnet-01 6~default--5835:s=subnet-01 1~default--5835:s=subnet-01 2~default--5835:s=subnet-01 9~default--5835:s=subnet-01 7~default--5835:s=subnet-01 vpcs := &vpcapi.VPCList{} if err := testCtx.kube.List(ctx, vpcs); err != nil { @@ -162,7 +162,10 @@ func vpcPeeringsStarterTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bo if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, nil, true); err != nil { return false, nil, fmt.Errorf("setting up peerings: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, err } @@ -171,7 +174,7 @@ func vpcPeeringsStarterTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bo // Test connectivity between all VPCs in a full mesh configuration, including all externals // Then, remove one external peering and test connectivity again. -func vpcPeeringsFullMeshAllExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func vpcPeeringsFullMeshAllExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec, 15) if err := populateFullMeshVpcPeerings(ctx, testCtx.kube, vpcPeerings); err != nil { return false, nil, fmt.Errorf("populating full mesh VPC peerings: %w", err) @@ -185,7 +188,10 @@ func vpcPeeringsFullMeshAllExternalsTest(ctx context.Context, testCtx *VPCPeerin if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, nil, true); err != nil { return false, nil, fmt.Errorf("setting up peerings: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, err } @@ -202,7 +208,10 @@ func vpcPeeringsFullMeshAllExternalsTest(ctx context.Context, testCtx *VPCPeerin if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, nil, true); err != nil { return false, nil, fmt.Errorf("setting up peerings: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, err } } @@ -211,7 +220,7 @@ func vpcPeeringsFullMeshAllExternalsTest(ctx context.Context, testCtx *VPCPeerin } // Test connectivity between all VPCs with no peering except of the external ones. -func vpcPeeringsOnlyExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func vpcPeeringsOnlyExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec, 0) externalPeerings := make(map[string]*vpcapi.ExternalPeeringSpec, 6) if err := populateAllExternalVpcPeerings(ctx, testCtx.kube, externalPeerings); err != nil { @@ -225,7 +234,10 @@ func vpcPeeringsOnlyExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCt if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, nil, true); err != nil { return false, nil, fmt.Errorf("setting up peerings: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, err } @@ -233,7 +245,7 @@ func vpcPeeringsOnlyExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCt } // Test connectivity between all VPCs in a full loop configuration, including all externals. -func vpcPeeringsFullLoopAllExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func vpcPeeringsFullLoopAllExternalsTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec, 6) if err := populateFullLoopVpcPeerings(ctx, testCtx.kube, vpcPeerings); err != nil { return false, nil, fmt.Errorf("populating full loop VPC peerings: %w", err) @@ -245,7 +257,10 @@ func vpcPeeringsFullLoopAllExternalsTest(ctx context.Context, testCtx *VPCPeerin if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, nil, true); err != nil { return false, nil, fmt.Errorf("setting up peerings: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, err } @@ -253,7 +268,7 @@ func vpcPeeringsFullLoopAllExternalsTest(ctx context.Context, testCtx *VPCPeerin } // Arbitrary configuration which again was shown to occasionally trigger the gNMI bug. -func vpcPeeringsSergeisSpecialTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func vpcPeeringsSergeisSpecialTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // 1+2 2+3 2+4 6+5 1~default--5835:s=subnet-01 vpcs := &vpcapi.VPCList{} if err := testCtx.kube.List(ctx, vpcs); err != nil { @@ -273,7 +288,10 @@ func vpcPeeringsSergeisSpecialTest(ctx context.Context, testCtx *VPCPeeringTestC if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, nil, true); err != nil { return false, nil, fmt.Errorf("setting up peerings: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, err } @@ -283,7 +301,7 @@ func vpcPeeringsSergeisSpecialTest(ctx context.Context, testCtx *VPCPeeringTestC // Test basic gateway peering connectivity between two VPCs. // Creates a gateway peering between the first two VPCs found, exposing all subnets // from each VPC, then tests connectivity to ensure traffic flows through the gateway. -func gatewayPeeringTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func gatewayPeeringTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcs := &vpcapi.VPCList{} if err := testCtx.kube.List(ctx, vpcs); err != nil { return false, nil, fmt.Errorf("listing VPCs: %w", err) @@ -308,7 +326,11 @@ func gatewayPeeringTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, return false, nil, fmt.Errorf("setting up gateway peerings: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing gateway peering connectivity: %w", err) } @@ -317,7 +339,7 @@ func gatewayPeeringTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, // Test gateway peering in a loop configuration where each VPC peers with the next one. // VPC1↔VPC2↔VPC3↔...↔VPCn↔VPC1. Test connectivity in a complete loop. -func gatewayPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func gatewayPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcs := &vpcapi.VPCList{} if err := testCtx.kube.List(ctx, vpcs); err != nil { return false, nil, fmt.Errorf("listing VPCs: %w", err) @@ -350,6 +372,10 @@ func gatewayPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bo return false, nil, fmt.Errorf("setting up gateway loop peerings: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + // Wait for EVPN Type-5 routes to be installed in leaf switch RIBs before // running connectivity tests. WaitReady only confirms frr-reload.py exited; // the ZEBRA CPI blast + BGP UPDATE propagation to leaves takes 26-38s more. @@ -387,7 +413,7 @@ func gatewayPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bo } } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing gateway loop connectivity: %w", err) } @@ -396,7 +422,7 @@ func gatewayPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bo // Test combining VPC peering and gateway peering in an alternating loop configuration. // Create alternating VPC and gateway peerings to form a complete loop through all VPCs. -func gatewayMixedPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func gatewayMixedPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcs := &vpcapi.VPCList{} if err := testCtx.kube.List(ctx, vpcs); err != nil { return false, nil, fmt.Errorf("listing VPCs: %w", err) @@ -435,7 +461,11 @@ func gatewayMixedPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx return false, nil, fmt.Errorf("setting up mixed peering loop: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing mixed peering loop connectivity: %w", err) } @@ -443,7 +473,7 @@ func gatewayMixedPeeringLoopTest(ctx context.Context, testCtx *VPCPeeringTestCtx } // Test combining external peering via fabric and via gateway. -func mixedGatewayAndFabricExternals(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func mixedGatewayAndFabricExternals(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcs := &vpcapi.VPCList{} if err := testCtx.kube.List(ctx, vpcs); err != nil { return false, nil, fmt.Errorf("listing VPCs: %w", err) @@ -476,7 +506,10 @@ func mixedGatewayAndFabricExternals(ctx context.Context, testCtx *VPCPeeringTest if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, gwPeerings, true); err != nil { return false, nil, fmt.Errorf("setting up mixed peering loop: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing mixed peering loop connectivity: %w", err) } @@ -489,7 +522,10 @@ func mixedGatewayAndFabricExternals(ctx context.Context, testCtx *VPCPeeringTest if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, gwPeerings, true); err != nil { return false, nil, fmt.Errorf("setting up mixed peering loop: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing mixed peering loop connectivity: %w", err) } diff --git a/pkg/hhfab/rt_nat_external_tests.go b/pkg/hhfab/rt_nat_external_tests.go index 6f60af926..08ec23c32 100644 --- a/pkg/hhfab/rt_nat_external_tests.go +++ b/pkg/hhfab/rt_nat_external_tests.go @@ -9,121 +9,91 @@ import ( "log/slog" "net/netip" "sort" - "strings" - "time" gwapi "go.githedgehog.com/fabric/api/gateway/v1alpha1" vpcapi "go.githedgehog.com/fabric/api/vpc/v1beta1" - wiringapi "go.githedgehog.com/fabric/api/wiring/v1beta1" - "go.githedgehog.com/fabric/pkg/util/apiutil" - "go.githedgehog.com/fabricator/pkg/util/sshutil" - "golang.org/x/sync/semaphore" ) -// gwNATPortForwardProbeTimeout is the maximum time to wait for the gateway's port-forward -// NAT rule to become active in the dataplane after the peering is applied. Unlike fabric -// route propagation (which waitForNATPoolInLeaves gates on), the gateway's DNAT rule -// programming has its own latency that no Kubernetes condition signals. -const gwNATPortForwardProbeTimeout = 2 * time.Minute - -// gwNATPortForwardProbeInterval is the polling interval between TCP-reachability probes. -const gwNATPortForwardProbeInterval = 5 * time.Second - -// waitForPortForwardReachable retries a TCP connect probe from the given server until it -// succeeds or gwNATPortForwardProbeTimeout elapses. Used to gate iperf3 port-forward tests -// on the actual reachability of the NAT-pool target IP:port; once a TCP handshake completes, -// iperf3 can run exactly once and any failure is a real test failure rather than a -// programming-lag race. -func (testCtx *VPCPeeringTestCtx) waitForPortForwardReachable(ctx context.Context, sshCfg *sshutil.Config, server, host string, port int) error { - // nc -z performs a TCP connect probe with no data; -w2 caps the connect attempt at 2s. - // Same primitive used in show-tech/control.sh, so we know it's available on flatcar. - cmd := fmt.Sprintf("nc -zw2 %s %d", host, port) - deadline := time.Now().Add(gwNATPortForwardProbeTimeout) - var lastErr error - for { - if _, _, err := retrySSHCmd(ctx, sshCfg, cmd, server); err == nil { - return nil - } else { //nolint:revive - lastErr = err - } - if time.Now().After(deadline) { - return fmt.Errorf("port-forward target %s:%d not reachable after %s: %w", host, port, gwNATPortForwardProbeTimeout, lastErr) - } - slog.Debug("Port-forward target not reachable yet, retrying", "server", server, "host", host, "port", port, "retryIn", gwNATPortForwardProbeInterval) - select { - case <-ctx.Done(): - return ctx.Err() - case <-time.After(gwNATPortForwardProbeInterval): - } +// overlayExternalSNAT marks every (server-in-vpcName → extName) Allow entry +// in the matrix with the given SNAT pool. +func overlayExternalSNAT(matrix *ConnectivityMatrix, vpcName, extName, sourcePoolCIDR string) error { + pool, err := netip.ParsePrefix(sourcePoolCIDR) + if err != nil { + return fmt.Errorf("parsing SNAT pool %s: %w", sourcePoolCIDR, err) } + + return OverlayMatrixNAT(matrix, ServerInVPC(vpcName), ExternalNamed(extName), func(_, _ *Endpoint, nat *TranslatedAddress) error { + nat.SourcePool = pool + + return nil + }) } -// testNATExternalConnectivity tests outbound connectivity from a VPC through a NAT gateway peering -// by curling 1.0.0.1 directly, bypassing the standard peering check that does not understand NAT -// expose CIDRs. Callers are expected to gate this on waitForNATPoolInLeaves first when the test -// depends on a freshly-applied NAT pool route reaching the fabric, so any failure here is a real -// connectivity failure, not a route-propagation race. -func (testCtx *VPCPeeringTestCtx) testNATExternalConnectivity(ctx context.Context, vpc *vpcapi.VPC, extName string) error { - servers := &wiringapi.ServerList{} - if err := testCtx.kube.List(ctx, servers); err != nil { - return fmt.Errorf("listing servers: %w", err) +// pingExternalStability runs a 10-ping probe from every server in vpcName to +// the external's BGP-neighbor IP (the actual remote device, not 1.0.0.1). +func (testCtx *VPCPeeringTestCtx) pingExternalStability(ctx context.Context, matrix *ConnectivityMatrix, vpcName, extName string) error { + remoteIP, err := getExternalRemoteIP(ctx, testCtx.kube, extName) + if err != nil { + return fmt.Errorf("getting external remote IP for ping: %w", err) + } + if remoteIP == "" { + return nil + } + remoteAddr, err := netip.ParseAddr(remoteIP) + if err != nil { + return fmt.Errorf("parsing external remote IP %s: %w", remoteIP, err) } - curlSem := semaphore.NewWeighted(1) - + seen := map[string]bool{} var tested int - for _, server := range servers.Items { - attachedSubnets, err := apiutil.GetAttachedSubnets(ctx, testCtx.kube, server.Name) - if err != nil { + for _, ep := range matrix.AllEndpoints { + if ep.Server == nil || ep.Server.VPC != vpcName { continue } - - inVPC := false - for subnetName := range attachedSubnets { - if strings.HasPrefix(subnetName, vpc.Name+"/") { - inVPC = true - - break - } - } - if !inVPC { + if seen[ep.Server.Name] { continue } + seen[ep.Server.Name] = true - sshCfg, err := testCtx.getSSH(ctx, server.Name) + sshCfg, err := testCtx.getSSH(ctx, ep.Server.Name) if err != nil { - return fmt.Errorf("getting ssh config for %s: %w", server.Name, err) + return fmt.Errorf("getting ssh config for %s: %w", ep.Server.Name, err) } - - slog.Debug("Testing NAT external connectivity via curl", "server", server.Name) - if curlErr := checkCurl(ctx, testCtx.tcOpts, curlSem, server.Name, sshCfg, "1.0.0.1", true); curlErr != nil { - return fmt.Errorf("NAT external connectivity check: %w", curlErr) - } - - if remoteIP, err := getExternalRemoteIP(ctx, testCtx.kube, extName); err != nil { - return fmt.Errorf("getting external remote IP for ping: %w", err) - } else if remoteIP != "" { - remoteAddr, err := netip.ParseAddr(remoteIP) - if err != nil { - return fmt.Errorf("parsing external remote IP %s: %w", remoteIP, err) - } - slog.Debug("Testing NAT external connectivity stability via ping", "server", server.Name, "target", remoteIP) - pingSem := semaphore.NewWeighted(1) - if pingErr := checkPing(ctx, 10, pingSem, server.Name, remoteIP, sshCfg, remoteAddr, nil, true); pingErr != nil { - return fmt.Errorf("NAT external connectivity ping stability check: %w", pingErr) - } + slog.Debug("Testing NAT external connectivity stability via ping", "server", ep.Server.Name, "target", remoteIP) + if pingErr := checkPing(ctx, 10, nil, ep.Server.Name, remoteIP, sshCfg, remoteAddr, nil, true); pingErr != nil { + return fmt.Errorf("NAT external connectivity ping stability check: %w", pingErr) } - tested++ } if tested == 0 { - return fmt.Errorf("no servers found in VPC %s for NAT external connectivity test", vpc.Name) //nolint:goerr113 + return fmt.Errorf("no servers found in VPC %s for ping stability check", vpcName) //nolint:goerr113 } return nil } +// overlayExternalPortForward marks every (server-in-vpcName → extName) Allow +// entry with a DNAT to destIP:destPort, telling the matrix-driven tester to +// exercise iperf3 against that virtual endpoint. Without any SNAT companion, +// the curl-to-external check correctly expects failure: the peering routes +// only the port-forward target, not arbitrary outbound. +func overlayExternalPortForward(matrix *ConnectivityMatrix, vpcName, extName string, destIP netip.Addr, destPort uint16) error { + if destPort == 0 { + return fmt.Errorf("destPort must be non-zero for port-forward overlay") //nolint:goerr113 + } + if !destIP.IsValid() { + return fmt.Errorf("destIP must be valid for port-forward overlay") //nolint:goerr113 + } + + return OverlayMatrixNAT(matrix, ServerInVPC(vpcName), ExternalNamed(extName), func(_, _ *Endpoint, nat *TranslatedAddress) error { + nat.DestinationIP = destIP + nat.DestinationPort = destPort + + return nil + }) +} + // Test gateway external peering with no NAT (baseline) // Peering spec: // @@ -137,7 +107,7 @@ func (testCtx *VPCPeeringTestCtx) testNATExternalConnectivity(ctx context.Contex // Expose: // Ips: // Cidr: 0.0.0.0/0 -func bgpExternalNoNatTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func bgpExternalNoNatTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.extName == "" { return true, nil, fmt.Errorf("no BGP external available for testing") //nolint:goerr113 } @@ -170,7 +140,11 @@ func bgpExternalNoNatTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing BGP external connectivity: %w", err) } @@ -194,7 +168,7 @@ func bgpExternalNoNatTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool // Expose: // Ips: // Cidr: 0.0.0.0/0 -func bgpExternalStaticNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func bgpExternalStaticNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.extName == "" { return true, nil, fmt.Errorf("no BGP external available for testing") //nolint:goerr113 } @@ -240,13 +214,23 @@ func bgpExternalStaticNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) ( return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, bgpNATCIDR); err != nil { return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - if err := testCtx.testNATExternalConnectivity(ctx, vpc, testCtx.extName); err != nil { + if err := overlayExternalSNAT(matrix, vpc.Name, testCtx.extName, bgpNATCIDR); err != nil { + return false, nil, fmt.Errorf("annotating matrix with BGP static SNAT pool: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing BGP external static NAT connectivity: %w", err) } + if err := testCtx.pingExternalStability(ctx, matrix, vpc.Name, testCtx.extName); err != nil { + return false, nil, fmt.Errorf("BGP external static NAT ping stability: %w", err) + } return false, nil, nil } @@ -269,7 +253,7 @@ func bgpExternalStaticNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) ( // Expose: // Ips: // Cidr: 0.0.0.0/0 -func bgpExternalMasqueradeNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func bgpExternalMasqueradeNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.extName == "" { return true, nil, fmt.Errorf("no BGP external available for testing") //nolint:goerr113 } @@ -315,13 +299,23 @@ func bgpExternalMasqueradeNATTest(ctx context.Context, testCtx *VPCPeeringTestCt return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, bgpNATCIDR); err != nil { return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - if err := testCtx.testNATExternalConnectivity(ctx, vpc, testCtx.extName); err != nil { + if err := overlayExternalSNAT(matrix, vpc.Name, testCtx.extName, bgpNATCIDR); err != nil { + return false, nil, fmt.Errorf("annotating matrix with BGP masquerade SNAT pool: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing BGP external masquerade NAT connectivity: %w", err) } + if err := testCtx.pingExternalStability(ctx, matrix, vpc.Name, testCtx.extName); err != nil { + return false, nil, fmt.Errorf("BGP external masquerade NAT ping stability: %w", err) + } return false, nil, nil } @@ -352,7 +346,7 @@ func bgpExternalMasqueradeNATTest(ctx context.Context, testCtx *VPCPeeringTestCt // - Protocol: TCP // Port: 5201 // As: 15201 -func bgpExternalPortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func bgpExternalPortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.extName == "" { return true, nil, fmt.Errorf("no BGP external available for testing") //nolint:goerr113 } @@ -418,11 +412,18 @@ func bgpExternalPortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestC return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, bgpNATCIDR); err != nil { return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - if err := testCtx.testIperfToExternal(ctx, vpc, bgpInvertedNATCIDR); err != nil { + if err := overlayExternalPortForward(matrix, vpc.Name, testCtx.extName, netip.AddrFrom4(b), 15201); err != nil { + return false, nil, fmt.Errorf("overlaying BGP external port-forward DNAT: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing BGP external port-forward via iperf3: %w", err) } @@ -456,7 +457,7 @@ func bgpExternalPortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestC // Expose: // Ips: // Cidr: 0.0.0.0/0 -func bgpExternalMasqueradePortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func bgpExternalMasqueradePortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.extName == "" { return true, nil, fmt.Errorf("no BGP external available for testing") //nolint:goerr113 } @@ -506,13 +507,26 @@ func bgpExternalMasqueradePortForwardNATTest(ctx context.Context, testCtx *VPCPe return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, bgpNATCIDR); err != nil { return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - if err := testCtx.testNATExternalConnectivity(ctx, vpc, testCtx.extName); err != nil { + // Only the outbound (VPC→ext via masquerade) direction is exercised + // here. The inbound port-forward (ext→VPC on 15201→5201) requires SSH + // to the external device, which the matrix doesn't model. + if err := overlayExternalSNAT(matrix, vpc.Name, testCtx.extName, bgpNATCIDR); err != nil { + return false, nil, fmt.Errorf("annotating matrix with BGP masquerade SNAT pool: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing BGP external masquerade+port-forward NAT connectivity: %w", err) } + if err := testCtx.pingExternalStability(ctx, matrix, vpc.Name, testCtx.extName); err != nil { + return false, nil, fmt.Errorf("BGP external masquerade+port-forward NAT ping stability: %w", err) + } return false, nil, nil } @@ -530,7 +544,7 @@ func bgpExternalMasqueradePortForwardNATTest(ctx context.Context, testCtx *VPCPe // Expose: // Ips: // Cidr: 0.0.0.0/0 -func staticExternalNoNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func staticExternalNoNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.staticExtName == "" { return true, nil, fmt.Errorf("no static external available for testing") //nolint:goerr113 } @@ -563,7 +577,11 @@ func staticExternalNoNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTest return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } - if err := DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts); err != nil { + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing static external connectivity: %w", err) } @@ -587,7 +605,7 @@ func staticExternalNoNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTest // Expose: // Ips: // Cidr: 0.0.0.0/0 -func staticExternalStaticNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func staticExternalStaticNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.staticExtName == "" { return true, nil, fmt.Errorf("no static external available for testing") //nolint:goerr113 } @@ -633,13 +651,23 @@ func staticExternalStaticNATGatewayTest(ctx context.Context, testCtx *VPCPeering return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, staticNATCIDR); err != nil { return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - if err := testCtx.testNATExternalConnectivity(ctx, vpc, testCtx.staticExtName); err != nil { + if err := overlayExternalSNAT(matrix, vpc.Name, testCtx.staticExtName, staticNATCIDR); err != nil { + return false, nil, fmt.Errorf("annotating matrix with static SNAT pool: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing static external static NAT connectivity: %w", err) } + if err := testCtx.pingExternalStability(ctx, matrix, vpc.Name, testCtx.staticExtName); err != nil { + return false, nil, fmt.Errorf("static external static NAT ping stability: %w", err) + } return false, nil, nil } @@ -662,7 +690,7 @@ func staticExternalStaticNATGatewayTest(ctx context.Context, testCtx *VPCPeering // Expose: // Ips: // Cidr: 0.0.0.0/0 -func staticExternalMasqueradeNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func staticExternalMasqueradeNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.staticExtName == "" { return true, nil, fmt.Errorf("no static external available for testing") //nolint:goerr113 } @@ -708,77 +736,25 @@ func staticExternalMasqueradeNATGatewayTest(ctx context.Context, testCtx *VPCPee return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } - if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, staticNATCIDR); err != nil { - return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) } - if err := testCtx.testNATExternalConnectivity(ctx, vpc, testCtx.staticExtName); err != nil { - return false, nil, fmt.Errorf("testing static external masquerade NAT connectivity: %w", err) + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, staticNATCIDR); err != nil { + return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - return false, nil, nil -} - -// testIperfToExternal runs iperf3 from a VPC server to invertedNATCIDR:15201, testing -// connectivity through the gateway's inverted port-forward NAT (external side has NAT). -// One server in the VPC is sufficient. Callers are expected to gate this on -// waitForNATPoolInLeaves first when the test depends on a freshly-applied NAT pool route, so any -// failure here is a real connectivity failure rather than a route-propagation race. -func (testCtx *VPCPeeringTestCtx) testIperfToExternal(ctx context.Context, vpc *vpcapi.VPC, invertedNATCIDR string) error { - servers := &wiringapi.ServerList{} - if err := testCtx.kube.List(ctx, servers); err != nil { - return fmt.Errorf("listing servers: %w", err) + if err := overlayExternalSNAT(matrix, vpc.Name, testCtx.staticExtName, staticNATCIDR); err != nil { + return false, nil, fmt.Errorf("annotating matrix with static masquerade SNAT pool: %w", err) } - - extNATIP := strings.SplitN(invertedNATCIDR, "/", 2)[0] - secs := testCtx.tcOpts.IPerfsSeconds - if secs <= 0 { - secs = 5 + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { + return false, nil, fmt.Errorf("testing static external masquerade NAT connectivity: %w", err) } - - for _, server := range servers.Items { - attachedSubnets, err := apiutil.GetAttachedSubnets(ctx, testCtx.kube, server.Name) - if err != nil { - continue - } - - inVPC := false - for subnetName := range attachedSubnets { - if strings.HasPrefix(subnetName, vpc.Name+"/") { - inVPC = true - - break - } - } - if !inVPC { - continue - } - - sshCfg, err := testCtx.getSSH(ctx, server.Name) - if err != nil { - return fmt.Errorf("getting ssh config for %s: %w", server.Name, err) - } - - // Gate iperf3 on TCP reachability: the gateway's port-forward DNAT rule has its own - // programming lag separate from fabric route propagation, and probing a TCP handshake - // is the precise signal that both halves of the path (fabric route + gateway DNAT) are - // active. After the probe succeeds, iperf3 runs once and any failure is a real test - // failure. - if err := testCtx.waitForPortForwardReachable(ctx, sshCfg, server.Name, extNATIP, 15201); err != nil { - return fmt.Errorf("waiting for port-forward target reachability: %w", err) - } - - cmd := fmt.Sprintf("toolbox -E LD_PRELOAD=/lib/x86_64-linux-gnu/libgcc_s.so.1 -q timeout %d iperf3 -J -c %s -p 15201 -t %d", - secs+25, extNATIP, secs) - slog.Debug("Testing iperf3 through inverted port-forward NAT", "server", server.Name, "target", extNATIP+":15201") - if _, _, iperfErr := retrySSHCmd(ctx, sshCfg, cmd, server.Name); iperfErr != nil { - return fmt.Errorf("iperf3 from %s to %s:15201: %w", server.Name, extNATIP, iperfErr) - } - - return nil + if err := testCtx.pingExternalStability(ctx, matrix, vpc.Name, testCtx.staticExtName); err != nil { + return false, nil, fmt.Errorf("static external masquerade NAT ping stability: %w", err) } - return fmt.Errorf("no servers found in VPC %s for iperf3 test", vpc.Name) //nolint:goerr113 + return false, nil, nil } // Test gateway static external peering with port-forward NAT (inverted: VPC→external). @@ -805,7 +781,7 @@ func (testCtx *VPCPeeringTestCtx) testIperfToExternal(ctx context.Context, vpc * // - Protocol: TCP // Port: 5201 // As: 15201 -func staticExternalPortForwardNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func staticExternalPortForwardNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.staticExtName == "" { return true, nil, fmt.Errorf("no static external available for testing") //nolint:goerr113 } @@ -871,11 +847,18 @@ func staticExternalPortForwardNATGatewayTest(ctx context.Context, testCtx *VPCPe return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, staticNATCIDR); err != nil { return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - if err := testCtx.testIperfToExternal(ctx, vpc, staticInvertedNATCIDR); err != nil { + if err := overlayExternalPortForward(matrix, vpc.Name, testCtx.staticExtName, netip.AddrFrom4(b), 15201); err != nil { + return false, nil, fmt.Errorf("overlaying static external port-forward DNAT: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing static external port-forward via iperf3: %w", err) } @@ -909,7 +892,7 @@ func staticExternalPortForwardNATGatewayTest(ctx context.Context, testCtx *VPCPe // Expose: // Ips: // Cidr: 0.0.0.0/0 -func staticExternalMasqueradePortForwardNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func staticExternalMasqueradePortForwardNATGatewayTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.staticExtName == "" { return true, nil, fmt.Errorf("no static external available for testing") //nolint:goerr113 } @@ -959,13 +942,26 @@ func staticExternalMasqueradePortForwardNATGatewayTest(ctx context.Context, test return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("refreshing matrix after peerings: %w", err) + } + if err := testCtx.waitForNATPoolInLeaves(ctx, vpc, staticNATCIDR); err != nil { return false, nil, fmt.Errorf("waiting for NAT pool route to propagate: %w", err) } - if err := testCtx.testNATExternalConnectivity(ctx, vpc, testCtx.staticExtName); err != nil { + // Only the outbound (VPC→ext via masquerade) direction is exercised + // here. The inbound port-forward (ext→VPC on 15201→5201) requires SSH + // to the external device, which the matrix doesn't model. + if err := overlayExternalSNAT(matrix, vpc.Name, testCtx.staticExtName, staticNATCIDR); err != nil { + return false, nil, fmt.Errorf("annotating matrix with static masquerade SNAT pool: %w", err) + } + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { return false, nil, fmt.Errorf("testing static external masquerade+port-forward NAT connectivity: %w", err) } + if err := testCtx.pingExternalStability(ctx, matrix, vpc.Name, testCtx.staticExtName); err != nil { + return false, nil, fmt.Errorf("static external masquerade+port-forward NAT ping stability: %w", err) + } return false, nil, nil } diff --git a/pkg/hhfab/rt_nat_tests.go b/pkg/hhfab/rt_nat_tests.go index 25c3a6649..f6eccdcef 100644 --- a/pkg/hhfab/rt_nat_tests.go +++ b/pkg/hhfab/rt_nat_tests.go @@ -10,24 +10,88 @@ import ( "log/slog" "net/netip" "sort" - "strings" "time" gwapi "go.githedgehog.com/fabric/api/gateway/v1alpha1" vpcapi "go.githedgehog.com/fabric/api/vpc/v1beta1" wiringapi "go.githedgehog.com/fabric/api/wiring/v1beta1" - "go.githedgehog.com/fabric/pkg/util/apiutil" "go.githedgehog.com/fabricator/pkg/util/sshutil" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kclient "sigs.k8s.io/controller-runtime/pkg/client" ) -// excludedInterfaces contains interface names to skip when discovering server IPs. -// These are system interfaces that don't carry VPC traffic. -var excludedInterfaces = map[string]bool{ - "lo": true, // loopback - "enp2s0": true, // management interface - "docker0": true, // docker bridge +// peeringSpecs bundles the three peering kinds a NAT test may need to +// install. Returned by natTestSpec.BuildSpec. +type peeringSpecs struct { + VPC map[string]*vpcapi.VPCPeeringSpec + External map[string]*vpcapi.ExternalPeeringSpec + Gateway map[string]*gwapi.PeeringSpec +} + +// emptyPeeringSpecs returns an initialized peeringSpecs that BuildSpec +// callbacks can populate without manually constructing each map. +func emptyPeeringSpecs() peeringSpecs { + return peeringSpecs{ + VPC: make(map[string]*vpcapi.VPCPeeringSpec), + External: make(map[string]*vpcapi.ExternalPeeringSpec), + Gateway: make(map[string]*gwapi.PeeringSpec), + } +} + +// natTestSpec describes a VPC-to-VPC NAT test that follows the standard +// driver shape: pick the first two VPCs (sorted alphabetically), build +// peerings, refresh the matrix, optionally overlay NAT info, then run the +// matrix-driven connectivity test. Tests that need a server move (overlap) +// or external CRD annotation lookup hand-roll instead. +type natTestSpec struct { + Name string + BuildSpec func(vpc1, vpc2 *vpcapi.VPC) (peeringSpecs, error) + Overlay func(vpc1, vpc2 *vpcapi.VPC, matrix *ConnectivityMatrix) error +} + +// runNATTest executes the standard NAT-test sequence defined by spec. +// Returns (skip=true) when fewer than two VPCs are available so the suite +// can mark the case as skipped. +func (testCtx *VPCPeeringTestCtx) runNATTest(ctx context.Context, matrix *ConnectivityMatrix, spec natTestSpec) (bool, []RevertFunc, error) { + vpcs := &vpcapi.VPCList{} + if err := testCtx.kube.List(ctx, vpcs); err != nil { + return false, nil, fmt.Errorf("listing VPCs: %w", err) + } + if len(vpcs.Items) < 2 { + return true, nil, fmt.Errorf("not enough VPCs for %s test", spec.Name) //nolint:goerr113 + } + sort.Slice(vpcs.Items, func(i, j int) bool { + return vpcs.Items[i].Name < vpcs.Items[j].Name + }) + vpc1 := &vpcs.Items[0] + vpc2 := &vpcs.Items[1] + + specs, err := spec.BuildSpec(vpc1, vpc2) + if err != nil { + return false, nil, fmt.Errorf("%s: building peering spec: %w", spec.Name, err) + } + + if err := DoSetupPeerings(ctx, testCtx.kube, specs.VPC, specs.External, specs.Gateway, true); err != nil { + return false, nil, fmt.Errorf("%s: setting up peerings: %w", spec.Name, err) + } + if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { + return false, nil, fmt.Errorf("%s: waiting for switches: %w", spec.Name, err) + } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, nil, fmt.Errorf("%s: refreshing matrix: %w", spec.Name, err) + } + + if spec.Overlay != nil { + if err := spec.Overlay(vpc1, vpc2, matrix); err != nil { + return false, nil, fmt.Errorf("%s: applying NAT overlay: %w", spec.Name, err) + } + } + + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { + return false, nil, fmt.Errorf("%s: testing connectivity: %w", spec.Name, err) + } + + return false, nil, nil } // calculateStaticNATIP calculates the expected NAT IP for a source IP using the static NAT offset algorithm. @@ -62,250 +126,132 @@ func calculateStaticNATIP(sourceIP, sourceSubnet, natPoolStart netip.Addr) (neti return netip.AddrFrom4(natIPBytes), nil } -// testNATGatewayConnectivity performs E2E connectivity testing for NAT gateway peering. -// It discovers server IPs, calculates expected NAT IPs, and performs ping/iperf3 tests -// using the shared checkPing and checkIPerf functions from testing.go. -// NOTE: This uses calculateStaticNATIP which couples to the dataplane NAT algorithm. -// The function supports both source NAT and destination NAT: -// - If vpc2NATPool is set: vpc1 pings vpc2 using vpc2's NAT IPs (destination NAT) -// - If vpc2NATPool is empty: vpc1 pings vpc2's real IPs (source NAT on vpc1 side) -// - If vpc1NATPool is set: also test vpc2 -> vpc1 direction (bidirectional NAT) -func (testCtx *VPCPeeringTestCtx) testNATGatewayConnectivity( - ctx context.Context, - vpc1, vpc2 *vpcapi.VPC, - vpc1NATPool, vpc2NATPool []string, -) error { - startTime := time.Now() - slog.Info("Testing NAT gateway peering connectivity") - - // Validate NAT pool parameters - we only support a single CIDR per VPC for now - if len(vpc1NATPool) > 1 || len(vpc2NATPool) > 1 { - return fmt.Errorf("multiple NAT CIDRs per VPC not supported, got vpc1=%d vpc2=%d", len(vpc1NATPool), len(vpc2NATPool)) //nolint:goerr113 +// overlayVPCToVPCStaticDNAT annotates every (server-in-srcVPCName → +// server-in-dstVPCName) matrix entry with the destination's static-NAT +// pool IP (computed from dst.Server.IP, dstSubnetCIDR, and dstNATPoolCIDR). +// The matrix-driven runner then targets the NAT IP instead of the real +// one for both ping and iperf3, matching what the gateway expects to see. +func overlayVPCToVPCStaticDNAT(matrix *ConnectivityMatrix, srcVPCName, dstVPCName, dstSubnetCIDR, dstNATPoolCIDR string) error { + subnetPrefix, err := netip.ParsePrefix(dstSubnetCIDR) + if err != nil { + return fmt.Errorf("parsing dst subnet %s: %w", dstSubnetCIDR, err) } - - servers := &wiringapi.ServerList{} - if err := testCtx.kube.List(ctx, servers); err != nil { - return fmt.Errorf("listing servers: %w", err) + poolPrefix, err := netip.ParsePrefix(dstNATPoolCIDR) + if err != nil { + return fmt.Errorf("parsing dst NAT pool %s: %w", dstNATPoolCIDR, err) } + subnetStart := subnetPrefix.Masked().Addr() + poolStart := poolPrefix.Masked().Addr() - // Get servers attached to each VPC - vpc1Servers := []string{} - vpc2Servers := []string{} - - for _, server := range servers.Items { - attachedSubnets, err := apiutil.GetAttachedSubnets(ctx, testCtx.kube, server.Name) + return OverlayMatrixNAT(matrix, ServerInVPC(srcVPCName), ServerInVPC(dstVPCName), func(_, dst *Endpoint, nat *TranslatedAddress) error { + if !dst.Server.IP.IsValid() { + return fmt.Errorf("matrix endpoint for %s has no IP", dst.Server.Name) //nolint:goerr113 + } + natIP, err := calculateStaticNATIP(dst.Server.IP, subnetStart, poolStart) if err != nil { - continue + return fmt.Errorf("calculating NAT IP for %s: %w", dst.Server.Name, err) } + nat.DestinationIP = natIP - for subnetName := range attachedSubnets { - if strings.HasPrefix(subnetName, vpc1.Name+"/") { - vpc1Servers = append(vpc1Servers, server.Name) - - break - } - if strings.HasPrefix(subnetName, vpc2.Name+"/") { - vpc2Servers = append(vpc2Servers, server.Name) + return nil + }) +} - break - } - } +// overlayVPCToVPCPortForwardDNAT marks every (server-in-srcVPCName → +// server-in-dstVPCName) entry with a port-forward DNAT to the destination's +// NAT IP on destPort. The matrix tester then runs iperf3 against +// (NAT IP, destPort) without ping for those pairs. +func overlayVPCToVPCPortForwardDNAT(matrix *ConnectivityMatrix, srcVPCName, dstVPCName, dstSubnetCIDR, dstNATPoolCIDR string, destPort uint16) error { + if destPort == 0 { + return fmt.Errorf("destPort must be non-zero for port-forward overlay") //nolint:goerr113 } - - if len(vpc1Servers) == 0 || len(vpc2Servers) == 0 { - return fmt.Errorf("need servers in both VPCs for NAT connectivity test") //nolint:err113 + subnetPrefix, err := netip.ParsePrefix(dstSubnetCIDR) + if err != nil { + return fmt.Errorf("parsing dst subnet %s: %w", dstSubnetCIDR, err) } + poolPrefix, err := netip.ParsePrefix(dstNATPoolCIDR) + if err != nil { + return fmt.Errorf("parsing dst NAT pool %s: %w", dstNATPoolCIDR, err) + } + subnetStart := subnetPrefix.Masked().Addr() + poolStart := poolPrefix.Masked().Addr() - slog.Debug("Found servers for NAT test", "vpc1", vpc1.Name, "servers", vpc1Servers, "vpc2", vpc2.Name, "servers", vpc2Servers) - - // Get SSH configs for servers - sshConfigs := map[string]*sshutil.Config{} - for _, serverName := range append(vpc1Servers, vpc2Servers...) { - // Find VM by name - var vm VM - found := false - for _, v := range testCtx.vlab.VMs { - if v.Name == serverName { - vm = v - found = true - - break - } - } - if !found { - return fmt.Errorf("VM not found for server %s", serverName) //nolint:err113 + return OverlayMatrixNAT(matrix, ServerInVPC(srcVPCName), ServerInVPC(dstVPCName), func(_, dst *Endpoint, nat *TranslatedAddress) error { + if !dst.Server.IP.IsValid() { + return fmt.Errorf("matrix endpoint for %s has no IP", dst.Server.Name) //nolint:goerr113 } - - sshCfg, err := testCtx.vlabCfg.SSHVM(ctx, testCtx.vlab, vm) + natIP, err := calculateStaticNATIP(dst.Server.IP, subnetStart, poolStart) if err != nil { - return fmt.Errorf("getting ssh config for %s: %w", serverName, err) + return fmt.Errorf("calculating NAT IP for %s: %w", dst.Server.Name, err) } + nat.DestinationIP = natIP + nat.DestinationPort = destPort - sshConfigs[serverName] = sshCfg - } + return nil + }) +} - // Discover server IPs - serverIPs := map[string]netip.Addr{} - for _, serverName := range append(vpc1Servers, vpc2Servers...) { - sshCfg := sshConfigs[serverName] - stdout, stderr, err := sshCfg.Run(ctx, "ip -o -4 addr show | awk '{print $2, $4}'") - if err != nil { - return fmt.Errorf("getting IP for %s: %w: %s", serverName, err, stderr) +// overrideVPCToVPCVerdict forces every (server-in-srcVPCName → +// server-in-dstVPCName) entry to the given verdict. +// Used to mark direction-asymmetric paths. +func overrideVPCToVPCVerdict(matrix *ConnectivityMatrix, srcVPCName, dstVPCName string, verdict ConnectivityVerdict) { + srcPred := ServerInVPC(srcVPCName) + dstPred := ServerInVPC(dstVPCName) + for _, src := range matrix.AllEndpoints { + if !srcPred(src) { + continue } - - var eligibleAddrs []netip.Addr - for _, line := range strings.Split(strings.TrimSpace(stdout), "\n") { - fields := strings.Fields(line) - if len(fields) != 2 { + for _, dst := range matrix.AllEndpoints { + if !dstPred(dst) { continue } - if excludedInterfaces[fields[0]] { - continue - } - - addr, err := netip.ParsePrefix(fields[1]) - if err != nil { - continue - } - - eligibleAddrs = append(eligibleAddrs, addr.Addr()) - } - - if len(eligibleAddrs) == 0 { - return fmt.Errorf("no IP found for server %s", serverName) //nolint:err113 - } - if len(eligibleAddrs) > 1 { - return fmt.Errorf("server %s has multiple IPs %v, NAT test requires single IP", serverName, eligibleAddrs) //nolint:err113 - } - serverIPs[serverName] = eligibleAddrs[0] - slog.Debug("Discovered server IP", "server", serverName, "ip", eligibleAddrs[0].String()) - } - - // Parse NAT pools - use Masked() to get the network address - var vpc1NATPoolStart, vpc2NATPoolStart netip.Addr - if len(vpc1NATPool) > 0 { - prefix, err := netip.ParsePrefix(vpc1NATPool[0]) - if err != nil { - return fmt.Errorf("parsing vpc1 NAT pool: %w", err) + existing := matrix.Lookup(src, dst, ProtoPort{}) + matrix.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: src, Destination: dst}, + Verdict: verdict, + Reason: ReachabilityReasonGatewayPeering, + Peering: existing.Peering, + NAT: existing.NAT, + }) } - vpc1NATPoolStart = prefix.Masked().Addr() - } - if len(vpc2NATPool) > 0 { - prefix, err := netip.ParsePrefix(vpc2NATPool[0]) - if err != nil { - return fmt.Errorf("parsing vpc2 NAT pool: %w", err) - } - vpc2NATPoolStart = prefix.Masked().Addr() } +} - // Get VPC subnet starts for offset calculation - // NAT test requires exactly one subnet per VPC to avoid ambiguity in offset calculation - if len(vpc1.Spec.Subnets) != 1 { - return fmt.Errorf("VPC %s has %d subnets, NAT test requires exactly one", vpc1.Name, len(vpc1.Spec.Subnets)) //nolint:err113 - } - if len(vpc2.Spec.Subnets) != 1 { - return fmt.Errorf("VPC %s has %d subnets, NAT test requires exactly one", vpc2.Name, len(vpc2.Spec.Subnets)) //nolint:err113 - } - var vpc1SubnetStart, vpc2SubnetStart netip.Addr - for _, subnet := range vpc1.Spec.Subnets { - prefix, err := netip.ParsePrefix(subnet.Subnet) - if err != nil { - return fmt.Errorf("parsing VPC %s subnet %s: %w", vpc1.Name, subnet.Subnet, err) - } - vpc1SubnetStart = prefix.Masked().Addr() - } - for _, subnet := range vpc2.Spec.Subnets { - prefix, err := netip.ParsePrefix(subnet.Subnet) - if err != nil { - return fmt.Errorf("parsing VPC %s subnet %s: %w", vpc2.Name, subnet.Subnet, err) - } - vpc2SubnetStart = prefix.Masked().Addr() +// rebindMatrixServerEndpoint refreshes the matrix's endpoint(s) for +// serverName to reflect the current cluster state. Used after a runtime +// attachment change (e.g., the overlap NAT test moves one server to a +// freshly created overlap VPC) so VPC-keyed overlays match the moved +// server. The (vpc, subnet) the server now belongs to is read from the +// VPCAttachment CRDs by CollectServerEndpoints — no need for the caller +// to restate it. Multi-IP servers produce multiple endpoints. +func (testCtx *VPCPeeringTestCtx) rebindMatrixServerEndpoint(ctx context.Context, matrix *ConnectivityMatrix, serverName string) error { + ssh := func(name string) (*sshutil.Config, error) { + return testCtx.getSSH(ctx, name) + } + newEPs, err := CollectServerEndpoints(ctx, testCtx.kube, ssh, []string{serverName}) + if err != nil { + return fmt.Errorf("collecting endpoints for %s: %w", serverName, err) } - - // Helper to calculate destination IP (NAT IP if pool configured, real IP otherwise) - getDestIP := func(serverName string, destSubnetStart, natPoolStart netip.Addr) (netip.Addr, error) { - realIP := serverIPs[serverName] - if natPoolStart.IsValid() { - natIP, err := calculateStaticNATIP(realIP, destSubnetStart, natPoolStart) - if err != nil { - return netip.Addr{}, fmt.Errorf("calculating NAT IP for %s: %w", serverName, err) - } - slog.Debug("Using NAT IP", "server", serverName, "real", realIP, "nat", natIP) - - return natIP, nil - } - - return realIP, nil + if len(newEPs) == 0 { + return fmt.Errorf("no endpoints discovered for server %s after rebind", serverName) //nolint:goerr113 } + matrix.ReplaceServerEndpoints(serverName, newEPs) - // Helper to test connectivity in one direction - testDirection := func(label string, fromServers, toServers []string, toSubnetStart, toNATPoolStart netip.Addr) error { - slog.Debug("Testing NAT connectivity", "direction", label) - - // Ping tests - var pingErrors []*PingError - for _, serverA := range fromServers { - for _, serverB := range toServers { - destIP, err := getDestIP(serverB, toSubnetStart, toNATPoolStart) - if err != nil { - return err - } - if pe := checkPing(ctx, testCtx.tcOpts.PingsCount, nil, serverA, serverB, sshConfigs[serverA], destIP, nil, true); pe != nil { - pingErrors = append(pingErrors, pe) - } - } - } - if len(pingErrors) > 0 { - var errMsgs []string - for _, pe := range pingErrors { - errMsgs = append(errMsgs, pe.Error()) - } - - return fmt.Errorf("NAT ping test (%s) failed with %d errors: %s", label, len(pingErrors), strings.Join(errMsgs, "; ")) //nolint:goerr113 - } - - // Iperf tests - slog.Debug("NAT ping tests completed, starting iperf3 tests", "direction", label) - reachability := Reachability{Reachable: true, Reason: ReachabilityReasonGatewayPeering} - var iperfErrors []*IperfError - for _, serverA := range fromServers { - for _, serverB := range toServers { - destIP, err := getDestIP(serverB, toSubnetStart, toNATPoolStart) - if err != nil { - return err - } - if ies := checkIPerf(ctx, testCtx.tcOpts, serverA, serverB, sshConfigs[serverA], destIP, reachability, false); len(ies) > 0 { - iperfErrors = append(iperfErrors, ies...) - } - } - } - if len(iperfErrors) > 0 { - var errMsgs []string - for _, ie := range iperfErrors { - errMsgs = append(errMsgs, ie.Error()) - } - - return fmt.Errorf("NAT iperf3 test (%s) failed with %d errors: %s", label, len(iperfErrors), strings.Join(errMsgs, "; ")) //nolint:goerr113 - } - - return nil - } + return nil +} - // Test vpc1 -> vpc2 direction (always) - if err := testDirection("vpc1->vpc2", vpc1Servers, vpc2Servers, vpc2SubnetStart, vpc2NATPoolStart); err != nil { - return err +// vpcFirstSubnetCIDR returns the (only) subnet CIDR for a VPC used in the +// NAT tests. The NAT overlays require exactly one subnet per VPC because +// the static-NAT offset algorithm is unambiguous only then. +func vpcFirstSubnetCIDR(vpc *vpcapi.VPC) (string, error) { + if len(vpc.Spec.Subnets) != 1 { + return "", fmt.Errorf("VPC %s has %d subnets, NAT test requires exactly one", vpc.Name, len(vpc.Spec.Subnets)) //nolint:goerr113 } - - // Test vpc2 -> vpc1 direction (only if vpc1 has static NAT - masquerade doesn't support being a destination) - if vpc1NATPoolStart.IsValid() { - if err := testDirection("vpc2->vpc1", vpc2Servers, vpc1Servers, vpc1SubnetStart, vpc1NATPoolStart); err != nil { - return err - } + for _, subnet := range vpc.Spec.Subnets { + return subnet.Subnet, nil } - slog.Info("NAT connectivity test (ping+iperf3) completed successfully", "took", time.Since(startTime)) - - return nil + return "", fmt.Errorf("VPC %s has empty subnet map", vpc.Name) //nolint:goerr113 } // Test gateway peering with masquerade source NAT (only VPC1 has masquerade NAT configured) @@ -329,53 +275,29 @@ func (testCtx *VPCPeeringTestCtx) testNATGatewayConnectivity( // Cidr: 10.50.2.0/24 // // NOTE: Masquerade NAT on both sides of a peering is not supported (see dataplane#1248) -func gatewayPeeringMasqueradeSourceNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { - vpcs := &vpcapi.VPCList{} - if err := testCtx.kube.List(ctx, vpcs); err != nil { - return false, nil, fmt.Errorf("listing VPCs: %w", err) - } - if len(vpcs.Items) < 2 { - return true, nil, fmt.Errorf("not enough VPCs for NAT gateway peering test") //nolint:goerr113 - } - - // Sort VPCs to ensure consistent selection - sort.Slice(vpcs.Items, func(i, j int) bool { - return vpcs.Items[i].Name < vpcs.Items[j].Name +func gatewayPeeringMasqueradeSourceNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { + return testCtx.runNATTest(ctx, matrix, natTestSpec{ + Name: "gateway masquerade source NAT", + BuildSpec: func(vpc1, vpc2 *vpcapi.VPC) (peeringSpecs, error) { + specs := emptyPeeringSpecs() + err := appendGwPeeringSpec(specs.Gateway, vpc1, vpc2, &GwPeeringOptions{ + VPC1NATCIDR: []string{"192.168.11.0/24"}, + VPC1NATMode: NATModeMasquerade, + }) + + return specs, err + }, + // vpc1→vpc2 works via masquerade SNAT (vpc2 sees real IPs as dst, + // no DNAT overlay needed — we just assert Allow). vpc2→vpc1 is + // blocked: masquerade is stateful and doesn't accept unsolicited + // inbound traffic on the NAT pool. + Overlay: func(vpc1, vpc2 *vpcapi.VPC, matrix *ConnectivityMatrix) error { + overrideVPCToVPCVerdict(matrix, vpc1.Name, vpc2.Name, VerdictAllow) + overrideVPCToVPCVerdict(matrix, vpc2.Name, vpc1.Name, VerdictDeny) + + return nil + }, }) - - vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec) - externalPeerings := make(map[string]*vpcapi.ExternalPeeringSpec) - gwPeerings := make(map[string]*gwapi.PeeringSpec) - - vpc1 := &vpcs.Items[0] - vpc2 := &vpcs.Items[1] - - // Only VPC1 has masquerade NAT - VPC1's traffic will be source-NATed - vpc1NATCIDR := []string{"192.168.11.0/24"} - - if err := appendGwPeeringSpec(gwPeerings, vpc1, vpc2, &GwPeeringOptions{ - VPC1NATCIDR: vpc1NATCIDR, - VPC1NATMode: NATModeMasquerade, - }); err != nil { - return false, nil, fmt.Errorf("setting up gateway peering: %w", err) - } - - if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, gwPeerings, true); err != nil { - return false, nil, fmt.Errorf("setting up NAT gateway peerings: %w", err) - } - - if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { - return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) - } - - // Test connectivity - VPC2 has no NAT, so we ping real IPs - // Note: For masquerade NAT, we only test VPC1 -> VPC2 direction (pass nil for vpc1NATPool) - // because masquerade is stateful and doesn't support VPC2 initiating connections to VPC1's NAT IPs - if err := testCtx.testNATGatewayConnectivity(ctx, vpc1, vpc2, nil, nil); err != nil { - return false, nil, fmt.Errorf("testing NAT gateway peering connectivity: %w", err) - } - - return false, nil, nil } // Test gateway peering with static source NAT (only VPC1 has NAT configured) @@ -397,49 +319,34 @@ func gatewayPeeringMasqueradeSourceNATTest(ctx context.Context, testCtx *VPCPeer // Expose: // Ips: // Cidr: 10.50.2.0/24 -func gatewayPeeringStaticSourceNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { - vpcs := &vpcapi.VPCList{} - if err := testCtx.kube.List(ctx, vpcs); err != nil { - return false, nil, fmt.Errorf("listing VPCs: %w", err) - } - if len(vpcs.Items) < 2 { - return true, nil, fmt.Errorf("not enough VPCs for static source NAT test") //nolint:goerr113 - } +func gatewayPeeringStaticSourceNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { + vpc1NATCIDR := "192.168.21.0/24" + + return testCtx.runNATTest(ctx, matrix, natTestSpec{ + Name: "gateway static source NAT", + BuildSpec: func(vpc1, vpc2 *vpcapi.VPC) (peeringSpecs, error) { + specs := emptyPeeringSpecs() + err := appendGwPeeringSpec(specs.Gateway, vpc1, vpc2, &GwPeeringOptions{ + VPC1NATCIDR: []string{vpc1NATCIDR}, + }) + + return specs, err + }, + // vpc1→vpc2 uses vpc2's real IPs (no NAT on vpc2) — populate + // can't see this Allow because the peering carries 'As', so we + // assert it explicitly. vpc2→vpc1 must target vpc1's NAT pool + // addresses: static NAT is bidirectional and the gateway only + // knows vpc1 by its NAT IPs from vpc2's side. + Overlay: func(vpc1, vpc2 *vpcapi.VPC, matrix *ConnectivityMatrix) error { + overrideVPCToVPCVerdict(matrix, vpc1.Name, vpc2.Name, VerdictAllow) + vpc1SubnetCIDR, err := vpcFirstSubnetCIDR(vpc1) + if err != nil { + return err + } - sort.Slice(vpcs.Items, func(i, j int) bool { - return vpcs.Items[i].Name < vpcs.Items[j].Name + return overlayVPCToVPCStaticDNAT(matrix, vpc2.Name, vpc1.Name, vpc1SubnetCIDR, vpc1NATCIDR) + }, }) - - vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec) - externalPeerings := make(map[string]*vpcapi.ExternalPeeringSpec) - gwPeerings := make(map[string]*gwapi.PeeringSpec) - - vpc1 := &vpcs.Items[0] - vpc2 := &vpcs.Items[1] - - // Only VPC1 has NAT - this means VPC1's traffic will be source-NATed - vpc1NATCIDR := []string{"192.168.21.0/24"} - - if err := appendGwPeeringSpec(gwPeerings, vpc1, vpc2, &GwPeeringOptions{ - VPC1NATCIDR: vpc1NATCIDR, - }); err != nil { - return false, nil, fmt.Errorf("setting up gateway peering: %w", err) - } - - if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, gwPeerings, true); err != nil { - return false, nil, fmt.Errorf("setting up static source NAT peerings: %w", err) - } - - if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { - return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) - } - - // Test connectivity - VPC2 has no NAT, so we ping real IPs - if err := testCtx.testNATGatewayConnectivity(ctx, vpc1, vpc2, vpc1NATCIDR, nil); err != nil { - return false, nil, fmt.Errorf("testing static source NAT connectivity: %w", err) - } - - return false, nil, nil } // Test gateway peering with bidirectional static NAT (both VPCs have NAT configured) @@ -465,50 +372,37 @@ func gatewayPeeringStaticSourceNATTest(ctx context.Context, testCtx *VPCPeeringT // Cidr: 10.50.2.0/24 // Nat: // Static: -func gatewayPeeringBidirectionalStaticNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { - vpcs := &vpcapi.VPCList{} - if err := testCtx.kube.List(ctx, vpcs); err != nil { - return false, nil, fmt.Errorf("listing VPCs: %w", err) - } - if len(vpcs.Items) < 2 { - return true, nil, fmt.Errorf("not enough VPCs for bidirectional NAT test") //nolint:goerr113 - } +func gatewayPeeringBidirectionalStaticNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { + vpc1NATCIDR := "192.168.31.0/24" + vpc2NATCIDR := "192.168.32.0/24" + + return testCtx.runNATTest(ctx, matrix, natTestSpec{ + Name: "gateway bidirectional static NAT", + BuildSpec: func(vpc1, vpc2 *vpcapi.VPC) (peeringSpecs, error) { + specs := emptyPeeringSpecs() + err := appendGwPeeringSpec(specs.Gateway, vpc1, vpc2, &GwPeeringOptions{ + VPC1NATCIDR: []string{vpc1NATCIDR}, + VPC2NATCIDR: []string{vpc2NATCIDR}, + }) + + return specs, err + }, + Overlay: func(vpc1, vpc2 *vpcapi.VPC, matrix *ConnectivityMatrix) error { + vpc1SubnetCIDR, err := vpcFirstSubnetCIDR(vpc1) + if err != nil { + return err + } + vpc2SubnetCIDR, err := vpcFirstSubnetCIDR(vpc2) + if err != nil { + return err + } + if err := overlayVPCToVPCStaticDNAT(matrix, vpc2.Name, vpc1.Name, vpc1SubnetCIDR, vpc1NATCIDR); err != nil { + return fmt.Errorf("overlaying vpc1 static DNAT: %w", err) + } - sort.Slice(vpcs.Items, func(i, j int) bool { - return vpcs.Items[i].Name < vpcs.Items[j].Name + return overlayVPCToVPCStaticDNAT(matrix, vpc1.Name, vpc2.Name, vpc2SubnetCIDR, vpc2NATCIDR) + }, }) - - vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec) - externalPeerings := make(map[string]*vpcapi.ExternalPeeringSpec) - gwPeerings := make(map[string]*gwapi.PeeringSpec) - - vpc1 := &vpcs.Items[0] - vpc2 := &vpcs.Items[1] - - // Both VPCs have NAT configured - each side sees the other's NAT addresses - vpc1NATCIDR := []string{"192.168.31.0/24"} - vpc2NATCIDR := []string{"192.168.32.0/24"} - - if err := appendGwPeeringSpec(gwPeerings, vpc1, vpc2, &GwPeeringOptions{ - VPC1NATCIDR: vpc1NATCIDR, - VPC2NATCIDR: vpc2NATCIDR, - }); err != nil { - return false, nil, fmt.Errorf("setting up gateway peering: %w", err) - } - - if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, gwPeerings, true); err != nil { - return false, nil, fmt.Errorf("setting up bidirectional NAT peerings: %w", err) - } - - if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { - return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) - } - - if err := testCtx.testNATGatewayConnectivity(ctx, vpc1, vpc2, vpc1NATCIDR, vpc2NATCIDR); err != nil { - return false, nil, fmt.Errorf("testing bidirectional NAT connectivity: %w", err) - } - - return false, nil, nil } // Test gateway peering with overlapping VPC subnets resolved via NAT. @@ -537,7 +431,7 @@ func gatewayPeeringBidirectionalStaticNATTest(ctx context.Context, testCtx *VPCP // Cidr: 10.50.1.0/24 // Nat: // Static: -func gatewayPeeringOverlapNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func gatewayPeeringOverlapNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { const ( overlapNSName = "overlap-ns" // max 11 chars for IPv4Namespace name overlapVPCName = "vpc-overlap" // max 11 chars for VPC name (VRF interface limit) @@ -778,6 +672,10 @@ func gatewayPeeringOverlapNATTest(ctx context.Context, testCtx *VPCPeeringTestCt return fmt.Errorf("reconfiguring server %s network: %w: %s", targetServer, err, stderr) } + if err := testCtx.rebindMatrixServerEndpoint(ctx, matrix, targetServer); err != nil { + return fmt.Errorf("rebinding server endpoint: %w", err) + } + return nil }) @@ -857,136 +755,36 @@ func gatewayPeeringOverlapNATTest(ctx context.Context, testCtx *VPCPeeringTestCt return false, reverts, fmt.Errorf("waiting for switches to be ready after peering: %w", err) } + if err := matrix.Repopulate(ctx, testCtx.kube); err != nil { + return false, reverts, fmt.Errorf("refreshing matrix after overlap peerings: %w", err) + } + // Test connectivity - both VPCs have overlapping subnets, NAT resolves them slog.Info("Testing connectivity between VPCs with overlapping subnets via NAT", "existingVPC", existingVPC.Name, "existingCIDR", existingSubnetCIDR, "overlapVPC", overlapVPC.Name, "overlapCIDR", existingSubnetCIDR, "existingNAT", existingVPCNATCIDR, "overlapNAT", overlapVPCNATCIDR) - if err := testCtx.testNATGatewayConnectivity(ctx, existingVPC, overlapVPC, existingVPCNATCIDR, overlapVPCNATCIDR); err != nil { - return false, reverts, fmt.Errorf("testing overlap NAT connectivity: %w", err) - } - - slog.Info("Overlap NAT test completed successfully") - - return false, reverts, nil -} - -// testPortForwardInboundConnectivity tests inbound port-forward NAT connectivity. -// The peering side with port-forward NAT (vpc1) exposes a NAT pool CIDR; the other side (vpc2) -// connects to vpc1's NAT IPs on the forwarded external port. The gateway translates to the -// real vpc1 server port (5201). This mirrors the port-forward semantics: inbound only. -func (testCtx *VPCPeeringTestCtx) testPortForwardInboundConnectivity( - ctx context.Context, - vpc1, vpc2 *vpcapi.VPC, - vpc1NATCIDRStr string, - externalPort int, -) error { - servers := &wiringapi.ServerList{} - if err := testCtx.kube.List(ctx, servers); err != nil { - return fmt.Errorf("listing servers: %w", err) + // Tell the matrix that the moved server now lives in the overlap VPC + // and pick up its DHCP-assigned IP from the new subnet. + if err := testCtx.rebindMatrixServerEndpoint(ctx, matrix, targetServer); err != nil { + return false, reverts, fmt.Errorf("rebinding moved server endpoint to overlap VPC: %w", err) } - var vpc1Servers, vpc2Servers []string - for _, server := range servers.Items { - attachedSubnets, err := apiutil.GetAttachedSubnets(ctx, testCtx.kube, server.Name) - if err != nil { - continue - } - for subnetName := range attachedSubnets { - if strings.HasPrefix(subnetName, vpc1.Name+"/") { - vpc1Servers = append(vpc1Servers, server.Name) - - break - } - if strings.HasPrefix(subnetName, vpc2.Name+"/") { - vpc2Servers = append(vpc2Servers, server.Name) - - break - } - } + if err := overlayVPCToVPCStaticDNAT(matrix, overlapVPCName, existingVPC.Name, existingSubnetCIDR, existingVPCNATCIDR[0]); err != nil { + return false, reverts, fmt.Errorf("overlaying existing VPC DNAT: %w", err) } - - if len(vpc1Servers) == 0 || len(vpc2Servers) == 0 { - return fmt.Errorf("need servers in both VPCs for port-forward test") //nolint:goerr113 + if err := overlayVPCToVPCStaticDNAT(matrix, existingVPC.Name, overlapVPCName, existingSubnetCIDR, overlapVPCNATCIDR[0]); err != nil { + return false, reverts, fmt.Errorf("overlaying overlap VPC DNAT: %w", err) } - sshConfigs := map[string]*sshutil.Config{} - for _, serverName := range append(vpc1Servers, vpc2Servers...) { - sshCfg, err := testCtx.getSSH(ctx, serverName) - if err != nil { - return fmt.Errorf("getting ssh config for %s: %w", serverName, err) - } - sshConfigs[serverName] = sshCfg - } - - // Discover vpc1 server IPs - serverIPs := map[string]netip.Addr{} - for _, serverName := range vpc1Servers { - stdout, stderr, err := sshConfigs[serverName].Run(ctx, "ip -o -4 addr show | awk '{print $2, $4}'") - if err != nil { - return fmt.Errorf("getting IP for %s: %w: %s", serverName, err, stderr) - } - var eligibleAddrs []netip.Addr - for _, line := range strings.Split(strings.TrimSpace(stdout), "\n") { - fields := strings.Fields(line) - if len(fields) != 2 { - continue - } - if excludedInterfaces[fields[0]] { - continue - } - addr, err := netip.ParsePrefix(fields[1]) - if err != nil { - continue - } - eligibleAddrs = append(eligibleAddrs, addr.Addr()) - } - if len(eligibleAddrs) != 1 { - return fmt.Errorf("server %s has %d eligible IPs, expected exactly 1", serverName, len(eligibleAddrs)) //nolint:goerr113 - } - serverIPs[serverName] = eligibleAddrs[0] - slog.Debug("Discovered vpc1 server IP", "server", serverName, "ip", eligibleAddrs[0].String()) - } - - natPrefix, err := netip.ParsePrefix(vpc1NATCIDRStr) - if err != nil { - return fmt.Errorf("parsing NAT CIDR %s: %w", vpc1NATCIDRStr, err) - } - natPoolStart := natPrefix.Masked().Addr() - - if len(vpc1.Spec.Subnets) != 1 { - return fmt.Errorf("VPC %s has %d subnets, port-forward test requires exactly one", vpc1.Name, len(vpc1.Spec.Subnets)) //nolint:goerr113 - } - var vpc1SubnetStart netip.Addr - for _, subnet := range vpc1.Spec.Subnets { - prefix, err := netip.ParsePrefix(subnet.Subnet) - if err != nil { - return fmt.Errorf("parsing VPC subnet: %w", err) - } - vpc1SubnetStart = prefix.Masked().Addr() + if err := DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix); err != nil { + return false, reverts, fmt.Errorf("testing overlap NAT connectivity: %w", err) } - // Test inbound port-forward: vpc2 server → vpc1's NAT IP:externalPort → vpc1 server:5201 - for _, serverB := range vpc1Servers { // iperf3 server side (behind NAT) - natIP, err := calculateStaticNATIP(serverIPs[serverB], vpc1SubnetStart, natPoolStart) - if err != nil { - return fmt.Errorf("calculating NAT IP for %s: %w", serverB, err) - } - - for _, serverA := range vpc2Servers { // iperf3 client side (initiates connection) - slog.Debug("Testing port-forward inbound", - "from", serverA, "to", serverB, "natIP", natIP, "externalPort", externalPort) - - cmd := fmt.Sprintf("toolbox -E LD_PRELOAD=/lib/x86_64-linux-gnu/libgcc_s.so.1 -q timeout %d iperf3 -c %s -p %d -t %d", - testCtx.tcOpts.IPerfsSeconds+25, natIP.String(), externalPort, testCtx.tcOpts.IPerfsSeconds) - if _, stderr, err := retrySSHCmd(ctx, sshConfigs[serverA], cmd, serverA); err != nil { - return fmt.Errorf("iperf3 client from %s to %s:%d: %w: %s", serverA, natIP, externalPort, err, stderr) - } - } - } + slog.Info("Overlap NAT test completed successfully") - return nil + return false, reverts, nil } // Test gateway peering with port-forwarding NAT only @@ -1013,54 +811,37 @@ func (testCtx *VPCPeeringTestCtx) testPortForwardInboundConnectivity( // Expose: // Ips: // Cidr: 10.50.2.0/24 -func gatewayPeeringPortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { - vpcs := &vpcapi.VPCList{} - if err := testCtx.kube.List(ctx, vpcs); err != nil { - return false, nil, fmt.Errorf("listing VPCs: %w", err) - } - if len(vpcs.Items) < 2 { - return true, nil, fmt.Errorf("not enough VPCs for port-forward NAT test") //nolint:goerr113 - } - - sort.Slice(vpcs.Items, func(i, j int) bool { - return vpcs.Items[i].Name < vpcs.Items[j].Name - }) - - vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec) - externalPeerings := make(map[string]*vpcapi.ExternalPeeringSpec) - gwPeerings := make(map[string]*gwapi.PeeringSpec) - - vpc1 := &vpcs.Items[0] - vpc2 := &vpcs.Items[1] - +func gatewayPeeringPortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { + const vpc1NATCIDR = "192.168.52.0/24" portForwardRules := []gwapi.PeeringNATPortForwardEntry{ {Protocol: gwapi.PeeringNATProtocolTCP, Port: "5201", As: "15201"}, } - if err := appendGwPeeringSpec(gwPeerings, vpc1, vpc2, &GwPeeringOptions{ - VPC1NATCIDR: []string{"192.168.52.0/24"}, - VPC1NATMode: NATModePortForward, - VPC1PortForwardRules: portForwardRules, - }); err != nil { - return false, nil, fmt.Errorf("setting up gateway peering: %w", err) - } - - if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, gwPeerings, true); err != nil { - return false, nil, fmt.Errorf("setting up port-forward NAT peerings: %w", err) - } - - if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { - return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) - } + return testCtx.runNATTest(ctx, matrix, natTestSpec{ + Name: "gateway port-forward NAT", + BuildSpec: func(vpc1, vpc2 *vpcapi.VPC) (peeringSpecs, error) { + specs := emptyPeeringSpecs() + err := appendGwPeeringSpec(specs.Gateway, vpc1, vpc2, &GwPeeringOptions{ + VPC1NATCIDR: []string{vpc1NATCIDR}, + VPC1NATMode: NATModePortForward, + VPC1PortForwardRules: portForwardRules, + }) - // Port-forward is INBOUND only: vpc2 connects to vpc1's NAT IP on the forwarded external port. - // vpc1 cannot initiate connections to vpc2 (no outbound NAT). - const vpc1NATCIDR = "192.168.52.0/24" - if err := testCtx.testPortForwardInboundConnectivity(ctx, vpc1, vpc2, vpc1NATCIDR, 15201); err != nil { - return false, nil, fmt.Errorf("testing port-forward inbound connectivity: %w", err) - } + return specs, err + }, + // Port-forward is INBOUND only: vpc2 connects to vpc1's NAT IP on + // the forwarded external port. vpc1 cannot initiate connections + // to vpc2 (no outbound NAT), so we force that direction to Deny. + Overlay: func(vpc1, vpc2 *vpcapi.VPC, matrix *ConnectivityMatrix) error { + overrideVPCToVPCVerdict(matrix, vpc1.Name, vpc2.Name, VerdictDeny) + vpc1SubnetCIDR, err := vpcFirstSubnetCIDR(vpc1) + if err != nil { + return err + } - return false, nil, nil + return overlayVPCToVPCPortForwardDNAT(matrix, vpc2.Name, vpc1.Name, vpc1SubnetCIDR, vpc1NATCIDR, 15201) + }, + }) } // Test gateway peering with combined masquerade and port-forwarding NAT @@ -1089,58 +870,39 @@ func gatewayPeeringPortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTe // Expose: // Ips: // Cidr: 10.50.2.0/24 -func gatewayPeeringMasqueradePortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { - vpcs := &vpcapi.VPCList{} - if err := testCtx.kube.List(ctx, vpcs); err != nil { - return false, nil, fmt.Errorf("listing VPCs: %w", err) - } - if len(vpcs.Items) < 2 { - return true, nil, fmt.Errorf("not enough VPCs for masquerade+port-forward NAT test") //nolint:goerr113 - } - - sort.Slice(vpcs.Items, func(i, j int) bool { - return vpcs.Items[i].Name < vpcs.Items[j].Name - }) - - vpcPeerings := make(map[string]*vpcapi.VPCPeeringSpec) - externalPeerings := make(map[string]*vpcapi.ExternalPeeringSpec) - gwPeerings := make(map[string]*gwapi.PeeringSpec) - - vpc1 := &vpcs.Items[0] - vpc2 := &vpcs.Items[1] - +func gatewayPeeringMasqueradePortForwardNATTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { + const vpc1NATCIDR = "192.168.51.0/24" portForwardRules := []gwapi.PeeringNATPortForwardEntry{ {Protocol: gwapi.PeeringNATProtocolTCP, Port: "5201", As: "15201"}, } - if err := appendGwPeeringSpec(gwPeerings, vpc1, vpc2, &GwPeeringOptions{ - VPC1NATCIDR: []string{"192.168.51.0/24"}, - VPC1NATMode: NATModeMasqueradePortForward, - VPC1PortForwardRules: portForwardRules, - }); err != nil { - return false, nil, fmt.Errorf("setting up gateway peering: %w", err) - } - - if err := DoSetupPeerings(ctx, testCtx.kube, vpcPeerings, externalPeerings, gwPeerings, true); err != nil { - return false, nil, fmt.Errorf("setting up masquerade+port-forward NAT peerings: %w", err) - } - - if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { - return false, nil, fmt.Errorf("waiting for switches to be ready: %w", err) - } + return testCtx.runNATTest(ctx, matrix, natTestSpec{ + Name: "gateway masquerade+port-forward NAT", + BuildSpec: func(vpc1, vpc2 *vpcapi.VPC) (peeringSpecs, error) { + specs := emptyPeeringSpecs() + err := appendGwPeeringSpec(specs.Gateway, vpc1, vpc2, &GwPeeringOptions{ + VPC1NATCIDR: []string{vpc1NATCIDR}, + VPC1NATMode: NATModeMasqueradePortForward, + VPC1PortForwardRules: portForwardRules, + }) - // Outbound direction: vpc1 reaches vpc2 via masquerade NAT (vpc1 traffic appears from 192.168.51.x) - if err := testCtx.testNATGatewayConnectivity(ctx, vpc1, vpc2, nil, nil); err != nil { - return false, nil, fmt.Errorf("testing masquerade+port-forward NAT outbound connectivity: %w", err) - } - - // Inbound direction: vpc2 connects to vpc1's NAT IP:15201, gateway forwards to vpc1 real IP:5201 - const vpc1NATCIDR = "192.168.51.0/24" - if err := testCtx.testPortForwardInboundConnectivity(ctx, vpc1, vpc2, vpc1NATCIDR, 15201); err != nil { - return false, nil, fmt.Errorf("testing masquerade+port-forward NAT inbound connectivity: %w", err) - } + return specs, err + }, + // vpc1→vpc2 rides masquerade SNAT against vpc2's real IPs; + // populate can't see this Allow because the peering carries 'As', + // so we assert it explicitly. vpc2→vpc1 must hit the port-forward + // virtual (NAT IP, 15201); the matrix runner treats DNAT+port as + // L4-only and skips ping for that direction. + Overlay: func(vpc1, vpc2 *vpcapi.VPC, matrix *ConnectivityMatrix) error { + overrideVPCToVPCVerdict(matrix, vpc1.Name, vpc2.Name, VerdictAllow) + vpc1SubnetCIDR, err := vpcFirstSubnetCIDR(vpc1) + if err != nil { + return err + } - return false, nil, nil + return overlayVPCToVPCPortForwardDNAT(matrix, vpc2.Name, vpc1.Name, vpc1SubnetCIDR, vpc1NATCIDR, 15201) + }, + }) } // getNATTestCases returns the NAT test cases to be added to the multi-VPC single-subnet suite diff --git a/pkg/hhfab/rt_no_vpc_suite.go b/pkg/hhfab/rt_no_vpc_suite.go index a7dbcfc5d..532f6c462 100644 --- a/pkg/hhfab/rt_no_vpc_suite.go +++ b/pkg/hhfab/rt_no_vpc_suite.go @@ -78,7 +78,7 @@ func makeNoVpcsSuite() *JUnitTestSuite { // 2. change breakout to some non default mode // 3. wait for all switches to be ready for 1 minute // 4. check that all agents report the breakout to be completed and that the port is in the expected mode -func breakoutTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func breakoutTest(ctx context.Context, testCtx *VPCPeeringTestCtx, _ *ConnectivityMatrix) (bool, []RevertFunc, error) { // get all agents in the fabric agents := &agentapi.AgentList{} if err := testCtx.kube.List(ctx, agents); err != nil { @@ -328,7 +328,7 @@ const ( AlloyCtrlComponent = "alloy-ctrl" ) -func lokiObservabilityTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func lokiObservabilityTest(ctx context.Context, testCtx *VPCPeeringTestCtx, _ *ConnectivityMatrix) (bool, []RevertFunc, error) { lokiEndpoint, _, env, err := getObservabilityQueryURLs(ctx, testCtx.kube) if err != nil { return true, nil, fmt.Errorf("error getting observability endpoints: %w", err) @@ -605,7 +605,7 @@ func lokiObservabilityTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (boo return false, nil, nil } -func prometheusObservabilityTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func prometheusObservabilityTest(ctx context.Context, testCtx *VPCPeeringTestCtx, _ *ConnectivityMatrix) (bool, []RevertFunc, error) { _, prometheusEndpoint, env, err := getObservabilityQueryURLs(ctx, testCtx.kube) if err != nil { return true, nil, fmt.Errorf("error getting observability endpoints: %w", err) diff --git a/pkg/hhfab/rt_on_ready_suite.go b/pkg/hhfab/rt_on_ready_suite.go index 280a2dd97..6b32e19b5 100644 --- a/pkg/hhfab/rt_on_ready_suite.go +++ b/pkg/hhfab/rt_on_ready_suite.go @@ -69,7 +69,7 @@ type ortServerInfo struct { // At the end of the test, we clean up all VPCs and peerings to leave a clean slate. // //nolint:cyclop -func newOnReadyTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func newOnReadyTest(ctx context.Context, testCtx *VPCPeeringTestCtx, _ *ConnectivityMatrix) (bool, []RevertFunc, error) { slog.Info("Starting new on-ready test: discovering resources") kube := testCtx.kube @@ -446,7 +446,15 @@ func newOnReadyTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []Re if err != nil { return false, nil, fmt.Errorf("parsing VPC B subnet: %w", err) } - hostBGPCmd, err := getServerHostBGPCmd(&hostBGPServer.conn, vlan, subPrefix, 1) + hostBGPCmd, err := getServerHostBGPCmd([]HostBGPParams{ + { + VPCLabel: vpcBName, + Connections: []*wiringapi.Connection{&hostBGPServer.conn}, + VLAN: vlan, + Subnet: subPrefix, + ServerOffset: 1, + }, + }) if err != nil { return false, nil, fmt.Errorf("hostBGP cmd for %s: %w", hostBGPServer.name, err) } diff --git a/pkg/hhfab/rt_single_vpc_suite.go b/pkg/hhfab/rt_single_vpc_suite.go index 8088be24f..df9238a07 100644 --- a/pkg/hhfab/rt_single_vpc_suite.go +++ b/pkg/hhfab/rt_single_vpc_suite.go @@ -126,7 +126,7 @@ func makeSingleVPCSuite() *JUnitTestSuite { // Basic test for mclag failover. // For each mclag connection, set one of the links down by shutting down the port on the switch, // then test connectivity. Repeat for the other link. -func mclagTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func mclagTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // list connections in the fabric, filter by MC-LAG connection type conns := &wiringapi.ConnectionList{} if err := testCtx.kube.List(ctx, conns, kclient.MatchingLabels{wiringapi.LabelConnectionType: wiringapi.ConnectionTypeMCLAG}); err != nil { @@ -143,7 +143,7 @@ func mclagTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertF return false, nil, fmt.Errorf("MCLAG connection %s has %d links, expected 2", conn.Name, len(conn.Spec.MCLAG.Links)) //nolint:goerr113 } for _, link := range conn.Spec.MCLAG.Links { - if err := shutDownLinkAndTest(ctx, testCtx, link); err != nil { + if err := shutDownLinkAndTest(ctx, testCtx, link, matrix); err != nil { return false, nil, err } // TODO: set other link down too and make sure that connectivity is lost @@ -161,7 +161,7 @@ func mclagTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertF // Basic test for eslag failover. // For each eslag connection, set one of the links down by shutting down the port on the switch, // then test connectivity. Repeat for the other link. -func eslagTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func eslagTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // l3vni mode is not compatible with ESLAG, so there will be no servers attached to ESLAG connections if testCtx.setupOpts.VPCMode == vpcapi.VPCModeL3VNI { return true, nil, fmt.Errorf("L3VNI mode is not compatible with ESLAG") //nolint:goerr113 @@ -182,7 +182,7 @@ func eslagTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertF return false, nil, fmt.Errorf("ESLAG connection %s has %d links, expected 2", conn.Name, len(conn.Spec.ESLAG.Links)) //nolint:goerr113 } for _, link := range conn.Spec.ESLAG.Links { - if err := shutDownLinkAndTest(ctx, testCtx, link); err != nil { + if err := shutDownLinkAndTest(ctx, testCtx, link, matrix); err != nil { return false, nil, err } // TODO: set other link down too and make sure that connectivity is lost @@ -200,7 +200,7 @@ func eslagTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertF // Basic test for bundled connection failover. // For each bundled connection, set one of the links down by shutting down the port on the switch, // then test connectivity. Repeat for the other link(s). -func bundledFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func bundledFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // list connections in the fabric, filter by bundled connection type conns := &wiringapi.ConnectionList{} if err := testCtx.kube.List(ctx, conns, kclient.MatchingLabels{wiringapi.LabelConnectionType: wiringapi.ConnectionTypeBundled}); err != nil { @@ -217,7 +217,7 @@ func bundledFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, return false, nil, fmt.Errorf("MCLAG connection %s has %d links, expected at least 2", conn.Name, len(conn.Spec.Bundled.Links)) //nolint:goerr113 } for _, link := range conn.Spec.Bundled.Links { - if err := shutDownLinkAndTest(ctx, testCtx, link); err != nil { + if err := shutDownLinkAndTest(ctx, testCtx, link, matrix); err != nil { return false, nil, err } // TODO: set other link down too and make sure that connectivity is lost @@ -235,7 +235,7 @@ func bundledFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, // Basic test for spine failover. // Iterate over the spine switches (skip the first one), and shut down all links towards them. // Test connectivity, then re-enable the links. -func spineFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func spineFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { var returnErr error // list spines, unfortunately we cannot filter by role @@ -366,7 +366,7 @@ outer: // that group, then shuts down all spine ports connected to the primary gateway. // After restoring, tests connectivity again. // Requires at least 2 gateways and 2 VPCs. -func gatewayFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func gatewayFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { var returnErr error // list gateways @@ -679,7 +679,7 @@ func gatewayFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, // Basic test for mesh failover. // Iterate over leaf switches, shutdown all mesh links except for one, test connectivity // as soon as we manage to test this on a leaf, return and renable all agents as part of the revert -func meshFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func meshFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // list leaves, unfortunately we cannot filter by role switches := &wiringapi.SwitchList{} if err := testCtx.kube.List(ctx, switches); err != nil { @@ -813,7 +813,7 @@ func meshFailoverTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, [] } // Vanilla test for VPC peering, just test connectivity without any further restriction -func noRestrictionsTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func noRestrictionsTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { return false, nil, fmt.Errorf("waiting for readiness: %w", err) } @@ -831,7 +831,7 @@ func noRestrictionsTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, // 3. Set both isolated and restricted flags in the third subnet, test connectivity // 4. Override isolation with explicit permit list, test connectivity // 5. Remove all restrictions -func singleVPCWithRestrictionsTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func singleVPCWithRestrictionsTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { var returnErr error vpcs := &vpcapi.VPCList{} @@ -966,7 +966,7 @@ outer: // for NTP, we check the output of timedatectl show-timesync; // for MTU, we check the output of "ip link" on the vlan interface; // for DHCP Lease, we check the output of "ip addr" on the server. -func dnsNtpMtuTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func dnsNtpMtuTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { vpcAttaches := &vpcapi.VPCAttachmentList{} if err := testCtx.kube.List(ctx, vpcAttaches); err != nil { return false, nil, fmt.Errorf("listing VPCAttachments: %w", err) @@ -1149,7 +1149,7 @@ func dnsNtpMtuTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []Rev // Uses 1 server by default, all servers in extended mode // Sets VPC DHCPOptions to a shorter lease and reconfigures servers via networkctl // Waits for DHCP renewal and checks lease time -func dhcpRenewalTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func dhcpRenewalTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // Find VPC with at least one server attached that has DHCP enabled vpcAttaches := &vpcapi.VPCAttachmentList{} if err := testCtx.kube.List(ctx, vpcAttaches); err != nil { @@ -1423,7 +1423,7 @@ func (testCtx *VPCPeeringTestCtx) testStaticIPAssignment(ctx context.Context, vp // Verifies that static IP assignments work correctly both within and outside the dynamic range. // The test finds any server on any subnet, saves the existing DHCP config (if any), // forces a hardcoded DHCP config, runs tests, and restores the original config. -func dhcpStaticLeaseTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func dhcpStaticLeaseTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // 1. Find any server attached to any VPC subnet (regardless of DHCP config) serverInfo, err := findAnyAttachedServer(ctx, testCtx.kube) if errors.Is(err, errNoAttachedServers) { @@ -1511,7 +1511,42 @@ func dhcpStaticLeaseTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, return fmt.Errorf("reverting VPC %s DHCP config: %w", serverInfo.VPCName, err) } - return WaitReady(ctx, testCtx.kube, testCtx.wrOpts) + if err := WaitReady(ctx, testCtx.kube, testCtx.wrOpts); err != nil { + return fmt.Errorf("waiting for ready after restoring DHCP config: %w", err) + } + + // update the connectivity matrix for all servers attached to the modified subnet + subnetServers, err := findAllServersInSubnet(ctx, testCtx.kube, serverInfo.VPCName, serverInfo.SubnetName) + if err != nil { + return fmt.Errorf("listing servers in subnet %s of VPC %s: %w", serverInfo.SubnetName, serverInfo.VPCName, err) + } + + for _, server := range subnetServers { + ssh, err := testCtx.getSSH(ctx, server.Name) + if err != nil { + return fmt.Errorf("getting ssh config for server %s: %w", server.Name, err) + } + + _, stderr, err := ssh.Run(ctx, fmt.Sprintf("sudo networkctl reconfigure %s", server.Interface)) + if err != nil { + if stderr != "" { + return fmt.Errorf("reconfiguring interface: %w (stderr: %s)", err, stderr) + } + + return fmt.Errorf("reconfiguring interface: %w", err) + } + // Give DHCP a moment to hand out a fresh lease + time.Sleep(5 * time.Second) + + // Refresh the matrix entry for this server. Without + // this, follow-up tests (MCLAG/ESLAG failover) probe the + // stale matrix IP and never reach the server. + if err := testCtx.rebindMatrixServerEndpoint(ctx, matrix, server.Name); err != nil { + return fmt.Errorf("refreshing matrix endpoint for %s after DHCP revert: %w", server.Name, err) + } + } + + return nil }, } @@ -1533,7 +1568,7 @@ func dhcpStaticLeaseTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, // Test RoCE functionality and DSCP traffic marking by enabling RoCE on a leaf switch // with servers, generating DSCP 24 marked traffic, and verifying UC3 queue counters. -func roceBasicTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func roceBasicTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { // this should never fail if len(testCtx.roceLeaves) == 0 { slog.Error("RoCE leaves not specified, skipping RoCE basic test") diff --git a/pkg/hhfab/rt_static_external.go b/pkg/hhfab/rt_static_external.go index 65764abf4..057713c8f 100644 --- a/pkg/hhfab/rt_static_external.go +++ b/pkg/hhfab/rt_static_external.go @@ -22,7 +22,7 @@ var ( // staticExternalPeeringTest tests static external connectivity using the External API // with spec.static (non-BGP mode). This test requires a static External with an ExternalAttachment // it creates the ExternalPeering dynamically to test VPC-to-External connectivity. -func staticExternalPeeringTest(ctx context.Context, testCtx *VPCPeeringTestCtx) (bool, []RevertFunc, error) { +func staticExternalPeeringTest(ctx context.Context, testCtx *VPCPeeringTestCtx, matrix *ConnectivityMatrix) (bool, []RevertFunc, error) { if testCtx.staticExtName == "" { return true, nil, errNoStaticExternalWithAttachment } diff --git a/pkg/hhfab/rt_utils.go b/pkg/hhfab/rt_utils.go index f58603505..717e7dcec 100644 --- a/pkg/hhfab/rt_utils.go +++ b/pkg/hhfab/rt_utils.go @@ -233,7 +233,7 @@ func changeSwitchPortStatus(ctx context.Context, ssh *sshutil.Config, deviceName } // disable agent, shutdown port on switch, test connectivity, enable agent, set port up -func shutDownLinkAndTest(ctx context.Context, testCtx *VPCPeeringTestCtx, link wiringapi.ServerToSwitchLink) (returnErr error) { +func shutDownLinkAndTest(ctx context.Context, testCtx *VPCPeeringTestCtx, link wiringapi.ServerToSwitchLink, matrix *ConnectivityMatrix) (returnErr error) { switchPort := link.Switch deviceName := switchPort.DeviceName() // get switch profile to find the port name in sonic-cli @@ -302,7 +302,14 @@ func shutDownLinkAndTest(ctx context.Context, testCtx *VPCPeeringTestCtx, link w slog.Debug("Waiting 5 seconds") time.Sleep(5 * time.Second) - return DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts) + var connCheckErr error + if matrix == nil { + connCheckErr = DoVLABTestConnectivity(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts) + } else { + connCheckErr = DoVLABTestConnectivityWithMatrix(ctx, testCtx.vlabCfg.WorkDir, testCtx.vlabCfg.CacheDir, testCtx.tcOpts, matrix) + } + + return connCheckErr } // getSwitchesForVPC returns the set of leaf switch names that have VPCAttachments for the given VPC. @@ -1226,3 +1233,52 @@ func findAnyAttachedServer(ctx context.Context, kube kclient.Client) (*AttachedS return nil, errNoAttachedServers } + +// finds all servers attached to a subnet in a VPC +func findAllServersInSubnet(ctx context.Context, kube kclient.Client, vpcName, subnetName string) ([]ServerWithInterface, error) { + servers := []ServerWithInterface{} + vpc := vpcapi.VPC{} + if err := kube.Get(ctx, kclient.ObjectKey{Name: vpcName, Namespace: kmetav1.NamespaceDefault}, &vpc); err != nil { + return servers, fmt.Errorf("getting VPC %s: %w", vpcName, err) + } + subnet, ok := vpc.Spec.Subnets[subnetName] + if !ok { + return servers, fmt.Errorf("no subnet %s in VPC %s", subnetName, vpcName) //nolint:goerr113 + } + attachList := vpcapi.VPCAttachmentList{} + if err := kube.List(ctx, &attachList, kclient.MatchingLabels{ + vpcapi.LabelSubnet: subnetName, + vpcapi.LabelVPC: vpcName, + }); err != nil { + return servers, fmt.Errorf("listing attachments to the target server vpc/subnet: %w", err) + } + for _, attach := range attachList.Items { + conn := &wiringapi.Connection{} + if err := kube.Get(ctx, kclient.ObjectKey{ + Namespace: kmetav1.NamespaceDefault, + Name: attach.Spec.Connection, + }, conn); err != nil { + return servers, fmt.Errorf("getting connection %s for attachment %s: %w", attach.Spec.Connection, attach.Name, err) + } + var ifName string + if conn.Spec.Unbundled != nil { + ifName = fmt.Sprintf("%s.%d", conn.Spec.Unbundled.Link.Server.LocalPortName(), subnet.VLAN) + } else { + ifName = fmt.Sprintf("bond0.%d", subnet.VLAN) + } + + _, serverNames, _, _, err := conn.Spec.Endpoints() + if err != nil { + return servers, fmt.Errorf("getting endpoints for connection %s: %w", conn.Name, err) + } else if len(serverNames) != 1 { + return servers, fmt.Errorf("expected 1 server for attachment %s, found %d", attach.Name, len(serverNames)) //nolint:goerr113 + } + + servers = append(servers, ServerWithInterface{ + Name: serverNames[0], + Interface: ifName, + }) + } + + return servers, nil +} diff --git a/pkg/hhfab/testing.go b/pkg/hhfab/testing.go index 58d986d50..c386e290a 100644 --- a/pkg/hhfab/testing.go +++ b/pkg/hhfab/testing.go @@ -60,6 +60,8 @@ const ( HashPolicyVLANAndSrcMAC = "vlan+srcmac" ) +var reachCheckUnsupported = errors.New("reachability check unsupported for this type of peering") + var HashPolicies = []string{ HashPolicyL2, HashPolicyL2And3, @@ -427,42 +429,63 @@ func GetServerNetconfCmd(conn *wiringapi.Connection, opts ServerNetconfOpts) (st return netconfCmd, nil } -// TODO: multi subnet support once test-connectivity supports it -func getServerHostBGPCmd(conn *wiringapi.Connection, vlan uint16, subnet netip.Prefix, serversInSubnet int) (string, error) { - if conn == nil { - return "", fmt.Errorf("connection is nil") +type HostBGPParams struct { + VPCLabel string + Connections []*wiringapi.Connection + VLAN uint16 + Subnet netip.Prefix + ServerOffset int +} + +func getServerHostBGPCmd(params []HostBGPParams) (string, error) { + if len(params) == 0 { + return "", fmt.Errorf("no params provided") } - cmd := fmt.Sprintf("vpc:v=%d:i=", vlan) - interfaces := []string{} - switch { - case conn.Spec.Unbundled != nil: - interfaces = append(interfaces, conn.Spec.Unbundled.Link.Server.LocalPortName()) - case conn.Spec.Bundled != nil: - for _, link := range conn.Spec.Bundled.Links { - interfaces = append(interfaces, link.Server.LocalPortName()) + cmd := "" + for i, param := range params { + if i > 0 { + cmd += " " } - case conn.Spec.MCLAG != nil: - for _, link := range conn.Spec.MCLAG.Links { - interfaces = append(interfaces, link.Server.LocalPortName()) + if len(param.Connections) == 0 { + return "", fmt.Errorf("no connections provided") } - case conn.Spec.ESLAG != nil: - for _, link := range conn.Spec.ESLAG.Links { - interfaces = append(interfaces, link.Server.LocalPortName()) + interfaces := []string{} + for _, conn := range param.Connections { + if conn == nil { + return "", fmt.Errorf("connection is nil") + } + switch { + case conn.Spec.Unbundled != nil: + interfaces = append(interfaces, conn.Spec.Unbundled.Link.Server.LocalPortName()) + case conn.Spec.Bundled != nil: + for _, link := range conn.Spec.Bundled.Links { + interfaces = append(interfaces, link.Server.LocalPortName()) + } + case conn.Spec.MCLAG != nil: + for _, link := range conn.Spec.MCLAG.Links { + interfaces = append(interfaces, link.Server.LocalPortName()) + } + case conn.Spec.ESLAG != nil: + for _, link := range conn.Spec.ESLAG.Links { + interfaces = append(interfaces, link.Server.LocalPortName()) + } + default: + return "", fmt.Errorf("unexpected connection type for conn %q", conn.Name) + } } - default: - return "", fmt.Errorf("unexpected connection type for conn %q", conn.Name) - } - cmd += strings.Join(interfaces, ":i=") - addr := subnet.Addr() - for range serversInSubnet { - addr = addr.Next() - } - if !addr.IsValid() { - return "", fmt.Errorf("failed to get IP address from subnet %s", subnet.String()) + cmd += fmt.Sprintf("%s:v=%d:i=", param.VPCLabel, param.VLAN) + cmd += strings.Join(interfaces, ":i=") + addr := param.Subnet.Addr() + for range param.ServerOffset { + addr = addr.Next() + } + if !addr.IsValid() { + return "", fmt.Errorf("failed to get IP address from subnet %s", param.Subnet.String()) + } + cmd += ":a=" + addr.String() + "/32" } - cmd += ":a=" + addr.String() + "/32" return cmd, nil } @@ -572,42 +595,48 @@ func ResolveDefaultServerMTU(ctx context.Context, kube kclient.Client, opts *Set return nil } -func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) error { +// SetupVPCs creates VPCs and VPC attachments per opts, configures servers, +// and returns one Endpoint per (server, vpc, subnet) attachment with the +// discovered IP and HostBGP flag populated. ESLAG servers skipped in non-L2VNI +// modes are not included in the returned list. Callers that want to drive +// matrix-based connectivity tests pass the result to BuildConnectivityMatrix; +// CLI and vlabrunner callers discard it. +func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) ([]*Endpoint, error) { ctx, cancel := context.WithTimeout(ctx, 30*time.Minute) defer cancel() start := time.Now() if opts.ServersPerSubnet <= 0 { - return fmt.Errorf("servers per subnet must be positive") + return nil, fmt.Errorf("servers per subnet must be positive") } if opts.SubnetsPerVPC <= 0 { - return fmt.Errorf("subnets per VPC must be positive") + return nil, fmt.Errorf("subnets per VPC must be positive") } if !slices.Contains(HashPolicies, opts.HashPolicy) { - return fmt.Errorf("invalid hash policy %q, must be one of %v", opts.HashPolicy, HashPolicies) + return nil, fmt.Errorf("invalid hash policy %q, must be one of %v", opts.HashPolicy, HashPolicies) } else if opts.HashPolicy != HashPolicyL2 && opts.HashPolicy != HashPolicyL2And3 { slog.Warn("The selected hash policy is not fully 802.3ad compliant, use layer2 or layer2+3 for full compliance", "hashPolicy", opts.HashPolicy) } if !slices.Contains(vpcapi.VPCModes, opts.VPCMode) { - return fmt.Errorf("invalid VPC mode %q, must be one of %v", opts.VPCMode, vpcapi.VPCModes) + return nil, fmt.Errorf("invalid VPC mode %q, must be one of %v", opts.VPCMode, vpcapi.VPCModes) } cacheCancel, kube, err := getKubeClientWithCache(ctx, c.WorkDir) if err != nil { - return fmt.Errorf("creating kube client: %w", err) + return nil, fmt.Errorf("creating kube client: %w", err) } defer cacheCancel() switchList := wiringapi.SwitchList{} if err := kube.List(ctx, &switchList); err != nil { - return fmt.Errorf("listing switches: %w", err) + return nil, fmt.Errorf("listing switches: %w", err) } allCumulus := true for _, sw := range switchList.Items { sp := &wiringapi.SwitchProfile{} if err := kube.Get(ctx, kclient.ObjectKey{Name: sw.Spec.Profile, Namespace: kmetav1.NamespaceDefault}, sp); err != nil { - return fmt.Errorf("getting switch profile %q: %w", sw.Spec.Profile, err) + return nil, fmt.Errorf("getting switch profile %q: %w", sw.Spec.Profile, err) } if !slices.Contains(meta.NOSTypesCumulus, sp.Spec.NOSType) { @@ -617,7 +646,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) } } if opts.P2P && !allCumulus { - return fmt.Errorf("P2P mode requires all switches to be cumulus") + return nil, fmt.Errorf("P2P mode requires all switches to be cumulus") } if !opts.P2P && allCumulus { opts.P2P = true @@ -625,7 +654,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) } if err := ResolveDefaultServerMTU(ctx, kube, &opts); err != nil { - return fmt.Errorf("resolving default server MTU: %w", err) + return nil, fmt.Errorf("resolving default server MTU: %w", err) } { @@ -648,7 +677,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) sshConfigs := map[string]*sshutil.Config{} for _, vm := range vlab.VMs { if sshCfg, err := c.SSHVM(ctx, vlab, vm); err != nil { - return fmt.Errorf("getting ssh config for vm %q: %w", vm.Name, err) + return nil, fmt.Errorf("getting ssh config for vm %q: %w", vm.Name, err) } else { sshConfigs[vm.Name] = sshCfg } @@ -663,32 +692,32 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) } if err := client.IgnoreNotFound(kube.DeleteAllOf(ctx, &vpcapi.VPCPeering{}, &delAllOpts)); err != nil { - return fmt.Errorf("cleaning up vpc peerings: %w", err) + return nil, fmt.Errorf("cleaning up vpc peerings: %w", err) } if err := client.IgnoreNotFound(kube.DeleteAllOf(ctx, &vpcapi.ExternalPeering{}, &delAllOpts)); err != nil { - return fmt.Errorf("cleaning up external peerings: %w", err) + return nil, fmt.Errorf("cleaning up external peerings: %w", err) } if c.Fab.Spec.Config.Gateway.Enable { if err := client.IgnoreNotFound(kube.DeleteAllOf(ctx, &gwapi.GatewayPeering{}, &delAllOpts)); err != nil { - return fmt.Errorf("cleaning up gateway peerings: %w", err) + return nil, fmt.Errorf("cleaning up gateway peerings: %w", err) } } } servers := &wiringapi.ServerList{} if err := kube.List(ctx, servers); err != nil { - return fmt.Errorf("listing servers: %w", err) + return nil, fmt.Errorf("listing servers: %w", err) } serverIDs := map[string]uint64{} for _, server := range servers.Items { if !strings.HasPrefix(server.Name, ServerNamePrefix) { - return fmt.Errorf("unexpected server name %q, should be %s", server.Name, ServerNamePrefix) + return nil, fmt.Errorf("unexpected server name %q, should be %s", server.Name, ServerNamePrefix) } serverID, err := strconv.ParseUint(server.Name[len(ServerNamePrefix):], 10, 64) if err != nil { - return fmt.Errorf("parsing server id: %w", err) + return nil, fmt.Errorf("parsing server id: %w", err) } serverIDs[server.Name] = serverID @@ -700,20 +729,20 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) vlanNS := &wiringapi.VLANNamespace{} if err := kube.Get(ctx, client.ObjectKey{Name: opts.VLANNamespace, Namespace: metav1.NamespaceDefault}, vlanNS); err != nil { - return fmt.Errorf("getting VLAN namespace %s: %w", opts.VLANNamespace, err) + return nil, fmt.Errorf("getting VLAN namespace %s: %w", opts.VLANNamespace, err) } nextVLAN, stopVLAN := iter.Pull(VLANsFrom(vlanNS.Spec.Ranges...)) defer stopVLAN() ipNS := &vpcapi.IPv4Namespace{} if err := kube.Get(ctx, client.ObjectKey{Name: opts.IPv4Namespace, Namespace: metav1.NamespaceDefault}, ipNS); err != nil { - return fmt.Errorf("getting IPv4 namespace %s: %w", opts.IPv4Namespace, err) + return nil, fmt.Errorf("getting IPv4 namespace %s: %w", opts.IPv4Namespace, err) } prefixes := []netip.Prefix{} for _, prefix := range ipNS.Spec.Subnets { prefix, err := netip.ParsePrefix(prefix) if err != nil { - return fmt.Errorf("parsing IPv4 namespace %s prefix %q: %w", opts.IPv4Namespace, prefix, err) + return nil, fmt.Errorf("parsing IPv4 namespace %s prefix %q: %w", opts.IPv4Namespace, prefix, err) } prefixes = append(prefixes, prefix) } @@ -748,7 +777,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) for _, server := range servers.Items { if opts.VPCMode != vpcapi.VPCModeL2VNI { if sa, err := getServerAttachState(ctx, kube, &server, false); err != nil { - return fmt.Errorf("checking server %q attachment state: %w", server.Name, err) + return nil, fmt.Errorf("checking server %q attachment state: %w", server.Name, err) } else if sa.ESLAG { eslagServers[server.Name] = true slog.Warn("Skipping ESLAG-connected server", "server", server.Name) @@ -768,20 +797,26 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) } conns := &wiringapi.ConnectionList{} + multihomed := false if err := kube.List(ctx, conns, wiringapi.MatchingLabelsForListLabelServer(server.Name)); err != nil { - return fmt.Errorf("listing connections for server %q: %w", server.Name, err) + return nil, fmt.Errorf("listing connections for server %q: %w", server.Name, err) } if len(conns.Items) == 0 { - return fmt.Errorf("no connections for server %q", server.Name) + return nil, fmt.Errorf("no connections for server %q", server.Name) } if len(conns.Items) > 1 { - return fmt.Errorf("multiple connections for server %q", server.Name) + for _, c := range conns.Items { + if c.Spec.Unbundled == nil { + return nil, fmt.Errorf("multiple connections for server %q of which some are not unbundled", server.Name) + } + } + multihomed = true } conn := conns.Items[0] switches, _, _, _, err := conn.Spec.Endpoints() if err != nil { - return fmt.Errorf("getting connection %q endpoints: %w", conn.Name, err) + return nil, fmt.Errorf("getting connection %q endpoints: %w", conn.Name, err) } isMclag := false for _, sw := range switches { @@ -791,10 +826,14 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) vpcName := fmt.Sprintf("vpc-%02d", vpcID+1) subnetName := fmt.Sprintf("subnet-%02d", subnetInVPC+1) - hostBGP := opts.HostBGPSubnet && !hostBGPDoneForVPC && conn.Spec.Unbundled != nil && !isMclag + hostBGP := multihomed || (opts.HostBGPSubnet && !hostBGPDoneForVPC && conn.Spec.Unbundled != nil && !isMclag) if hostBGP { hostBGPDoneForVPC = true } + // hostBGP and P2P are mutually exclusive per server: a hostBGP host uses + // its real subnet and a /32 VIP, so the P2P /31 semantics must not apply + // (relevant when P2P is force-enabled on an all-Cumulus fabric). + useP2P := opts.P2P && !hostBGP var vpc *vpcapi.VPC if len(vpcs) > 0 && vpcs[len(vpcs)-1].Name == vpcName { @@ -816,7 +855,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) if vpc.Spec.Subnets[subnetName] == nil { subnet, ok := nextPrefix() if !ok { - return fmt.Errorf("no more subnets available") + return nil, fmt.Errorf("no more subnets available") } var dhcpOpts *vpcapi.VPCDHCPOptions @@ -830,7 +869,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) vlan, ok := nextVLAN() if !ok { - return fmt.Errorf("no more vlans available") + return nil, fmt.Errorf("no more vlans available") } dhcp := vpcapi.VPCDHCP{} if !hostBGP && !opts.P2P { @@ -847,10 +886,10 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) } p2p := netip.Prefix{} - if opts.P2P { + if useP2P { subnet, err := netip.ParsePrefix(vpc.Spec.Subnets[subnetName].Subnet) if err != nil { - return fmt.Errorf("parsing vpc subnet %s/%s %q: %w", vpcName, subnetName, vpc.Spec.Subnets[subnetName].Subnet, err) + return nil, fmt.Errorf("parsing vpc subnet %s/%s %q: %w", vpcName, subnetName, vpc.Spec.Subnets[subnetName].Subnet, err) } b := subnet.Masked().Addr().As4() @@ -864,47 +903,65 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) expectedSubnet, err := netip.ParsePrefix(vpc.Spec.Subnets[subnetName].Subnet) if err != nil { - return fmt.Errorf("parsing vpc subnet %s/%s %q: %w", vpcName, subnetName, vpc.Spec.Subnets[subnetName].Subnet, err) + return nil, fmt.Errorf("parsing vpc subnet %s/%s %q: %w", vpcName, subnetName, vpc.Spec.Subnets[subnetName].Subnet, err) } expectedSubnets[server.Name] = expectedSubnet - if opts.P2P { + if useP2P { expectedSubnets[server.Name] = p2p } if hostBGP { hostBGPServers[server.Name] = true } - attachName := fmt.Sprintf("%s--%s--%s", conn.Name, vpcName, subnetName) - attachNames[attachName] = true - attachAnns := map[string]string{} - if opts.P2P { - attachAnns[vpcapi.AnnotationVPCAttachmentP2PLink] = p2p.String() + // Connections to attach: the single connection normally, all of them for + // a multihomed server (which shares one hostBGP subnet across every link). + attachConns := []*wiringapi.Connection{&conn} + if multihomed { + attachConns = nil + for i := range conns.Items { + attachConns = append(attachConns, &conns.Items[i]) + } } - attach := &vpcapi.VPCAttachment{ - ObjectMeta: metav1.ObjectMeta{ - Name: attachName, - Namespace: metav1.NamespaceDefault, - Annotations: attachAnns, - }, - Spec: vpcapi.VPCAttachmentSpec{ - Connection: conn.Name, - Subnet: fmt.Sprintf("%s/%s", vpcName, subnetName), - }, - } - attaches = append(attaches, attach) + for _, ac := range attachConns { + attachName := fmt.Sprintf("%s--%s--%s", ac.Name, vpcName, subnetName) + attachNames[attachName] = true + attachAnns := map[string]string{} + if useP2P { + attachAnns[vpcapi.AnnotationVPCAttachmentP2PLink] = p2p.String() + } - vlan := uint16(0) - if !attach.Spec.NativeVLAN { - vlan = vpc.Spec.Subnets[subnetName].VLAN + attaches = append(attaches, &vpcapi.VPCAttachment{ + ObjectMeta: metav1.ObjectMeta{ + Name: attachName, + Namespace: metav1.NamespaceDefault, + Annotations: attachAnns, + }, + Spec: vpcapi.VPCAttachmentSpec{ + Connection: ac.Name, + Subnet: fmt.Sprintf("%s/%s", vpcName, subnetName), + }, + }) } + // NativeVLAN is never set on the attachments we build above, so the VLAN + // is always the subnet's VLAN. + vlan := vpc.Spec.Subnets[subnetName].VLAN + var confCmd string var confErr error if hostBGP { - confCmd, confErr = getServerHostBGPCmd(&conn, vlan, expectedSubnet, serverInSubnet) - } else if opts.P2P { + confCmd, confErr = getServerHostBGPCmd([]HostBGPParams{ + { + VPCLabel: vpcName, + Connections: attachConns, + VLAN: vlan, + Subnet: expectedSubnet, + ServerOffset: serverInSubnet, + }, + }) + } else if useP2P { confCmd, confErr = GetServerNetconfCmd(&conn, ServerNetconfOpts{ P2P: p2p.String(), }) @@ -916,7 +973,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) }) } if confErr != nil { - return fmt.Errorf("getting conf cmd for server %q: %w", server.Name, confErr) + return nil, fmt.Errorf("getting conf cmd for server %q: %w", server.Name, confErr) } netconfs[server.Name] = confCmd @@ -924,7 +981,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) if opts.WaitSwitchesReady { if err := WaitReady(ctx, kube, WaitReadyOpts{AppliedFor: 15 * time.Second, Timeout: 10 * time.Minute}); err != nil { - return fmt.Errorf("waiting for ready: %w", err) + return nil, fmt.Errorf("waiting for ready: %w", err) } } @@ -934,12 +991,12 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) existingAttaches := &vpcapi.VPCAttachmentList{} if err := kube.List(ctx, existingAttaches); err != nil { - return fmt.Errorf("listing existing attachments: %w", err) + return nil, fmt.Errorf("listing existing attachments: %w", err) } for _, attach := range existingAttaches.Items { if opts.ForceCleanup || !attachNames[attach.Name] { if err := kube.Delete(ctx, &attach); err != nil { - return fmt.Errorf("deleting attachment %q: %w", attach.Name, err) + return nil, fmt.Errorf("deleting attachment %q: %w", attach.Name, err) } slog.Info("Deleted", "attachment", attach.Name) changed = true @@ -948,13 +1005,13 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) existingVPCs := &vpcapi.VPCList{} if err := kube.List(ctx, existingVPCs); err != nil { - return fmt.Errorf("listing existing VPCs: %w", err) + return nil, fmt.Errorf("listing existing VPCs: %w", err) } deletedVPCs := false for _, vpc := range existingVPCs.Items { if opts.ForceCleanup || !vpcNames[vpc.Name] { if err := kube.Delete(ctx, &vpc); err != nil { - return fmt.Errorf("deleting VPC %q: %w", vpc.Name, err) + return nil, fmt.Errorf("deleting VPC %q: %w", vpc.Name, err) } slog.Info("Deleted", "vpc", vpc.Name) changed = true @@ -971,7 +1028,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) if deletedVPCs && opts.WaitSwitchesReady { if err := WaitReady(ctx, kube, WaitReadyOpts{AppliedFor: 15 * time.Second, Timeout: 10 * time.Minute}); err != nil { - return fmt.Errorf("waiting for switches after VPC deletion: %w", err) + return nil, fmt.Errorf("waiting for switches after VPC deletion: %w", err) } } } @@ -979,7 +1036,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) for _, vpc := range vpcs { iterChanged, err := CreateOrUpdateVpc(ctx, kube, vpc) if err != nil { - return fmt.Errorf("creating or updating vpc %q: %w", vpc.Name, err) + return nil, fmt.Errorf("creating or updating vpc %q: %w", vpc.Name, err) } changed = changed || iterChanged } @@ -994,7 +1051,7 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) return nil }) if err != nil { - return fmt.Errorf("creating or updating vpc attachment %q: %w", attach.Name, err) + return nil, fmt.Errorf("creating or updating vpc attachment %q: %w", attach.Name, err) } switch res { @@ -1011,12 +1068,12 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) // TODO remove it when we can actually know that changes to VPC/VPCAttachment are reflected in agents select { case <-ctx.Done(): - return fmt.Errorf("sleeping before waiting for ready: %w", ctx.Err()) + return nil, fmt.Errorf("sleeping before waiting for ready: %w", ctx.Err()) case <-time.After(15 * time.Second): } if err := WaitReady(ctx, kube, WaitReadyOpts{AppliedFor: 15 * time.Second, Timeout: 10 * time.Minute}); err != nil { - return fmt.Errorf("waiting for ready: %w", err) + return nil, fmt.Errorf("waiting for ready: %w", err) } } @@ -1123,12 +1180,17 @@ func (c *Config) SetupVPCs(ctx context.Context, vlab *VLAB, opts SetupVPCsOpts) } if err := g.Wait(); err != nil { - return fmt.Errorf("configuring servers: %w", err) + return nil, fmt.Errorf("configuring servers: %w", err) } slog.Info("All servers configured and verified", "took", time.Since(start)) - return nil + endpoints, err := CollectServerEndpoints(ctx, kube, SSHResolverFromMap(sshConfigs), nil) + if err != nil { + return nil, fmt.Errorf("collecting server endpoints: %w", err) + } + + return endpoints, nil } type SetupPeeringsOpts struct { @@ -1136,6 +1198,9 @@ type SetupPeeringsOpts struct { Requests []string } +// SetupPeerings creates/updates peerings per opts.Requests. It is matrix- +// agnostic: callers that want a post-peering connectivity expectation map +// invoke BuildConnectivityMatrix separately. See matrix.go for the model. func (c *Config) SetupPeerings(ctx context.Context, vlab *VLAB, opts SetupPeeringsOpts) error { ctx, cancel := context.WithTimeout(ctx, 30*time.Minute) defer cancel() @@ -1804,12 +1869,118 @@ func (c *Config) SetupPeerings(ctx context.Context, vlab *VLAB, opts SetupPeerin if err := DoSetupPeerings(ctx, kube, vpcPeerings, externalPeerings, gwPeerings, opts.WaitSwitchesReady); err != nil { return err } + slog.Info("VPC and External Peerings setup complete", "took", time.Since(start)) return nil } -func DoSetupPeerings(ctx context.Context, kube client.Client, vpcPeerings map[string]*vpcapi.VPCPeeringSpec, externalPeerings map[string]*vpcapi.ExternalPeeringSpec, gwPeerings map[string]*gwapi.PeeringSpec, waitReady bool) error { +// buildExternalEndpoints constructs one *Endpoint per External CRD. +// Prefixes are currently hardcoded to 0.0.0.0/0, which is what we usually +// use in test. +// SourceIP is left empty; callers that need to model external-originated +// traffic must populate it manually. +func buildExternalEndpoints(externals []vpcapi.External) []*Endpoint { + names := make([]string, 0, len(externals)) + for _, ext := range externals { + names = append(names, ext.Name) + } + slices.Sort(names) + + out := make([]*Endpoint, 0, len(names)) + defaultPrefix, _ := netip.ParsePrefix("0.0.0.0/0") + for _, name := range names { + + out = append(out, &Endpoint{ + External: &ExternalEndpoint{ + ExternalName: name, + Prefixes: []netip.Prefix{defaultPrefix}, + }, + }) + } + + return out +} + +// populateConnectivityMatrix iterates all endpoint pairs in m.AllEndpoints +// and adds an allow expectation when the cluster's current peering state +// reports the pair as reachable. Reverse-direction expectations are emitted +// independently. NAT translation details (SNAT/DNAT/port-forward/masquerade +// pools) and external-originated traffic are NOT modeled — callers must +// Add() explicit entries for those. +// +// TODO: IsServerReachable takes only (srcName, dstName) and so returns the +// same verdict for every (src_ep, dst_ep) pair sharing those names. For a +// server with multiple (vpc, subnet) attachments this is incorrect when +// the attachments live in mutually-isolated VPCs. Fixing it needs a +// reachability API that takes (server, vpc, subnet) on both sides; +// tracked as a follow-up to the endpoint-collection refactor. +func populateConnectivityMatrix(ctx context.Context, kube kclient.Reader, m *ConnectivityMatrix, gatewayEnabled bool) error { + // first reset all connectivity expectations + m.entries = make(map[EndpointPair]map[ProtoPort]ConnectivityExpectation) + for _, src := range m.AllEndpoints { + for _, dst := range m.AllEndpoints { + if src == dst { + continue + } + switch { + case src.Server != nil && dst.Server != nil: + if src.Server.Name == dst.Server.Name { + continue + } + r, err := IsServerReachable(ctx, kube, src.Server.Name, dst.Server.Name, gatewayEnabled) + if err != nil { + if errors.Is(err, reachCheckUnsupported) { + continue + } + return fmt.Errorf("checking %s -> %s: %w", src.Server.Name, dst.Server.Name, err) + } + if r.Reachable { + m.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: src, Destination: dst}, + Verdict: VerdictAllow, + Reason: r.Reason, + Peering: r.Peering, + }) + } + case src.Server != nil && dst.External != nil: + for _, prefix := range dst.External.Prefixes { + r, err := IsExternalSubnetReachable(ctx, kube, src.Server.Name, prefix.String(), gatewayEnabled) + if err != nil { + if errors.Is(err, reachCheckUnsupported) { + continue + } + return fmt.Errorf("checking %s -> %s/%s: %w", src.Server.Name, dst.External.ExternalName, prefix.String(), err) + } + if r.Reachable { + m.Add(ConnectivityExpectation{ + Pair: EndpointPair{Source: src, Destination: dst}, + Verdict: VerdictAllow, + Reason: r.Reason, + Peering: r.Peering, + }) + + break + } + } + // external-as-source and external-to-external paths are not + // auto-generated; the reachability helpers don't model them. + } + } + } + + return nil +} + +// DoSetupPeerings reconciles VPC/external/gateway peerings to match the +// caller's specs (deletes anything not in the spec, creates or updates the +// rest), optionally waits for switches to converge, and returns. It is +// matrix-agnostic: callers that need a post-peering ConnectivityMatrix +// invoke BuildConnectivityMatrix afterward. +func DoSetupPeerings(ctx context.Context, kube client.Client, vpcPeerings map[string]*vpcapi.VPCPeeringSpec, + externalPeerings map[string]*vpcapi.ExternalPeeringSpec, gwPeerings map[string]*gwapi.PeeringSpec, + waitReady bool, +) error { f, _, _, err := fab.GetFabAndNodes(ctx, kube, fab.GetFabAndNodesOpts{AllowNotHydrated: true}) if err != nil { return fmt.Errorf("getting fab: %w", err) @@ -2099,42 +2270,32 @@ type TestConnectivityOpts struct { RequireAllServers bool } -func (c *Config) TestConnectivity(ctx context.Context, vlab *VLAB, opts TestConnectivityOpts) error { - if opts.PingsCount == 0 && opts.IPerfsSeconds == 0 && opts.CurlsCount == 0 { - return fmt.Errorf("at least one of pings, iperfs or curls should be enabled") - } - start := time.Now() - - if opts.PingsParallel <= 0 { - opts.PingsParallel = 50 - } - if opts.IPerfsParallel <= 0 { - opts.IPerfsParallel = 1 - } - if opts.CurlsParallel <= 0 { - opts.CurlsParallel = 50 - } - - slog.Info("Testing server to server and server to external connectivity") - - sshConfigs := map[string]*sshutil.Config{} +// prepareConnectivityTest does the shared prelude work for connectivity +// tests: SSH config map for all vlab VMs, kube client with a cache, switch +// list and the IPerfsMinSpeed adjustment based on switch types, and +// optional WaitReady. The caller must call cleanup when finished to release +// the kube cache. opts is mutated in place to apply the IPerfsMinSpeed +// adjustment. +func (c *Config) prepareConnectivityTest(ctx context.Context, vlab *VLAB, opts *TestConnectivityOpts) (sshConfigs map[string]*sshutil.Config, kube kclient.Client, cleanup func(), err error) { + sshConfigs = map[string]*sshutil.Config{} for _, vm := range vlab.VMs { - if sshCfg, err := c.SSHVM(ctx, vlab, vm); err != nil { - return fmt.Errorf("getting ssh config for vm %q: %w", vm.Name, err) - } else { - sshConfigs[vm.Name] = sshCfg + sshCfg, err := c.SSHVM(ctx, vlab, vm) + if err != nil { + return nil, nil, nil, fmt.Errorf("getting ssh config for vm %q: %w", vm.Name, err) } + sshConfigs[vm.Name] = sshCfg } cacheCancel, kube, err := getKubeClientWithCache(ctx, c.WorkDir) if err != nil { - return fmt.Errorf("creating kube client: %w", err) + return nil, nil, nil, fmt.Errorf("creating kube client: %w", err) } - defer cacheCancel() switches := &wiringapi.SwitchList{} if err := kube.List(ctx, switches); err != nil { - return fmt.Errorf("listing switches: %w", err) + cacheCancel() + + return nil, nil, nil, fmt.Errorf("listing switches: %w", err) } allVirtual := len(switches.Items) > 0 allCumulusVX := true @@ -2160,9 +2321,96 @@ func (c *Config) TestConnectivity(ctx context.Context, vlab *VLAB, opts TestConn if opts.WaitSwitchesReady { if err := WaitReady(ctx, kube, WaitReadyOpts{AppliedFor: 15 * time.Second, Timeout: 10 * time.Minute}); err != nil { - return fmt.Errorf("waiting for ready: %w", err) + cacheCancel() + + return nil, nil, nil, fmt.Errorf("waiting for ready: %w", err) + } + } + + return sshConfigs, kube, cacheCancel, nil +} + +// pingIperfPairArgs holds the resolved inputs for one directional +// ping+iperf check between two endpoints. The caller is responsible for +// looking up the source SSH config, destination IP, expected reachability, +// and the bidir flag before invoking runPingIperfPair. +type pingIperfPairArgs struct { + From string + To string + FromSSH *sshutil.Config + ToIP netip.Addr + Expected Reachability + Bidir bool + Pings *semaphore.Weighted + Iperfs *semaphore.Weighted +} + +// runPingIperfPair runs ping and (when enabled and not lex-skipped in bidir +// mode) iperf for one (From → To) pair, returning every encountered error +// so the caller can route them. +func runPingIperfPair(ctx context.Context, opts TestConnectivityOpts, args pingIperfPairArgs) []error { + logArgs := []any{ + "from", args.From, + "to", args.To, + "expected", args.Expected.Reachable, + } + if args.Expected.Reachable { + logArgs = append(logArgs, "reason", args.Expected.Reason) + if args.Expected.Peering != "" { + logArgs = append(logArgs, "peering", args.Expected.Peering) } } + slog.Debug("Checking connectivity", logArgs...) + + var errs []error + if pe := checkPing(ctx, opts.PingsCount, args.Pings, args.From, args.To, args.FromSSH, args.ToIP, nil, args.Expected.Reachable); pe != nil { + return append(errs, pe) + } + + if opts.IPerfsSeconds <= 0 { + return errs + } + + // In bidir mode the lex-larger direction has nothing left to drive. + if args.Bidir && args.From > args.To { + return errs + } + + if err := args.Iperfs.Acquire(ctx, 1); err != nil { + return append(errs, fmt.Errorf("acquiring iperf3 semaphore: %w", err)) + } + defer args.Iperfs.Release(1) + + for _, ie := range checkIPerf(ctx, opts, args.From, args.To, args.FromSSH, args.ToIP, args.Expected, args.Bidir) { + errs = append(errs, ie) + } + + return errs +} + +func (c *Config) TestConnectivity(ctx context.Context, vlab *VLAB, opts TestConnectivityOpts) error { + if opts.PingsCount == 0 && opts.IPerfsSeconds == 0 && opts.CurlsCount == 0 { + return fmt.Errorf("at least one of pings, iperfs or curls should be enabled") + } + start := time.Now() + + if opts.PingsParallel <= 0 { + opts.PingsParallel = 50 + } + if opts.IPerfsParallel <= 0 { + opts.IPerfsParallel = 1 + } + if opts.CurlsParallel <= 0 { + opts.CurlsParallel = 50 + } + + slog.Info("Testing server to server and server to external connectivity") + + sshConfigs, kube, cacheCancel, err := c.prepareConnectivityTest(ctx, vlab, &opts) + if err != nil { + return err + } + defer cacheCancel() servers := &wiringapi.ServerList{} if err := kube.List(ctx, servers); err != nil { @@ -2219,55 +2467,28 @@ func (c *Config) TestConnectivity(ctx context.Context, vlab *VLAB, opts TestConn ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) defer cancel() - if err := func() error { - sshConfig, ok := sshConfigs[server] - if !ok { - return fmt.Errorf("missing ssh config for %q", server) - } - sshs.Store(server, sshConfig) - - stdout, stderr, err := sshConfig.Run(ctx, "ip -o -4 addr show | awk '{print $2, $4}'") - if err != nil { - return fmt.Errorf("running ip addr show: %w: %s", err, stderr) - } - - found := false - lines := strings.SplitSeq(strings.TrimSpace(stdout), "\n") - for line := range lines { - fields := strings.Fields(line) - if len(fields) != 2 { - return fmt.Errorf("unexpected ip addr line %q", line) - } - - if (fields[0] == "lo" && fields[1] == "127.0.0.1/8") || fields[0] == "enp2s0" || fields[0] == "docker0" { - continue - } - - if found { - return fmt.Errorf("unexpected multiple ip addrs") - } - - addr, err := netip.ParsePrefix(fields[1]) - if err != nil { - return fmt.Errorf("parsing ip addr %q: %w", fields[1], err) - } - - found = true - ips.Store(server, addr) - - slog.Info("Found", "server", server, "addr", addr.String()) - } - - if !found { - slog.Debug("No IP addr found", "server", server, "stdout", stdout, "stderr", stderr) - - return fmt.Errorf("no IP addr found") - } + sshConfig, ok := sshConfigs[server] + if !ok { + return fmt.Errorf("missing ssh config for %q", server) + } + sshs.Store(server, sshConfig) - return nil - }(); err != nil { + found, err := discoverServerIPs(ctx, sshConfig, server) + if err != nil { return fmt.Errorf("getting server %q IP: %w", server, err) } + // Legacy TestConnectivity assumes one VPC IP per server; the + // matrix-driven path (TestConnectivityWithMatrix) is the one + // that handles multi-IP servers correctly. + switch len(found) { + case 0: + return fmt.Errorf("no IP discovered for server %q", server) //nolint:goerr113 + case 1: + ips.Store(server, found[0].prefix.Addr()) + slog.Info("Found", "server", server, "addr", found[0].prefix.String()) + default: + return fmt.Errorf("server %q has multiple eligible IPs, use matrix-based connectivity test", server) //nolint:goerr113 + } return nil }) @@ -2325,73 +2546,53 @@ func (c *Config) TestConnectivity(ctx context.Context, vlab *VLAB, opts TestConn if opts.PingsCount > 0 || opts.IPerfsSeconds > 0 { wg.Go(func() { - if err := func() error { - expectedReachable, err := IsServerReachable(ctx, kube, serverA, serverB, c.Fab.Spec.Config.Gateway.Enable) - if err != nil { - return fmt.Errorf("checking if should be reachable: %w", err) - } - - logArgs := []any{ - "from", serverA, - "to", serverB, - "expected", expectedReachable.Reachable, - } - if expectedReachable.Reachable { - logArgs = append(logArgs, "reason", expectedReachable.Reason) - if expectedReachable.Peering != "" { - logArgs = append(logArgs, "peering", expectedReachable.Peering) - } - } - slog.Debug("Checking connectivity", logArgs...) + expectedReachable, err := IsServerReachable(ctx, kube, serverA, serverB, c.Fab.Spec.Config.Gateway.Enable) + if err != nil { + errChan <- fmt.Errorf("checking if should be reachable: %w", err) - ipBR, ok := ips.Load(serverB) - if !ok { - return fmt.Errorf("missing IP for %q", serverB) - } - ipB := ipBR.(netip.Prefix) + return + } - clientAR, ok := sshs.Load(serverA) - if !ok { - return fmt.Errorf("missing ssh client for %q", serverA) - } - clientA := clientAR.(*sshutil.Config) + ipBR, ok := ips.Load(serverB) + if !ok { + errChan <- fmt.Errorf("missing IP for %q", serverB) - if pe := checkPing(ctx, opts.PingsCount, pings, serverA, serverB, clientA, ipB.Addr(), nil, expectedReachable.Reachable); pe != nil { - return pe - } + return + } + ipB := ipBR.(netip.Addr) - if opts.IPerfsSeconds <= 0 { - return nil - } + clientAR, ok := sshs.Load(serverA) + if !ok { + errChan <- fmt.Errorf("missing ssh client for %q", serverA) - bidir := false - if expectedReachable.Reachable && requestedPairs[[2]string{serverB, serverA}] { - revReachable, err := IsServerReachable(ctx, kube, serverB, serverA, c.Fab.Spec.Config.Gateway.Enable) - if err != nil { - return fmt.Errorf("checking reverse reachability for bidir: %w", err) - } - if revReachable.Reachable { - bidir = true - } - } + return + } + clientA := clientAR.(*sshutil.Config) - // In bidir mode the lex-larger direction has nothing left to drive. - if bidir && serverA > serverB { - return nil - } + bidir := false + if opts.IPerfsSeconds > 0 && expectedReachable.Reachable && requestedPairs[[2]string{serverB, serverA}] { + revReachable, err := IsServerReachable(ctx, kube, serverB, serverA, c.Fab.Spec.Config.Gateway.Enable) + if err != nil { + errChan <- fmt.Errorf("checking reverse reachability for bidir: %w", err) - if err := iperfs.Acquire(ctx, 1); err != nil { - return fmt.Errorf("acquiring iperf3 semaphore: %s", err) + return } - defer iperfs.Release(1) - - for _, ie := range checkIPerf(ctx, opts, serverA, serverB, clientA, ipB.Addr(), expectedReachable, bidir) { - errChan <- ie + if revReachable.Reachable { + bidir = true } + } - return nil - }(); err != nil { - errChan <- err + for _, e := range runPingIperfPair(ctx, opts, pingIperfPairArgs{ + From: serverA, + To: serverB, + FromSSH: clientA, + ToIP: ipB, + Expected: expectedReachable, + Bidir: bidir, + Pings: pings, + Iperfs: iperfs, + }) { + errChan <- e } }) } @@ -2756,7 +2957,7 @@ func isVPCSubnetPresentInPeering(peering *gwapi.PeeringEntry, vpc gwapi.VPCInfo, } if len(expose.As) > 0 { - return false, fmt.Errorf("expose as %s is not supported yet", expose.As) + return false, fmt.Errorf("%w: gw peering with non-empty expose 'As' %s", reachCheckUnsupported, expose.As) } for _, exposeEntry := range expose.IPs { @@ -2775,7 +2976,7 @@ func isVPCSubnetPresentInPeering(peering *gwapi.PeeringEntry, vpc gwapi.VPCInfo, } } } else { - return false, fmt.Errorf("only cidr and vpcSubnet are supported as expose entries: %s", exposeEntry) + return false, fmt.Errorf("%w: gw peering with non-empty expose 'not' %s in IPs", reachCheckUnsupported, exposeEntry.Not) } if exposeSubnetName == vpcSubnet { diff --git a/pkg/hhfab/vlabbuilder.go b/pkg/hhfab/vlabbuilder.go index f8a032a09..cc51c88bd 100644 --- a/pkg/hhfab/vlabbuilder.go +++ b/pkg/hhfab/vlabbuilder.go @@ -55,7 +55,7 @@ type VLABBuilderDefault struct { ESLAGServers uint8 // number of ESLAG servers to generate for ESLAG switches UnbundledServers uint8 // number of unbundled servers to generate for switches (only for one of the first switch in the redundancy group or orphan switch) BundledServers uint8 // number of bundled servers to generate for switches (only for one of the second switch in the redundancy group or orphan switch) - MultiHomedServers uint8 // number of multi-homed servers (2 connections to 2 different orphan leaves) + MultiHomedServers uint8 // number of multi-homed servers (2 connections to 2 different leaves, preferably orphans) NoSwitches bool // do not generate any switches GatewayUplinks uint8 // number of uplinks for gateway node to the spines GatewayDriver string // gateway driver to use for gateway node @@ -219,8 +219,8 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri b.ESLAGServers = 0 } - if b.MultiHomedServers > 0 && b.OrphanLeafsCount < 2 { - return fmt.Errorf("at least two orphan leaves are needed for multihomed servers") //nolint:goerr113 + if b.MultiHomedServers > 0 && b.OrphanLeafsCount+totalESLAGLeafs < 2 { + return fmt.Errorf("at least two leaves are needed for multihomed servers") //nolint:goerr113 } if b.ExtESLAGConnCount > totalESLAGLeafs { @@ -364,6 +364,7 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri externalConns := []wiringapi.Connection{} extESLAGConns := uint8(0) extOrphanConns := uint8(0) + mhLeaves := []string{} for eslagID := uint8(0); eslagID < uint8(len(eslagLeafGroups)); eslagID++ { //nolint:gosec sg := fmt.Sprintf("eslag-%d", eslagID+1) @@ -376,6 +377,10 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri for eslagLeafID := uint8(0); eslagLeafID < leafs; eslagLeafID++ { leafName := fmt.Sprintf("leaf-%02d", leafID+eslagLeafID) leafNames = append(leafNames, leafName) + // add eslag leaves to candidates for multihomed servers if there are not enough orphan laves + if b.OrphanLeafsCount < 2 { + mhLeaves = append(mhLeaves, leafName) + } if _, err := b.createSwitch(ctx, leafName, wiringapi.SwitchSpec{ Role: wiringapi.SwitchRoleServerLeaf, @@ -485,7 +490,6 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri } } - orphanLeaves := []string{} for idx := uint8(1); idx <= b.OrphanLeafsCount; idx++ { leafName := fmt.Sprintf("leaf-%02d", leafID) @@ -495,7 +499,7 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri }, nil); err != nil { return err } - orphanLeaves = append(orphanLeaves, leafName) + mhLeaves = append(mhLeaves, leafName) if extOrphanConns < b.ExtOrphanConnCount { var err error @@ -562,16 +566,16 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri } } - orphanIdx := 0 + mhIdx := 0 for range int(b.MultiHomedServers) { serverName := fmt.Sprintf("server-%02d", serverID) - orphan1 := orphanLeaves[orphanIdx] - orphanIdx = (orphanIdx + 1) % len(orphanLeaves) - orphan2 := orphanLeaves[orphanIdx] - orphanIdx = (orphanIdx + 1) % len(orphanLeaves) + leaf1 := mhLeaves[mhIdx] + mhIdx = (mhIdx + 1) % len(mhLeaves) + leaf2 := mhLeaves[mhIdx] + mhIdx = (mhIdx + 1) % len(mhLeaves) if _, err := b.createServer(ctx, serverName, wiringapi.ServerSpec{ - Description: fmt.Sprintf("S-%02d MultiHomed %s + %s", serverID, orphan1, orphan2), + Description: fmt.Sprintf("S-%02d MultiHomed %s + %s", serverID, leaf1, leaf2), }); err != nil { return err } @@ -580,7 +584,7 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri Unbundled: &wiringapi.ConnUnbundled{ Link: wiringapi.ServerToSwitchLink{ Server: wiringapi.BasePortName{Port: b.nextServerPort(serverName)}, - Switch: wiringapi.BasePortName{Port: b.nextSwitchPort(orphan1)}, + Switch: wiringapi.BasePortName{Port: b.nextSwitchPort(leaf1)}, }, }, }); err != nil { @@ -590,7 +594,7 @@ func (b *VLABBuilderDefault) Build(ctx context.Context, l *apiutil.Loader, fabri Unbundled: &wiringapi.ConnUnbundled{ Link: wiringapi.ServerToSwitchLink{ Server: wiringapi.BasePortName{Port: b.nextServerPort(serverName)}, - Switch: wiringapi.BasePortName{Port: b.nextSwitchPort(orphan2)}, + Switch: wiringapi.BasePortName{Port: b.nextSwitchPort(leaf2)}, }, }, }); err != nil { diff --git a/pkg/hhfab/vlabrunner.go b/pkg/hhfab/vlabrunner.go index 77105c859..0284c5d10 100644 --- a/pkg/hhfab/vlabrunner.go +++ b/pkg/hhfab/vlabrunner.go @@ -694,7 +694,7 @@ func (c *Config) VLABRun(ctx context.Context, vlab *VLAB, opts VLABRunOpts) erro InterfaceMTU: opts.InterfaceMTU, } slog.Debug("Running setup-vpcs", "opts", setupVPCsOpts) - if err := c.SetupVPCs(ctx, vlab, setupVPCsOpts); err != nil { + if _, err := c.SetupVPCs(ctx, vlab, setupVPCsOpts); err != nil { return c.handleShutdownWithPause(ctx, vlab, opts, fmt.Errorf("setting up VPCs: %w", err)) } case OnReadySetupPeerings: