From 9208f0b8ed80d016905f3b8d0d893c604db4c8f8 Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Wed, 22 Jul 2026 18:45:56 +0200 Subject: [PATCH 1/9] Add experimental prepared Agent startup --- cmd/agent/subcommands/run/BUILD.bazel | 2 + cmd/agent/subcommands/run/command.go | 10 +- cmd/trace-agent/subcommands/run/BUILD.bazel | 2 + cmd/trace-agent/subcommands/run/command.go | 6 +- comp/core/agentlifecycle/def/BUILD.bazel | 8 + comp/core/agentlifecycle/def/component.go | 37 +++ comp/core/agentlifecycle/fx/BUILD.bazel | 12 + comp/core/agentlifecycle/fx/fx.go | 19 ++ comp/core/agentlifecycle/impl/BUILD.bazel | 28 +++ .../agentlifecycle/impl/agent_lifecycle.go | 211 +++++++++++++++++ .../impl/agent_lifecycle_test.go | 222 ++++++++++++++++++ comp/core/healthprobe/impl/BUILD.bazel | 1 + comp/core/healthprobe/impl/healthprobe.go | 17 +- .../core/healthprobe/impl/healthprobe_test.go | 26 +- go.mod | 2 +- pkg/config/setup/BUILD.bazel | 1 + pkg/config/setup/agent_lifecycle.go | 23 ++ pkg/config/setup/config.go | 1 + pkg/config/setup/config_test.go | 8 + pkg/util/fxutil/BUILD.bazel | 2 + pkg/util/fxutil/oneshot.go | 50 +++- pkg/util/fxutil/run.go | 36 ++- pkg/util/fxutil/startup_gate.go | 47 ++++ pkg/util/fxutil/startup_gate_test.go | 119 ++++++++++ 24 files changed, 868 insertions(+), 22 deletions(-) create mode 100644 comp/core/agentlifecycle/def/BUILD.bazel create mode 100644 comp/core/agentlifecycle/def/component.go create mode 100644 comp/core/agentlifecycle/fx/BUILD.bazel create mode 100644 comp/core/agentlifecycle/fx/fx.go create mode 100644 comp/core/agentlifecycle/impl/BUILD.bazel create mode 100644 comp/core/agentlifecycle/impl/agent_lifecycle.go create mode 100644 comp/core/agentlifecycle/impl/agent_lifecycle_test.go create mode 100644 pkg/config/setup/agent_lifecycle.go create mode 100644 pkg/util/fxutil/startup_gate.go create mode 100644 pkg/util/fxutil/startup_gate_test.go diff --git a/cmd/agent/subcommands/run/BUILD.bazel b/cmd/agent/subcommands/run/BUILD.bazel index 10e887047385..dc652db69335 100644 --- a/cmd/agent/subcommands/run/BUILD.bazel +++ b/cmd/agent/subcommands/run/BUILD.bazel @@ -44,6 +44,8 @@ go_library( "//comp/collector/collector/impl", "//comp/connectivitychecker/fx", "//comp/core", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/agenttelemetry/def", "//comp/core/agenttelemetry/fx", "//comp/core/autodiscovery/def", diff --git a/cmd/agent/subcommands/run/command.go b/cmd/agent/subcommands/run/command.go index 3416aa989e2c..a5ab699cfe87 100644 --- a/cmd/agent/subcommands/run/command.go +++ b/cmd/agent/subcommands/run/command.go @@ -74,6 +74,8 @@ import ( collectorimpl "github.com/DataDog/datadog-agent/comp/collector/collector/impl" connectivitycheckerfx "github.com/DataDog/datadog-agent/comp/connectivitychecker/fx" "github.com/DataDog/datadog-agent/comp/core" + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + agentlifecyclefx "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx" autodiscovery "github.com/DataDog/datadog-agent/comp/core/autodiscovery/def" adfx "github.com/DataDog/datadog-agent/comp/core/autodiscovery/fx" "github.com/DataDog/datadog-agent/comp/core/autodiscovery/providers" @@ -235,7 +237,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { config.WithExtraConfFiles(cliParams.ExtraConfFilePath), config.WithFleetPoliciesDirPath(cliParams.FleetPoliciesDirPath), } - return fxutil.OneShot(run, + return fxutil.OneShotWithStartupGate[agentlifecycle.Component](run, fx.Invoke(func(_ log.Component) { ddruntime.SetMaxProcs() }), @@ -245,6 +247,8 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { LogParams: log.ForDaemon(command.LoggerName, "log_file", defaultpaths.GetDefaultLogFile()), }), fx.Supply(pidimpl.NewParams(cliParams.pidfilePath)), + fx.Supply(agentlifecycle.Params{ComponentName: "core-agent"}), + agentlifecyclefx.Module(), logging.EnableFxLoggingOnDebug[log.Component](), fxinstrumentation.Module(), getSharedFxOption(), @@ -273,6 +277,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { // run starts the main loop. func run(log log.Component, cfg config.Component, + agentLifecycle agentlifecycle.Component, flare flare.Component, tlm telemetry.Component, sysprobeConf sysprobeconfig.Component, @@ -392,6 +397,9 @@ func run(log log.Component, ); err != nil { return err } + if err := agentLifecycle.MarkActive(); err != nil { + return err + } agentStarted := tlm.NewCounter( "runtime", diff --git a/cmd/trace-agent/subcommands/run/BUILD.bazel b/cmd/trace-agent/subcommands/run/BUILD.bazel index e097ff24924e..d0a9a90c68b1 100644 --- a/cmd/trace-agent/subcommands/run/BUILD.bazel +++ b/cmd/trace-agent/subcommands/run/BUILD.bazel @@ -14,6 +14,8 @@ go_library( "//cmd/trace-agent/subcommands", "//comp/agent/autoexit/def", "//comp/agent/autoexit/fx", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/agenttelemetry/def", "//comp/core/agenttelemetry/fx", "//comp/core/config", diff --git a/cmd/trace-agent/subcommands/run/command.go b/cmd/trace-agent/subcommands/run/command.go index c0cb625ce004..a1528fb7a224 100644 --- a/cmd/trace-agent/subcommands/run/command.go +++ b/cmd/trace-agent/subcommands/run/command.go @@ -16,6 +16,8 @@ import ( "github.com/DataDog/datadog-agent/cmd/trace-agent/subcommands" autoexit "github.com/DataDog/datadog-agent/comp/agent/autoexit/def" autoexitfx "github.com/DataDog/datadog-agent/comp/agent/autoexit/fx" + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + agentlifecyclefx "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx" agenttelemetry "github.com/DataDog/datadog-agent/comp/core/agenttelemetry/def" agenttelemetryfx "github.com/DataDog/datadog-agent/comp/core/agenttelemetry/fx" coreconfig "github.com/DataDog/datadog-agent/comp/core/config" @@ -81,11 +83,13 @@ func runTraceAgentProcess(ctx context.Context, cliParams *Params, defaultConfPat if cliParams.ConfPath == "" { cliParams.ConfPath = defaultConfPath } - err := fxutil.Run( + err := fxutil.RunWithStartupGate[agentlifecycle.Component]( // ctx is required to be supplied from here, as Windows needs to inject its own context // to allow the agent to work as a service. fx.Provide(func() context.Context { return ctx }), // fx.Supply(ctx) fails with a missing type error. fx.Supply(coreconfig.NewAgentParams(cliParams.ConfPath, coreconfig.WithFleetPoliciesDirPath(cliParams.FleetPoliciesDirPath))), + fx.Supply(agentlifecycle.Params{ComponentName: "trace-agent"}), + agentlifecyclefx.Module(), secretsfx.Module(), delegatedauthfx.Module(), telemetryfx.Module(), diff --git a/comp/core/agentlifecycle/def/BUILD.bazel b/comp/core/agentlifecycle/def/BUILD.bazel new file mode 100644 index 000000000000..df4b1ab12f4a --- /dev/null +++ b/comp/core/agentlifecycle/def/BUILD.bazel @@ -0,0 +1,8 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "def", + srcs = ["component.go"], + importpath = "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def", + visibility = ["//visibility:public"], +) diff --git a/comp/core/agentlifecycle/def/component.go b/comp/core/agentlifecycle/def/component.go new file mode 100644 index 000000000000..2a4949cac182 --- /dev/null +++ b/comp/core/agentlifecycle/def/component.go @@ -0,0 +1,37 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package agentlifecycle defines the experimental prepared/active Agent lifecycle component. +package agentlifecycle + +import "context" + +// team: agent-runtimes + +const ( + // StatePrepared means construction is complete and the process is waiting for node ownership. + StatePrepared = "prepared" + // StateActivating means node ownership was acquired and Agent components are starting. + StateActivating = "activating" + // StateActive means the Agent process completed startup. + StateActive = "active" + // StateStopped means the Agent stopped and released node ownership. + StateStopped = "stopped" +) + +// Params identifies the Agent process using the lifecycle gate. +type Params struct { + ComponentName string +} + +// Component gates Agent startup on node-local ownership. +type Component interface { + // Wait reports Prepared and blocks until this process owns the node-local lock. + Wait(context.Context) error + // MarkActive reports that Agent startup completed. + MarkActive() error + // Close releases node-local ownership after Agent components have stopped. + Close() error +} diff --git a/comp/core/agentlifecycle/fx/BUILD.bazel b/comp/core/agentlifecycle/fx/BUILD.bazel new file mode 100644 index 000000000000..fcbbb058b9c8 --- /dev/null +++ b/comp/core/agentlifecycle/fx/BUILD.bazel @@ -0,0 +1,12 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "fx", + srcs = ["fx.go"], + importpath = "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx", + visibility = ["//visibility:public"], + deps = [ + "//comp/core/agentlifecycle/impl", + "//pkg/util/fxutil", + ], +) diff --git a/comp/core/agentlifecycle/fx/fx.go b/comp/core/agentlifecycle/fx/fx.go new file mode 100644 index 000000000000..7adcdfd60126 --- /dev/null +++ b/comp/core/agentlifecycle/fx/fx.go @@ -0,0 +1,19 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package fx provides the experimental Agent lifecycle component. +package fx + +import ( + agentlifecycleimpl "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/impl" + "github.com/DataDog/datadog-agent/pkg/util/fxutil" +) + +// Module defines the fx options for the experimental Agent lifecycle component. +func Module() fxutil.Module { + return fxutil.Component( + fxutil.ProvideComponentConstructor(agentlifecycleimpl.NewComponent), + ) +} diff --git a/comp/core/agentlifecycle/impl/BUILD.bazel b/comp/core/agentlifecycle/impl/BUILD.bazel new file mode 100644 index 000000000000..f12ad087ceba --- /dev/null +++ b/comp/core/agentlifecycle/impl/BUILD.bazel @@ -0,0 +1,28 @@ +load("@rules_go//go:def.bzl", "go_library") +load("//bazel/rules/go:dd_agent_go_test.bzl", "dd_agent_go_test") + +go_library( + name = "impl", + srcs = ["agent_lifecycle.go"], + importpath = "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/impl", + visibility = ["//visibility:public"], + deps = [ + "//comp/core/agentlifecycle/def", + "//comp/core/config", + "//comp/core/log/def", + "//comp/def", + "@com_github_gofrs_flock//:flock", + ], +) + +dd_agent_go_test( + name = "impl_test", + srcs = ["agent_lifecycle_test.go"], + embed = [":impl"], + deps = [ + "//comp/core/agentlifecycle/def", + "//comp/core/config", + "//comp/core/log/mock", + "@com_github_stretchr_testify//require", + ], +) diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle.go b/comp/core/agentlifecycle/impl/agent_lifecycle.go new file mode 100644 index 000000000000..ab343f3256f2 --- /dev/null +++ b/comp/core/agentlifecycle/impl/agent_lifecycle.go @@ -0,0 +1,211 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package agentlifecycleimpl implements the experimental prepared/active Agent lifecycle. +package agentlifecycleimpl + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "sync" + "time" + + "github.com/gofrs/flock" + + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + "github.com/DataDog/datadog-agent/comp/core/config" + log "github.com/DataDog/datadog-agent/comp/core/log/def" + compdef "github.com/DataDog/datadog-agent/comp/def" +) + +const ( + lockRetryInterval = 100 * time.Millisecond + rolloutEnabledKey = "experimental.node_agent_rollout.enabled" + rolloutLockPathKey = "experimental.node_agent_rollout.lock_path" + rolloutStatePathKey = "experimental.node_agent_rollout.state_path" +) + +type fileLocker interface { + TryLockContext(context.Context, time.Duration) (bool, error) + Unlock() error +} + +type dependencies struct { + compdef.In + + Config config.Component + Log log.Component + Params agentlifecycle.Params +} + +type component struct { + enabled bool + componentName string + lockPath string + statePath string + log log.Component + locker fileLocker + + mu sync.Mutex + acquired bool + closed bool +} + +var _ agentlifecycle.Component = (*component)(nil) + +// NewComponent creates the experimental Agent lifecycle component. +func NewComponent(deps dependencies) (agentlifecycle.Component, error) { + return newComponent(deps, func(path string) fileLocker { return flock.New(path) }) +} + +func newComponent(deps dependencies, newLocker func(string) fileLocker) (agentlifecycle.Component, error) { + if !deps.Config.GetBool(rolloutEnabledKey) { + return &component{}, nil + } + + lockPath := deps.Config.GetString(rolloutLockPathKey) + statePath := deps.Config.GetString(rolloutStatePathKey) + if deps.Params.ComponentName == "" { + return nil, errors.New("experimental node Agent rollout requires a component name") + } + if !filepath.IsAbs(lockPath) { + return nil, fmt.Errorf("%s must be an absolute path", rolloutLockPathKey) + } + if !filepath.IsAbs(statePath) { + return nil, fmt.Errorf("%s must be an absolute path", rolloutStatePathKey) + } + if filepath.Clean(lockPath) == filepath.Clean(statePath) { + return nil, errors.New("experimental node Agent rollout lock and state paths must differ") + } + if err := os.MkdirAll(filepath.Dir(lockPath), 0o755); err != nil { + return nil, fmt.Errorf("create Agent rollout lock directory: %w", err) + } + if err := os.MkdirAll(filepath.Dir(statePath), 0o755); err != nil { + return nil, fmt.Errorf("create Agent rollout state directory: %w", err) + } + + return &component{ + enabled: true, + componentName: deps.Params.ComponentName, + lockPath: lockPath, + statePath: statePath, + log: deps.Log, + locker: newLocker(lockPath), + }, nil +} + +func (c *component) Wait(ctx context.Context) error { + if !c.enabled { + return nil + } + + c.mu.Lock() + if c.closed { + c.mu.Unlock() + return errors.New("experimental Agent lifecycle is already closed") + } + if c.acquired { + c.mu.Unlock() + return errors.New("experimental Agent lifecycle already owns the node lock") + } + if err := c.writeState(agentlifecycle.StatePrepared); err != nil { + c.mu.Unlock() + return err + } + c.mu.Unlock() + + c.log.Infof("%s is prepared and waiting for node ownership at %s", c.componentName, c.lockPath) + locked, err := c.locker.TryLockContext(ctx, lockRetryInterval) + if err != nil { + return fmt.Errorf("acquire Agent rollout lock %q: %w", c.lockPath, err) + } + if !locked { + if ctx.Err() != nil { + return ctx.Err() + } + return fmt.Errorf("failed to acquire Agent rollout lock %q", c.lockPath) + } + + c.mu.Lock() + c.acquired = true + if err := c.writeState(agentlifecycle.StateActivating); err != nil { + c.acquired = false + c.mu.Unlock() + unlockErr := c.locker.Unlock() + return errors.Join(err, unlockErr) + } + c.mu.Unlock() + c.log.Infof("%s acquired node ownership and is activating", c.componentName) + return nil +} + +func (c *component) MarkActive() error { + if !c.enabled { + return nil + } + + c.mu.Lock() + defer c.mu.Unlock() + if c.closed { + return errors.New("cannot mark a closed experimental Agent lifecycle active") + } + if !c.acquired { + return errors.New("cannot mark the experimental Agent lifecycle active before acquiring node ownership") + } + if err := c.writeState(agentlifecycle.StateActive); err != nil { + return err + } + c.log.Infof("%s is active", c.componentName) + return nil +} + +func (c *component) Close() error { + if !c.enabled { + return nil + } + + c.mu.Lock() + defer c.mu.Unlock() + if c.closed { + return nil + } + c.closed = true + + stateErr := c.writeState(agentlifecycle.StateStopped) + var unlockErr error + if c.acquired { + unlockErr = c.locker.Unlock() + c.acquired = false + } + return errors.Join(stateErr, unlockErr) +} + +func (c *component) writeState(state string) error { + tmp, err := os.CreateTemp(filepath.Dir(c.statePath), ".agent-rollout-state-") + if err != nil { + return fmt.Errorf("create temporary Agent rollout state: %w", err) + } + tmpPath := tmp.Name() + defer os.Remove(tmpPath) + + if _, err := fmt.Fprintln(tmp, state); err != nil { + _ = tmp.Close() + return fmt.Errorf("write Agent rollout state: %w", err) + } + if err := tmp.Chmod(0o644); err != nil { + _ = tmp.Close() + return fmt.Errorf("set Agent rollout state permissions: %w", err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("close Agent rollout state: %w", err) + } + if err := os.Rename(tmpPath, c.statePath); err != nil { + return fmt.Errorf("publish Agent rollout state: %w", err) + } + return nil +} diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go new file mode 100644 index 000000000000..4fc7c2ebe942 --- /dev/null +++ b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go @@ -0,0 +1,222 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +//go:build test + +package agentlifecycleimpl + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" + + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + "github.com/DataDog/datadog-agent/comp/core/config" + logmock "github.com/DataDog/datadog-agent/comp/core/log/mock" +) + +type fakeLocker struct { + attempted chan struct{} + allow chan struct{} + unlocks atomic.Int32 +} + +func newFakeLocker() *fakeLocker { + return &fakeLocker{attempted: make(chan struct{}), allow: make(chan struct{})} +} + +func (l *fakeLocker) TryLockContext(ctx context.Context, _ time.Duration) (bool, error) { + close(l.attempted) + select { + case <-l.allow: + return true, nil + case <-ctx.Done(): + return false, ctx.Err() + } +} + +func (l *fakeLocker) Unlock() error { + l.unlocks.Add(1) + return nil +} + +func TestDisabledLifecycleIsNoop(t *testing.T) { + deps := dependencies{Config: config.NewMock(t), Log: logmock.New(t)} + comp, err := newComponent(deps, func(string) fileLocker { + t.Fatal("disabled lifecycle must not create a lock") + return nil + }) + require.NoError(t, err) + require.NoError(t, comp.Wait(context.Background())) + require.NoError(t, comp.MarkActive()) + require.NoError(t, comp.Close()) +} + +func TestLifecycleStateTransitions(t *testing.T) { + comp, locker, statePath := newEnabledComponent(t) + + waitResult := make(chan error, 1) + go func() { waitResult <- comp.Wait(context.Background()) }() + + <-locker.attempted + require.Equal(t, agentlifecycle.StatePrepared, readState(t, statePath)) + close(locker.allow) + require.NoError(t, <-waitResult) + require.Equal(t, agentlifecycle.StateActivating, readState(t, statePath)) + + require.NoError(t, comp.MarkActive()) + require.Equal(t, agentlifecycle.StateActive, readState(t, statePath)) + + require.NoError(t, comp.Close()) + require.Equal(t, agentlifecycle.StateStopped, readState(t, statePath)) + require.EqualValues(t, 1, locker.unlocks.Load()) + require.NoError(t, comp.Close(), "Close must be idempotent") + require.EqualValues(t, 1, locker.unlocks.Load()) +} + +func TestLifecycleWaitCancellation(t *testing.T) { + comp, locker, statePath := newEnabledComponent(t) + ctx, cancel := context.WithCancel(context.Background()) + waitResult := make(chan error, 1) + go func() { waitResult <- comp.Wait(ctx) }() + + <-locker.attempted + require.Equal(t, agentlifecycle.StatePrepared, readState(t, statePath)) + cancel() + require.ErrorIs(t, <-waitResult, context.Canceled) + require.NoError(t, comp.Close()) + require.Zero(t, locker.unlocks.Load()) +} + +func TestRealFileLockHandsOwnershipToPreparedReplacement(t *testing.T) { + dir := t.TempDir() + lockPath := filepath.Join(dir, "locks", "agent.lock") + newProcess := func(componentName, stateFile string) agentlifecycle.Component { + t.Helper() + deps := dependencies{ + Config: config.NewMockWithOverrides(t, map[string]interface{}{ + rolloutEnabledKey: true, + rolloutLockPathKey: lockPath, + rolloutStatePathKey: filepath.Join(dir, "state", stateFile), + }), + Log: logmock.New(t), + Params: agentlifecycle.Params{ComponentName: componentName}, + } + comp, err := NewComponent(deps) + require.NoError(t, err) + return comp + } + + oldProcess := newProcess("old-agent", "old.state") + replacement := newProcess("replacement-agent", "replacement.state") + require.NoError(t, oldProcess.Wait(context.Background())) + + replacementResult := make(chan error, 1) + go func() { replacementResult <- replacement.Wait(context.Background()) }() + require.Eventually(t, func() bool { + contents, err := os.ReadFile(filepath.Join(dir, "state", "replacement.state")) + return err == nil && strings.TrimSpace(string(contents)) == agentlifecycle.StatePrepared + }, time.Second, 10*time.Millisecond) + select { + case err := <-replacementResult: + t.Fatalf("replacement acquired ownership before the old process stopped: %v", err) + case <-time.After(200 * time.Millisecond): + } + + require.NoError(t, oldProcess.Close()) + select { + case err := <-replacementResult: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("replacement did not acquire ownership after the old process stopped") + } + require.Equal(t, agentlifecycle.StateActivating, readState(t, filepath.Join(dir, "state", "replacement.state"))) + require.NoError(t, replacement.Close()) +} + +func TestLifecycleRequiresValidPaths(t *testing.T) { + tests := map[string]map[string]interface{}{ + "relative lock": { + rolloutLockPathKey: "lock", + rolloutStatePathKey: filepath.Join(t.TempDir(), "state"), + }, + "relative state": { + rolloutLockPathKey: filepath.Join(t.TempDir(), "lock"), + rolloutStatePathKey: "state", + }, + "same path": { + rolloutLockPathKey: filepath.Join(t.TempDir(), "same"), + rolloutStatePathKey: "", + }, + } + for name, overrides := range tests { + t.Run(name, func(t *testing.T) { + overrides[rolloutEnabledKey] = true + if name == "same path" { + overrides[rolloutStatePathKey] = overrides[rolloutLockPathKey] + } + deps := dependencies{ + Config: config.NewMockWithOverrides(t, overrides), + Log: logmock.New(t), + Params: agentlifecycle.Params{ComponentName: "test-agent"}, + } + _, err := newComponent(deps, func(string) fileLocker { return newFakeLocker() }) + require.Error(t, err) + }) + } +} + +func TestMarkActiveRequiresOwnership(t *testing.T) { + comp, _, _ := newEnabledComponent(t) + require.Error(t, comp.MarkActive()) +} + +func TestWaitPropagatesLockerError(t *testing.T) { + comp, locker, _ := newEnabledComponent(t) + expected := errors.New("lock failed") + locker.allow = nil + comp.(*component).locker = errorLocker{err: expected} + require.ErrorIs(t, comp.Wait(context.Background()), expected) +} + +type errorLocker struct{ err error } + +func (l errorLocker) TryLockContext(context.Context, time.Duration) (bool, error) { + return false, l.err +} +func (errorLocker) Unlock() error { return nil } + +func newEnabledComponent(t *testing.T) (agentlifecycle.Component, *fakeLocker, string) { + t.Helper() + dir := t.TempDir() + statePath := filepath.Join(dir, "state", "agent.state") + locker := newFakeLocker() + deps := dependencies{ + Config: config.NewMockWithOverrides(t, map[string]interface{}{ + rolloutEnabledKey: true, + rolloutLockPathKey: filepath.Join(dir, "lock", "agent.lock"), + rolloutStatePathKey: statePath, + }), + Log: logmock.New(t), + Params: agentlifecycle.Params{ComponentName: "test-agent"}, + } + comp, err := newComponent(deps, func(string) fileLocker { return locker }) + require.NoError(t, err) + return comp, locker, statePath +} + +func readState(t *testing.T, path string) string { + t.Helper() + contents, err := os.ReadFile(path) + require.NoError(t, err) + return strings.TrimSpace(string(contents)) +} diff --git a/comp/core/healthprobe/impl/BUILD.bazel b/comp/core/healthprobe/impl/BUILD.bazel index 2de39edc5075..f8d90d1bdca7 100644 --- a/comp/core/healthprobe/impl/BUILD.bazel +++ b/comp/core/healthprobe/impl/BUILD.bazel @@ -26,5 +26,6 @@ dd_agent_go_test( "//comp/def", "//pkg/status/health", "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", ], ) diff --git a/comp/core/healthprobe/impl/healthprobe.go b/comp/core/healthprobe/impl/healthprobe.go index aa5d7e537b9a..b13efc61471d 100644 --- a/comp/core/healthprobe/impl/healthprobe.go +++ b/comp/core/healthprobe/impl/healthprobe.go @@ -48,6 +48,11 @@ type healthprobe struct { } func (h *healthprobe) start() error { + ln, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%v", h.options.Port)) + if err != nil { + return err + } + h.listener = ln h.log.Debugf("Health check listening on port %d", h.options.Port) go h.server.Serve(h.listener) //nolint:errcheck @@ -72,18 +77,12 @@ func NewComponent(reqs Requires) (Provides, error) { return provides, nil } - ln, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%v", healthPort)) - if err != nil { - return provides, err - } - server := buildServer(reqs.Options, reqs.Log) probe := &healthprobe{ - options: reqs.Options, - log: reqs.Log, - server: server, - listener: ln, + options: reqs.Options, + log: reqs.Log, + server: server, } reqs.Lc.Append(compdef.Hook{ diff --git a/comp/core/healthprobe/impl/healthprobe_test.go b/comp/core/healthprobe/impl/healthprobe_test.go index d3262a28a285..150bb6663214 100644 --- a/comp/core/healthprobe/impl/healthprobe_test.go +++ b/comp/core/healthprobe/impl/healthprobe_test.go @@ -9,11 +9,14 @@ package healthprobeimpl import ( "context" "errors" + "fmt" + "net" "net/http" "net/http/httptest" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" healthprobeComponent "github.com/DataDog/datadog-agent/comp/core/healthprobe/def" logmock "github.com/DataDog/datadog-agent/comp/core/log/mock" @@ -22,6 +25,10 @@ import ( ) func TestServer(t *testing.T) { + reserved, err := net.Listen("tcp", "0.0.0.0:0") + require.NoError(t, err) + port := reserved.Addr().(*net.TCPAddr).Port + require.NoError(t, reserved.Close()) lc := compdef.NewTestLifecycle(t) logComponent := logmock.New(t) @@ -30,21 +37,30 @@ func TestServer(t *testing.T) { Lc: lc, Log: logComponent, Options: healthprobeComponent.Options{ - Port: 7869, + Port: port, }, } provides, err := NewComponent(requires) - assert.NoError(t, err) + require.NoError(t, err) + + require.NotNil(t, provides.Comp) - assert.NotNil(t, provides.Comp) + beforeStart, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%d", port)) + require.NoError(t, err, "constructing the component must not claim the health port") + require.NoError(t, beforeStart.Close()) ctx := context.Background() lc.AssertHooksNumber(1) - assert.NoError(t, lc.Start(ctx)) - assert.NoError(t, lc.Stop(ctx)) + require.NoError(t, lc.Start(ctx)) + conflicting, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%d", port)) + require.Error(t, err, "starting the component must claim the health port") + if conflicting != nil { + require.NoError(t, conflicting.Close()) + } + require.NoError(t, lc.Stop(ctx)) } func TestServerNoHealthPort(t *testing.T) { diff --git a/go.mod b/go.mod index 3826ae6b697a..5515e3c7ebe7 100644 --- a/go.mod +++ b/go.mod @@ -722,7 +722,7 @@ require ( github.com/gocql/gocql v1.6.0 // indirect github.com/godbus/dbus v4.1.0+incompatible // indirect github.com/godror/knownpb v0.3.0 // indirect - github.com/gofrs/flock v0.13.0 // indirect + github.com/gofrs/flock v0.13.0 github.com/gofrs/uuid v4.4.0+incompatible // indirect github.com/gogo/googleapis v1.4.1 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect diff --git a/pkg/config/setup/BUILD.bazel b/pkg/config/setup/BUILD.bazel index 2b3253de04b4..713a266311d8 100644 --- a/pkg/config/setup/BUILD.bazel +++ b/pkg/config/setup/BUILD.bazel @@ -4,6 +4,7 @@ load("//bazel/rules/go:dd_agent_go_test.bzl", "dd_agent_go_test") go_library( name = "setup", srcs = [ + "agent_lifecycle.go", "apm_settings.go", "common_settings.go", "config.go", diff --git a/pkg/config/setup/agent_lifecycle.go b/pkg/config/setup/agent_lifecycle.go new file mode 100644 index 000000000000..47f18780631b --- /dev/null +++ b/pkg/config/setup/agent_lifecycle.go @@ -0,0 +1,23 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package setup + +import pkgconfigmodel "github.com/DataDog/datadog-agent/pkg/config/model" + +const ( + // ExperimentalNodeAgentRolloutEnabled enables the experimental prepared/active Agent lifecycle. + ExperimentalNodeAgentRolloutEnabled = "experimental.node_agent_rollout.enabled" + // ExperimentalNodeAgentRolloutLockPath is the node-local ownership lock used by one Agent process. + ExperimentalNodeAgentRolloutLockPath = "experimental.node_agent_rollout.lock_path" + // ExperimentalNodeAgentRolloutStatePath reports the local lifecycle state of one Agent process. + ExperimentalNodeAgentRolloutStatePath = "experimental.node_agent_rollout.state_path" +) + +func setupExperimentalNodeAgentRollout(config pkgconfigmodel.Setup) { + config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutEnabled, false) + config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutLockPath, "") + config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutStatePath, "") +} diff --git a/pkg/config/setup/config.go b/pkg/config/setup/config.go index d87d9fb9cf35..27985e40d0c3 100644 --- a/pkg/config/setup/config.go +++ b/pkg/config/setup/config.go @@ -282,6 +282,7 @@ func initCommonConfigComponents(config pkgconfigmodel.Setup) { // settings that are only initialized by the full agent, not serverless func initFullAgentOnlyComponents(config pkgconfigmodel.Setup) { comps := []func(pkgconfigmodel.Setup){ + setupExperimentalNodeAgentRollout, setupProcesses, setupPrivateActionRunner, remoteflags, diff --git a/pkg/config/setup/config_test.go b/pkg/config/setup/config_test.go index de8fd47209fb..0ad415ff3a26 100644 --- a/pkg/config/setup/config_test.go +++ b/pkg/config/setup/config_test.go @@ -772,6 +772,14 @@ func TestHealthPlatformDefaults(t *testing.T) { assert.Equal(t, true, config.GetBool("health_platform.invalidconfig_check.enabled")) } +func TestExperimentalNodeAgentRolloutDefaults(t *testing.T) { + config := confFromYAML(t, "") + + assert.False(t, config.GetBool(ExperimentalNodeAgentRolloutEnabled)) + assert.Empty(t, config.GetString(ExperimentalNodeAgentRolloutLockPath)) + assert.Empty(t, config.GetString(ExperimentalNodeAgentRolloutStatePath)) +} + func TestInfrastructureModeNoneDisablesECSTaskCollection(t *testing.T) { datadogYaml := ` infrastructure_mode: none diff --git a/pkg/util/fxutil/BUILD.bazel b/pkg/util/fxutil/BUILD.bazel index 70c03b5dd8d9..d2f2690b28e0 100644 --- a/pkg/util/fxutil/BUILD.bazel +++ b/pkg/util/fxutil/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "provide_comp.go", "provide_optional.go", "run.go", + "startup_gate.go", "test.go", "timeout.go", ], @@ -37,6 +38,7 @@ dd_agent_go_test( "errorunwrapper_test.go", "group_test.go", "provide_comp_test.go", + "startup_gate_test.go", "test_test.go", ], embed = [":fxutil"], diff --git a/pkg/util/fxutil/oneshot.go b/pkg/util/fxutil/oneshot.go index f970f13efd4d..98c83aef2f17 100644 --- a/pkg/util/fxutil/oneshot.go +++ b/pkg/util/fxutil/oneshot.go @@ -23,6 +23,23 @@ func OneShot(oneShotFunc interface{}, opts ...fx.Option) error { if fxAppTestOverride != nil { return fxAppTestOverride(oneShotFunc, opts) } + return oneShot(oneShotFunc, nil, opts...) +} + +// OneShotWithStartupGate constructs an Fx application, waits for the supplied gate before +// starting its lifecycle, invokes oneShotFunc, and releases the gate after lifecycle shutdown. +// The one-shot function is responsible for calling MarkActive after its non-Fx startup completes. +func OneShotWithStartupGate[T StartupGate](oneShotFunc interface{}, opts ...fx.Option) error { + if fxAppTestOverride != nil { + return fxAppTestOverride(oneShotFunc, opts) + } + + var gate T + opts = append(opts, fx.Populate(&gate)) + return oneShot(oneShotFunc, func() StartupGate { return gate }, opts...) +} + +func oneShot(oneShotFunc interface{}, gateProvider func() StartupGate, opts ...fx.Option) error { // Use a delayed Fx invocation to capture arguments for oneShotFunc during // application setup, but not actually invoke the question until all @@ -43,20 +60,47 @@ func OneShot(oneShotFunc interface{}, opts ...fx.Option) error { ) app := fx.New(opts...) + var gate StartupGate + if gateProvider != nil { + if err := app.Err(); err != nil { + return UnwrapIfErrArgumentsFailed(err) + } + gate = gateProvider() + if err := WaitForStartupGate(context.Background(), app, gate); err != nil { + if errors.Is(err, ErrStartupGateShutdown) { + return gate.Close() + } + return errors.Join(err, gate.Close()) + } + } + // start the app startCtx, cancel := context.WithTimeout(context.Background(), app.StartTimeout()) defer cancel() if err := app.Start(startCtx); err != nil { - return errors.Join(UnwrapIfErrArgumentsFailed(err), stopApp(app)) + return errors.Join(UnwrapIfErrArgumentsFailed(err), stopAppAndCloseGate(app, gate)) } // call the original oneShotFunc with the args captured during app startup err := delayedCall.call() if err != nil { - return errors.Join(err, stopApp(app)) + return errors.Join(err, stopAppAndCloseGate(app, gate)) } - return stopApp(app) + return stopAppAndCloseGate(app, gate) +} + +// stopAppAndCloseGate preserves ownership when Fx cannot prove that every +// lifecycle hook stopped. In that case process teardown and the kernel closing +// the lock descriptor remain the safety boundary. +func stopAppAndCloseGate(app *fx.App, gate StartupGate) error { + if err := stopApp(app); err != nil { + return err + } + if gate != nil { + return gate.Close() + } + return nil } func stopApp(app *fx.App) error { diff --git a/pkg/util/fxutil/run.go b/pkg/util/fxutil/run.go index d094927ee7e2..6356b4ef9552 100644 --- a/pkg/util/fxutil/run.go +++ b/pkg/util/fxutil/run.go @@ -20,6 +20,22 @@ func Run(opts ...fx.Option) error { if fxAppTestOverride != nil { return fxAppTestOverride(func() {}, opts) } + return run(nil, opts...) +} + +// RunWithStartupGate constructs an Fx application, waits for the supplied gate before +// starting its lifecycle, marks it active after startup, and releases it after shutdown. +func RunWithStartupGate[T StartupGate](opts ...fx.Option) error { + if fxAppTestOverride != nil { + return fxAppTestOverride(func() {}, opts) + } + + var gate T + opts = append(opts, fx.Populate(&gate)) + return run(func() StartupGate { return gate }, opts...) +} + +func run(gateProvider func() StartupGate, opts ...fx.Option) error { opts = append(opts, FxAgentBase()) // Temporarily increase timeout for all fxutil.Run calls until we can better characterize our @@ -34,14 +50,30 @@ func Run(opts ...fx.Option) error { return err } + var gate StartupGate + if gateProvider != nil { + gate = gateProvider() + if err := WaitForStartupGate(context.Background(), app, gate); err != nil { + if errors.Is(err, ErrStartupGateShutdown) { + return gate.Close() + } + return errors.Join(err, gate.Close()) + } + } + startCtx, cancel := context.WithTimeout(context.Background(), app.StartTimeout()) defer cancel() if err := app.Start(startCtx); err != nil { - return errors.Join(UnwrapIfErrArgumentsFailed(err), stopApp(app)) + return errors.Join(UnwrapIfErrArgumentsFailed(err), stopAppAndCloseGate(app, gate)) + } + if gate != nil { + if err := gate.MarkActive(); err != nil { + return errors.Join(err, stopAppAndCloseGate(app, gate)) + } } <-app.Done() - return stopApp(app) + return stopAppAndCloseGate(app, gate) } diff --git a/pkg/util/fxutil/startup_gate.go b/pkg/util/fxutil/startup_gate.go new file mode 100644 index 000000000000..8183442c2239 --- /dev/null +++ b/pkg/util/fxutil/startup_gate.go @@ -0,0 +1,47 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package fxutil + +import ( + "context" + "errors" + + "go.uber.org/fx" +) + +// StartupGate delays lifecycle startup until an external ownership condition is satisfied. +type StartupGate interface { + Wait(context.Context) error + MarkActive() error + Close() error +} + +// ErrStartupGateShutdown means an Fx shutdown signal arrived while the gate was waiting. +var ErrStartupGateShutdown = errors.New("shutdown while waiting on startup gate") + +// WaitForStartupGate waits for a gate and cancels it when the caller or Fx requests shutdown. +func WaitForStartupGate(ctx context.Context, app *fx.App, gate StartupGate) error { + waitCtx, cancel := context.WithCancel(ctx) + defer cancel() + + waitResult := make(chan error, 1) + go func() { + waitResult <- gate.Wait(waitCtx) + }() + + select { + case err := <-waitResult: + return err + case <-app.Done(): + cancel() + <-waitResult + return ErrStartupGateShutdown + case <-ctx.Done(): + cancel() + <-waitResult + return ctx.Err() + } +} diff --git a/pkg/util/fxutil/startup_gate_test.go b/pkg/util/fxutil/startup_gate_test.go new file mode 100644 index 000000000000..ff1c68c0f5de --- /dev/null +++ b/pkg/util/fxutil/startup_gate_test.go @@ -0,0 +1,119 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package fxutil + +import ( + "context" + "errors" + "sync" + "testing" + + "github.com/stretchr/testify/require" + "go.uber.org/fx" +) + +type gateRecorder struct { + mu sync.Mutex + events []string +} + +func (r *gateRecorder) add(event string) { + r.mu.Lock() + defer r.mu.Unlock() + r.events = append(r.events, event) +} + +func (r *gateRecorder) snapshot() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.events...) +} + +type recordingGate struct{ recorder *gateRecorder } + +func (g *recordingGate) Wait(context.Context) error { g.recorder.add("wait"); return nil } +func (g *recordingGate) MarkActive() error { g.recorder.add("active"); return nil } +func (g *recordingGate) Close() error { g.recorder.add("close"); return nil } + +type recordingComponent struct{} + +func TestOneShotWithStartupGateOrdering(t *testing.T) { + recorder := &gateRecorder{} + gate := &recordingGate{recorder: recorder} + + err := OneShotWithStartupGate[*recordingGate]( + func(_ *recordingComponent) { recorder.add("run") }, + fx.Supply(gate), + fx.Provide(func(lc fx.Lifecycle) *recordingComponent { + recorder.add("construct") + lc.Append(fx.Hook{ + OnStart: func(context.Context) error { recorder.add("start"); return nil }, + OnStop: func(context.Context) error { recorder.add("stop"); return nil }, + }) + return &recordingComponent{} + }), + ) + require.NoError(t, err) + require.Equal(t, []string{"construct", "wait", "start", "run", "stop", "close"}, recorder.snapshot()) +} + +func TestRunWithStartupGateOrdering(t *testing.T) { + recorder := &gateRecorder{} + gate := &recordingGate{recorder: recorder} + + err := RunWithStartupGate[*recordingGate]( + fx.Supply(gate), + fx.Invoke(func(lc fx.Lifecycle, shutdowner fx.Shutdowner) { + lc.Append(fx.Hook{ + OnStart: func(context.Context) error { + recorder.add("start") + return shutdowner.Shutdown() + }, + OnStop: func(context.Context) error { recorder.add("stop"); return nil }, + }) + }), + ) + require.NoError(t, err) + require.Equal(t, []string{"wait", "start", "active", "stop", "close"}, recorder.snapshot()) +} + +func TestOneShotWithStartupGateKeepsOwnershipWhenStopFails(t *testing.T) { + recorder := &gateRecorder{} + gate := &recordingGate{recorder: recorder} + stopErr := errors.New("stop failed") + + err := OneShotWithStartupGate[*recordingGate]( + func(_ *recordingComponent) { recorder.add("run") }, + fx.Supply(gate), + fx.Provide(func(lc fx.Lifecycle) *recordingComponent { + lc.Append(fx.Hook{ + OnStart: func(context.Context) error { recorder.add("start"); return nil }, + OnStop: func(context.Context) error { recorder.add("stop"); return stopErr }, + }) + return &recordingComponent{} + }), + ) + require.ErrorIs(t, err, stopErr) + require.Equal(t, []string{"wait", "start", "run", "stop"}, recorder.snapshot()) +} + +func TestRunWithStartupGateKeepsOwnershipWhenStopFails(t *testing.T) { + recorder := &gateRecorder{} + gate := &recordingGate{recorder: recorder} + stopErr := errors.New("stop failed") + + err := RunWithStartupGate[*recordingGate]( + fx.Supply(gate), + fx.Invoke(func(lc fx.Lifecycle, shutdowner fx.Shutdowner) { + lc.Append(fx.Hook{ + OnStart: func(context.Context) error { return shutdowner.Shutdown() }, + OnStop: func(context.Context) error { recorder.add("stop"); return stopErr }, + }) + }), + ) + require.ErrorIs(t, err, stopErr) + require.Equal(t, []string{"wait", "active", "stop"}, recorder.snapshot()) +} From 8cf90e343f2e8c681e2428ef5fd1a381d7cc98e7 Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Thu, 23 Jul 2026 12:04:59 +0200 Subject: [PATCH 2/9] Refresh Agent component index --- comp/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/comp/README.md b/comp/README.md index ef04613ecaba..9177c97bc741 100644 --- a/comp/README.md +++ b/comp/README.md @@ -99,6 +99,10 @@ Package collector defines the collector component. Package core implements the "core" bundle, providing services common to all agent flavors and binaries. +### [comp/core/agentlifecycle](https://pkg.go.dev/github.com/DataDog/datadog-agent/comp/core/agentlifecycle) + +Package agentlifecycle defines the experimental prepared/active Agent lifecycle component. + ### [comp/core/agenttelemetry](https://pkg.go.dev/github.com/DataDog/datadog-agent/comp/core/agenttelemetry) Package agenttelemetry implements a component to generate Agent telemetry From a56519827e2eb4252ebbd0c27a86890b50657499 Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Thu, 23 Jul 2026 13:56:37 +0200 Subject: [PATCH 3/9] Harden experimental Agent rollout startup --- cmd/agent/subcommands/run/BUILD.bazel | 4 + cmd/agent/subcommands/run/command.go | 2 +- cmd/agent/subcommands/run/command_windows.go | 10 +++ .../subcommands/run/command_windows_test.go | 22 +++++ comp/core/agentlifecycle/impl/BUILD.bazel | 1 + .../agentlifecycle/impl/agent_lifecycle.go | 45 +++++++++- .../impl/agent_lifecycle_test.go | 88 +++++++++++++++---- pkg/config/schema/yaml/core_schema.yaml | 20 +++++ pkg/config/setup/agent_lifecycle.go | 4 +- 9 files changed, 174 insertions(+), 22 deletions(-) diff --git a/cmd/agent/subcommands/run/BUILD.bazel b/cmd/agent/subcommands/run/BUILD.bazel index dc652db69335..a430494bf0ce 100644 --- a/cmd/agent/subcommands/run/BUILD.bazel +++ b/cmd/agent/subcommands/run/BUILD.bazel @@ -178,6 +178,7 @@ go_library( "//pkg/commonchecks", "//pkg/config/remote/data", "//pkg/config/settings", + "//pkg/config/setup", "//pkg/config/utils", "//pkg/diagnose/connectivity", "//pkg/diagnose/firewallscanner", @@ -304,8 +305,11 @@ dd_agent_go_test( deps = [ "//cmd/agent/command", "//comp/core", + "//comp/core/config", "//comp/core/pid/impl", + "//pkg/config/setup", "//pkg/util/fxutil", "@com_github_stretchr_testify//require", + "@org_uber_go_fx//:fx", ], ) diff --git a/cmd/agent/subcommands/run/command.go b/cmd/agent/subcommands/run/command.go index a5ab699cfe87..bc53bf088784 100644 --- a/cmd/agent/subcommands/run/command.go +++ b/cmd/agent/subcommands/run/command.go @@ -247,7 +247,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { LogParams: log.ForDaemon(command.LoggerName, "log_file", defaultpaths.GetDefaultLogFile()), }), fx.Supply(pidimpl.NewParams(cliParams.pidfilePath)), - fx.Supply(agentlifecycle.Params{ComponentName: "core-agent"}), + fx.Supply(agentlifecycle.Params{ComponentName: "agent"}), agentlifecyclefx.Module(), logging.EnableFxLoggingOnDebug[log.Component](), fxinstrumentation.Module(), diff --git a/cmd/agent/subcommands/run/command_windows.go b/cmd/agent/subcommands/run/command_windows.go index 0cf67e08ba4f..395177394be0 100644 --- a/cmd/agent/subcommands/run/command_windows.go +++ b/cmd/agent/subcommands/run/command_windows.go @@ -10,6 +10,7 @@ package run import ( "context" + "errors" _ "expvar" // Blank import used because this isn't directly used in this file _ "net/http/pprof" // Blank import used because this isn't directly used in this file @@ -84,6 +85,7 @@ import ( rcclient "github.com/DataDog/datadog-agent/comp/remote-config/rcclient/def" snmpscanmanager "github.com/DataDog/datadog-agent/comp/snmpscanmanager/def" softwareinventoryfx "github.com/DataDog/datadog-agent/comp/softwareinventory/fx" + configsetup "github.com/DataDog/datadog-agent/pkg/config/setup" "github.com/DataDog/datadog-agent/pkg/serializer" "github.com/DataDog/datadog-agent/pkg/util/defaultpaths" "github.com/DataDog/datadog-agent/pkg/util/fxutil" @@ -210,6 +212,7 @@ func StartAgentWithDefaults(ctxChan <-chan context.Context) (<-chan error, error SysprobeConfigParams: sysprobeconfigimpl.NewParams(), LogParams: log.ForDaemon(command.LoggerName, "log_file", defaultpaths.GetDefaultLogFile()), }), + fx.Invoke(validateWindowsPreparedRollout), getSharedFxOption(), getPlatformModules(), ) @@ -228,6 +231,13 @@ func StartAgentWithDefaults(ctxChan <-chan context.Context) (<-chan error, error return errChan, nil } +func validateWindowsPreparedRollout(config config.Component) error { + if config.GetBool(configsetup.ExperimentalNodeAgentRolloutEnabled) { + return errors.New("experimental node Agent rollout is not supported by the Windows service") + } + return nil +} + // Re-register the ctrl handler because Go uses SetConsoleCtrlHandler and Python uses posix signal calls. // This is only needed when using the embedded Python module. // When Python imports the signal module, it overrides the ctrl handler set by Go and installs the Windows posix signal handler. diff --git a/cmd/agent/subcommands/run/command_windows_test.go b/cmd/agent/subcommands/run/command_windows_test.go index a3937ab9fb2a..6f4969e07442 100644 --- a/cmd/agent/subcommands/run/command_windows_test.go +++ b/cmd/agent/subcommands/run/command_windows_test.go @@ -11,8 +11,11 @@ import ( "context" "testing" + coreconfig "github.com/DataDog/datadog-agent/comp/core/config" + configsetup "github.com/DataDog/datadog-agent/pkg/config/setup" "github.com/DataDog/datadog-agent/pkg/util/fxutil" "github.com/stretchr/testify/require" + "go.uber.org/fx" ) func TestStartAgentWithDefaults(t *testing.T) { @@ -23,3 +26,22 @@ func TestStartAgentWithDefaults(t *testing.T) { require.NoError(t, err) }) } + +func TestWindowsServiceRejectsPreparedRolloutBeforeStart(t *testing.T) { + cfg := coreconfig.NewMockWithOverrides(t, map[string]interface{}{ + configsetup.ExperimentalNodeAgentRolloutEnabled: true, + }) + started := false + app := fx.New( + fx.Supply(cfg), + fx.Invoke(validateWindowsPreparedRollout), + fx.Invoke(func(lifecycle fx.Lifecycle) { + lifecycle.Append(fx.Hook{OnStart: func(context.Context) error { + started = true + return nil + }}) + }), + ) + require.ErrorContains(t, app.Err(), "not supported") + require.False(t, started) +} diff --git a/comp/core/agentlifecycle/impl/BUILD.bazel b/comp/core/agentlifecycle/impl/BUILD.bazel index f12ad087ceba..062a0e844dff 100644 --- a/comp/core/agentlifecycle/impl/BUILD.bazel +++ b/comp/core/agentlifecycle/impl/BUILD.bazel @@ -23,6 +23,7 @@ dd_agent_go_test( "//comp/core/agentlifecycle/def", "//comp/core/config", "//comp/core/log/mock", + "@com_github_gofrs_flock//:flock", "@com_github_stretchr_testify//require", ], ) diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle.go b/comp/core/agentlifecycle/impl/agent_lifecycle.go index ab343f3256f2..ac3a5816c33b 100644 --- a/comp/core/agentlifecycle/impl/agent_lifecycle.go +++ b/comp/core/agentlifecycle/impl/agent_lifecycle.go @@ -12,6 +12,8 @@ import ( "fmt" "os" "path/filepath" + "runtime" + "strings" "sync" "time" @@ -64,15 +66,28 @@ func NewComponent(deps dependencies) (agentlifecycle.Component, error) { } func newComponent(deps dependencies, newLocker func(string) fileLocker) (agentlifecycle.Component, error) { + return newComponentForPlatform(deps, newLocker, runtime.GOOS) +} + +func newComponentForPlatform(deps dependencies, newLocker func(string) fileLocker, goos string) (agentlifecycle.Component, error) { if !deps.Config.GetBool(rolloutEnabledKey) { return &component{}, nil } + if err := validatePlatform(goos); err != nil { + return nil, err + } - lockPath := deps.Config.GetString(rolloutLockPathKey) - statePath := deps.Config.GetString(rolloutStatePathKey) if deps.Params.ComponentName == "" { return nil, errors.New("experimental node Agent rollout requires a component name") } + lockPath, err := resolveComponentPath(deps.Config.GetString(rolloutLockPathKey), deps.Params.ComponentName, ".lock", rolloutLockPathKey) + if err != nil { + return nil, err + } + statePath, err := resolveComponentPath(deps.Config.GetString(rolloutStatePathKey), deps.Params.ComponentName, ".state", rolloutStatePathKey) + if err != nil { + return nil, err + } if !filepath.IsAbs(lockPath) { return nil, fmt.Errorf("%s must be an absolute path", rolloutLockPathKey) } @@ -99,6 +114,32 @@ func newComponent(deps dependencies, newLocker func(string) fileLocker) (agentli }, nil } +func validatePlatform(goos string) error { + if goos != "linux" { + return fmt.Errorf("experimental node Agent rollout is Linux-only (running on %s)", goos) + } + return nil +} + +// resolveComponentPath makes the process identity part of every coordination +// path. A shared datadog.yaml can use the {component} token, while callers such +// as the Operator can continue supplying an already-expanded process path. +func resolveComponentPath(configuredPath, componentName, suffix, configKey string) (string, error) { + if filepath.Base(componentName) != componentName || componentName == "." || componentName == ".." { + return "", errors.New("experimental node Agent rollout component name must be a path-safe base name") + } + + if strings.Contains(configuredPath, "{component}") { + return strings.ReplaceAll(configuredPath, "{component}", componentName), nil + } + + expectedBase := componentName + suffix + if filepath.Base(configuredPath) != expectedBase { + return "", fmt.Errorf("%s must contain {component} or end in %q", configKey, expectedBase) + } + return configuredPath, nil +} + func (c *component) Wait(ctx context.Context) error { if !c.enabled { return nil diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go index 4fc7c2ebe942..2e34fcfec1b9 100644 --- a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go +++ b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + "github.com/gofrs/flock" "github.com/stretchr/testify/require" agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" @@ -99,31 +100,31 @@ func TestLifecycleWaitCancellation(t *testing.T) { func TestRealFileLockHandsOwnershipToPreparedReplacement(t *testing.T) { dir := t.TempDir() - lockPath := filepath.Join(dir, "locks", "agent.lock") - newProcess := func(componentName, stateFile string) agentlifecycle.Component { + lockPath := filepath.Join(dir, "locks", "{component}.lock") + newProcess := func(componentName string) agentlifecycle.Component { t.Helper() deps := dependencies{ Config: config.NewMockWithOverrides(t, map[string]interface{}{ rolloutEnabledKey: true, rolloutLockPathKey: lockPath, - rolloutStatePathKey: filepath.Join(dir, "state", stateFile), + rolloutStatePathKey: filepath.Join(dir, "state", "{component}.state"), }), Log: logmock.New(t), Params: agentlifecycle.Params{ComponentName: componentName}, } - comp, err := NewComponent(deps) + comp, err := newComponentForPlatform(deps, func(path string) fileLocker { return flock.New(path) }, "linux") require.NoError(t, err) return comp } - oldProcess := newProcess("old-agent", "old.state") - replacement := newProcess("replacement-agent", "replacement.state") + oldProcess := newProcess("agent") + replacement := newProcess("agent") require.NoError(t, oldProcess.Wait(context.Background())) replacementResult := make(chan error, 1) go func() { replacementResult <- replacement.Wait(context.Background()) }() require.Eventually(t, func() bool { - contents, err := os.ReadFile(filepath.Join(dir, "state", "replacement.state")) + contents, err := os.ReadFile(filepath.Join(dir, "state", "agent.state")) return err == nil && strings.TrimSpace(string(contents)) == agentlifecycle.StatePrepared }, time.Second, 10*time.Millisecond) select { @@ -139,24 +140,32 @@ func TestRealFileLockHandsOwnershipToPreparedReplacement(t *testing.T) { case <-time.After(2 * time.Second): t.Fatal("replacement did not acquire ownership after the old process stopped") } - require.Equal(t, agentlifecycle.StateActivating, readState(t, filepath.Join(dir, "state", "replacement.state"))) + require.Equal(t, agentlifecycle.StateActivating, readState(t, filepath.Join(dir, "state", "agent.state"))) require.NoError(t, replacement.Close()) } func TestLifecycleRequiresValidPaths(t *testing.T) { tests := map[string]map[string]interface{}{ "relative lock": { - rolloutLockPathKey: "lock", - rolloutStatePathKey: filepath.Join(t.TempDir(), "state"), + rolloutLockPathKey: "test-agent.lock", + rolloutStatePathKey: filepath.Join(t.TempDir(), "test-agent.state"), }, "relative state": { - rolloutLockPathKey: filepath.Join(t.TempDir(), "lock"), - rolloutStatePathKey: "state", + rolloutLockPathKey: filepath.Join(t.TempDir(), "test-agent.lock"), + rolloutStatePathKey: "test-agent.state", }, "same path": { - rolloutLockPathKey: filepath.Join(t.TempDir(), "same"), + rolloutLockPathKey: filepath.Join(t.TempDir(), "test-agent.lock"), rolloutStatePathKey: "", }, + "shared lock path": { + rolloutLockPathKey: filepath.Join(t.TempDir(), "agent.lock"), + rolloutStatePathKey: filepath.Join(t.TempDir(), "test-agent.state"), + }, + "shared state path": { + rolloutLockPathKey: filepath.Join(t.TempDir(), "test-agent.lock"), + rolloutStatePathKey: filepath.Join(t.TempDir(), "agent.state"), + }, } for name, overrides := range tests { t.Run(name, func(t *testing.T) { @@ -169,12 +178,57 @@ func TestLifecycleRequiresValidPaths(t *testing.T) { Log: logmock.New(t), Params: agentlifecycle.Params{ComponentName: "test-agent"}, } - _, err := newComponent(deps, func(string) fileLocker { return newFakeLocker() }) + _, err := newComponentForPlatform(deps, func(string) fileLocker { return newFakeLocker() }, "linux") require.Error(t, err) }) } } +func TestComponentPathResolution(t *testing.T) { + tests := map[string]struct { + configured string + component string + suffix string + expected string + }{ + "template": { + configured: "/var/run/datadog/{component}.lock", + component: "core-agent", + suffix: ".lock", + expected: "/var/run/datadog/core-agent.lock", + }, + "operator expanded path": { + configured: "/var/run/datadog/trace-agent.state", + component: "trace-agent", + suffix: ".state", + expected: "/var/run/datadog/trace-agent.state", + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + resolved, err := resolveComponentPath(test.configured, test.component, test.suffix, "test.path") + require.NoError(t, err) + require.Equal(t, test.expected, resolved) + }) + } +} + +func TestComponentPathRejectsSharedLiteral(t *testing.T) { + _, err := resolveComponentPath("/var/run/datadog/agent.lock", "core-agent", ".lock", "test.path") + require.ErrorContains(t, err, "must contain {component}") +} + +func TestPreparedRolloutRejectsWindows(t *testing.T) { + require.ErrorContains(t, validatePlatform("windows"), "Linux-only") + require.ErrorContains(t, validatePlatform("darwin"), "Linux-only") + require.NoError(t, validatePlatform("linux")) +} + +func TestComponentPathRejectsTraversalName(t *testing.T) { + _, err := resolveComponentPath("/var/run/datadog/{component}.lock", "..", ".lock", "test.path") + require.ErrorContains(t, err, "path-safe") +} + func TestMarkActiveRequiresOwnership(t *testing.T) { comp, _, _ := newEnabledComponent(t) require.Error(t, comp.MarkActive()) @@ -198,18 +252,18 @@ func (errorLocker) Unlock() error { return nil } func newEnabledComponent(t *testing.T) (agentlifecycle.Component, *fakeLocker, string) { t.Helper() dir := t.TempDir() - statePath := filepath.Join(dir, "state", "agent.state") + statePath := filepath.Join(dir, "state", "test-agent.state") locker := newFakeLocker() deps := dependencies{ Config: config.NewMockWithOverrides(t, map[string]interface{}{ rolloutEnabledKey: true, - rolloutLockPathKey: filepath.Join(dir, "lock", "agent.lock"), + rolloutLockPathKey: filepath.Join(dir, "lock", "test-agent.lock"), rolloutStatePathKey: statePath, }), Log: logmock.New(t), Params: agentlifecycle.Params{ComponentName: "test-agent"}, } - comp, err := newComponent(deps, func(string) fileLocker { return locker }) + comp, err := newComponentForPlatform(deps, func(string) fileLocker { return locker }, "linux") require.NoError(t, err) return comp, locker, statePath } diff --git a/pkg/config/schema/yaml/core_schema.yaml b/pkg/config/schema/yaml/core_schema.yaml index e9aff7c87968..99c6465b5bd7 100644 --- a/pkg/config/schema/yaml/core_schema.yaml +++ b/pkg/config/schema/yaml/core_schema.yaml @@ -8312,6 +8312,26 @@ properties: tags: - golang_type:duration comment: Duration during which the host tags will be submitted with metrics. + experimental: + node_type: section + type: object + properties: + node_agent_rollout: + node_type: section + type: object + properties: + enabled: + node_type: setting + type: boolean + default: false + lock_path: + node_type: setting + type: string + default: '' + state_path: + node_type: setting + type: string + default: '' external_metrics: node_type: section type: object diff --git a/pkg/config/setup/agent_lifecycle.go b/pkg/config/setup/agent_lifecycle.go index 47f18780631b..595813565efb 100644 --- a/pkg/config/setup/agent_lifecycle.go +++ b/pkg/config/setup/agent_lifecycle.go @@ -10,9 +10,9 @@ import pkgconfigmodel "github.com/DataDog/datadog-agent/pkg/config/model" const ( // ExperimentalNodeAgentRolloutEnabled enables the experimental prepared/active Agent lifecycle. ExperimentalNodeAgentRolloutEnabled = "experimental.node_agent_rollout.enabled" - // ExperimentalNodeAgentRolloutLockPath is the node-local ownership lock used by one Agent process. + // ExperimentalNodeAgentRolloutLockPath is the node-local ownership lock used by one Agent process. Shared configuration must include {component}. ExperimentalNodeAgentRolloutLockPath = "experimental.node_agent_rollout.lock_path" - // ExperimentalNodeAgentRolloutStatePath reports the local lifecycle state of one Agent process. + // ExperimentalNodeAgentRolloutStatePath reports the local lifecycle state of one Agent process. Shared configuration must include {component}. ExperimentalNodeAgentRolloutStatePath = "experimental.node_agent_rollout.state_path" ) From 292d1c1214d2e8c0ec40008be6f2595d5c4a57ab Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Thu, 23 Jul 2026 14:05:09 +0200 Subject: [PATCH 4/9] Refresh Agent Bazel dependencies --- cmd/agent/subcommands/run/BUILD.bazel | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cmd/agent/subcommands/run/BUILD.bazel b/cmd/agent/subcommands/run/BUILD.bazel index a430494bf0ce..e33c487ba9dc 100644 --- a/cmd/agent/subcommands/run/BUILD.bazel +++ b/cmd/agent/subcommands/run/BUILD.bazel @@ -178,7 +178,6 @@ go_library( "//pkg/commonchecks", "//pkg/config/remote/data", "//pkg/config/settings", - "//pkg/config/setup", "//pkg/config/utils", "//pkg/diagnose/connectivity", "//pkg/diagnose/firewallscanner", @@ -289,6 +288,7 @@ go_library( "//comp/trace/etwtracer/def", "//comp/trace/etwtracer/fx", "//pkg/config/model", + "//pkg/config/setup", "//pkg/util/winutil", ], "//conditions:default": [], @@ -305,11 +305,15 @@ dd_agent_go_test( deps = [ "//cmd/agent/command", "//comp/core", - "//comp/core/config", "//comp/core/pid/impl", - "//pkg/config/setup", "//pkg/util/fxutil", "@com_github_stretchr_testify//require", - "@org_uber_go_fx//:fx", - ], + ] + select({ + "@rules_go//go/platform:windows": [ + "//comp/core/config", + "//pkg/config/setup", + "@org_uber_go_fx//:fx", + ], + "//conditions:default": [], + }), ) From 00d47ac6ce84ee2ccf9673dfa2f4276156ac5adb Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Thu, 23 Jul 2026 15:09:12 +0200 Subject: [PATCH 5/9] Fix Windows prepared rollout test wiring --- cmd/agent/subcommands/run/command_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/subcommands/run/command_windows_test.go b/cmd/agent/subcommands/run/command_windows_test.go index 6f4969e07442..44d0ac11ce62 100644 --- a/cmd/agent/subcommands/run/command_windows_test.go +++ b/cmd/agent/subcommands/run/command_windows_test.go @@ -33,7 +33,7 @@ func TestWindowsServiceRejectsPreparedRolloutBeforeStart(t *testing.T) { }) started := false app := fx.New( - fx.Supply(cfg), + fx.Provide(func() coreconfig.Component { return cfg }), fx.Invoke(validateWindowsPreparedRollout), fx.Invoke(func(lifecycle fx.Lifecycle) { lifecycle.Append(fx.Hook{OnStart: func(context.Context) error { From 839ba53a59c1b76d02219c9d6479e54966cd70ef Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Fri, 24 Jul 2026 13:19:23 +0200 Subject: [PATCH 6/9] Use kubelet siblings for prepared Agent rollout --- comp/core/agentlifecycle/def/component.go | 4 +- comp/core/agentlifecycle/impl/BUILD.bazel | 9 +- .../agentlifecycle/impl/agent_lifecycle.go | 278 ++++++++++---- .../impl/agent_lifecycle_test.go | 346 +++++++++++------- .../agentlifecycle/impl/local_pods_kubelet.go | 54 +++ .../impl/local_pods_nokubelet.go | 23 ++ go.mod | 2 +- pkg/config/schema/yaml/core_schema.yaml | 2 +- pkg/config/setup/agent_lifecycle.go | 6 +- pkg/config/setup/config_test.go | 2 +- pkg/util/fxutil/oneshot.go | 5 +- 11 files changed, 524 insertions(+), 207 deletions(-) create mode 100644 comp/core/agentlifecycle/impl/local_pods_kubelet.go create mode 100644 comp/core/agentlifecycle/impl/local_pods_nokubelet.go diff --git a/comp/core/agentlifecycle/def/component.go b/comp/core/agentlifecycle/def/component.go index 2a4949cac182..a03cb58aa149 100644 --- a/comp/core/agentlifecycle/def/component.go +++ b/comp/core/agentlifecycle/def/component.go @@ -26,9 +26,9 @@ type Params struct { ComponentName string } -// Component gates Agent startup on node-local ownership. +// Component gates Agent startup until older sibling Pods have left the node. type Component interface { - // Wait reports Prepared and blocks until this process owns the node-local lock. + // Wait reports Prepared and blocks until no older Pod owned by the same DaemonSet remains on the node. Wait(context.Context) error // MarkActive reports that Agent startup completed. MarkActive() error diff --git a/comp/core/agentlifecycle/impl/BUILD.bazel b/comp/core/agentlifecycle/impl/BUILD.bazel index 062a0e844dff..83a4da49a6a1 100644 --- a/comp/core/agentlifecycle/impl/BUILD.bazel +++ b/comp/core/agentlifecycle/impl/BUILD.bazel @@ -3,7 +3,11 @@ load("//bazel/rules/go:dd_agent_go_test.bzl", "dd_agent_go_test") go_library( name = "impl", - srcs = ["agent_lifecycle.go"], + srcs = [ + "agent_lifecycle.go", + "local_pods_kubelet.go", + "local_pods_nokubelet.go", + ], importpath = "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/impl", visibility = ["//visibility:public"], deps = [ @@ -11,7 +15,7 @@ go_library( "//comp/core/config", "//comp/core/log/def", "//comp/def", - "@com_github_gofrs_flock//:flock", + "//pkg/util/kubernetes/kubelet", ], ) @@ -23,7 +27,6 @@ dd_agent_go_test( "//comp/core/agentlifecycle/def", "//comp/core/config", "//comp/core/log/mock", - "@com_github_gofrs_flock//:flock", "@com_github_stretchr_testify//require", ], ) diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle.go b/comp/core/agentlifecycle/impl/agent_lifecycle.go index ac3a5816c33b..4f76ca01d608 100644 --- a/comp/core/agentlifecycle/impl/agent_lifecycle.go +++ b/comp/core/agentlifecycle/impl/agent_lifecycle.go @@ -13,12 +13,11 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "sync" "time" - "github.com/gofrs/flock" - agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" "github.com/DataDog/datadog-agent/comp/core/config" log "github.com/DataDog/datadog-agent/comp/core/log/def" @@ -26,15 +25,28 @@ import ( ) const ( - lockRetryInterval = 100 * time.Millisecond + siblingPollInterval = time.Second rolloutEnabledKey = "experimental.node_agent_rollout.enabled" - rolloutLockPathKey = "experimental.node_agent_rollout.lock_path" + rolloutPodUIDKey = "experimental.node_agent_rollout.pod_uid" rolloutStatePathKey = "experimental.node_agent_rollout.state_path" ) -type fileLocker interface { - TryLockContext(context.Context, time.Duration) (bool, error) - Unlock() error +type podOwner struct { + kind string + uid string + controller bool +} + +type localPod struct { + uid string + name string + namespace string + createdAt time.Time + owners []podOwner +} + +type localPodSource interface { + ListLocalPods(context.Context) ([]localPod, error) } type dependencies struct { @@ -48,28 +60,27 @@ type dependencies struct { type component struct { enabled bool componentName string - lockPath string + podUID string statePath string + processPID int + processStart string log log.Component - locker fileLocker + pods localPodSource + pollInterval time.Duration - mu sync.Mutex - acquired bool - closed bool + mu sync.Mutex + activating bool + closed bool } var _ agentlifecycle.Component = (*component)(nil) // NewComponent creates the experimental Agent lifecycle component. func NewComponent(deps dependencies) (agentlifecycle.Component, error) { - return newComponent(deps, func(path string) fileLocker { return flock.New(path) }) + return newComponent(deps, newLocalPodSource(), runtime.GOOS, currentProcessIdentity) } -func newComponent(deps dependencies, newLocker func(string) fileLocker) (agentlifecycle.Component, error) { - return newComponentForPlatform(deps, newLocker, runtime.GOOS) -} - -func newComponentForPlatform(deps dependencies, newLocker func(string) fileLocker, goos string) (agentlifecycle.Component, error) { +func newComponent(deps dependencies, pods localPodSource, goos string, processIdentity func() (int, string, error)) (agentlifecycle.Component, error) { if !deps.Config.GetBool(rolloutEnabledKey) { return &component{}, nil } @@ -80,40 +91,68 @@ func newComponentForPlatform(deps dependencies, newLocker func(string) fileLocke if deps.Params.ComponentName == "" { return nil, errors.New("experimental node Agent rollout requires a component name") } - lockPath, err := resolveComponentPath(deps.Config.GetString(rolloutLockPathKey), deps.Params.ComponentName, ".lock", rolloutLockPathKey) - if err != nil { - return nil, err + podUID := strings.TrimSpace(deps.Config.GetString(rolloutPodUIDKey)) + if podUID == "" { + return nil, fmt.Errorf("%s must identify this Pod", rolloutPodUIDKey) } statePath, err := resolveComponentPath(deps.Config.GetString(rolloutStatePathKey), deps.Params.ComponentName, ".state", rolloutStatePathKey) if err != nil { return nil, err } - if !filepath.IsAbs(lockPath) { - return nil, fmt.Errorf("%s must be an absolute path", rolloutLockPathKey) - } if !filepath.IsAbs(statePath) { return nil, fmt.Errorf("%s must be an absolute path", rolloutStatePathKey) } - if filepath.Clean(lockPath) == filepath.Clean(statePath) { - return nil, errors.New("experimental node Agent rollout lock and state paths must differ") - } - if err := os.MkdirAll(filepath.Dir(lockPath), 0o755); err != nil { - return nil, fmt.Errorf("create Agent rollout lock directory: %w", err) - } if err := os.MkdirAll(filepath.Dir(statePath), 0o755); err != nil { return nil, fmt.Errorf("create Agent rollout state directory: %w", err) } + if err := os.Remove(statePath); err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, fmt.Errorf("clear stale Agent rollout state: %w", err) + } + processPID, processStart, err := processIdentity() + if err != nil { + return nil, err + } return &component{ enabled: true, componentName: deps.Params.ComponentName, - lockPath: lockPath, + podUID: podUID, statePath: statePath, + processPID: processPID, + processStart: processStart, log: deps.Log, - locker: newLocker(lockPath), + pods: pods, + pollInterval: siblingPollInterval, }, nil } +// currentProcessIdentity returns values that an exec probe can independently +// verify through /proc. Container filesystems such as an EmptyDir survive a +// container restart, so state alone is insufficient: it must be tied to the +// exact process generation that published it. +func currentProcessIdentity() (int, string, error) { + pid := os.Getpid() + contents, err := os.ReadFile("/proc/self/stat") + if err != nil { + return 0, "", fmt.Errorf("read Agent process identity: %w", err) + } + // The parenthesized comm field may contain spaces or right parentheses. + // Fields after its final ") " begin at field 3; starttime is field 22. + end := strings.LastIndex(string(contents), ") ") + if end < 0 { + return 0, "", errors.New("read Agent process identity: malformed /proc/self/stat") + } + fields := strings.Fields(string(contents)[end+2:]) + const startTimeIndex = 22 - 3 + if len(fields) <= startTimeIndex { + return 0, "", errors.New("read Agent process identity: incomplete /proc/self/stat") + } + if _, err := strconv.ParseUint(fields[startTimeIndex], 10, 64); err != nil { + return 0, "", fmt.Errorf("read Agent process identity: invalid start time: %w", err) + } + return pid, fields[startTimeIndex], nil +} + func validatePlatform(goos string) error { if goos != "linux" { return fmt.Errorf("experimental node Agent rollout is Linux-only (running on %s)", goos) @@ -121,9 +160,9 @@ func validatePlatform(goos string) error { return nil } -// resolveComponentPath makes the process identity part of every coordination -// path. A shared datadog.yaml can use the {component} token, while callers such -// as the Operator can continue supplying an already-expanded process path. +// resolveComponentPath makes the process identity part of every state path. A +// shared datadog.yaml can use the {component} token, while the Operator can +// continue supplying an already-expanded process path. func resolveComponentPath(configuredPath, componentName, suffix, configKey string) (string, error) { if filepath.Base(componentName) != componentName || componentName == "." || componentName == ".." { return "", errors.New("experimental node Agent rollout component name must be a path-safe base name") @@ -150,38 +189,156 @@ func (c *component) Wait(ctx context.Context) error { c.mu.Unlock() return errors.New("experimental Agent lifecycle is already closed") } - if c.acquired { + if c.activating { c.mu.Unlock() - return errors.New("experimental Agent lifecycle already owns the node lock") + return errors.New("experimental Agent lifecycle is already activating") + } + c.mu.Unlock() + + ticker := time.NewTicker(c.pollInterval) + defer ticker.Stop() + prepared := false + + for { + if err := ctx.Err(); err != nil { + return err + } + pods, err := c.pods.ListLocalPods(ctx) + if err == nil { + var siblings []localPod + siblings, err = siblingPods(pods, c.podUID) + if err == nil && len(siblings) == 0 { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + return c.beginActivation() + } + if err == nil && !prepared { + if err = c.markPrepared(); err == nil { + prepared = true + } + } + } + if err != nil { + // A failed or incomplete kubelet response must never be interpreted as + // proof that the old Pod is gone. Remain prepared and retry. + c.log.Warnf("%s cannot verify node-local sibling Pods; remaining inactive: %v", c.componentName, err) + } + + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + } + } +} + +func (c *component) markPrepared() error { + c.mu.Lock() + defer c.mu.Unlock() + if c.closed { + return errors.New("cannot prepare a closed experimental Agent lifecycle") } if err := c.writeState(agentlifecycle.StatePrepared); err != nil { - c.mu.Unlock() return err } - c.mu.Unlock() + c.log.Infof("%s verified an older DaemonSet sibling and is prepared while it waits", c.componentName) + return nil +} + +func siblingPods(pods []localPod, selfUID string) ([]localPod, error) { + var self *localPod + for i := range pods { + if pods[i].uid == selfUID { + if self != nil { + return nil, fmt.Errorf("kubelet returned duplicate entries for self Pod UID %q", selfUID) + } + self = &pods[i] + } + } + if self == nil { + return nil, fmt.Errorf("self Pod UID %q is absent from the kubelet Pod list", selfUID) + } - c.log.Infof("%s is prepared and waiting for node ownership at %s", c.componentName, c.lockPath) - locked, err := c.locker.TryLockContext(ctx, lockRetryInterval) + ownerUID, err := daemonSetOwnerUID(*self) if err != nil { - return fmt.Errorf("acquire Agent rollout lock %q: %w", c.lockPath, err) + return nil, err } - if !locked { - if ctx.Err() != nil { - return ctx.Err() + if self.createdAt.IsZero() { + return nil, fmt.Errorf("self Pod %s/%s has no creation timestamp", self.namespace, self.name) + } + + var siblings []localPod + for i := range pods { + pod := pods[i] + if pod.uid == selfUID || pod.namespace != self.namespace { + continue + } + candidateOwnerUID, ownerErr := daemonSetOwnerUID(pod) + if ownerErr == nil && candidateOwnerUID == ownerUID { + precedes, precedesErr := podPrecedes(pod, *self) + if precedesErr != nil { + return nil, precedesErr + } + if precedes { + siblings = append(siblings, pod) + } } - return fmt.Errorf("failed to acquire Agent rollout lock %q", c.lockPath) } + return siblings, nil +} +// podPrecedes fails closed when Kubernetes' second-precision creation +// timestamps cannot establish an order. Pod UIDs and resourceVersions are not +// creation-order values and must not be used to guess which process is active. +func podPrecedes(candidate, self localPod) (bool, error) { + if candidate.createdAt.IsZero() { + // An incomplete kubelet record must not be interpreted as a newer Pod. + return true, nil + } + if candidate.createdAt.Before(self.createdAt) { + return true, nil + } + if candidate.createdAt.After(self.createdAt) { + return false, nil + } + return false, fmt.Errorf("cannot order same-timestamp Pods %s/%s and %s/%s", candidate.namespace, candidate.name, self.namespace, self.name) +} + +func daemonSetOwnerUID(pod localPod) (string, error) { + var ownerUID string + for _, owner := range pod.owners { + if owner.kind != "DaemonSet" || !owner.controller { + continue + } + if owner.uid == "" { + return "", fmt.Errorf("Pod %s/%s has a DaemonSet controller with an empty UID", pod.namespace, pod.name) + } + if ownerUID != "" { + return "", fmt.Errorf("Pod %s/%s has multiple DaemonSet controllers", pod.namespace, pod.name) + } + ownerUID = owner.uid + } + if ownerUID == "" { + return "", fmt.Errorf("Pod %s/%s is not controlled by a DaemonSet", pod.namespace, pod.name) + } + return ownerUID, nil +} + +func (c *component) beginActivation() error { c.mu.Lock() - c.acquired = true + defer c.mu.Unlock() + if c.closed { + return errors.New("cannot activate a closed experimental Agent lifecycle") + } + if c.activating { + return errors.New("experimental Agent lifecycle is already activating") + } if err := c.writeState(agentlifecycle.StateActivating); err != nil { - c.acquired = false - c.mu.Unlock() - unlockErr := c.locker.Unlock() - return errors.Join(err, unlockErr) + return err } - c.mu.Unlock() - c.log.Infof("%s acquired node ownership and is activating", c.componentName) + c.activating = true + c.log.Infof("%s has no older DaemonSet sibling on the node and is activating", c.componentName) return nil } @@ -195,8 +352,8 @@ func (c *component) MarkActive() error { if c.closed { return errors.New("cannot mark a closed experimental Agent lifecycle active") } - if !c.acquired { - return errors.New("cannot mark the experimental Agent lifecycle active before acquiring node ownership") + if !c.activating { + return errors.New("cannot mark the experimental Agent lifecycle active before the sibling check") } if err := c.writeState(agentlifecycle.StateActive); err != nil { return err @@ -216,14 +373,7 @@ func (c *component) Close() error { return nil } c.closed = true - - stateErr := c.writeState(agentlifecycle.StateStopped) - var unlockErr error - if c.acquired { - unlockErr = c.locker.Unlock() - c.acquired = false - } - return errors.Join(stateErr, unlockErr) + return c.writeState(agentlifecycle.StateStopped) } func (c *component) writeState(state string) error { @@ -234,7 +384,7 @@ func (c *component) writeState(state string) error { tmpPath := tmp.Name() defer os.Remove(tmpPath) - if _, err := fmt.Fprintln(tmp, state); err != nil { + if _, err := fmt.Fprintln(tmp, state, c.processPID, c.processStart); err != nil { _ = tmp.Close() return fmt.Errorf("write Agent rollout state: %w", err) } diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go index 2e34fcfec1b9..da05425b7c10 100644 --- a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go +++ b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go @@ -12,12 +12,12 @@ import ( "errors" "os" "path/filepath" + "strconv" "strings" - "sync/atomic" + "sync" "testing" "time" - "github.com/gofrs/flock" "github.com/stretchr/testify/require" agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" @@ -25,160 +25,213 @@ import ( logmock "github.com/DataDog/datadog-agent/comp/core/log/mock" ) -type fakeLocker struct { - attempted chan struct{} - allow chan struct{} - unlocks atomic.Int32 +const ( + selfPodUID = "new-pod-uid" + daemonUID = "daemonset-uid" +) + +type scriptedPodSource struct { + mu sync.Mutex + responses []podResponse + calls chan struct{} } -func newFakeLocker() *fakeLocker { - return &fakeLocker{attempted: make(chan struct{}), allow: make(chan struct{})} +type podResponse struct { + pods []localPod + err error } -func (l *fakeLocker) TryLockContext(ctx context.Context, _ time.Duration) (bool, error) { - close(l.attempted) +func (s *scriptedPodSource) ListLocalPods(context.Context) ([]localPod, error) { + s.mu.Lock() + defer s.mu.Unlock() select { - case <-l.allow: - return true, nil - case <-ctx.Done(): - return false, ctx.Err() + case s.calls <- struct{}{}: + default: + } + if len(s.responses) == 0 { + return nil, errors.New("no scripted kubelet response") } + response := s.responses[0] + if len(s.responses) > 1 { + s.responses = s.responses[1:] + } + return response.pods, response.err } -func (l *fakeLocker) Unlock() error { - l.unlocks.Add(1) - return nil +func (s *scriptedPodSource) setResponses(responses ...podResponse) { + s.mu.Lock() + defer s.mu.Unlock() + s.responses = responses } func TestDisabledLifecycleIsNoop(t *testing.T) { deps := dependencies{Config: config.NewMock(t), Log: logmock.New(t)} - comp, err := newComponent(deps, func(string) fileLocker { - t.Fatal("disabled lifecycle must not create a lock") - return nil - }) + comp, err := newComponent(deps, nil, "linux", testProcessIdentity) require.NoError(t, err) require.NoError(t, comp.Wait(context.Background())) require.NoError(t, comp.MarkActive()) require.NoError(t, comp.Close()) } -func TestLifecycleStateTransitions(t *testing.T) { - comp, locker, statePath := newEnabledComponent(t) +func TestFreshPodActivatesAfterConstruction(t *testing.T) { + comp, _, statePath := newEnabledComponent(t, podResponse{pods: []localPod{selfPod()}}) + + require.NoError(t, comp.Wait(context.Background())) + require.Equal(t, agentlifecycle.StateActivating, readState(t, statePath)) + require.NoError(t, comp.MarkActive()) + require.Equal(t, agentlifecycle.StateActive, readState(t, statePath)) + require.NoError(t, comp.Close()) + require.Equal(t, agentlifecycle.StateStopped, readState(t, statePath)) + require.NoError(t, comp.Close(), "Close must be idempotent") +} + +func TestConstructionClearsStalePreparedState(t *testing.T) { + statePath := filepath.Join(t.TempDir(), "state", "test-agent.state") + require.NoError(t, os.MkdirAll(filepath.Dir(statePath), 0o755)) + require.NoError(t, os.WriteFile(statePath, []byte(agentlifecycle.StatePrepared), 0o644)) + deps := dependencies{ + Config: config.NewMockWithOverrides(t, map[string]interface{}{ + rolloutEnabledKey: true, + rolloutPodUIDKey: selfPodUID, + rolloutStatePathKey: statePath, + }), + Log: logmock.New(t), + Params: agentlifecycle.Params{ComponentName: "test-agent"}, + } + _, err := newComponent(deps, &scriptedPodSource{}, "linux", testProcessIdentity) + require.NoError(t, err) + require.NoFileExists(t, statePath) +} + +func TestStateIsBoundToCurrentProcessGeneration(t *testing.T) { + comp, _, statePath := newEnabledComponent(t, podResponse{pods: []localPod{selfPod()}}) + require.NoError(t, comp.Wait(context.Background())) + + contents, err := os.ReadFile(statePath) + require.NoError(t, err) + fields := strings.Fields(string(contents)) + require.Len(t, fields, 3) + require.Equal(t, agentlifecycle.StateActivating, fields[0]) + pid, started, err := testProcessIdentity() + require.NoError(t, err) + require.Equal(t, strconv.Itoa(pid), fields[1]) + require.Equal(t, started, fields[2]) +} + +func TestReplacementRemainsPreparedUntilSiblingDisappears(t *testing.T) { + withOld := []localPod{selfPod(), siblingPod("old-pod-uid", "old-agent")} + comp, source, statePath := newEnabledComponent(t, podResponse{pods: withOld}) waitResult := make(chan error, 1) go func() { waitResult <- comp.Wait(context.Background()) }() - <-locker.attempted - require.Equal(t, agentlifecycle.StatePrepared, readState(t, statePath)) - close(locker.allow) + <-source.calls + requireStateEventually(t, statePath, agentlifecycle.StatePrepared) + select { + case err := <-waitResult: + t.Fatalf("replacement activated while the old sibling was present: %v", err) + case <-time.After(20 * time.Millisecond): + } + + source.setResponses(podResponse{pods: []localPod{selfPod()}}) require.NoError(t, <-waitResult) require.Equal(t, agentlifecycle.StateActivating, readState(t, statePath)) +} - require.NoError(t, comp.MarkActive()) - require.Equal(t, agentlifecycle.StateActive, readState(t, statePath)) +func TestKubeletErrorsFailClosedThenRecover(t *testing.T) { + withOld := []localPod{selfPod(), siblingPod("old-pod-uid", "old-agent")} + comp, source, statePath := newEnabledComponent(t, podResponse{err: errors.New("kubelet unavailable")}) - require.NoError(t, comp.Close()) - require.Equal(t, agentlifecycle.StateStopped, readState(t, statePath)) - require.EqualValues(t, 1, locker.unlocks.Load()) - require.NoError(t, comp.Close(), "Close must be idempotent") - require.EqualValues(t, 1, locker.unlocks.Load()) + waitResult := make(chan error, 1) + go func() { waitResult <- comp.Wait(context.Background()) }() + <-source.calls + require.NoFileExists(t, statePath, "a replacement must not become Ready before kubelet safety is established") + select { + case err := <-waitResult: + t.Fatalf("kubelet failure opened the activation gate: %v", err) + case <-time.After(20 * time.Millisecond): + } + source.setResponses(podResponse{pods: withOld}) + require.Eventually(t, func() bool { + contents, err := os.ReadFile(statePath) + return err == nil && len(strings.Fields(string(contents))) > 0 && strings.Fields(string(contents))[0] == agentlifecycle.StatePrepared + }, time.Second, time.Millisecond) + require.Equal(t, agentlifecycle.StatePrepared, readState(t, statePath)) + source.setResponses(podResponse{pods: []localPod{selfPod()}}) + require.NoError(t, <-waitResult) } -func TestLifecycleWaitCancellation(t *testing.T) { - comp, locker, statePath := newEnabledComponent(t) +func TestMissingSelfPodFailsClosed(t *testing.T) { + comp, source, statePath := newEnabledComponent(t, podResponse{pods: []localPod{siblingPod("old-pod-uid", "old-agent")}}) ctx, cancel := context.WithCancel(context.Background()) waitResult := make(chan error, 1) go func() { waitResult <- comp.Wait(ctx) }() - - <-locker.attempted - require.Equal(t, agentlifecycle.StatePrepared, readState(t, statePath)) + <-source.calls + require.NoFileExists(t, statePath) cancel() require.ErrorIs(t, <-waitResult, context.Canceled) - require.NoError(t, comp.Close()) - require.Zero(t, locker.unlocks.Load()) } -func TestRealFileLockHandsOwnershipToPreparedReplacement(t *testing.T) { - dir := t.TempDir() - lockPath := filepath.Join(dir, "locks", "{component}.lock") - newProcess := func(componentName string) agentlifecycle.Component { - t.Helper() - deps := dependencies{ - Config: config.NewMockWithOverrides(t, map[string]interface{}{ - rolloutEnabledKey: true, - rolloutLockPathKey: lockPath, - rolloutStatePathKey: filepath.Join(dir, "state", "{component}.state"), - }), - Log: logmock.New(t), - Params: agentlifecycle.Params{ComponentName: componentName}, - } - comp, err := newComponentForPlatform(deps, func(path string) fileLocker { return flock.New(path) }, "linux") - require.NoError(t, err) - return comp - } +func TestDifferentDaemonSetOrNamespaceDoesNotBlock(t *testing.T) { + otherDaemon := siblingPod("other-daemon-pod", "other-daemon") + otherDaemon.owners[0].uid = "other-daemonset-uid" + otherNamespace := siblingPod("other-namespace-pod", "other-namespace") + otherNamespace.namespace = "other" - oldProcess := newProcess("agent") - replacement := newProcess("agent") - require.NoError(t, oldProcess.Wait(context.Background())) + comp, _, _ := newEnabledComponent(t, podResponse{pods: []localPod{selfPod(), otherDaemon, otherNamespace}}) + require.NoError(t, comp.Wait(context.Background())) +} - replacementResult := make(chan error, 1) - go func() { replacementResult <- replacement.Wait(context.Background()) }() - require.Eventually(t, func() bool { - contents, err := os.ReadFile(filepath.Join(dir, "state", "agent.state")) - return err == nil && strings.TrimSpace(string(contents)) == agentlifecycle.StatePrepared - }, time.Second, 10*time.Millisecond) - select { - case err := <-replacementResult: - t.Fatalf("replacement acquired ownership before the old process stopped: %v", err) - case <-time.After(200 * time.Millisecond): - } +func TestNonControllerOwnerDoesNotIdentifySelf(t *testing.T) { + self := selfPod() + self.owners[0].controller = false + comp, source, statePath := newEnabledComponent(t, podResponse{pods: []localPod{self}}) + ctx, cancel := context.WithCancel(context.Background()) + waitResult := make(chan error, 1) + go func() { waitResult <- comp.Wait(ctx) }() + <-source.calls + require.NoFileExists(t, statePath) + cancel() + require.ErrorIs(t, <-waitResult, context.Canceled) +} - require.NoError(t, oldProcess.Close()) - select { - case err := <-replacementResult: - require.NoError(t, err) - case <-time.After(2 * time.Second): - t.Fatal("replacement did not acquire ownership after the old process stopped") - } - require.Equal(t, agentlifecycle.StateActivating, readState(t, filepath.Join(dir, "state", "agent.state"))) - require.NoError(t, replacement.Close()) +func TestLifecycleWaitCancellation(t *testing.T) { + withOld := []localPod{selfPod(), siblingPod("old-pod-uid", "old-agent")} + comp, source, statePath := newEnabledComponent(t, podResponse{pods: withOld}) + ctx, cancel := context.WithCancel(context.Background()) + waitResult := make(chan error, 1) + go func() { waitResult <- comp.Wait(ctx) }() + + <-source.calls + requireStateEventually(t, statePath, agentlifecycle.StatePrepared) + cancel() + require.ErrorIs(t, <-waitResult, context.Canceled) + require.NoError(t, comp.Close()) } -func TestLifecycleRequiresValidPaths(t *testing.T) { +func TestLifecycleRequiresValidConfiguration(t *testing.T) { tests := map[string]map[string]interface{}{ - "relative lock": { - rolloutLockPathKey: "test-agent.lock", + "missing Pod UID": { rolloutStatePathKey: filepath.Join(t.TempDir(), "test-agent.state"), }, "relative state": { - rolloutLockPathKey: filepath.Join(t.TempDir(), "test-agent.lock"), + rolloutPodUIDKey: selfPodUID, rolloutStatePathKey: "test-agent.state", }, - "same path": { - rolloutLockPathKey: filepath.Join(t.TempDir(), "test-agent.lock"), - rolloutStatePathKey: "", - }, - "shared lock path": { - rolloutLockPathKey: filepath.Join(t.TempDir(), "agent.lock"), - rolloutStatePathKey: filepath.Join(t.TempDir(), "test-agent.state"), - }, "shared state path": { - rolloutLockPathKey: filepath.Join(t.TempDir(), "test-agent.lock"), + rolloutPodUIDKey: selfPodUID, rolloutStatePathKey: filepath.Join(t.TempDir(), "agent.state"), }, } for name, overrides := range tests { t.Run(name, func(t *testing.T) { overrides[rolloutEnabledKey] = true - if name == "same path" { - overrides[rolloutStatePathKey] = overrides[rolloutLockPathKey] - } deps := dependencies{ Config: config.NewMockWithOverrides(t, overrides), Log: logmock.New(t), Params: agentlifecycle.Params{ComponentName: "test-agent"}, } - _, err := newComponentForPlatform(deps, func(string) fileLocker { return newFakeLocker() }, "linux") + _, err := newComponent(deps, &scriptedPodSource{}, "linux", testProcessIdentity) require.Error(t, err) }) } @@ -188,89 +241,124 @@ func TestComponentPathResolution(t *testing.T) { tests := map[string]struct { configured string component string - suffix string expected string }{ "template": { - configured: "/var/run/datadog/{component}.lock", + configured: "/var/run/datadog/{component}.state", component: "core-agent", - suffix: ".lock", - expected: "/var/run/datadog/core-agent.lock", + expected: "/var/run/datadog/core-agent.state", }, "operator expanded path": { configured: "/var/run/datadog/trace-agent.state", component: "trace-agent", - suffix: ".state", expected: "/var/run/datadog/trace-agent.state", }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - resolved, err := resolveComponentPath(test.configured, test.component, test.suffix, "test.path") + resolved, err := resolveComponentPath(test.configured, test.component, ".state", "test.path") require.NoError(t, err) require.Equal(t, test.expected, resolved) }) } } -func TestComponentPathRejectsSharedLiteral(t *testing.T) { - _, err := resolveComponentPath("/var/run/datadog/agent.lock", "core-agent", ".lock", "test.path") - require.ErrorContains(t, err, "must contain {component}") -} - -func TestPreparedRolloutRejectsWindows(t *testing.T) { +func TestPreparedRolloutRejectsUnsupportedPlatform(t *testing.T) { require.ErrorContains(t, validatePlatform("windows"), "Linux-only") require.ErrorContains(t, validatePlatform("darwin"), "Linux-only") require.NoError(t, validatePlatform("linux")) } func TestComponentPathRejectsTraversalName(t *testing.T) { - _, err := resolveComponentPath("/var/run/datadog/{component}.lock", "..", ".lock", "test.path") + _, err := resolveComponentPath("/var/run/datadog/{component}.state", "..", ".state", "test.path") require.ErrorContains(t, err, "path-safe") } -func TestMarkActiveRequiresOwnership(t *testing.T) { - comp, _, _ := newEnabledComponent(t) +func TestMarkActiveRequiresSiblingCheck(t *testing.T) { + comp, _, _ := newEnabledComponent(t, podResponse{pods: []localPod{selfPod()}}) require.Error(t, comp.MarkActive()) } -func TestWaitPropagatesLockerError(t *testing.T) { - comp, locker, _ := newEnabledComponent(t) - expected := errors.New("lock failed") - locker.allow = nil - comp.(*component).locker = errorLocker{err: expected} - require.ErrorIs(t, comp.Wait(context.Background()), expected) +func TestSiblingSelectionRejectsDuplicateSelf(t *testing.T) { + _, err := siblingPods([]localPod{selfPod(), selfPod()}, selfPodUID) + require.ErrorContains(t, err, "duplicate") +} + +func TestOlderPodWinsAfterSimultaneousRestart(t *testing.T) { + oldSelf := selfPod() + oldSelf.createdAt = time.Unix(100, 0) + newer := siblingPod("newer-pod-uid", "newer-agent") + newer.createdAt = time.Unix(300, 0) + + blocking, err := siblingPods([]localPod{oldSelf, newer}, selfPodUID) + require.NoError(t, err) + require.Empty(t, blocking, "an older Pod must reactivate instead of deadlocking with a newer replacement") } -type errorLocker struct{ err error } +func TestSameTimestampFailsClosed(t *testing.T) { + self := selfPod() + other := siblingPod("other-pod-uid", "other-agent") + other.createdAt = self.createdAt -func (l errorLocker) TryLockContext(context.Context, time.Duration) (bool, error) { - return false, l.err + _, err := siblingPods([]localPod{self, other}, selfPodUID) + require.ErrorContains(t, err, "same-timestamp") } -func (errorLocker) Unlock() error { return nil } -func newEnabledComponent(t *testing.T) (agentlifecycle.Component, *fakeLocker, string) { +func newEnabledComponent(t *testing.T, responses ...podResponse) (agentlifecycle.Component, *scriptedPodSource, string) { t.Helper() dir := t.TempDir() statePath := filepath.Join(dir, "state", "test-agent.state") - locker := newFakeLocker() + source := &scriptedPodSource{responses: responses, calls: make(chan struct{}, 10)} deps := dependencies{ Config: config.NewMockWithOverrides(t, map[string]interface{}{ rolloutEnabledKey: true, - rolloutLockPathKey: filepath.Join(dir, "lock", "test-agent.lock"), + rolloutPodUIDKey: selfPodUID, rolloutStatePathKey: statePath, }), Log: logmock.New(t), Params: agentlifecycle.Params{ComponentName: "test-agent"}, } - comp, err := newComponentForPlatform(deps, func(string) fileLocker { return locker }, "linux") + comp, err := newComponent(deps, source, "linux", testProcessIdentity) require.NoError(t, err) - return comp, locker, statePath + comp.(*component).pollInterval = time.Millisecond + return comp, source, statePath +} + +func testProcessIdentity() (int, string, error) { + return 4242, "123456", nil +} + +func selfPod() localPod { + return localPod{ + uid: selfPodUID, + name: "new-agent", + namespace: "datadog-agent", + createdAt: time.Unix(200, 0), + owners: []podOwner{{kind: "DaemonSet", uid: daemonUID, controller: true}}, + } +} + +func siblingPod(uid, name string) localPod { + pod := selfPod() + pod.uid = uid + pod.name = name + pod.createdAt = time.Unix(100, 0) + return pod } func readState(t *testing.T, path string) string { t.Helper() contents, err := os.ReadFile(path) require.NoError(t, err) - return strings.TrimSpace(string(contents)) + fields := strings.Fields(string(contents)) + require.NotEmpty(t, fields) + return fields[0] +} + +func requireStateEventually(t *testing.T, path, expected string) { + t.Helper() + require.Eventually(t, func() bool { + contents, err := os.ReadFile(path) + return err == nil && len(strings.Fields(string(contents))) > 0 && strings.Fields(string(contents))[0] == expected + }, time.Second, time.Millisecond) } diff --git a/comp/core/agentlifecycle/impl/local_pods_kubelet.go b/comp/core/agentlifecycle/impl/local_pods_kubelet.go new file mode 100644 index 000000000000..60e5add23f48 --- /dev/null +++ b/comp/core/agentlifecycle/impl/local_pods_kubelet.go @@ -0,0 +1,54 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +//go:build kubelet + +package agentlifecycleimpl + +import ( + "context" + + "github.com/DataDog/datadog-agent/pkg/util/kubernetes/kubelet" +) + +type kubeletLocalPodSource struct{} + +func newLocalPodSource() localPodSource { + return kubeletLocalPodSource{} +} + +func (kubeletLocalPodSource) ListLocalPods(ctx context.Context) ([]localPod, error) { + kubeUtil, err := kubelet.GetKubeUtil() + if err != nil { + return nil, err + } + kubeletPods, err := kubeUtil.GetLocalPodList(ctx) + if err != nil { + return nil, err + } + + pods := make([]localPod, 0, len(kubeletPods)) + for _, pod := range kubeletPods { + if pod == nil { + continue + } + owners := make([]podOwner, 0, len(pod.Owners())) + for _, owner := range pod.Owners() { + owners = append(owners, podOwner{ + kind: owner.Kind, + uid: owner.ID, + controller: owner.Controller != nil && *owner.Controller, + }) + } + pods = append(pods, localPod{ + uid: pod.Metadata.UID, + name: pod.Metadata.Name, + namespace: pod.Metadata.Namespace, + createdAt: pod.Metadata.CreationTimestamp, + owners: owners, + }) + } + return pods, nil +} diff --git a/comp/core/agentlifecycle/impl/local_pods_nokubelet.go b/comp/core/agentlifecycle/impl/local_pods_nokubelet.go new file mode 100644 index 000000000000..011ccc9ebe15 --- /dev/null +++ b/comp/core/agentlifecycle/impl/local_pods_nokubelet.go @@ -0,0 +1,23 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +//go:build !kubelet + +package agentlifecycleimpl + +import ( + "context" + "errors" +) + +type unsupportedLocalPodSource struct{} + +func newLocalPodSource() localPodSource { + return unsupportedLocalPodSource{} +} + +func (unsupportedLocalPodSource) ListLocalPods(context.Context) ([]localPod, error) { + return nil, errors.New("experimental node Agent rollout requires an Agent built with kubelet support") +} diff --git a/go.mod b/go.mod index 5515e3c7ebe7..3826ae6b697a 100644 --- a/go.mod +++ b/go.mod @@ -722,7 +722,7 @@ require ( github.com/gocql/gocql v1.6.0 // indirect github.com/godbus/dbus v4.1.0+incompatible // indirect github.com/godror/knownpb v0.3.0 // indirect - github.com/gofrs/flock v0.13.0 + github.com/gofrs/flock v0.13.0 // indirect github.com/gofrs/uuid v4.4.0+incompatible // indirect github.com/gogo/googleapis v1.4.1 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect diff --git a/pkg/config/schema/yaml/core_schema.yaml b/pkg/config/schema/yaml/core_schema.yaml index 99c6465b5bd7..17e9ece304ac 100644 --- a/pkg/config/schema/yaml/core_schema.yaml +++ b/pkg/config/schema/yaml/core_schema.yaml @@ -8324,7 +8324,7 @@ properties: node_type: setting type: boolean default: false - lock_path: + pod_uid: node_type: setting type: string default: '' diff --git a/pkg/config/setup/agent_lifecycle.go b/pkg/config/setup/agent_lifecycle.go index 595813565efb..2e35a099d07f 100644 --- a/pkg/config/setup/agent_lifecycle.go +++ b/pkg/config/setup/agent_lifecycle.go @@ -10,14 +10,14 @@ import pkgconfigmodel "github.com/DataDog/datadog-agent/pkg/config/model" const ( // ExperimentalNodeAgentRolloutEnabled enables the experimental prepared/active Agent lifecycle. ExperimentalNodeAgentRolloutEnabled = "experimental.node_agent_rollout.enabled" - // ExperimentalNodeAgentRolloutLockPath is the node-local ownership lock used by one Agent process. Shared configuration must include {component}. - ExperimentalNodeAgentRolloutLockPath = "experimental.node_agent_rollout.lock_path" + // ExperimentalNodeAgentRolloutPodUID identifies the Pod that must wait for older siblings owned by the same DaemonSet. + ExperimentalNodeAgentRolloutPodUID = "experimental.node_agent_rollout.pod_uid" // ExperimentalNodeAgentRolloutStatePath reports the local lifecycle state of one Agent process. Shared configuration must include {component}. ExperimentalNodeAgentRolloutStatePath = "experimental.node_agent_rollout.state_path" ) func setupExperimentalNodeAgentRollout(config pkgconfigmodel.Setup) { config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutEnabled, false) - config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutLockPath, "") + config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutPodUID, "") config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutStatePath, "") } diff --git a/pkg/config/setup/config_test.go b/pkg/config/setup/config_test.go index 0ad415ff3a26..28d00f7f209b 100644 --- a/pkg/config/setup/config_test.go +++ b/pkg/config/setup/config_test.go @@ -776,7 +776,7 @@ func TestExperimentalNodeAgentRolloutDefaults(t *testing.T) { config := confFromYAML(t, "") assert.False(t, config.GetBool(ExperimentalNodeAgentRolloutEnabled)) - assert.Empty(t, config.GetString(ExperimentalNodeAgentRolloutLockPath)) + assert.Empty(t, config.GetString(ExperimentalNodeAgentRolloutPodUID)) assert.Empty(t, config.GetString(ExperimentalNodeAgentRolloutStatePath)) } diff --git a/pkg/util/fxutil/oneshot.go b/pkg/util/fxutil/oneshot.go index 98c83aef2f17..b71cc9dc5d23 100644 --- a/pkg/util/fxutil/oneshot.go +++ b/pkg/util/fxutil/oneshot.go @@ -90,9 +90,8 @@ func oneShot(oneShotFunc interface{}, gateProvider func() StartupGate, opts ...f return stopAppAndCloseGate(app, gate) } -// stopAppAndCloseGate preserves ownership when Fx cannot prove that every -// lifecycle hook stopped. In that case process teardown and the kernel closing -// the lock descriptor remain the safety boundary. +// stopAppAndCloseGate preserves the active lifecycle state when Fx cannot prove +// that every hook stopped. Process teardown remains the final safety boundary. func stopAppAndCloseGate(app *fx.App, gate StartupGate) error { if err := stopApp(app); err != nil { return err From 1cc57a9db9adc4f41afdc1b02217c12ef427689c Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Tue, 28 Jul 2026 13:35:23 +0200 Subject: [PATCH 7/9] Gate surged Agent containers before initialization --- cmd/agent/subcommands/run/command.go | 5 - cmd/host-profiler/subcommands/run/BUILD.bazel | 4 + cmd/host-profiler/subcommands/run/command.go | 6 +- cmd/otel-agent/subcommands/run/BUILD.bazel | 2 + cmd/otel-agent/subcommands/run/command.go | 15 +- .../subcommands/run/BUILD.bazel | 2 + .../subcommands/run/command.go | 6 +- cmd/process-agent/command/BUILD.bazel | 2 + cmd/process-agent/command/main_common.go | 16 +- cmd/system-probe/subcommands/run/BUILD.bazel | 2 + cmd/system-probe/subcommands/run/command.go | 6 +- comp/core/agentlifecycle/def/component.go | 21 +- .../agentlifecycle/impl/agent_lifecycle.go | 391 +++++++----------- .../impl/agent_lifecycle_test.go | 386 +++++++---------- .../agentlifecycle/impl/local_pods_kubelet.go | 25 +- pkg/util/fxutil/errorunwrapper.go | 11 +- pkg/util/fxutil/errorunwrapper_test.go | 1 + pkg/util/fxutil/oneshot.go | 28 +- pkg/util/fxutil/run.go | 26 +- pkg/util/fxutil/startup_gate.go | 36 +- pkg/util/fxutil/startup_gate_test.go | 13 +- tasks/build_tags.bzl | 3 +- 22 files changed, 418 insertions(+), 589 deletions(-) diff --git a/cmd/agent/subcommands/run/command.go b/cmd/agent/subcommands/run/command.go index bc53bf088784..b1a92faaefa6 100644 --- a/cmd/agent/subcommands/run/command.go +++ b/cmd/agent/subcommands/run/command.go @@ -277,7 +277,6 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { // run starts the main loop. func run(log log.Component, cfg config.Component, - agentLifecycle agentlifecycle.Component, flare flare.Component, tlm telemetry.Component, sysprobeConf sysprobeconfig.Component, @@ -397,10 +396,6 @@ func run(log log.Component, ); err != nil { return err } - if err := agentLifecycle.MarkActive(); err != nil { - return err - } - agentStarted := tlm.NewCounter( "runtime", "started", diff --git a/cmd/host-profiler/subcommands/run/BUILD.bazel b/cmd/host-profiler/subcommands/run/BUILD.bazel index 00c002275c99..168fc5d8feac 100644 --- a/cmd/host-profiler/subcommands/run/BUILD.bazel +++ b/cmd/host-profiler/subcommands/run/BUILD.bazel @@ -11,6 +11,8 @@ go_library( "//cmd/agent/command", "//cmd/host-profiler/globalparams", "//comp/core", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/config", "//comp/core/configsync/def", "//comp/core/configsync/fx", @@ -46,6 +48,8 @@ go_library( "//cmd/agent/command", "//cmd/host-profiler/globalparams", "//comp/core", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/config", "//comp/core/configsync/def", "//comp/core/configsync/fx", diff --git a/cmd/host-profiler/subcommands/run/command.go b/cmd/host-profiler/subcommands/run/command.go index 57136a5a1e0f..f64a05f6c4e5 100644 --- a/cmd/host-profiler/subcommands/run/command.go +++ b/cmd/host-profiler/subcommands/run/command.go @@ -22,6 +22,8 @@ import ( "github.com/DataDog/datadog-agent/cmd/agent/command" "github.com/DataDog/datadog-agent/cmd/host-profiler/globalparams" "github.com/DataDog/datadog-agent/comp/core" + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + agentlifecyclefx "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx" "github.com/DataDog/datadog-agent/comp/core/config" configsync "github.com/DataDog/datadog-agent/comp/core/configsync/def" configsyncfx "github.com/DataDog/datadog-agent/comp/core/configsync/fx" @@ -90,6 +92,8 @@ func runHostProfilerCommand(ctx context.Context, cliParams *cliParams) error { } var opts = []fx.Option{ + fx.Supply(agentlifecycle.Params{ComponentName: "host-profiler"}), + agentlifecyclefx.Module(), hostprofiler.Bundle(collectorimpl.NewParams(cliParams.GlobalParams.ConfigURI(), cliParams.GoRuntimeMetrics)), logging.DefaultFxLoggingOption(), } @@ -120,7 +124,7 @@ func runHostProfilerCommand(ctx context.Context, cliParams *cliParams) error { ) } - return fxutil.OneShot(run, opts...) + return fxutil.OneShotWithStartupGate[agentlifecycle.Component](run, opts...) } func run(collector collector.Component) error { diff --git a/cmd/otel-agent/subcommands/run/BUILD.bazel b/cmd/otel-agent/subcommands/run/BUILD.bazel index f25e2b0c1ff8..6a04b6978a04 100644 --- a/cmd/otel-agent/subcommands/run/BUILD.bazel +++ b/cmd/otel-agent/subcommands/run/BUILD.bazel @@ -15,6 +15,8 @@ go_library( deps = [ "//cmd/otel-agent/config", "//cmd/otel-agent/subcommands", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/agenttelemetry/fx", "//comp/core/config", "//comp/core/configsync/def", diff --git a/cmd/otel-agent/subcommands/run/command.go b/cmd/otel-agent/subcommands/run/command.go index d2a1441d05ac..5c13f90b434b 100644 --- a/cmd/otel-agent/subcommands/run/command.go +++ b/cmd/otel-agent/subcommands/run/command.go @@ -20,6 +20,8 @@ import ( agentConfig "github.com/DataDog/datadog-agent/cmd/otel-agent/config" "github.com/DataDog/datadog-agent/cmd/otel-agent/subcommands" + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + agentlifecyclefx "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx" agenttelemetryfx "github.com/DataDog/datadog-agent/comp/core/agenttelemetry/fx" coreconfig "github.com/DataDog/datadog-agent/comp/core/config" configsync "github.com/DataDog/datadog-agent/comp/core/configsync/def" @@ -132,11 +134,16 @@ func runOTelAgentCommand(ctx context.Context, params *cliParams, opts ...fx.Opti fmt.Println("*** OpenTelemetry Collector is not enabled, exiting application ***. Set the config option `otelcollector.enabled` or the environment variable `DD_OTELCOLLECTOR_ENABLED` at true to enable it.") return nil } + gateOptions := []fx.Option{ + fx.Supply(agentlifecycle.Params{ComponentName: "otel-agent"}), + agentlifecyclefx.Module(), + } uris := buildConfigURIs(params) if err == agentConfig.ErrNoDDExporter { - return fxutil.Run( + return fxutil.RunWithStartupGate[agentlifecycle.Component]( + fx.Options(gateOptions...), fx.Supply(uris), fx.Provide(func() coreconfig.Component { return acfg @@ -170,12 +177,14 @@ func runOTelAgentCommand(ctx context.Context, params *cliParams, opts ...fx.Opti } if acfg.GetBool("otel_standalone") { - return fxutil.Run( + return fxutil.RunWithStartupGate[agentlifecycle.Component]( + fx.Options(gateOptions...), commonAgentFxOptions(ctx, params, acfg, uris, opts...), standaloneAgentFxOptions(params), ) } - return fxutil.Run( + return fxutil.RunWithStartupGate[agentlifecycle.Component]( + fx.Options(gateOptions...), commonAgentFxOptions(ctx, params, acfg, uris, opts...), connectedAgentFxOptions(params), ) diff --git a/cmd/privateactionrunner/subcommands/run/BUILD.bazel b/cmd/privateactionrunner/subcommands/run/BUILD.bazel index 6aa84c5c8f0c..084345047c2b 100644 --- a/cmd/privateactionrunner/subcommands/run/BUILD.bazel +++ b/cmd/privateactionrunner/subcommands/run/BUILD.bazel @@ -14,6 +14,8 @@ go_library( deps = [ "//cmd/privateactionrunner/command", "//comp/core", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/config", "//comp/core/hostname/remotehostnameimpl", "//comp/core/ipc/fx", diff --git a/cmd/privateactionrunner/subcommands/run/command.go b/cmd/privateactionrunner/subcommands/run/command.go index fccbfaf4ccc8..c72d7a35fee9 100644 --- a/cmd/privateactionrunner/subcommands/run/command.go +++ b/cmd/privateactionrunner/subcommands/run/command.go @@ -21,6 +21,8 @@ import ( "github.com/DataDog/datadog-agent/cmd/privateactionrunner/command" "github.com/DataDog/datadog-agent/comp/core" + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + agentlifecyclefx "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx" "github.com/DataDog/datadog-agent/comp/core/config" ipcfx "github.com/DataDog/datadog-agent/comp/core/ipc/fx" log "github.com/DataDog/datadog-agent/comp/core/log/def" @@ -103,6 +105,8 @@ func runPrivateActionRunner(ctx context.Context, confPath string, extraConfFiles ConfigParams: config.NewAgentParams(confPath, config.WithExtraConfFiles(extraConfFiles)), LogParams: log.ForDaemon(command.LoggerName, pkgconfigsetup.PARLogFile, defaultpaths.GetDefaultPrivateActionRunnerLogFile())}), core.Bundle(core.WithSecrets()), + fx.Supply(agentlifecycle.Params{ComponentName: "private-action-runner"}), + agentlifecyclefx.Module(), fx.Provide(func(c config.Component) settings.Params { return settings.Params{ Settings: map[string]settings.RuntimeSetting{ @@ -127,7 +131,7 @@ func runPrivateActionRunner(ctx context.Context, confPath string, extraConfFiles privateactionrunnerfx.Module(), } - err := fxutil.Run(fxOptions...) + err := fxutil.RunWithStartupGate[agentlifecycle.Component](fxOptions...) if errors.Is(err, privateactionrunner.ErrNotEnabled) { return nil } diff --git a/cmd/process-agent/command/BUILD.bazel b/cmd/process-agent/command/BUILD.bazel index f54e82900374..9641fedf42b8 100644 --- a/cmd/process-agent/command/BUILD.bazel +++ b/cmd/process-agent/command/BUILD.bazel @@ -17,6 +17,8 @@ go_library( "//comp/agent/autoexit/def", "//comp/agent/autoexit/fx", "//comp/core", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/config", "//comp/core/configstreamconsumer/def", "//comp/core/configstreamconsumer/fx", diff --git a/cmd/process-agent/command/main_common.go b/cmd/process-agent/command/main_common.go index 07491589512c..3b262ce185a8 100644 --- a/cmd/process-agent/command/main_common.go +++ b/cmd/process-agent/command/main_common.go @@ -18,6 +18,8 @@ import ( autoexit "github.com/DataDog/datadog-agent/comp/agent/autoexit/def" autoexitfx "github.com/DataDog/datadog-agent/comp/agent/autoexit/fx" "github.com/DataDog/datadog-agent/comp/core" + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + agentlifecyclefx "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx" "github.com/DataDog/datadog-agent/comp/core/config" configstreamconsumer "github.com/DataDog/datadog-agent/comp/core/configstreamconsumer/def" configstreamconsumerfx "github.com/DataDog/datadog-agent/comp/core/configstreamconsumer/fx" @@ -108,7 +110,9 @@ func runApp(ctx context.Context, globalParams *GlobalParams) error { RCClient rcclient.Component WorkloadMeta workloadmeta.Component } + var startupGate fxutil.StartupGate app := fx.New( + fxutil.StartupGateOption[agentlifecycle.Component](ctx, &startupGate), fx.Supply( core.BundleParams{ SysprobeConfigParams: sysprobeconfigimpl.NewParams( @@ -119,6 +123,8 @@ func runApp(ctx context.Context, globalParams *GlobalParams) error { LogParams: DaemonLogParams, }, ), + fx.Supply(agentlifecycle.Params{ComponentName: "process-agent"}), + agentlifecyclefx.Module(), fx.Supply( status.Params{ PythonVersionGetFunc: python.GetPythonVersion, @@ -233,6 +239,15 @@ func runApp(ctx context.Context, globalParams *GlobalParams) error { return nil }), ) + if err := app.Err(); err != nil && !errors.Is(err, errAgentDisabled) { + if startupGate != nil { + return errors.Join(fxutil.UnwrapIfErrArgumentsFailed(err), startupGate.Close()) + } + return fxutil.UnwrapIfErrArgumentsFailed(err) + } + if startupGate != nil { + defer startupGate.Close() + } err := app.Start(ctx) if err != nil { @@ -252,7 +267,6 @@ func runApp(ctx context.Context, globalParams *GlobalParams) error { return err } } - // Wait for exit signal select { case <-exitSignal: diff --git a/cmd/system-probe/subcommands/run/BUILD.bazel b/cmd/system-probe/subcommands/run/BUILD.bazel index d3bb1fca9d8c..51f4806c139e 100644 --- a/cmd/system-probe/subcommands/run/BUILD.bazel +++ b/cmd/system-probe/subcommands/run/BUILD.bazel @@ -17,6 +17,8 @@ go_library( "//cmd/system-probe/common", "//comp/agent/autoexit/def", "//comp/agent/autoexit/fx", + "//comp/core/agentlifecycle/def", + "//comp/core/agentlifecycle/fx", "//comp/core/config", "//comp/core/configstreamconsumer/def", "//comp/core/configstreamconsumer/fx", diff --git a/cmd/system-probe/subcommands/run/command.go b/cmd/system-probe/subcommands/run/command.go index 57591d2c7839..ff0b502d48cc 100644 --- a/cmd/system-probe/subcommands/run/command.go +++ b/cmd/system-probe/subcommands/run/command.go @@ -28,6 +28,8 @@ import ( "github.com/DataDog/datadog-agent/cmd/system-probe/common" autoexit "github.com/DataDog/datadog-agent/comp/agent/autoexit/def" autoexitfx "github.com/DataDog/datadog-agent/comp/agent/autoexit/fx" + agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" + agentlifecyclefx "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/fx" "github.com/DataDog/datadog-agent/comp/core/config" configstreamconsumer "github.com/DataDog/datadog-agent/comp/core/configstreamconsumer/def" configstreamconsumerfx "github.com/DataDog/datadog-agent/comp/core/configstreamconsumer/fx" @@ -122,6 +124,8 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { Long: `Runs the system-probe in the foreground`, RunE: func(_ *cobra.Command, _ []string) error { opts := []fx.Option{ + fx.Supply(agentlifecycle.Params{ComponentName: "system-probe"}), + agentlifecyclefx.Module(), fx.Invoke(func(_ log.Component) { ddruntime.SetMaxProcs() }), @@ -135,7 +139,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { configstreamconsumerfx.Module(), getSharedFxOption(), } - return fxutil.OneShot(run, opts...) + return fxutil.OneShotWithStartupGate[agentlifecycle.Component](run, opts...) }, } runCmd.Flags().StringVarP(&cliParams.pidfilePath, "pid", "p", "", "path to the pidfile") diff --git a/comp/core/agentlifecycle/def/component.go b/comp/core/agentlifecycle/def/component.go index a03cb58aa149..98ce22e3cd04 100644 --- a/comp/core/agentlifecycle/def/component.go +++ b/comp/core/agentlifecycle/def/component.go @@ -3,35 +3,22 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2026-present Datadog, Inc. -// Package agentlifecycle defines the experimental prepared/active Agent lifecycle component. +// Package agentlifecycle defines the experimental Agent startup gate. package agentlifecycle import "context" // team: agent-runtimes -const ( - // StatePrepared means construction is complete and the process is waiting for node ownership. - StatePrepared = "prepared" - // StateActivating means node ownership was acquired and Agent components are starting. - StateActivating = "activating" - // StateActive means the Agent process completed startup. - StateActive = "active" - // StateStopped means the Agent stopped and released node ownership. - StateStopped = "stopped" -) - // Params identifies the Agent process using the lifecycle gate. type Params struct { ComponentName string } -// Component gates Agent startup until older sibling Pods have left the node. +// Component gates Agent construction until the older same-name container has stopped. type Component interface { - // Wait reports Prepared and blocks until no older Pod owned by the same DaemonSet remains on the node. + // Wait blocks until no older same-name container owned by the same DaemonSet remains on the node. Wait(context.Context) error - // MarkActive reports that Agent startup completed. - MarkActive() error - // Close releases node-local ownership after Agent components have stopped. + // Close releases gate resources. Close() error } diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle.go b/comp/core/agentlifecycle/impl/agent_lifecycle.go index 4f76ca01d608..db72f4ab6ed2 100644 --- a/comp/core/agentlifecycle/impl/agent_lifecycle.go +++ b/comp/core/agentlifecycle/impl/agent_lifecycle.go @@ -3,19 +3,15 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2026-present Datadog, Inc. -// Package agentlifecycleimpl implements the experimental prepared/active Agent lifecycle. +// Package agentlifecycleimpl implements the experimental Agent startup gate. package agentlifecycleimpl import ( "context" "errors" "fmt" - "os" - "path/filepath" "runtime" - "strconv" "strings" - "sync" "time" agentlifecycle "github.com/DataDog/datadog-agent/comp/core/agentlifecycle/def" @@ -28,7 +24,9 @@ const ( siblingPollInterval = time.Second rolloutEnabledKey = "experimental.node_agent_rollout.enabled" rolloutPodUIDKey = "experimental.node_agent_rollout.pod_uid" - rolloutStatePathKey = "experimental.node_agent_rollout.state_path" + + freshInstallObservations = 2 + missingOlderObservations = 2 ) type podOwner struct { @@ -37,12 +35,21 @@ type podOwner struct { controller bool } +type localContainer struct { + name string + terminated bool + ready bool +} + type localPod struct { - uid string - name string - namespace string - createdAt time.Time - owners []podOwner + uid string + name string + namespace string + createdAt time.Time + deletionTimestamp *time.Time + owners []podOwner + declaredContainers []string + containers []localContainer } type localPodSource interface { @@ -61,168 +68,101 @@ type component struct { enabled bool componentName string podUID string - statePath string - processPID int - processStart string log log.Component pods localPodSource pollInterval time.Duration - - mu sync.Mutex - activating bool - closed bool } var _ agentlifecycle.Component = (*component)(nil) -// NewComponent creates the experimental Agent lifecycle component. +// NewComponent creates the experimental Agent startup gate. func NewComponent(deps dependencies) (agentlifecycle.Component, error) { - return newComponent(deps, newLocalPodSource(), runtime.GOOS, currentProcessIdentity) + return newComponent(deps, newLocalPodSource(), runtime.GOOS) } -func newComponent(deps dependencies, pods localPodSource, goos string, processIdentity func() (int, string, error)) (agentlifecycle.Component, error) { +func newComponent(deps dependencies, pods localPodSource, goos string) (agentlifecycle.Component, error) { if !deps.Config.GetBool(rolloutEnabledKey) { return &component{}, nil } - if err := validatePlatform(goos); err != nil { - return nil, err + if goos != "linux" { + return nil, fmt.Errorf("experimental node Agent rollout is Linux-only (running on %s)", goos) } - - if deps.Params.ComponentName == "" { - return nil, errors.New("experimental node Agent rollout requires a component name") + if deps.Params.ComponentName == "" || strings.ContainsAny(deps.Params.ComponentName, `/\\`) { + return nil, errors.New("experimental node Agent rollout requires a path-safe component name") } podUID := strings.TrimSpace(deps.Config.GetString(rolloutPodUIDKey)) if podUID == "" { return nil, fmt.Errorf("%s must identify this Pod", rolloutPodUIDKey) } - statePath, err := resolveComponentPath(deps.Config.GetString(rolloutStatePathKey), deps.Params.ComponentName, ".state", rolloutStatePathKey) - if err != nil { - return nil, err - } - if !filepath.IsAbs(statePath) { - return nil, fmt.Errorf("%s must be an absolute path", rolloutStatePathKey) - } - if err := os.MkdirAll(filepath.Dir(statePath), 0o755); err != nil { - return nil, fmt.Errorf("create Agent rollout state directory: %w", err) - } - if err := os.Remove(statePath); err != nil && !errors.Is(err, os.ErrNotExist) { - return nil, fmt.Errorf("clear stale Agent rollout state: %w", err) - } - processPID, processStart, err := processIdentity() - if err != nil { - return nil, err - } - - return &component{ + c := &component{ enabled: true, componentName: deps.Params.ComponentName, podUID: podUID, - statePath: statePath, - processPID: processPID, - processStart: processStart, log: deps.Log, pods: pods, pollInterval: siblingPollInterval, - }, nil -} - -// currentProcessIdentity returns values that an exec probe can independently -// verify through /proc. Container filesystems such as an EmptyDir survive a -// container restart, so state alone is insufficient: it must be tied to the -// exact process generation that published it. -func currentProcessIdentity() (int, string, error) { - pid := os.Getpid() - contents, err := os.ReadFile("/proc/self/stat") - if err != nil { - return 0, "", fmt.Errorf("read Agent process identity: %w", err) - } - // The parenthesized comm field may contain spaces or right parentheses. - // Fields after its final ") " begin at field 3; starttime is field 22. - end := strings.LastIndex(string(contents), ") ") - if end < 0 { - return 0, "", errors.New("read Agent process identity: malformed /proc/self/stat") - } - fields := strings.Fields(string(contents)[end+2:]) - const startTimeIndex = 22 - 3 - if len(fields) <= startTimeIndex { - return 0, "", errors.New("read Agent process identity: incomplete /proc/self/stat") - } - if _, err := strconv.ParseUint(fields[startTimeIndex], 10, 64); err != nil { - return 0, "", fmt.Errorf("read Agent process identity: invalid start time: %w", err) - } - return pid, fields[startTimeIndex], nil -} - -func validatePlatform(goos string) error { - if goos != "linux" { - return fmt.Errorf("experimental node Agent rollout is Linux-only (running on %s)", goos) } - return nil + return c, nil } -// resolveComponentPath makes the process identity part of every state path. A -// shared datadog.yaml can use the {component} token, while the Operator can -// continue supplying an already-expanded process path. -func resolveComponentPath(configuredPath, componentName, suffix, configKey string) (string, error) { - if filepath.Base(componentName) != componentName || componentName == "." || componentName == ".." { - return "", errors.New("experimental node Agent rollout component name must be a path-safe base name") - } - - if strings.Contains(configuredPath, "{component}") { - return strings.ReplaceAll(configuredPath, "{component}", componentName), nil - } - - expectedBase := componentName + suffix - if filepath.Base(configuredPath) != expectedBase { - return "", fmt.Errorf("%s must contain {component} or end in %q", configKey, expectedBase) - } - return configuredPath, nil -} - -func (c *component) Wait(ctx context.Context) error { +// Wait leaves the executable asleep before later Fx construction until the +// older instance of this same container has stopped. Kubelet errors fail closed. +func (c *component) Wait(ctx context.Context) (err error) { if !c.enabled { return nil } - - c.mu.Lock() - if c.closed { - c.mu.Unlock() - return errors.New("experimental Agent lifecycle is already closed") - } - if c.activating { - c.mu.Unlock() - return errors.New("experimental Agent lifecycle is already activating") - } - c.mu.Unlock() - ticker := time.NewTicker(c.pollInterval) defer ticker.Stop() - prepared := false + // The value records that kubelet has shown deletion beginning for the Pod. + // A later omission is only a safe handoff signal after that observation. + knownOlder := map[string]bool{} + missingOlder := map[string]int{} + olderObservedLogged := false + emptyObservations := 0 + pollErrorLogged := false for { - if err := ctx.Err(); err != nil { - return err - } pods, err := c.pods.ListLocalPods(ctx) if err == nil { - var siblings []localPod - siblings, err = siblingPods(pods, c.podUID) - if err == nil && len(siblings) == 0 { - if ctxErr := ctx.Err(); ctxErr != nil { - return ctxErr + var older []localPod + older, err = olderSiblingPods(pods, c.podUID) + if err == nil { + for i := range older { + knownOlder[older[i].uid] = knownOlder[older[i].uid] || older[i].deletionTimestamp != nil } - return c.beginActivation() - } - if err == nil && !prepared { - if err = c.markPrepared(); err == nil { - prepared = true + + if len(knownOlder) == 0 { + emptyObservations++ + if emptyObservations >= freshInstallObservations && (c.componentName == "agent" || replacementCoreReady(pods, c.podUID)) { + c.log.Infof("%s found no older container on the node and is starting", c.componentName) + return nil + } + } else { + emptyObservations = 0 + if !olderObservedLogged { + olderObservedLogged = true + c.log.Infof("%s is waiting for its older container to stop", c.componentName) + } + if err == nil && olderContainersStopped(older, knownOlder, missingOlder, c.componentName) { + if c.componentName == "agent" { + c.log.Info("the older core Agent container stopped; starting") + return nil + } + if replacementCoreReady(pods, c.podUID) { + c.log.Infof("the older %s container stopped and the replacement core Agent is ready; starting", c.componentName) + return nil + } + } } } } if err != nil { - // A failed or incomplete kubelet response must never be interpreted as - // proof that the old Pod is gone. Remain prepared and retry. - c.log.Warnf("%s cannot verify node-local sibling Pods; remaining inactive: %v", c.componentName, err) + if !pollErrorLogged { + c.log.Warnf("%s cannot verify node-local containers; remaining asleep: %v", c.componentName, err) + pollErrorLogged = true + } + } else { + pollErrorLogged = false } select { @@ -233,20 +173,23 @@ func (c *component) Wait(ctx context.Context) error { } } -func (c *component) markPrepared() error { - c.mu.Lock() - defer c.mu.Unlock() - if c.closed { - return errors.New("cannot prepare a closed experimental Agent lifecycle") - } - if err := c.writeState(agentlifecycle.StatePrepared); err != nil { - return err +func replacementCoreReady(pods []localPod, selfUID string) bool { + for i := range pods { + if pods[i].uid != selfUID { + continue + } + for j := range pods[i].containers { + container := pods[i].containers[j] + if container.name == "agent" { + return container.ready && !container.terminated + } + } + return false } - c.log.Infof("%s verified an older DaemonSet sibling and is prepared while it waits", c.componentName) - return nil + return false } -func siblingPods(pods []localPod, selfUID string) ([]localPod, error) { +func olderSiblingPods(pods []localPod, selfUID string) ([]localPod, error) { var self *localPod for i := range pods { if pods[i].uid == selfUID { @@ -259,7 +202,6 @@ func siblingPods(pods []localPod, selfUID string) ([]localPod, error) { if self == nil { return nil, fmt.Errorf("self Pod UID %q is absent from the kubelet Pod list", selfUID) } - ownerUID, err := daemonSetOwnerUID(*self) if err != nil { return nil, err @@ -268,41 +210,71 @@ func siblingPods(pods []localPod, selfUID string) ([]localPod, error) { return nil, fmt.Errorf("self Pod %s/%s has no creation timestamp", self.namespace, self.name) } - var siblings []localPod + var older []localPod for i := range pods { - pod := pods[i] - if pod.uid == selfUID || pod.namespace != self.namespace { + candidate := pods[i] + if candidate.uid == selfUID || candidate.namespace != self.namespace { continue } - candidateOwnerUID, ownerErr := daemonSetOwnerUID(pod) - if ownerErr == nil && candidateOwnerUID == ownerUID { - precedes, precedesErr := podPrecedes(pod, *self) - if precedesErr != nil { - return nil, precedesErr - } - if precedes { - siblings = append(siblings, pod) - } + candidateOwnerUID, ownerErr := daemonSetOwnerUID(candidate) + if ownerErr != nil || candidateOwnerUID != ownerUID { + continue + } + if candidate.createdAt.IsZero() || candidate.createdAt.Before(self.createdAt) { + older = append(older, candidate) + continue + } + if candidate.createdAt.Equal(self.createdAt) { + return nil, fmt.Errorf("cannot order same-timestamp Pods %s/%s and %s/%s", candidate.namespace, candidate.name, self.namespace, self.name) } } - return siblings, nil + return older, nil } -// podPrecedes fails closed when Kubernetes' second-precision creation -// timestamps cannot establish an order. Pod UIDs and resourceVersions are not -// creation-order values and must not be used to guess which process is active. -func podPrecedes(candidate, self localPod) (bool, error) { - if candidate.createdAt.IsZero() { - // An incomplete kubelet record must not be interpreted as a newer Pod. - return true, nil - } - if candidate.createdAt.Before(self.createdAt) { - return true, nil - } - if candidate.createdAt.After(self.createdAt) { - return false, nil +func olderContainersStopped(current []localPod, known map[string]bool, missing map[string]int, componentName string) bool { + byUID := make(map[string]localPod, len(current)) + for i := range current { + byUID[current[i].uid] = current[i] + } + for uid, deletionObserved := range known { + pod, present := byUID[uid] + if !present { + missing[uid]++ + if !deletionObserved && missing[uid] < missingOlderObservations { + return false + } + continue + } + missing[uid] = 0 + if olderContainerStillExists(pod, componentName) { + return false + } } - return false, fmt.Errorf("cannot order same-timestamp Pods %s/%s and %s/%s", candidate.namespace, candidate.name, self.namespace, self.name) + return len(known) > 0 +} + +func olderContainerStillExists(pod localPod, componentName string) bool { + for i := range pod.containers { + container := pod.containers[i] + if container.name != componentName { + continue + } + // A crashed container may be Terminated briefly before kubelet restarts + // it. Only use Terminated as a handoff signal once Pod deletion proves + // that Kubernetes intends to stop the old generation. + return pod.deletionTimestamp == nil || !container.terminated + } + declared := false + for _, name := range pod.declaredContainers { + declared = declared || name == componentName + } + if !declared { + return false + } + // A missing status is not positive evidence that the old runtime process + // exited. Wait for either a Terminated state or Pod removal after deletion + // was observed. + return true } func daemonSetOwnerUID(pod localPod) (string, error) { @@ -311,11 +283,8 @@ func daemonSetOwnerUID(pod localPod) (string, error) { if owner.kind != "DaemonSet" || !owner.controller { continue } - if owner.uid == "" { - return "", fmt.Errorf("Pod %s/%s has a DaemonSet controller with an empty UID", pod.namespace, pod.name) - } - if ownerUID != "" { - return "", fmt.Errorf("Pod %s/%s has multiple DaemonSet controllers", pod.namespace, pod.name) + if owner.uid == "" || ownerUID != "" { + return "", fmt.Errorf("Pod %s/%s has an invalid DaemonSet controller", pod.namespace, pod.name) } ownerUID = owner.uid } @@ -325,78 +294,6 @@ func daemonSetOwnerUID(pod localPod) (string, error) { return ownerUID, nil } -func (c *component) beginActivation() error { - c.mu.Lock() - defer c.mu.Unlock() - if c.closed { - return errors.New("cannot activate a closed experimental Agent lifecycle") - } - if c.activating { - return errors.New("experimental Agent lifecycle is already activating") - } - if err := c.writeState(agentlifecycle.StateActivating); err != nil { - return err - } - c.activating = true - c.log.Infof("%s has no older DaemonSet sibling on the node and is activating", c.componentName) - return nil -} - -func (c *component) MarkActive() error { - if !c.enabled { - return nil - } - - c.mu.Lock() - defer c.mu.Unlock() - if c.closed { - return errors.New("cannot mark a closed experimental Agent lifecycle active") - } - if !c.activating { - return errors.New("cannot mark the experimental Agent lifecycle active before the sibling check") - } - if err := c.writeState(agentlifecycle.StateActive); err != nil { - return err - } - c.log.Infof("%s is active", c.componentName) - return nil -} - func (c *component) Close() error { - if !c.enabled { - return nil - } - - c.mu.Lock() - defer c.mu.Unlock() - if c.closed { - return nil - } - c.closed = true - return c.writeState(agentlifecycle.StateStopped) -} - -func (c *component) writeState(state string) error { - tmp, err := os.CreateTemp(filepath.Dir(c.statePath), ".agent-rollout-state-") - if err != nil { - return fmt.Errorf("create temporary Agent rollout state: %w", err) - } - tmpPath := tmp.Name() - defer os.Remove(tmpPath) - - if _, err := fmt.Fprintln(tmp, state, c.processPID, c.processStart); err != nil { - _ = tmp.Close() - return fmt.Errorf("write Agent rollout state: %w", err) - } - if err := tmp.Chmod(0o644); err != nil { - _ = tmp.Close() - return fmt.Errorf("set Agent rollout state permissions: %w", err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("close Agent rollout state: %w", err) - } - if err := os.Rename(tmpPath, c.statePath); err != nil { - return fmt.Errorf("publish Agent rollout state: %w", err) - } return nil } diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go index da05425b7c10..7baa227fb544 100644 --- a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go +++ b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go @@ -10,10 +10,6 @@ package agentlifecycleimpl import ( "context" "errors" - "os" - "path/filepath" - "strconv" - "strings" "sync" "testing" "time" @@ -66,275 +62,220 @@ func (s *scriptedPodSource) setResponses(responses ...podResponse) { func TestDisabledLifecycleIsNoop(t *testing.T) { deps := dependencies{Config: config.NewMock(t), Log: logmock.New(t)} - comp, err := newComponent(deps, nil, "linux", testProcessIdentity) + comp, err := newComponent(deps, nil, "linux") require.NoError(t, err) require.NoError(t, comp.Wait(context.Background())) - require.NoError(t, comp.MarkActive()) require.NoError(t, comp.Close()) } -func TestFreshPodActivatesAfterConstruction(t *testing.T) { - comp, _, statePath := newEnabledComponent(t, podResponse{pods: []localPod{selfPod()}}) - +func TestFreshPodStartsAfterTwoSuccessfulObservations(t *testing.T) { + comp, source := newEnabledComponent(t, podResponse{pods: []localPod{selfPod()}}) require.NoError(t, comp.Wait(context.Background())) - require.Equal(t, agentlifecycle.StateActivating, readState(t, statePath)) - require.NoError(t, comp.MarkActive()) - require.Equal(t, agentlifecycle.StateActive, readState(t, statePath)) - require.NoError(t, comp.Close()) - require.Equal(t, agentlifecycle.StateStopped, readState(t, statePath)) - require.NoError(t, comp.Close(), "Close must be idempotent") + require.GreaterOrEqual(t, len(source.calls), freshInstallObservations) } -func TestConstructionClearsStalePreparedState(t *testing.T) { - statePath := filepath.Join(t.TempDir(), "state", "test-agent.state") - require.NoError(t, os.MkdirAll(filepath.Dir(statePath), 0o755)) - require.NoError(t, os.WriteFile(statePath, []byte(agentlifecycle.StatePrepared), 0o644)) - deps := dependencies{ - Config: config.NewMockWithOverrides(t, map[string]interface{}{ - rolloutEnabledKey: true, - rolloutPodUIDKey: selfPodUID, - rolloutStatePathKey: statePath, - }), - Log: logmock.New(t), - Params: agentlifecycle.Params{ComponentName: "test-agent"}, +func TestReplacementWaitsForSameContainerToTerminate(t *testing.T) { + old := siblingPod("old-pod-uid", "old-agent") + old.containers = []localContainer{{name: "test-agent"}} + comp, source := newEnabledComponent(t, podResponse{pods: []localPod{selfPod(), old}}) + + result := make(chan error, 1) + go func() { result <- comp.Wait(context.Background()) }() + <-source.calls + <-source.calls + select { + case err := <-result: + t.Fatalf("replacement started while old container was running: %v", err) + default: } - _, err := newComponent(deps, &scriptedPodSource{}, "linux", testProcessIdentity) - require.NoError(t, err) - require.NoFileExists(t, statePath) -} -func TestStateIsBoundToCurrentProcessGeneration(t *testing.T) { - comp, _, statePath := newEnabledComponent(t, podResponse{pods: []localPod{selfPod()}}) - require.NoError(t, comp.Wait(context.Background())) + now := time.Now() + old.deletionTimestamp = &now + old.containers[0] = localContainer{name: "test-agent", terminated: true} + source.setResponses(podResponse{pods: []localPod{selfPod(), old}}) + require.NoError(t, <-result) +} - contents, err := os.ReadFile(statePath) - require.NoError(t, err) - fields := strings.Fields(string(contents)) - require.Len(t, fields, 3) - require.Equal(t, agentlifecycle.StateActivating, fields[0]) - pid, started, err := testProcessIdentity() - require.NoError(t, err) - require.Equal(t, strconv.Itoa(pid), fields[1]) - require.Equal(t, started, fields[2]) +func TestCrashedOlderContainerDoesNotReleaseGate(t *testing.T) { + old := siblingPod("old-pod-uid", "old-agent") + old.containers = []localContainer{{name: "test-agent", terminated: true}} + known := map[string]bool{old.uid: false} + missing := map[string]int{} + require.False(t, olderContainersStopped([]localPod{old}, known, missing, "test-agent")) + + now := time.Now() + old.deletionTimestamp = &now + known[old.uid] = true + require.True(t, olderContainersStopped([]localPod{old}, known, missing, "test-agent")) } -func TestReplacementRemainsPreparedUntilSiblingDisappears(t *testing.T) { - withOld := []localPod{selfPod(), siblingPod("old-pod-uid", "old-agent")} - comp, source, statePath := newEnabledComponent(t, podResponse{pods: withOld}) +func TestContainersHandOffIndependently(t *testing.T) { + old := siblingPod("old-pod-uid", "old-agent") + old.containers = []localContainer{ + {name: "agent", terminated: true}, + {name: "system-probe"}, + } + now := time.Now() + old.deletionTimestamp = &now + known := map[string]bool{old.uid: true} + missing := map[string]int{} + require.True(t, olderContainersStopped([]localPod{old}, known, missing, "agent")) + require.False(t, olderContainersStopped([]localPod{old}, known, missing, "system-probe")) +} - waitResult := make(chan error, 1) - go func() { waitResult <- comp.Wait(context.Background()) }() +func TestNonCoreWaitsForReplacementCoreReadiness(t *testing.T) { + old := siblingPod("old-pod-uid", "old-agent") + old.containers = []localContainer{{name: "test-agent", terminated: true}} + now := time.Now() + old.deletionTimestamp = &now + self := selfPod() + self.containers[0].ready = false + comp, source := newEnabledComponent(t, podResponse{pods: []localPod{self, old}}) + result := make(chan error, 1) + go func() { result <- comp.Wait(context.Background()) }() + <-source.calls <-source.calls - requireStateEventually(t, statePath, agentlifecycle.StatePrepared) select { - case err := <-waitResult: - t.Fatalf("replacement activated while the old sibling was present: %v", err) - case <-time.After(20 * time.Millisecond): + case err := <-result: + t.Fatalf("non-core component started before replacement core was ready: %v", err) + default: } + self.containers[0].ready = true + source.setResponses(podResponse{pods: []localPod{self, old}}) + require.NoError(t, <-result) +} - source.setResponses(podResponse{pods: []localPod{selfPod()}}) - require.NoError(t, <-waitResult) - require.Equal(t, agentlifecycle.StateActivating, readState(t, statePath)) +func TestDisappearedOlderPodReleasesGateAfterDeletionWasObserved(t *testing.T) { + old := siblingPod("old-pod-uid", "old-agent") + old.containers = []localContainer{{name: "test-agent"}} + known := map[string]bool{old.uid: false} + missing := map[string]int{} + require.False(t, olderContainersStopped([]localPod{old}, known, missing, "test-agent")) + + now := time.Now() + old.deletionTimestamp = &now + known[old.uid] = true + require.False(t, olderContainersStopped([]localPod{old}, known, missing, "test-agent")) + require.True(t, olderContainersStopped(nil, known, missing, "test-agent")) } -func TestKubeletErrorsFailClosedThenRecover(t *testing.T) { - withOld := []localPod{selfPod(), siblingPod("old-pod-uid", "old-agent")} - comp, source, statePath := newEnabledComponent(t, podResponse{err: errors.New("kubelet unavailable")}) +func TestMissingOlderPodWithoutObservedDeletionRequiresTwoSnapshots(t *testing.T) { + known := map[string]bool{"old-pod-uid": false} + missing := map[string]int{} + require.False(t, olderContainersStopped(nil, known, missing, "test-agent")) + require.True(t, olderContainersStopped(nil, known, missing, "test-agent")) +} - waitResult := make(chan error, 1) - go func() { waitResult <- comp.Wait(context.Background()) }() +func TestKubeletErrorsFailClosed(t *testing.T) { + comp, source := newEnabledComponent(t, podResponse{err: errors.New("kubelet unavailable")}) + ctx, cancel := context.WithCancel(context.Background()) + result := make(chan error, 1) + go func() { result <- comp.Wait(ctx) }() + <-source.calls <-source.calls - require.NoFileExists(t, statePath, "a replacement must not become Ready before kubelet safety is established") select { - case err := <-waitResult: - t.Fatalf("kubelet failure opened the activation gate: %v", err) - case <-time.After(20 * time.Millisecond): + case err := <-result: + t.Fatalf("kubelet error opened the gate: %v", err) + default: } - source.setResponses(podResponse{pods: withOld}) - require.Eventually(t, func() bool { - contents, err := os.ReadFile(statePath) - return err == nil && len(strings.Fields(string(contents))) > 0 && strings.Fields(string(contents))[0] == agentlifecycle.StatePrepared - }, time.Second, time.Millisecond) - require.Equal(t, agentlifecycle.StatePrepared, readState(t, statePath)) - source.setResponses(podResponse{pods: []localPod{selfPod()}}) - require.NoError(t, <-waitResult) + cancel() + require.ErrorIs(t, <-result, context.Canceled) } -func TestMissingSelfPodFailsClosed(t *testing.T) { - comp, source, statePath := newEnabledComponent(t, podResponse{pods: []localPod{siblingPod("old-pod-uid", "old-agent")}}) - ctx, cancel := context.WithCancel(context.Background()) - waitResult := make(chan error, 1) - go func() { waitResult <- comp.Wait(ctx) }() - <-source.calls - require.NoFileExists(t, statePath) - cancel() - require.ErrorIs(t, <-waitResult, context.Canceled) +func TestMissingContainerStatusRequiresPodDeletion(t *testing.T) { + old := siblingPod("old-pod-uid", "old-agent") + old.declaredContainers = []string{"test-agent"} + known := map[string]bool{old.uid: false} + missing := map[string]int{} + require.False(t, olderContainersStopped([]localPod{old}, known, missing, "test-agent")) + now := time.Now() + old.deletionTimestamp = &now + known[old.uid] = true + require.False(t, olderContainersStopped([]localPod{old}, known, missing, "test-agent")) } -func TestDifferentDaemonSetOrNamespaceDoesNotBlock(t *testing.T) { +func TestComponentAddedByNewRevisionDoesNotWait(t *testing.T) { + old := siblingPod("old-pod-uid", "old-agent") + old.declaredContainers = []string{"agent"} + known := map[string]bool{old.uid: false} + require.True(t, olderContainersStopped([]localPod{old}, known, map[string]int{}, "system-probe")) +} + +func TestDifferentDaemonSetAndNewerPodDoNotBlock(t *testing.T) { otherDaemon := siblingPod("other-daemon-pod", "other-daemon") otherDaemon.owners[0].uid = "other-daemonset-uid" - otherNamespace := siblingPod("other-namespace-pod", "other-namespace") - otherNamespace.namespace = "other" - - comp, _, _ := newEnabledComponent(t, podResponse{pods: []localPod{selfPod(), otherDaemon, otherNamespace}}) - require.NoError(t, comp.Wait(context.Background())) -} + newer := siblingPod("newer-pod", "newer-agent") + newer.createdAt = time.Unix(300, 0) -func TestNonControllerOwnerDoesNotIdentifySelf(t *testing.T) { - self := selfPod() - self.owners[0].controller = false - comp, source, statePath := newEnabledComponent(t, podResponse{pods: []localPod{self}}) - ctx, cancel := context.WithCancel(context.Background()) - waitResult := make(chan error, 1) - go func() { waitResult <- comp.Wait(ctx) }() - <-source.calls - require.NoFileExists(t, statePath) - cancel() - require.ErrorIs(t, <-waitResult, context.Canceled) + older, err := olderSiblingPods([]localPod{selfPod(), otherDaemon, newer}, selfPodUID) + require.NoError(t, err) + require.Empty(t, older) } -func TestLifecycleWaitCancellation(t *testing.T) { - withOld := []localPod{selfPod(), siblingPod("old-pod-uid", "old-agent")} - comp, source, statePath := newEnabledComponent(t, podResponse{pods: withOld}) - ctx, cancel := context.WithCancel(context.Background()) - waitResult := make(chan error, 1) - go func() { waitResult <- comp.Wait(ctx) }() - - <-source.calls - requireStateEventually(t, statePath, agentlifecycle.StatePrepared) - cancel() - require.ErrorIs(t, <-waitResult, context.Canceled) - require.NoError(t, comp.Close()) +func TestSameTimestampFailsClosed(t *testing.T) { + other := siblingPod("other-pod", "other-agent") + other.createdAt = selfPod().createdAt + _, err := olderSiblingPods([]localPod{selfPod(), other}, selfPodUID) + require.ErrorContains(t, err, "same-timestamp") } func TestLifecycleRequiresValidConfiguration(t *testing.T) { - tests := map[string]map[string]interface{}{ - "missing Pod UID": { - rolloutStatePathKey: filepath.Join(t.TempDir(), "test-agent.state"), - }, - "relative state": { - rolloutPodUIDKey: selfPodUID, - rolloutStatePathKey: "test-agent.state", - }, - "shared state path": { - rolloutPodUIDKey: selfPodUID, - rolloutStatePathKey: filepath.Join(t.TempDir(), "agent.state"), - }, + tests := map[string]struct { + component string + podUID string + platform string + }{ + "missing component": {podUID: selfPodUID, platform: "linux"}, + "unsafe component": {component: "../agent", podUID: selfPodUID, platform: "linux"}, + "missing Pod UID": {component: "test-agent", platform: "linux"}, + "non Linux": {component: "test-agent", podUID: selfPodUID, platform: "windows"}, } - for name, overrides := range tests { + for name, test := range tests { t.Run(name, func(t *testing.T) { - overrides[rolloutEnabledKey] = true deps := dependencies{ - Config: config.NewMockWithOverrides(t, overrides), + Config: config.NewMockWithOverrides(t, map[string]interface{}{ + rolloutEnabledKey: true, + rolloutPodUIDKey: test.podUID, + }), Log: logmock.New(t), - Params: agentlifecycle.Params{ComponentName: "test-agent"}, + Params: agentlifecycle.Params{ComponentName: test.component}, } - _, err := newComponent(deps, &scriptedPodSource{}, "linux", testProcessIdentity) + _, err := newComponent(deps, &scriptedPodSource{}, test.platform) require.Error(t, err) }) } } -func TestComponentPathResolution(t *testing.T) { - tests := map[string]struct { - configured string - component string - expected string - }{ - "template": { - configured: "/var/run/datadog/{component}.state", - component: "core-agent", - expected: "/var/run/datadog/core-agent.state", - }, - "operator expanded path": { - configured: "/var/run/datadog/trace-agent.state", - component: "trace-agent", - expected: "/var/run/datadog/trace-agent.state", - }, - } - for name, test := range tests { - t.Run(name, func(t *testing.T) { - resolved, err := resolveComponentPath(test.configured, test.component, ".state", "test.path") - require.NoError(t, err) - require.Equal(t, test.expected, resolved) - }) - } -} - -func TestPreparedRolloutRejectsUnsupportedPlatform(t *testing.T) { - require.ErrorContains(t, validatePlatform("windows"), "Linux-only") - require.ErrorContains(t, validatePlatform("darwin"), "Linux-only") - require.NoError(t, validatePlatform("linux")) -} - -func TestComponentPathRejectsTraversalName(t *testing.T) { - _, err := resolveComponentPath("/var/run/datadog/{component}.state", "..", ".state", "test.path") - require.ErrorContains(t, err, "path-safe") -} - -func TestMarkActiveRequiresSiblingCheck(t *testing.T) { - comp, _, _ := newEnabledComponent(t, podResponse{pods: []localPod{selfPod()}}) - require.Error(t, comp.MarkActive()) -} - -func TestSiblingSelectionRejectsDuplicateSelf(t *testing.T) { - _, err := siblingPods([]localPod{selfPod(), selfPod()}, selfPodUID) - require.ErrorContains(t, err, "duplicate") -} - -func TestOlderPodWinsAfterSimultaneousRestart(t *testing.T) { - oldSelf := selfPod() - oldSelf.createdAt = time.Unix(100, 0) - newer := siblingPod("newer-pod-uid", "newer-agent") - newer.createdAt = time.Unix(300, 0) - - blocking, err := siblingPods([]localPod{oldSelf, newer}, selfPodUID) +func newEnabledComponent(t *testing.T, responses ...podResponse) (*component, *scriptedPodSource) { + t.Helper() + source := &scriptedPodSource{responses: responses, calls: make(chan struct{}, 20)} + comp, err := newComponent(enabledDependencies(t), source, "linux") require.NoError(t, err) - require.Empty(t, blocking, "an older Pod must reactivate instead of deadlocking with a newer replacement") -} - -func TestSameTimestampFailsClosed(t *testing.T) { - self := selfPod() - other := siblingPod("other-pod-uid", "other-agent") - other.createdAt = self.createdAt - - _, err := siblingPods([]localPod{self, other}, selfPodUID) - require.ErrorContains(t, err, "same-timestamp") + result := comp.(*component) + result.pollInterval = time.Millisecond + return result, source } -func newEnabledComponent(t *testing.T, responses ...podResponse) (agentlifecycle.Component, *scriptedPodSource, string) { - t.Helper() - dir := t.TempDir() - statePath := filepath.Join(dir, "state", "test-agent.state") - source := &scriptedPodSource{responses: responses, calls: make(chan struct{}, 10)} - deps := dependencies{ +func enabledDependencies(t *testing.T) dependencies { + return dependencies{ Config: config.NewMockWithOverrides(t, map[string]interface{}{ - rolloutEnabledKey: true, - rolloutPodUIDKey: selfPodUID, - rolloutStatePathKey: statePath, + rolloutEnabledKey: true, + rolloutPodUIDKey: selfPodUID, }), Log: logmock.New(t), Params: agentlifecycle.Params{ComponentName: "test-agent"}, } - comp, err := newComponent(deps, source, "linux", testProcessIdentity) - require.NoError(t, err) - comp.(*component).pollInterval = time.Millisecond - return comp, source, statePath -} - -func testProcessIdentity() (int, string, error) { - return 4242, "123456", nil } func selfPod() localPod { return localPod{ - uid: selfPodUID, - name: "new-agent", - namespace: "datadog-agent", - createdAt: time.Unix(200, 0), - owners: []podOwner{{kind: "DaemonSet", uid: daemonUID, controller: true}}, + uid: selfPodUID, + name: "new-agent", + namespace: "datadog-agent", + createdAt: time.Unix(200, 0), + owners: []podOwner{{kind: "DaemonSet", uid: daemonUID, controller: true}}, + containers: []localContainer{{name: "agent", ready: true}}, } } @@ -345,20 +286,3 @@ func siblingPod(uid, name string) localPod { pod.createdAt = time.Unix(100, 0) return pod } - -func readState(t *testing.T, path string) string { - t.Helper() - contents, err := os.ReadFile(path) - require.NoError(t, err) - fields := strings.Fields(string(contents)) - require.NotEmpty(t, fields) - return fields[0] -} - -func requireStateEventually(t *testing.T, path, expected string) { - t.Helper() - require.Eventually(t, func() bool { - contents, err := os.ReadFile(path) - return err == nil && len(strings.Fields(string(contents))) > 0 && strings.Fields(string(contents))[0] == expected - }, time.Second, time.Millisecond) -} diff --git a/comp/core/agentlifecycle/impl/local_pods_kubelet.go b/comp/core/agentlifecycle/impl/local_pods_kubelet.go index 60e5add23f48..7d67270d99d5 100644 --- a/comp/core/agentlifecycle/impl/local_pods_kubelet.go +++ b/comp/core/agentlifecycle/impl/local_pods_kubelet.go @@ -42,12 +42,27 @@ func (kubeletLocalPodSource) ListLocalPods(ctx context.Context) ([]localPod, err controller: owner.Controller != nil && *owner.Controller, }) } + containers := make([]localContainer, 0, len(pod.Status.Containers)) + for _, container := range pod.Status.Containers { + containers = append(containers, localContainer{ + name: container.Name, + terminated: container.State.Terminated != nil, + ready: container.Ready, + }) + } + declaredContainers := make([]string, 0, len(pod.Spec.Containers)) + for _, container := range pod.Spec.Containers { + declaredContainers = append(declaredContainers, container.Name) + } pods = append(pods, localPod{ - uid: pod.Metadata.UID, - name: pod.Metadata.Name, - namespace: pod.Metadata.Namespace, - createdAt: pod.Metadata.CreationTimestamp, - owners: owners, + uid: pod.Metadata.UID, + name: pod.Metadata.Name, + namespace: pod.Metadata.Namespace, + createdAt: pod.Metadata.CreationTimestamp, + deletionTimestamp: pod.Metadata.DeletionTimestamp, + owners: owners, + declaredContainers: declaredContainers, + containers: containers, }) } return pods, nil diff --git a/pkg/util/fxutil/errorunwrapper.go b/pkg/util/fxutil/errorunwrapper.go index a64f84895d76..a7edefcf744d 100644 --- a/pkg/util/fxutil/errorunwrapper.go +++ b/pkg/util/fxutil/errorunwrapper.go @@ -8,17 +8,18 @@ package fxutil import ( "errors" "reflect" - "regexp" ) // UnwrapIfErrArgumentsFailed unwrap the error if the error was returned by an FX invoke method otherwise return the error. func UnwrapIfErrArgumentsFailed(err error) error { // This is a workaround until https://github.com/uber-go/fx/issues/988 will be done. if reflect.TypeOf(err).Name() == "errArgumentsFailed" { - re := regexp.MustCompile(`.*received non-nil error from function.*\(.*\): (.*)`) - matches := re.FindStringSubmatch(err.Error()) - if len(matches) == 2 { - return errors.New(matches[1]) + for { + cause := errors.Unwrap(err) + if cause == nil { + return err + } + err = cause } } return err diff --git a/pkg/util/fxutil/errorunwrapper_test.go b/pkg/util/fxutil/errorunwrapper_test.go index 65b8c1d487c2..62fe3a85794b 100644 --- a/pkg/util/fxutil/errorunwrapper_test.go +++ b/pkg/util/fxutil/errorunwrapper_test.go @@ -19,4 +19,5 @@ func TestUnwrapIfErrArgumentsFailed(t *testing.T) { fx.Provide(func() (*struct{}, error) { return nil, expectedError }), ) require.Equal(t, expectedError.Error(), err.Error()) + require.ErrorIs(t, err, expectedError) } diff --git a/pkg/util/fxutil/oneshot.go b/pkg/util/fxutil/oneshot.go index b71cc9dc5d23..68f4ecd92431 100644 --- a/pkg/util/fxutil/oneshot.go +++ b/pkg/util/fxutil/oneshot.go @@ -28,14 +28,13 @@ func OneShot(oneShotFunc interface{}, opts ...fx.Option) error { // OneShotWithStartupGate constructs an Fx application, waits for the supplied gate before // starting its lifecycle, invokes oneShotFunc, and releases the gate after lifecycle shutdown. -// The one-shot function is responsible for calling MarkActive after its non-Fx startup completes. func OneShotWithStartupGate[T StartupGate](oneShotFunc interface{}, opts ...fx.Option) error { if fxAppTestOverride != nil { return fxAppTestOverride(oneShotFunc, opts) } - var gate T - opts = append(opts, fx.Populate(&gate)) + var gate StartupGate + opts = append([]fx.Option{StartupGateOption[T](context.Background(), &gate)}, opts...) return oneShot(oneShotFunc, func() StartupGate { return gate }, opts...) } @@ -62,16 +61,13 @@ func oneShot(oneShotFunc interface{}, gateProvider func() StartupGate, opts ...f var gate StartupGate if gateProvider != nil { - if err := app.Err(); err != nil { - return UnwrapIfErrArgumentsFailed(err) - } gate = gateProvider() - if err := WaitForStartupGate(context.Background(), app, gate); err != nil { - if errors.Is(err, ErrStartupGateShutdown) { - return gate.Close() - } - return errors.Join(err, gate.Close()) + } + if err := app.Err(); err != nil { + if gate != nil { + return errors.Join(UnwrapIfErrArgumentsFailed(err), gate.Close()) } + return UnwrapIfErrArgumentsFailed(err) } // start the app @@ -90,16 +86,12 @@ func oneShot(oneShotFunc interface{}, gateProvider func() StartupGate, opts ...f return stopAppAndCloseGate(app, gate) } -// stopAppAndCloseGate preserves the active lifecycle state when Fx cannot prove -// that every hook stopped. Process teardown remains the final safety boundary. func stopAppAndCloseGate(app *fx.App, gate StartupGate) error { - if err := stopApp(app); err != nil { - return err - } + stopErr := stopApp(app) if gate != nil { - return gate.Close() + return errors.Join(stopErr, gate.Close()) } - return nil + return stopErr } func stopApp(app *fx.App) error { diff --git a/pkg/util/fxutil/run.go b/pkg/util/fxutil/run.go index 6356b4ef9552..011a77d44126 100644 --- a/pkg/util/fxutil/run.go +++ b/pkg/util/fxutil/run.go @@ -24,14 +24,14 @@ func Run(opts ...fx.Option) error { } // RunWithStartupGate constructs an Fx application, waits for the supplied gate before -// starting its lifecycle, marks it active after startup, and releases it after shutdown. +// starting its lifecycle, and releases it after shutdown. func RunWithStartupGate[T StartupGate](opts ...fx.Option) error { if fxAppTestOverride != nil { return fxAppTestOverride(func() {}, opts) } - var gate T - opts = append(opts, fx.Populate(&gate)) + var gate StartupGate + opts = append([]fx.Option{StartupGateOption[T](context.Background(), &gate)}, opts...) return run(func() StartupGate { return gate }, opts...) } @@ -46,19 +46,15 @@ func run(gateProvider func() StartupGate, opts ...fx.Option) error { ) app := fx.New(opts...) - if err := app.Err(); err != nil { - return err - } - var gate StartupGate if gateProvider != nil { gate = gateProvider() - if err := WaitForStartupGate(context.Background(), app, gate); err != nil { - if errors.Is(err, ErrStartupGateShutdown) { - return gate.Close() - } - return errors.Join(err, gate.Close()) + } + if err := app.Err(); err != nil { + if gate != nil { + return errors.Join(UnwrapIfErrArgumentsFailed(err), gate.Close()) } + return UnwrapIfErrArgumentsFailed(err) } startCtx, cancel := context.WithTimeout(context.Background(), app.StartTimeout()) @@ -67,12 +63,6 @@ func run(gateProvider func() StartupGate, opts ...fx.Option) error { if err := app.Start(startCtx); err != nil { return errors.Join(UnwrapIfErrArgumentsFailed(err), stopAppAndCloseGate(app, gate)) } - if gate != nil { - if err := gate.MarkActive(); err != nil { - return errors.Join(err, stopAppAndCloseGate(app, gate)) - } - } - <-app.Done() return stopAppAndCloseGate(app, gate) diff --git a/pkg/util/fxutil/startup_gate.go b/pkg/util/fxutil/startup_gate.go index 8183442c2239..ec6ceeba6284 100644 --- a/pkg/util/fxutil/startup_gate.go +++ b/pkg/util/fxutil/startup_gate.go @@ -7,41 +7,21 @@ package fxutil import ( "context" - "errors" "go.uber.org/fx" ) -// StartupGate delays lifecycle startup until an external ownership condition is satisfied. +// StartupGate delays lifecycle startup until an external condition is satisfied. type StartupGate interface { Wait(context.Context) error - MarkActive() error Close() error } -// ErrStartupGateShutdown means an Fx shutdown signal arrived while the gate was waiting. -var ErrStartupGateShutdown = errors.New("shutdown while waiting on startup gate") - -// WaitForStartupGate waits for a gate and cancels it when the caller or Fx requests shutdown. -func WaitForStartupGate(ctx context.Context, app *fx.App, gate StartupGate) error { - waitCtx, cancel := context.WithCancel(ctx) - defer cancel() - - waitResult := make(chan error, 1) - go func() { - waitResult <- gate.Wait(waitCtx) - }() - - select { - case err := <-waitResult: - return err - case <-app.Done(): - cancel() - <-waitResult - return ErrStartupGateShutdown - case <-ctx.Done(): - cancel() - <-waitResult - return ctx.Err() - } +// StartupGateOption installs the gate as the first Fx invoke. Fx providers are +// lazy, so later constructors and invokes are not evaluated until Wait returns. +func StartupGateOption[T StartupGate](ctx context.Context, captured *StartupGate) fx.Option { + return fx.Invoke(func(gate T) error { + *captured = gate + return gate.Wait(ctx) + }) } diff --git a/pkg/util/fxutil/startup_gate_test.go b/pkg/util/fxutil/startup_gate_test.go index ff1c68c0f5de..4cc6ec3ca080 100644 --- a/pkg/util/fxutil/startup_gate_test.go +++ b/pkg/util/fxutil/startup_gate_test.go @@ -35,7 +35,6 @@ func (r *gateRecorder) snapshot() []string { type recordingGate struct{ recorder *gateRecorder } func (g *recordingGate) Wait(context.Context) error { g.recorder.add("wait"); return nil } -func (g *recordingGate) MarkActive() error { g.recorder.add("active"); return nil } func (g *recordingGate) Close() error { g.recorder.add("close"); return nil } type recordingComponent struct{} @@ -57,7 +56,7 @@ func TestOneShotWithStartupGateOrdering(t *testing.T) { }), ) require.NoError(t, err) - require.Equal(t, []string{"construct", "wait", "start", "run", "stop", "close"}, recorder.snapshot()) + require.Equal(t, []string{"wait", "construct", "start", "run", "stop", "close"}, recorder.snapshot()) } func TestRunWithStartupGateOrdering(t *testing.T) { @@ -77,10 +76,10 @@ func TestRunWithStartupGateOrdering(t *testing.T) { }), ) require.NoError(t, err) - require.Equal(t, []string{"wait", "start", "active", "stop", "close"}, recorder.snapshot()) + require.Equal(t, []string{"wait", "start", "stop", "close"}, recorder.snapshot()) } -func TestOneShotWithStartupGateKeepsOwnershipWhenStopFails(t *testing.T) { +func TestOneShotWithStartupGateClosesAfterStopFailure(t *testing.T) { recorder := &gateRecorder{} gate := &recordingGate{recorder: recorder} stopErr := errors.New("stop failed") @@ -97,10 +96,10 @@ func TestOneShotWithStartupGateKeepsOwnershipWhenStopFails(t *testing.T) { }), ) require.ErrorIs(t, err, stopErr) - require.Equal(t, []string{"wait", "start", "run", "stop"}, recorder.snapshot()) + require.Equal(t, []string{"wait", "start", "run", "stop", "close"}, recorder.snapshot()) } -func TestRunWithStartupGateKeepsOwnershipWhenStopFails(t *testing.T) { +func TestRunWithStartupGateClosesAfterStopFailure(t *testing.T) { recorder := &gateRecorder{} gate := &recordingGate{recorder: recorder} stopErr := errors.New("stop failed") @@ -115,5 +114,5 @@ func TestRunWithStartupGateKeepsOwnershipWhenStopFails(t *testing.T) { }), ) require.ErrorIs(t, err, stopErr) - require.Equal(t, []string{"wait", "active", "stop"}, recorder.snapshot()) + require.Equal(t, []string{"wait", "stop", "close"}, recorder.snapshot()) } diff --git a/tasks/build_tags.bzl b/tasks/build_tags.bzl index 3731fae968e7..0076b3a38b96 100644 --- a/tasks/build_tags.bzl +++ b/tasks/build_tags.bzl @@ -236,6 +236,7 @@ SERVERLESS_TAGS = set(["serverless", "otlp"]) SYSTEM_PROBE_TAGS = set([ "datadog.no_waf", "ec2", + "kubelet", "linux_bpf", "netcgo", "npm", @@ -278,7 +279,7 @@ LOADER_TAGS = set() # imported by https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/f963ab53ee55aeb56d58617ed12c840e8b07cc53/receiver/prometheusreceiver/factory.go#L10 HOST_PROFILER_TAGS = set(["remove_all_sd", "docker", "kubelet"]) -PRIVATEACTIONRUNNER_TAGS = set(["zlib", "zstd"]) +PRIVATEACTIONRUNNER_TAGS = set(["kubelet", "zlib", "zstd"]) SECRET_GENERIC_CONNECTOR_TAGS = set() From 6256379aad7652369c212fa59af9c8e3d2d7a181 Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Tue, 28 Jul 2026 14:22:15 +0200 Subject: [PATCH 8/9] Fix Agent lifecycle race tests --- comp/README.md | 2 +- .../core/agentlifecycle/impl/agent_lifecycle_test.go | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/comp/README.md b/comp/README.md index 9177c97bc741..491b0dc13614 100644 --- a/comp/README.md +++ b/comp/README.md @@ -101,7 +101,7 @@ agent flavors and binaries. ### [comp/core/agentlifecycle](https://pkg.go.dev/github.com/DataDog/datadog-agent/comp/core/agentlifecycle) -Package agentlifecycle defines the experimental prepared/active Agent lifecycle component. +Package agentlifecycle defines the experimental Agent startup gate. ### [comp/core/agenttelemetry](https://pkg.go.dev/github.com/DataDog/datadog-agent/comp/core/agenttelemetry) diff --git a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go index 7baa227fb544..d41c7e197d7b 100644 --- a/comp/core/agentlifecycle/impl/agent_lifecycle_test.go +++ b/comp/core/agentlifecycle/impl/agent_lifecycle_test.go @@ -90,9 +90,10 @@ func TestReplacementWaitsForSameContainerToTerminate(t *testing.T) { } now := time.Now() - old.deletionTimestamp = &now - old.containers[0] = localContainer{name: "test-agent", terminated: true} - source.setResponses(podResponse{pods: []localPod{selfPod(), old}}) + terminatedOld := siblingPod("old-pod-uid", "old-agent") + terminatedOld.deletionTimestamp = &now + terminatedOld.containers = []localContainer{{name: "test-agent", terminated: true}} + source.setResponses(podResponse{pods: []localPod{selfPod(), terminatedOld}}) require.NoError(t, <-result) } @@ -141,8 +142,9 @@ func TestNonCoreWaitsForReplacementCoreReadiness(t *testing.T) { t.Fatalf("non-core component started before replacement core was ready: %v", err) default: } - self.containers[0].ready = true - source.setResponses(podResponse{pods: []localPod{self, old}}) + readySelf := selfPod() + readySelf.containers[0].ready = true + source.setResponses(podResponse{pods: []localPod{readySelf, old}}) require.NoError(t, <-result) } From 2c9af7f8f11b3ea25a45d791675848cdc4b50a35 Mon Sep 17 00:00:00 2001 From: "ali.benabdallah" Date: Tue, 28 Jul 2026 15:52:56 +0200 Subject: [PATCH 9/9] Remove obsolete rollout lifecycle hooks --- comp/core/healthprobe/impl/BUILD.bazel | 1 - comp/core/healthprobe/impl/healthprobe.go | 17 ++++++------ .../core/healthprobe/impl/healthprobe_test.go | 26 ++++--------------- pkg/config/setup/agent_lifecycle.go | 3 --- pkg/config/setup/config_test.go | 1 - 5 files changed, 14 insertions(+), 34 deletions(-) diff --git a/comp/core/healthprobe/impl/BUILD.bazel b/comp/core/healthprobe/impl/BUILD.bazel index f8d90d1bdca7..2de39edc5075 100644 --- a/comp/core/healthprobe/impl/BUILD.bazel +++ b/comp/core/healthprobe/impl/BUILD.bazel @@ -26,6 +26,5 @@ dd_agent_go_test( "//comp/def", "//pkg/status/health", "@com_github_stretchr_testify//assert", - "@com_github_stretchr_testify//require", ], ) diff --git a/comp/core/healthprobe/impl/healthprobe.go b/comp/core/healthprobe/impl/healthprobe.go index b13efc61471d..aa5d7e537b9a 100644 --- a/comp/core/healthprobe/impl/healthprobe.go +++ b/comp/core/healthprobe/impl/healthprobe.go @@ -48,11 +48,6 @@ type healthprobe struct { } func (h *healthprobe) start() error { - ln, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%v", h.options.Port)) - if err != nil { - return err - } - h.listener = ln h.log.Debugf("Health check listening on port %d", h.options.Port) go h.server.Serve(h.listener) //nolint:errcheck @@ -77,12 +72,18 @@ func NewComponent(reqs Requires) (Provides, error) { return provides, nil } + ln, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%v", healthPort)) + if err != nil { + return provides, err + } + server := buildServer(reqs.Options, reqs.Log) probe := &healthprobe{ - options: reqs.Options, - log: reqs.Log, - server: server, + options: reqs.Options, + log: reqs.Log, + server: server, + listener: ln, } reqs.Lc.Append(compdef.Hook{ diff --git a/comp/core/healthprobe/impl/healthprobe_test.go b/comp/core/healthprobe/impl/healthprobe_test.go index 150bb6663214..d3262a28a285 100644 --- a/comp/core/healthprobe/impl/healthprobe_test.go +++ b/comp/core/healthprobe/impl/healthprobe_test.go @@ -9,14 +9,11 @@ package healthprobeimpl import ( "context" "errors" - "fmt" - "net" "net/http" "net/http/httptest" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" healthprobeComponent "github.com/DataDog/datadog-agent/comp/core/healthprobe/def" logmock "github.com/DataDog/datadog-agent/comp/core/log/mock" @@ -25,10 +22,6 @@ import ( ) func TestServer(t *testing.T) { - reserved, err := net.Listen("tcp", "0.0.0.0:0") - require.NoError(t, err) - port := reserved.Addr().(*net.TCPAddr).Port - require.NoError(t, reserved.Close()) lc := compdef.NewTestLifecycle(t) logComponent := logmock.New(t) @@ -37,30 +30,21 @@ func TestServer(t *testing.T) { Lc: lc, Log: logComponent, Options: healthprobeComponent.Options{ - Port: port, + Port: 7869, }, } provides, err := NewComponent(requires) - require.NoError(t, err) - - require.NotNil(t, provides.Comp) + assert.NoError(t, err) - beforeStart, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%d", port)) - require.NoError(t, err, "constructing the component must not claim the health port") - require.NoError(t, beforeStart.Close()) + assert.NotNil(t, provides.Comp) ctx := context.Background() lc.AssertHooksNumber(1) - require.NoError(t, lc.Start(ctx)) - conflicting, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%d", port)) - require.Error(t, err, "starting the component must claim the health port") - if conflicting != nil { - require.NoError(t, conflicting.Close()) - } - require.NoError(t, lc.Stop(ctx)) + assert.NoError(t, lc.Start(ctx)) + assert.NoError(t, lc.Stop(ctx)) } func TestServerNoHealthPort(t *testing.T) { diff --git a/pkg/config/setup/agent_lifecycle.go b/pkg/config/setup/agent_lifecycle.go index 2e35a099d07f..8d9d40e6963f 100644 --- a/pkg/config/setup/agent_lifecycle.go +++ b/pkg/config/setup/agent_lifecycle.go @@ -12,12 +12,9 @@ const ( ExperimentalNodeAgentRolloutEnabled = "experimental.node_agent_rollout.enabled" // ExperimentalNodeAgentRolloutPodUID identifies the Pod that must wait for older siblings owned by the same DaemonSet. ExperimentalNodeAgentRolloutPodUID = "experimental.node_agent_rollout.pod_uid" - // ExperimentalNodeAgentRolloutStatePath reports the local lifecycle state of one Agent process. Shared configuration must include {component}. - ExperimentalNodeAgentRolloutStatePath = "experimental.node_agent_rollout.state_path" ) func setupExperimentalNodeAgentRollout(config pkgconfigmodel.Setup) { config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutEnabled, false) config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutPodUID, "") - config.BindEnvAndSetDefault(ExperimentalNodeAgentRolloutStatePath, "") } diff --git a/pkg/config/setup/config_test.go b/pkg/config/setup/config_test.go index 28d00f7f209b..0cbda83c7f5b 100644 --- a/pkg/config/setup/config_test.go +++ b/pkg/config/setup/config_test.go @@ -777,7 +777,6 @@ func TestExperimentalNodeAgentRolloutDefaults(t *testing.T) { assert.False(t, config.GetBool(ExperimentalNodeAgentRolloutEnabled)) assert.Empty(t, config.GetString(ExperimentalNodeAgentRolloutPodUID)) - assert.Empty(t, config.GetString(ExperimentalNodeAgentRolloutStatePath)) } func TestInfrastructureModeNoneDisablesECSTaskCollection(t *testing.T) {