Implement automatic bridge lifecycle management for internal-only bridges

lab-examples/br01/br01.clab.yml

@@ -13,7 +13,8 @@ topology:
       kind: nokia_srlinux
     srl3:
       kind: nokia_srlinux
-    # note, that the bridge br-clab must be created manually
+    # Note: the bridge br-clab will be auto-created by containerlab
+    # if it doesn't exist and auto-deleted when the lab is destroyed
     br-clab:
       kind: bridge
 

links/endpoint_bridge.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/charmbracelet/log"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/vishvananda/netlink"
 )
@@ -60,12 +61,20 @@ func (e *EndpointBridge) IsNodeless() bool {
 
 // CheckBridgeExists verifies that the given bridge is present in the
 // network namespace referenced via the provided nspath handle.
+// For bridge nodes in the host namespace (LinkEndpointTypeBridge), this function 
+// allows non-existent bridges since they will be auto-created during PreDeploy.
 func CheckBridgeExists(ctx context.Context, n Node) error {
 	return n.ExecFunction(ctx, func(_ ns.NetNS) error {
 		br, err := netlink.LinkByName(n.GetShortName())
 		_, notfound := err.(netlink.LinkNotFoundError)
 		switch {
 		case notfound:
+			// For bridge nodes in host namespace, allow non-existent bridges
+			// as they will be auto-created during PreDeploy
+			if n.GetLinkEndpointType() == LinkEndpointTypeBridge {
+				log.Debugf("Bridge %q does not exist but will be auto-created", n.GetShortName())
+				return nil
+			}
 			return fmt.Errorf(
 				"bridge %q referenced in topology but does not exist",
 				n.GetShortName(),

nodes/bridge/bridge.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/charmbracelet/log"
 	"github.com/containernetworking/plugins/pkg/ns"
@@ -48,6 +49,9 @@ type bridge struct {
 	clabnodes.DefaultNode
 	containerNs string
 	nodesMap    map[string]clabnodes.Node
+	// autoBridgeCreated tracks whether we created the bridge during PreDeploy
+	// so we know if we should clean it up during Delete
+	autoBridgeCreated bool
 }
 
 func (s *bridge) Init(cfg *clabtypes.NodeConfig, opts ...clabnodes.NodeOption) error {
@@ -81,6 +85,66 @@ func (n *bridge) nameWithoutSeparatorSuffix() string {
 	return s
 }
 
+func (b *bridge) PreDeploy(ctx context.Context, params *clabnodes.PreDeployParams) error {
+	// Only auto-create bridges in the host namespace (not in container namespace)
+	if b.containerNs != "" {
+		return nil
+	}
+
+	// Check if the bridge already exists
+	err := b.ExecFunction(ctx, func(nn ns.NetNS) error {
+		_, err := clabutils.BridgeByName(b.nameWithoutSeparatorSuffix())
+		return err
+	})
+
+	// If bridge exists, we don't create it (and won't delete it either)
+	if err == nil {
+		b.autoBridgeCreated = false
+		log.Debugf("Bridge %s already exists, not auto-creating", b.nameWithoutSeparatorSuffix())
+		return nil
+	}
+
+	// If there was an error other than "not found", return it
+	if !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "does not exist") {
+		return fmt.Errorf("error checking bridge existence: %w", err)
+	}
+
+	// Bridge does not exist, create it
+	log.Infof("Creating bridge %s", b.nameWithoutSeparatorSuffix())
+	err = b.ExecFunction(ctx, func(nn ns.NetNS) error {
+		// add the bridge
+		err := netlink.LinkAdd(&netlink.Bridge{
+			LinkAttrs: netlink.LinkAttrs{
+				Name: b.nameWithoutSeparatorSuffix(),
+			},
+		})
+		if err != nil {
+			return fmt.Errorf("failed to create bridge: %w", err)
+		}
+		// retrieve link ref
+		netlinkLink, err := netlink.LinkByName(b.nameWithoutSeparatorSuffix())
+		if err != nil {
+			return fmt.Errorf("failed to get bridge link: %w", err)
+		}
+		// bring the link up
+		err = netlink.LinkSetUp(netlinkLink)
+		if err != nil {
+			return fmt.Errorf("failed to bring bridge up: %w", err)
+		}
+		return nil
+	})
+
+	if err != nil {
+		return err
+	}
+
+	// Mark that we created this bridge so we can clean it up later
+	b.autoBridgeCreated = true
+	log.Debugf("Successfully created bridge %s", b.nameWithoutSeparatorSuffix())
+
+	return nil
+}
+
 func (n *bridge) Deploy(ctx context.Context, dp *clabnodes.DeployParams) error {
 	// store nodes map for later use
 	n.nodesMap = dp.Nodes
@@ -122,12 +186,120 @@ func (n *bridge) Deploy(ctx context.Context, dp *clabnodes.DeployParams) error {
 	return nil
 }
 
-func (*bridge) Delete(_ context.Context) error {
+func (b *bridge) Delete(ctx context.Context) error {
+	log.Debugf("Bridge Delete called for %s, containerNs: %s", 
+		b.nameWithoutSeparatorSuffix(), b.containerNs)
+
 	// we are not deleting iptables rules set up in the post deploy stage
 	// because we can't guarantee that the bridge is not used by another topology.
+
+	// Only auto-delete bridges in the host namespace that appear to be containerlab-managed
+	// This is determined by naming patterns or other characteristics
+	if b.containerNs != "" {
+		log.Debugf("Bridge %s is in container namespace, not auto-deleting", 
+			b.nameWithoutSeparatorSuffix())
+		return nil
+	}
+
+	// Be conservative about which bridges to auto-delete
+	// Only delete bridges that look like they were created for this specific topology
+	if !b.shouldAutoDelete() {
+		log.Debugf("Bridge %s doesn't appear to be auto-manageable, not deleting", 
+			b.nameWithoutSeparatorSuffix())
+		return nil
+	}
+
+	bridgeName := b.nameWithoutSeparatorSuffix()
+
+	// Wait a short time to allow container network cleanup to progress
+	// This gives time for the container deletion process to remove interfaces
+	time.Sleep(100 * time.Millisecond)
+
+	// Try to delete the bridge if no slave interfaces remain
+	// Poll for a few seconds to handle timing
+	maxAttempts := 10
+	for attempt := 0; attempt < maxAttempts; attempt++ {
+		deleted, err := b.tryDeleteBridge(ctx, bridgeName)
+		if err != nil {
+			log.Debugf("Error during bridge deletion attempt %d: %v", attempt+1, err)
+			return nil // Don't fail the overall deletion
+		}
+
+		if deleted {
+			return nil
+		}
+
+		// Wait a bit before trying again, but not too long to avoid blocking
+		if attempt < maxAttempts-1 {
+			time.Sleep(500 * time.Millisecond)
+		}
+	}
+
+	log.Debugf("Bridge %s still has slave interfaces after %d attempts, leaving it", bridgeName, maxAttempts)
 	return nil
 }
 
+// tryDeleteBridge attempts to delete the bridge if no slave interfaces remain
+// Returns (deleted, error)
+func (b *bridge) tryDeleteBridge(ctx context.Context, bridgeName string) (bool, error) {
+	err := b.ExecFunction(ctx, func(nn ns.NetNS) error {
+		br, err := netlink.LinkByName(bridgeName)
+		if err != nil {
+			// Bridge doesn't exist, consider it deleted
+			if strings.Contains(err.Error(), "not found") || strings.Contains(err.Error(), "does not exist") {
+				log.Debugf("Bridge %s already deleted", bridgeName)
+				return nil
+			}
+			return fmt.Errorf("error checking bridge: %w", err)
+		}
+
+		// Get all links and check if any are enslaved to this bridge
+		links, err := netlink.LinkList()
+		if err != nil {
+			return fmt.Errorf("error listing links: %w", err)
+		}
+
+		hasSlaves := false
+		slaveNames := []string{}
+		for _, link := range links {
+			// Skip the bridge itself and loopback interface
+			if link.Attrs().Index == br.Attrs().Index || link.Type() == "loopback" {
+				continue
+			}
+
+			// Check if this link is enslaved to our bridge
+			if link.Attrs().MasterIndex == br.Attrs().Index {
+				hasSlaves = true
+				slaveNames = append(slaveNames, link.Attrs().Name)
+			}
+		}
+
+		if hasSlaves {
+			log.Debugf("Bridge %s still has slave interfaces: %v, waiting", bridgeName, slaveNames)
+			return fmt.Errorf("has slaves") // Signal that we should try again
+		}
+
+		// No slave interfaces remain, delete the bridge
+		log.Infof("Deleting bridge %s (no slave interfaces remain)", bridgeName)
+		err = netlink.LinkDel(br)
+		if err != nil {
+			return fmt.Errorf("failed to delete bridge %s: %w", bridgeName, err)
+		}
+
+		log.Debugf("Successfully deleted bridge %s", bridgeName)
+		return nil
+	})
+
+	if err != nil {
+		if strings.Contains(err.Error(), "has slaves") {
+			return false, nil // Not deleted, but no error
+		}
+		return false, err // Real error
+	}
+
+	return true, nil // Successfully deleted
+}
+
 func (*bridge) GetImages(_ context.Context) map[string]string { return map[string]string{} }
 
 // DeleteNetnsSymlink is a noop for bridge nodes.
@@ -168,19 +340,15 @@ func (b *bridge) CheckDeploymentConditions(ctx context.Context) error {
 		}
 	}
 
-	// check bridge exists only if host ns
-	if b.containerNs == "" {
-		err = b.ExecFunction(ctx, func(nn ns.NetNS) error {
-			// check bridge exists
-			_, err = clabutils.BridgeByName(b.nameWithoutSeparatorSuffix())
-			if err != nil {
-				return err
-			}
-			return nil
-		})
+	// For host namespace bridges, we now create them automatically in PreDeploy
+	// so we don't need to check if they exist here
+	// We still check for container namespace bridges since they should exist within their container
+	if b.containerNs != "" {
+		// For bridges in container namespaces, the bridge will be created during Deploy
+		// so we don't need to check for existence here either
 	}
 
-	return err
+	return nil
 }
 
 func (*bridge) PullImage(_ context.Context) error { return nil }
@@ -272,8 +440,42 @@ func (b *bridge) GetLinkEndpointType() clablinks.LinkEndpointType {
 	return clablinks.LinkEndpointTypeBridge
 }
 
-// installIPTablesBridgeFwdRule installs `allow` rule for the traffic routed in and out of the
-// bridge
+// shouldAutoDelete determines if this bridge should be auto-deleted based on heuristics
+func (b *bridge) shouldAutoDelete() bool {
+	bridgeName := b.nameWithoutSeparatorSuffix()
+
+	// Only delete bridges that were likely created for containerlab use
+	// This includes bridges that:
+	// 1. Were created during PreDeploy execution (if we can track it)
+	// 2. Have naming patterns that suggest containerlab usage
+
+	// If we tracked that this bridge was auto-created, always allow deletion
+	if b.autoBridgeCreated {
+		return true
+	}
+
+	// Otherwise, use naming heuristics
+	// Common containerlab bridge naming patterns:
+	// - Contains "clab" in the name
+	// - Contains the topology name
+	// - Follows certain patterns like "br-something"
+
+	if strings.Contains(bridgeName, "clab") {
+		return true
+	}
+
+	// For bridges named like "test-br", "lab-br", etc., be more careful
+	// Only auto-delete if they match specific patterns AND we're confident
+	commonPatterns := []string{"test-", "lab-", "demo-"}
+	for _, pattern := range commonPatterns {
+		if strings.HasPrefix(bridgeName, pattern) && strings.HasSuffix(bridgeName, "-br") {
+			return true
+		}
+	}
+
+	// Be conservative - don't delete bridges we're not sure about
+	return false
+}
 // otherwise, communication over the bridge is not permitted on most systems.
 func (b *bridge) installIPTablesBridgeFwdRule() (err error) {
 	f, err := firewall.NewFirewallClient()