diff --git a/changelog/fragments/1696516399-Avoid-possible-data-race-when-diring-a-component-start-the-Elastic-Agent-fails-to-write-to-the-component's-stdin.yaml b/changelog/fragments/1696516399-Avoid-possible-data-race-when-diring-a-component-start-the-Elastic-Agent-fails-to-write-to-the-component's-stdin.yaml new file mode 100644 index 00000000000..2e0a88a0d80 --- /dev/null +++ b/changelog/fragments/1696516399-Avoid-possible-data-race-when-diring-a-component-start-the-Elastic-Agent-fails-to-write-to-the-component's-stdin.yaml @@ -0,0 +1,32 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug + +# Change summary; a 80ish characters long description of the change. +summary: Avoid possible data race when diring a component start the Elastic Agent fails to write to the component's stdin + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; a word indicating the component this changeset affects. +component: component/runtime + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +pr: https://github.com/owner/repo/1234 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +#issue: https://github.com/owner/repo/1234 diff --git a/pkg/component/runtime/command.go b/pkg/component/runtime/command.go index df02656b3f2..140e9d22557 100644 --- a/pkg/component/runtime/command.go +++ b/pkg/component/runtime/command.go @@ -422,7 +422,7 @@ func (c *commandRuntime) startWatcher(info *process.Info, comm Communicator) { go func() { err := comm.WriteConnInfo(info.Stdin) if err != nil { - c.forceCompState(client.UnitStateFailed, fmt.Sprintf("Failed: failed to provide connection information to spawned pid '%d': %s", info.PID, err)) + _, _ = c.logErr.Write([]byte(fmt.Sprintf("Failed: failed to provide connection information to spawned pid '%d': %s", info.PID, err))) // kill instantly _ = info.Kill() } else { diff --git a/pkg/component/runtime/manager_test.go b/pkg/component/runtime/manager_test.go index cbce15b13c5..010c9f0478f 100644 --- a/pkg/component/runtime/manager_test.go +++ b/pkg/component/runtime/manager_test.go @@ -2481,18 +2481,21 @@ func TestManager_FakeInput_RestartsOnMissedCheckins(t *testing.T) { return case state := <-sub.Ch(): t.Logf("component state changed: %+v", state) - if state.State == client.UnitStateStarting || state.State == client.UnitStateHealthy { + + switch state.State { + case client.UnitStateStarting: + case client.UnitStateHealthy: // starting and healthy are allowed - } else if state.State == client.UnitStateDegraded { + case client.UnitStateDegraded: // should go to degraded first wasDegraded = true - } else if state.State == client.UnitStateFailed { + case client.UnitStateFailed: if wasDegraded { subErrCh <- nil } else { subErrCh <- errors.New("should have been degraded before failed") } - } else { + default: subErrCh <- fmt.Errorf("unknown component state: %v", state.State) } } diff --git a/pkg/component/runtime/state.go b/pkg/component/runtime/state.go index 37392b6d4bc..203f453fa76 100644 --- a/pkg/component/runtime/state.go +++ b/pkg/component/runtime/state.go @@ -462,7 +462,9 @@ func (s *ComponentState) cleanupStopped() bool { return cleaned } -// forceState force updates the state for the entire component, forcing that state on all units. +// forceState force updates the state for the entire component, forcing that +// state on all units. It returns true if either the component state or any of +// the units state changed, false otherwise. func (s *ComponentState) forceState(state client.UnitState, msg string) bool { changed := false if s.State != state || s.Message != msg {