From c08b225ee25815900702ab3ac7ed30823af27bd8 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Thu, 12 Dec 2024 22:02:00 +0100 Subject: [PATCH] Fix bug in nebula TUN device naming, causing it to force nebula to reload too much --- go/daemon/children/children.go | 29 ++++++++----- go/daemon/children/nebula.go | 13 ++++-- go/daemon/children/nebula_device_namer.go | 51 +++++++++++++++++++++++ go/daemon/network/loader.go | 6 +++ go/daemon/network/network.go | 30 ++++++++----- go/daemon/network/network_it_util_test.go | 23 ++++++---- go/nebula/device.go | 18 -------- tasks/v0.0.3/code/todo.md | 8 ---- 8 files changed, 121 insertions(+), 57 deletions(-) create mode 100644 go/daemon/children/nebula_device_namer.go delete mode 100644 go/nebula/device.go delete mode 100644 tasks/v0.0.3/code/todo.md diff --git a/go/daemon/children/children.go b/go/daemon/children/children.go index fa6d7ef..0f850d2 100644 --- a/go/daemon/children/children.go +++ b/go/daemon/children/children.go @@ -23,10 +23,11 @@ import ( // - dnsmasq // - garage (0 or more, depending on configured storage allocations) type Children struct { - logger *mlog.Logger - binDirPath string - runtimeDir toolkit.Dir - garageAdminToken string + logger *mlog.Logger + binDirPath string + runtimeDir toolkit.Dir + garageAdminToken string + nebulaDeviceNamer *NebulaDeviceNamer garageRPCSecret string @@ -45,6 +46,7 @@ func New( networkConfig daecommon.NetworkConfig, runtimeDir toolkit.Dir, garageAdminToken string, + nebulaDeviceNamer *NebulaDeviceNamer, hostBootstrap bootstrap.Bootstrap, ) ( *Children, error, @@ -56,11 +58,12 @@ func New( } c := &Children{ - logger: logger, - binDirPath: binDirPath, - runtimeDir: runtimeDir, - garageAdminToken: garageAdminToken, - garageRPCSecret: garageRPCSecret, + logger: logger, + binDirPath: binDirPath, + runtimeDir: runtimeDir, + garageAdminToken: garageAdminToken, + nebulaDeviceNamer: nebulaDeviceNamer, + garageRPCSecret: garageRPCSecret, } if c.nebulaProc, err = nebulaPmuxProc( @@ -68,6 +71,7 @@ func New( c.logger, c.runtimeDir.Path, c.binDirPath, + c.nebulaDeviceNamer, networkConfig, hostBootstrap, ); err != nil { @@ -145,7 +149,12 @@ func (c *Children) reloadNebula( hostBootstrap bootstrap.Bootstrap, ) error { if _, changed, err := nebulaWriteConfig( - ctx, c.logger, c.runtimeDir.Path, networkConfig, hostBootstrap, + ctx, + c.logger, + c.runtimeDir.Path, + c.nebulaDeviceNamer, + networkConfig, + hostBootstrap, ); err != nil { return fmt.Errorf("writing a new nebula config: %w", err) } else if !changed { diff --git a/go/daemon/children/nebula.go b/go/daemon/children/nebula.go index 3b4e6f7..c7bb968 100644 --- a/go/daemon/children/nebula.go +++ b/go/daemon/children/nebula.go @@ -6,7 +6,6 @@ import ( "io" "isle/bootstrap" "isle/daemon/daecommon" - "isle/nebula" "isle/toolkit" "isle/yamlutil" "net" @@ -53,6 +52,7 @@ func waitForNebula( } func nebulaConfig( + deviceNamer *NebulaDeviceNamer, networkConfig daecommon.NetworkConfig, hostBootstrap bootstrap.Bootstrap, ) ( @@ -119,7 +119,10 @@ func nebulaConfig( "respond": true, }, "tun": m{ - "dev": nebula.GetDeviceName(hostBootstrap.NetworkCreationParams.ID), + "dev": deviceNamer.getName( + hostBootstrap.NetworkCreationParams.ID, + hostBootstrap.ThisHost().IP(), + ), }, "firewall": firewall, } @@ -169,12 +172,13 @@ func nebulaWriteConfig( ctx context.Context, logger *mlog.Logger, runtimeDirPath string, + deviceNamer *NebulaDeviceNamer, networkConfig daecommon.NetworkConfig, hostBootstrap bootstrap.Bootstrap, ) ( string, bool, error, ) { - config, err := nebulaConfig(networkConfig, hostBootstrap) + config, err := nebulaConfig(deviceNamer, networkConfig, hostBootstrap) if err != nil { return "", false, fmt.Errorf("creating nebula config: %w", err) } @@ -199,13 +203,14 @@ func nebulaPmuxProc( ctx context.Context, logger *mlog.Logger, runtimeDirPath, binDirPath string, + deviceNamer *NebulaDeviceNamer, networkConfig daecommon.NetworkConfig, hostBootstrap bootstrap.Bootstrap, ) ( *pmuxlib.Process, error, ) { nebulaYmlPath, _, err := nebulaWriteConfig( - ctx, logger, runtimeDirPath, networkConfig, hostBootstrap, + ctx, logger, runtimeDirPath, deviceNamer, networkConfig, hostBootstrap, ) if err != nil { return nil, fmt.Errorf("writing nebula config: %w", err) diff --git a/go/daemon/children/nebula_device_namer.go b/go/daemon/children/nebula_device_namer.go new file mode 100644 index 0000000..2cb4a23 --- /dev/null +++ b/go/daemon/children/nebula_device_namer.go @@ -0,0 +1,51 @@ +package children + +import ( + "fmt" + "net/netip" + "sync" +) + +type nebulaDeviceNamerKey struct { + networkID string + deviceIP netip.Addr +} + +// NebulaDeviceNamer is used to assign unique TUN device names to different +// nebula networks running on the same host. It is intended to be shared amongst +// all running Children instances so that they do not accidentally conflict in +// TUN device names. +// +// NebulaDeviceNamer is thread-safe. +type NebulaDeviceNamer struct { + l sync.Mutex + counter uint64 + assigned map[nebulaDeviceNamerKey]string +} + +// NewNebulaDeviceNamer initializes and returns a new instance. +func NewNebulaDeviceNamer() *NebulaDeviceNamer { + return &NebulaDeviceNamer{ + assigned: map[nebulaDeviceNamerKey]string{}, + } +} + +// getName returns a unique TUN device name for the given network and IP, +// generating a new one if the ID has never been given before. +func (n *NebulaDeviceNamer) getName(networkID string, ip netip.Addr) string { + key := nebulaDeviceNamerKey{networkID, ip} + + n.l.Lock() + defer n.l.Unlock() + + if name, ok := n.assigned[key]; ok { + return name + } + + i := n.counter + n.counter++ + + name := fmt.Sprintf("isle%d-%s", i, networkID) + n.assigned[key] = name + return name +} diff --git a/go/daemon/network/loader.go b/go/daemon/network/loader.go index 9bf7d0e..9de4237 100644 --- a/go/daemon/network/loader.go +++ b/go/daemon/network/loader.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "isle/bootstrap" + "isle/daemon/children" "isle/daemon/daecommon" "isle/nebula" "isle/toolkit" @@ -126,6 +127,7 @@ type loader struct { envBinDirPath string networksStateDir toolkit.Dir networksRuntimeDir toolkit.Dir + nebulaDeviceNamer *children.NebulaDeviceNamer } // NewLoader returns a new Loader which will use the given directories to load @@ -167,6 +169,7 @@ func NewLoader( envBinDirPath, networksStateDir, networksRuntimeDir, + children.NewNebulaDeviceNamer(), }, nil } @@ -222,6 +225,7 @@ func (l *loader) Load( ctx, logger.WithNamespace("network"), l.envBinDirPath, + l.nebulaDeviceNamer, networkStateDir, networkRuntimeDir, opts, @@ -254,6 +258,7 @@ func (l *loader) Join( ctx, logger.WithNamespace("network"), l.envBinDirPath, + l.nebulaDeviceNamer, joiningBootstrap, networkStateDir, networkRuntimeDir, @@ -286,6 +291,7 @@ func (l *loader) Create( ctx, logger.WithNamespace("network"), l.envBinDirPath, + l.nebulaDeviceNamer, networkStateDir, networkRuntimeDir, creationParams, diff --git a/go/daemon/network/network.go b/go/daemon/network/network.go index 3b6ab70..88400b1 100644 --- a/go/daemon/network/network.go +++ b/go/daemon/network/network.go @@ -166,9 +166,10 @@ func (o *Opts) withDefaults() *Opts { type network struct { logger *mlog.Logger - envBinDirPath string - stateDir toolkit.Dir - runtimeDir toolkit.Dir + envBinDirPath string + nebulaDeviceNamer *children.NebulaDeviceNamer + stateDir toolkit.Dir + runtimeDir toolkit.Dir opts *Opts @@ -191,6 +192,7 @@ func newNetwork( ctx context.Context, logger *mlog.Logger, envBinDirPath string, + nebulaDeviceNamer *children.NebulaDeviceNamer, stateDir toolkit.Dir, runtimeDir toolkit.Dir, dirsMayExist bool, @@ -202,13 +204,14 @@ func newNetwork( var ( n = &network{ - logger: logger, - envBinDirPath: envBinDirPath, - stateDir: stateDir, - runtimeDir: runtimeDir, - opts: opts.withDefaults(), - workerCtx: ctx, - workerCancel: cancel, + logger: logger, + envBinDirPath: envBinDirPath, + nebulaDeviceNamer: nebulaDeviceNamer, + stateDir: stateDir, + runtimeDir: runtimeDir, + opts: opts.withDefaults(), + workerCtx: ctx, + workerCancel: cancel, } err error ) @@ -253,6 +256,7 @@ func load( ctx context.Context, logger *mlog.Logger, envBinDirPath string, + nebulaDeviceNamer *children.NebulaDeviceNamer, stateDir toolkit.Dir, runtimeDir toolkit.Dir, opts *Opts, @@ -263,6 +267,7 @@ func load( ctx, logger, envBinDirPath, + nebulaDeviceNamer, stateDir, runtimeDir, true, @@ -292,6 +297,7 @@ func join( ctx context.Context, logger *mlog.Logger, envBinDirPath string, + nebulaDeviceNamer *children.NebulaDeviceNamer, joiningBootstrap JoiningBootstrap, stateDir toolkit.Dir, runtimeDir toolkit.Dir, @@ -303,6 +309,7 @@ func join( ctx, logger, envBinDirPath, + nebulaDeviceNamer, stateDir, runtimeDir, false, @@ -329,6 +336,7 @@ func create( ctx context.Context, logger *mlog.Logger, envBinDirPath string, + nebulaDeviceNamer *children.NebulaDeviceNamer, stateDir toolkit.Dir, runtimeDir toolkit.Dir, creationParams bootstrap.CreationParams, @@ -349,6 +357,7 @@ func create( ctx, logger, envBinDirPath, + nebulaDeviceNamer, stateDir, runtimeDir, false, @@ -461,6 +470,7 @@ func (n *network) initialize( n.networkConfig, n.runtimeDir, n.opts.GarageAdminToken, + n.nebulaDeviceNamer, n.currBootstrap, ) if err != nil { diff --git a/go/daemon/network/network_it_util_test.go b/go/daemon/network/network_it_util_test.go index 6fd5796..833bd68 100644 --- a/go/daemon/network/network_it_util_test.go +++ b/go/daemon/network/network_it_util_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "isle/bootstrap" + "isle/daemon/children" "isle/daemon/daecommon" "isle/garage" "isle/nebula" @@ -60,10 +61,11 @@ func newPublicAddr() string { } type integrationHarness struct { - ctx context.Context - logger *mlog.Logger - rootDir toolkit.Dir - dirCounter atomic.Uint64 + ctx context.Context + logger *mlog.Logger + rootDir toolkit.Dir + dirCounter atomic.Uint64 + nebulaDeviceNamer *children.NebulaDeviceNamer } func newIntegrationHarness(t *testing.T) *integrationHarness { @@ -86,9 +88,10 @@ func newIntegrationHarness(t *testing.T) *integrationHarness { }) return &integrationHarness{ - ctx: context.Background(), - logger: toolkit.NewTestLogger(t), - rootDir: toolkit.Dir{Path: rootDir}, + ctx: context.Background(), + logger: toolkit.NewTestLogger(t), + rootDir: toolkit.Dir{Path: rootDir}, + nebulaDeviceNamer: children.NewNebulaDeviceNamer(), } } @@ -170,6 +173,7 @@ type integrationHarnessNetwork struct { logger *mlog.Logger hostName nebula.HostName stateDir, runtimeDir toolkit.Dir + nebulaDeviceNamer *children.NebulaDeviceNamer opts *Opts } @@ -204,6 +208,7 @@ func (h *integrationHarness) createNetwork( h.ctx, logger, getEnvBinDirPath(), + h.nebulaDeviceNamer, stateDir, runtimeDir, opts.creationParams, @@ -222,6 +227,7 @@ func (h *integrationHarness) createNetwork( hostName, stateDir, runtimeDir, + h.nebulaDeviceNamer, networkOpts, } @@ -286,6 +292,7 @@ func (h *integrationHarness) joinNetwork( h.ctx, logger, getEnvBinDirPath(), + h.nebulaDeviceNamer, joiningBootstrap, stateDir, runtimeDir, @@ -302,6 +309,7 @@ func (h *integrationHarness) joinNetwork( hostName, stateDir, runtimeDir, + h.nebulaDeviceNamer, networkOpts, } @@ -327,6 +335,7 @@ func (nh *integrationHarnessNetwork) restart(t *testing.T) { nh.ctx, nh.logger, getEnvBinDirPath(), + nh.nebulaDeviceNamer, nh.stateDir, nh.runtimeDir, nh.opts, diff --git a/go/nebula/device.go b/go/nebula/device.go deleted file mode 100644 index a9100f1..0000000 --- a/go/nebula/device.go +++ /dev/null @@ -1,18 +0,0 @@ -package nebula - -import ( - "fmt" - "sync/atomic" -) - -var deviceCounter = new(atomic.Uint64) - -// GetDeviceName returns the network device name to use for a particular -// network. Each returns name is gauranteed to be unique for the lifetime of the -// process. -func GetDeviceName(networkID string) string { - i := deviceCounter.Add(1) - 1 - // the returned string will be too long for linux, but it will get - // automatically truncated. - return fmt.Sprintf("isle%d-%s", i, networkID) -} diff --git a/tasks/v0.0.3/code/todo.md b/tasks/v0.0.3/code/todo.md deleted file mode 100644 index 8bffd05..0000000 --- a/tasks/v0.0.3/code/todo.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -type: tasks ---- - -# Address All TODOs - -All TODOs which remain in the codebase must be addressed or moved into a task -file.