avoid data race caused by calling forceCompState outside commandRunt…

…ime.Run (#3537) (#3555) Instead of startWatcher setting the component state on comm.WriteConnInfo failure, now it logs the error using the component stderr logger. (cherry picked from commit 39f324f) Co-authored-by: Anderson Queiroz <[email protected]>
elastic · Oct 6, 2023 · cb50e97 · cb50e97
1 parent e4cd988
commit cb50e97
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git 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
@@ -2474,18 +2474,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
@@ -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 {