vlab: generate wiring for externals

[edipascale]

Generate wirings for externals as part of hhfab vlab gen. Add parameters to determine how many externals and external connections are created, and where the ext. connections are placed (i.e. mclag, eslag or orphan leaves).

All attachments are currently vlan tagged, I didn't want to over complicate things with extra params for untagged.
Also all externals are in the same namespace, and attachments are created for all externals and all connections.

Closes: #521

[edipascale]

I've added commits to also address #522, but we have a problem here:

• I've set the default to 1 external and 1 connection on an eslag leaf, similar to what we have in physical envs
• however, in collapsed-core mode only mclag switches are created, which means that the defaults are now invalid for this mode
• and on the other hand, we don't want to default to mclag externals because of test-connectivity failure with (virtual) external peerings from mclag leaves #524

In terms of having the ci passing, it would be easy enough to add explicit values to the vlab-gen for either the collapsed-core (if we keep the defaults as described above) or the spine-leaf cases (if we do not touch the current default). The question is what is the "least undesirable" option @Frostman

[Frostman]

We should probably generated conns to external from the the from both switches of the last redundancy group configured. Let's chat next week and discuss what would be the best approach.

[edipascale]

current status:

• the changes in release-test to use the first external have to be reverted or at least adapted; we cannot use virtual externals for most scenarios, as multiple external peerings (together with the limitation on virtual switches and ACLs) mean that VPCs will be able to ping each other even when they shouldn't. We can either revert the change, or fetch the annotations for externals and discard non-hardware ones.
• on top of that, there is https://github.com/githedgehog/internal/issues/145 which makes test-connectivity jobs with the new external peering fail in the ci

[edipascale]

current status:

• the changes in release-test to use the first external have to be reverted or at least adapted; we cannot use virtual externals for most scenarios, as multiple external peerings (together with the limitation on virtual switches and ACLs) mean that VPCs will be able to ping each other even when they shouldn't. We can either revert the change, or fetch the annotations for externals and discard non-hardware ones.

I went for the second option but it's not perfect. Technically the issue is not with virtual externals, but with virtual switches attached to any type of externals. Because right now we only have hardware externals connected to hardware switches and virtual externals connected to virtual switches, discarding virtual externals is the easy solution... but we probably want to revisit it.

• on top of that, there is troubleshoot virtual external peering with active connections on multiple leaves internal#145 which makes test-connectivity jobs with the new external peering fail in the ci

I've already got an answer from BCM on the case I opened requesting more info, which I provided; let's see if we can get unblocked there. In the meanwhile I reduced the autogenerated external connections from 2 to 1, which should work even with this strange issue.

Also I'm seeing several failures in the CI, both here and in other PRs. but they do not appear related to the changes. I'll dig a bit more tomorrow.

[edipascale]

@Frostman this is technically ready to review now. I've removed the default external generation as discussed, and added warnings in case an option with multiple external connections is selected. The only thing missing is a run on a physical environment to check whether we need any additional fix for https://github.com/githedgehog/lab/issues/247, unsure whether we want to wait for that before tagging this as ready

[edipascale]

I could finally check this on physical environments, please see https://github.com/githedgehog/lab/issues/247 for details but this should be ready to be reviewed

[github-actions]

Test Results

  9 files  ±0  27 suites  ±0   4h 39m 48s ⏱️ +37s
 16 tests ±0  16 ✅ ±0   0 💤 ±0  0 ❌ ±0 
144 runs  ±0  56 ✅ ±0  88 💤 ±0  0 ❌ ±0 

Results for commit f1b3c2e. ± Comparison against base commit 8408af0.

[edipascale]

@Frostman I've also added an iptable rule to mimic the ACL to drop fabric-to-fabric traffic that we recently added on the ds2000-01, it appears to work:

core@control-1 ~ $ kubectl get externalpeering
NAME                  VPC      EXTERNAL      AGE
vpc-01--external-01   vpc-01   external-01   6m18s
vpc-08--external-02   vpc-08   external-02   6m18s

and

