Skip to content

Commit

Permalink
avoid reporting empty version_info for components that just started
Browse files Browse the repository at this point in the history
Given a design choice, the coordinator's reported state might be stale, leading to healthy components lacking version_info. 
The state diagnostics hook now re-fetches once the state when necessary, improving the accuracy of component status reporting.
  • Loading branch information
AndersonQ committed Aug 21, 2024
1 parent 3544522 commit db1be75
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 14 deletions.
37 changes: 29 additions & 8 deletions internal/pkg/agent/application/coordinator/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ func New(logger *logger.Logger, cfg *configuration.Configuration, logLevel logp.
return c
}

// State returns the current state for the coordinator.
// State returns the current state for the coordinator. It might be a state
// behind the actual state. See broadcaster.New for details.
// Called by external goroutines.
func (c *Coordinator) State() State {
return c.stateBroadcaster.Get()
Expand Down Expand Up @@ -904,15 +905,35 @@ func (c *Coordinator) DiagnosticHooks() diagnostics.Hooks {
UpgradeDetails *details.Details `yaml:"upgrade_details,omitempty"`
}

s := c.State()
n := len(s.Components)
compStates := make([]StateComponentOutput, n)
for i := 0; i < n; i++ {
compStates[i] = StateComponentOutput{
ID: s.Components[i].Component.ID,
State: s.Components[i].State,
getState := func(c *Coordinator) (State, []StateComponentOutput) {
s := c.State()
compStates := make([]StateComponentOutput, len(s.Components))
for i, comp := range s.Components {
compStates[i] = StateComponentOutput{
ID: comp.Component.ID,
State: comp.State,
}
}
return s, compStates
}
s, compStates := getState(c)

// c.State() may return a stale state (up to 1 version behind)
// due to potential delays in state updates.
// If the state is HEALTH but `version_info` is missing, it
// indicates staleness.
// In this case, the state is re-fetched to obtain the latest
// information. After the second fetch, no more checks are
// performed and the state is returned as is.
for _, cs := range compStates {
if cs.State.State == client.UnitStateStarting ||
(cs.State.VersionInfo.BuildHash == "" ||
cs.State.VersionInfo.Meta["commit"] == "") {
s, compStates = getState(c)
break
}
}

output := StateHookOutput{
State: s.State,
Message: s.Message,
Expand Down
19 changes: 13 additions & 6 deletions internal/pkg/agent/application/coordinator/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ components:
// the state diagnostic reports it.
func TestDiagnosticState(t *testing.T) {
now := time.Now().UTC()
state := State{
state1 := State{
State: agentclient.Starting,
Message: "starting up",
FleetState: agentclient.Configuring,
Expand All @@ -422,11 +422,12 @@ func TestDiagnosticState(t *testing.T) {
{
Component: component.Component{ID: "comp-1"},
State: runtime.ComponentState{
State: client.UnitStateDegraded,
Message: "degraded message",
State: client.UnitStateStarting,
Message: "starting message",
VersionInfo: runtime.ComponentVersionInfo{
Name: "version name",
BuildHash: "a-build-hash",
Meta: map[string]string{},
},
},
},
Expand All @@ -444,6 +445,9 @@ func TestDiagnosticState(t *testing.T) {
},
}

state2 := state1
state2.Components[0].State.VersionInfo.Meta["commit"] = "a-commit-hash"

expected := fmt.Sprintf(`
state: 0
message: "starting up"
Expand All @@ -454,14 +458,16 @@ components:
- id: "comp-1"
state:
pid: 0
state: 3
message: "degraded message"
state: 0
message: "starting message"
features_idx: 0
component_idx: 0
units: {}
version_info:
name: "version name"
build_hash: "a-build-hash"
meta:
commit: "a-commit-hash"
upgrade_details:
target_version: 8.12.0
state: UPG_DOWNLOADING
Expand All @@ -476,9 +482,10 @@ upgrade_details:
coord := &Coordinator{
// This test needs a broadcaster since the components-actual diagnostic
// fetches the state via State().
stateBroadcaster: broadcaster.New(state, 0, 0),
stateBroadcaster: broadcaster.New(state1, 0, 0),
}

coord.stateBroadcaster.InputChan <- state2
hook, ok := diagnosticHooksMap(coord)["state"]
require.True(t, ok, "diagnostic hooks should have an entry for state")

Expand Down

0 comments on commit db1be75

Please sign in to comment.