From bae6b06089b5d207086c851417e4b4be763bef7f Mon Sep 17 00:00:00 2001 From: Vadim Berezniker Date: Mon, 9 Dec 2024 14:18:19 -0800 Subject: [PATCH] Make task ip range configurable. (#7986) --- .../server/cmd/executor/executor_linux.go | 2 +- .../firecracker/firecracker_test.go | 4 +- .../containers/ociruntime/BUILD | 1 + .../containers/ociruntime/ociruntime_test.go | 3 + server/util/networking/networking.go | 68 +++++++++++++------ server/util/networking/networking_test.go | 10 +-- 6 files changed, 61 insertions(+), 27 deletions(-) diff --git a/enterprise/server/cmd/executor/executor_linux.go b/enterprise/server/cmd/executor/executor_linux.go index 97b9edad8f0..8af31d651b3 100644 --- a/enterprise/server/cmd/executor/executor_linux.go +++ b/enterprise/server/cmd/executor/executor_linux.go @@ -90,7 +90,7 @@ func setupNetworking(rootContext context.Context) { log.Debugf("Error cleaning up old net namespaces: %s", err) } } - if err := networking.ConfigureRoutingForIsolation(rootContext); err != nil { + if err := networking.Configure(rootContext); err != nil { fmt.Printf("Error configuring secondary network: %s", err) os.Exit(1) } diff --git a/enterprise/server/remote_execution/containers/firecracker/firecracker_test.go b/enterprise/server/remote_execution/containers/firecracker/firecracker_test.go index 66c00816167..866726ee591 100644 --- a/enterprise/server/remote_execution/containers/firecracker/firecracker_test.go +++ b/enterprise/server/remote_execution/containers/firecracker/firecracker_test.go @@ -185,8 +185,10 @@ type envOpts struct { } func getTestEnv(ctx context.Context, t *testing.T, opts envOpts) *testenv.TestEnv { + err := networking.Configure(ctx) + require.NoError(t, err) testnetworking.Setup(t) - err := networking.EnableMasquerading(ctx) + err = networking.EnableMasquerading(ctx) require.NoError(t, err) // Set up a lockfile directory to coordinate network locking across sharded // test processes on the host. diff --git a/enterprise/server/remote_execution/containers/ociruntime/BUILD b/enterprise/server/remote_execution/containers/ociruntime/BUILD index 457f09d297b..d2442260bcd 100644 --- a/enterprise/server/remote_execution/containers/ociruntime/BUILD +++ b/enterprise/server/remote_execution/containers/ociruntime/BUILD @@ -91,6 +91,7 @@ go_test( "//server/testutil/testshell", "//server/testutil/testtar", "//server/util/disk", + "//server/util/networking", "//server/util/proto", "//server/util/status", "//server/util/testing/flags", diff --git a/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go b/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go index 4708b5d3e72..9e1a3cbb51e 100644 --- a/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go +++ b/enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go @@ -34,6 +34,7 @@ import ( "github.com/buildbuddy-io/buildbuddy/server/testutil/testshell" "github.com/buildbuddy-io/buildbuddy/server/testutil/testtar" "github.com/buildbuddy-io/buildbuddy/server/util/disk" + "github.com/buildbuddy-io/buildbuddy/server/util/networking" "github.com/buildbuddy-io/buildbuddy/server/util/proto" "github.com/buildbuddy-io/buildbuddy/server/util/status" "github.com/buildbuddy-io/buildbuddy/server/util/testing/flags" @@ -66,6 +67,8 @@ func init() { } func setupNetworking(t *testing.T) { + err := networking.Configure(context.Background()) + require.NoError(t, err) testnetworking.Setup(t) // Disable network pooling in tests to simplify cleanup. flags.Set(t, "executor.oci.network_pool_size", 0) diff --git a/server/util/networking/networking.go b/server/util/networking/networking.go index dade70fdcd3..18dc408badc 100644 --- a/server/util/networking/networking.go +++ b/server/util/networking/networking.go @@ -6,6 +6,7 @@ import ( "flag" "fmt" "net" + "net/netip" "os" "os/exec" "path/filepath" @@ -32,6 +33,7 @@ var ( preserveExistingNetNamespaces = flag.Bool("executor.preserve_existing_netns", false, "Preserve existing bb-executor net namespaces. By default all \"bb-executor\" net namespaces are removed on executor startup, but if multiple executors are running on the same machine this behavior should be disabled to prevent them interfering with each other.") natSourcePortRange = flag.String("executor.nat_source_port_range", "", "If set, restrict the source ports for NATed traffic to this range. ") networkLockDir = flag.String("executor.network_lock_directory", "", "If set, use this directory to store lockfiles for allocated IP ranges. This is required if running multiple executors within the same networking environment.") + taskIPRange = flag.String("executor.task_ip_range", "192.168.0.0/16", "Subnet to allocate IP addresses from for actions that require network access. Must be a /16 range.") // Private IP ranges, as defined in RFC1918. PrivateIPRanges = []string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "169.254.0.0/16"} @@ -349,6 +351,8 @@ func (p *ContainerNetworkPool) Shutdown(ctx context.Context) error { // HostNetAllocator assigns unique /30 networks from the host for use in VMs. type HostNetAllocator struct { + baseAddr [4]byte + mu sync.Mutex // Next index to try locking; wraps around at numAssignableNetworks. // Since most tasks are short-lived, this usually will point to an index @@ -363,20 +367,34 @@ type HostNetAllocator struct { } } -var hostNetAllocator = &HostNetAllocator{} +func NewHostNetAllocator(ipRange string) (*HostNetAllocator, error) { + p, err := netip.ParsePrefix(ipRange) + if err != nil { + return nil, err + } + if !p.Addr().Is4() { + return nil, fmt.Errorf("ipRange not an IPv4 address") + } + if p.Bits() != 16 { + return nil, fmt.Errorf("ipRange is not a /16") + } + return &HostNetAllocator{baseAddr: p.Addr().As4()}, nil +} + +var hostNetAllocator *HostNetAllocator // HostNet represents a reserved /30 network from the host for use in a VM. type HostNet struct { - netIdx int - unlock func() -} - -func (n *HostNet) CIDR() string { - return fmt.Sprintf("192.168.%d.%d", n.netIdx/30, (n.netIdx%30)*8+4) + cidrSuffix + baseAddr [4]byte + netIdx int + unlock func() } func (n *HostNet) HostIP() string { - return fmt.Sprintf("192.168.%d.%d", n.netIdx/30, (n.netIdx%30)*8+5) + ip := n.baseAddr + ip[2] = byte(n.netIdx / 30) + ip[3] = byte(n.netIdx%30)*8 + 5 + return netip.AddrFrom4(ip).String() } func (n *HostNet) HostIPWithCIDR() string { @@ -384,7 +402,10 @@ func (n *HostNet) HostIPWithCIDR() string { } func (n *HostNet) NamespacedIP() string { - return fmt.Sprintf("192.168.%d.%d", n.netIdx/30, ((n.netIdx%30)*8)+6) + ip := n.baseAddr + ip[2] = byte(n.netIdx / 30) + ip[3] = byte(n.netIdx%30)*8 + 6 + return netip.AddrFrom4(ip).String() } func (n *HostNet) NamespacedIPWithCIDR() string { @@ -432,8 +453,9 @@ func (a *HostNetAllocator) Get() (*HostNet, error) { net.locked = true return &HostNet{ - netIdx: netIdx, - unlock: func() { a.unlock(netIdx) }, + baseAddr: a.baseAddr, + netIdx: netIdx, + unlock: func() { a.unlock(netIdx) }, }, nil } return nil, status.ResourceExhaustedError("host IP address space exhausted") @@ -957,19 +979,23 @@ func routingTableContainsTable(tableEntry string) (bool, error) { return false, nil } -// ConfigureRoutingForIsolation sets up a routing table for handling network -// isolation via either a secondary network interface or blackholing. -func ConfigureRoutingForIsolation(ctx context.Context) error { - if !IsSecondaryNetworkEnabled() { - // No need to add IP rule when we don't use secondary network - return nil +// Configure setups networking related infrastructure, such as traffic isolation +// and IP allocation. +func Configure(ctx context.Context) error { + a, err := NewHostNetAllocator(*taskIPRange) + if err != nil { + return status.WrapError(err, "could not create host network allocator") } + hostNetAllocator = a - // Adds a new routing table - if err := addRoutingTableEntryIfNotPresent(ctx); err != nil { - return err + if IsSecondaryNetworkEnabled() { + // Adds a new routing table + if err := addRoutingTableEntryIfNotPresent(ctx); err != nil { + return err + } + return configurePolicyBasedRoutingForSecondaryNetwork(ctx) } - return configurePolicyBasedRoutingForSecondaryNetwork(ctx) + return nil } // configurePolicyBasedRoutingForNetworkWIthRoutePrefix configures policy routing for secondary diff --git a/server/util/networking/networking_test.go b/server/util/networking/networking_test.go index 7b059a97a5a..e722b724f5b 100644 --- a/server/util/networking/networking_test.go +++ b/server/util/networking/networking_test.go @@ -31,9 +31,9 @@ const ( func TestHostNetAllocator(t *testing.T) { const n = 1000 - a := &networking.HostNetAllocator{} + a, err := networking.NewHostNetAllocator("192.168.0.0/16") + require.NoError(t, err) var nets [n]*networking.HostNet - var err error uniqueCIDRs := map[string]struct{}{} // Reserve all possible CIDRs @@ -88,9 +88,11 @@ func TestHostNetAllocator(t *testing.T) { } func TestConcurrentSetupAndCleanup(t *testing.T) { + ctx := context.Background() + err := networking.Configure(ctx) + require.NoError(t, err) testnetworking.Setup(t) - ctx := context.Background() eg, gCtx := errgroup.WithContext(ctx) eg.SetLimit(8) for i := 0; i < 20; i++ { @@ -111,7 +113,7 @@ func TestConcurrentSetupAndCleanup(t *testing.T) { return nil }) } - err := eg.Wait() + err = eg.Wait() require.NoError(t, err) }