$ ./hhfab-ve vlab test-connectivity --source server-01 --source server-08 --destination server-01 --destination server-08 -v
12:10:12 INF Hedgehog Fabricator version=v0.40.0-107-gc9be7a7c-edp
12:10:12 INF Wiring hydrated successfully mode=if-not-present
12:10:12 INF VLAB config loaded file=vlab/config.yaml
12:10:12 INF Testing server to server and server to external connectivity
12:10:12 WRN All switches are virtual, ignoring IPerf min speed
12:10:12 INF Waiting for switches and gateways ready appliedFor=1m0s timeout=30m0s
12:10:12 INF Expected switches agent=v0.86.0 switches="[leaf-02 leaf-03 leaf-04 leaf-05 spine-01 spine-02 leaf-01]"
12:10:12 INF All switches and gateways are ready took=404.595918ms
12:10:13 INF Discovering server IPs servers=10
12:10:13 INF Found server=server-01 addr=10.0.1.2/24
12:10:13 INF Found server=server-08 addr=10.0.8.2/24
12:10:13 INF Running pings, iperfs and curls servers=10
12:10:13 DBG Checking external connectivity from=server-01 reachable=true
12:10:13 DBG Running curls from=server-01 to=1.0.0.1 count=3
12:10:13 DBG Checking external connectivity from=server-08 reachable=true
12:10:13 DBG Running curls from=server-08 to=1.0.0.1 count=3
12:10:13 DBG Checking connectivity from=server-08 to=server-01 expected=false
12:10:13 DBG Checking connectivity from=server-01 to=server-08 expected=false
12:10:13 DBG Running ping from=server-08 to=10.0.1.2
12:10:13 DBG Running ping from=server-01 to=10.0.8.2
12:10:13 DBG Curl result from=server-01 to=1.0.0.1 expected=true ok=true fail=false err=<nil> out="<html>\r\n<head><title>301 Moved Permanently</title></head>\r\n<body>\r\n<center><h1>301 Moved Permanently</h1></center>\r\n<hr><center>cloudflare</center>\r\n</body>\r\n</html>"
12:10:13 DBG Curl result from=server-08 to=1.0.0.1 expected=true ok=true fail=false err=<nil> out="<html>\r\n<head><title>301 Moved Permanently</title></head>\r\n<body>\r\n<center><h1>301 Moved Permanently</h1></center>\r\n<hr><center>cloudflare</center>\r\n</body>\r\n</html>"
12:10:13 DBG Curl result from=server-01 to=1.0.0.1 expected=true ok=true fail=false err=<nil> out="<html>\r\n<head><title>301 Moved Permanently</title></head>\r\n<body>\r\n<center><h1>301 Moved Permanently</h1></center>\r\n<hr><center>cloudflare</center>\r\n</body>\r\n</html>"
12:10:13 DBG Curl result from=server-08 to=1.0.0.1 expected=true ok=true fail=false err=<nil> out="<html>\r\n<head><title>301 Moved Permanently</title></head>\r\n<body>\r\n<center><h1>301 Moved Permanently</h1></center>\r\n<hr><center>cloudflare</center>\r\n</body>\r\n</html>"
12:10:13 DBG Curl result from=server-01 to=1.0.0.1 expected=true ok=true fail=false err=<nil> out="<html>\r\n<head><title>301 Moved Permanently</title></head>\r\n<body>\r\n<center><h1>301 Moved Permanently</h1></center>\r\n<hr><center>cloudflare</center>\r\n</body>\r\n</html>"
12:10:13 DBG Curl result from=server-08 to=1.0.0.1 expected=true ok=true fail=false err=<nil> out="<html>\r\n<head><title>301 Moved Permanently</title></head>\r\n<body>\r\n<center><h1>301 Moved Permanently</h1></center>\r\n<hr><center>cloudflare</center>\r\n</body>\r\n</html>"
12:10:18 DBG Ping result from=server-08 to=server-01 expected=false ok=false fail=true err="Process exited with status 1" out="PING 10.0.1.2 (10.0.1.2) 56(84) bytes of data.\n\n--- 10.0.1.2 ping statistics ---\n5 packets transmitted, 0 received, 100% packet loss, time 4119ms"
12:10:18 DBG Ping result from=server-01 to=server-08 expected=false ok=false fail=true err="Process exited with status 1" out="PING 10.0.8.2 (10.0.8.2) 56(84) bytes of data.\n\n--- 10.0.8.2 ping statistics ---\n5 packets transmitted, 0 received, 100% packet loss, time 4137ms"
12:10:18 INF All connectivity tested successfully took=6.580151402s

[Copilot]

Pull Request Overview

This PR adds functionality to generate wiring for external connections as part of the VLAB generation process. It introduces parameters to control the number of externals and external connections created, with placement options for MCLAG, ESLAG, or orphan leaves.

• Adds new CLI flags for configuring external generation parameters
• Improves external connection validation and error handling in VLAB config
• Updates BGP configuration template for better multipath support

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/hhfab/vlabconfig.go	Enhanced external connection validation logic with better error handling for mixed hardware/virtual scenarios
pkg/hhfab/vlabbuilder.go	Added external generation functionality with new parameters and connection creation methods
pkg/hhfab/vlab_external_butane.tmpl.yaml	Updated BGP configuration to enable multipath-relax and improved routing policies
pkg/hhfab/release.go	Refactored external detection logic and removed NamedExternal skip flag
cmd/hhfab/main.go	Added CLI flags for external configuration parameters

Comments suppressed due to low confidence (2)

pkg/hhfab/vlabbuilder.go:932

• [nitpick] The comment removal //nolint:unparam suggests this function now returns different error types, but the function signature and implementation appear unchanged. Consider adding back the nolint comment if the function still always returns the same error pattern, or document why it was removed.

func (b *VLABBuilder) createConnection(ctx context.Context, spec wiringapi.ConnectionSpec) (*wiringapi.Connection, error) {

pkg/hhfab/vlabbuilder.go:782

• [nitpick] The use of double hyphens '--' in the attachment name could be confusing and may cause parsing issues. Consider using a single hyphen or underscore for better readability and consistency.

				extAttachName := fmt.Sprintf("%s--%s", conn.Spec.External.Link.Switch.DeviceName(), ext.Name)