Honor per-unit status_reporting in agent health rollup#15192
Conversation
|
💚 CLA has been signed |
|
This pull request does not have a backport label. Could you fix it @Oddly? 🙏
|
Oddly
force-pushed
the
suppress-health-degradation
branch
3 times, most recently
from
419671d to
b47f34c
Compare
The Coordinator health rollup (generateReportableState/hasState) now reads a per-unit status_reporting config from the unit's expected config. Setting report_degraded: false and/or report_failed: false excludes that unit from the agent's aggregate health, so broad policies with inputs that are not running on every enrolled host (or that fail) no longer drag the whole agent into a degraded state. The unit's own state is still reported per component; only the aggregate rollup ignores it. Works for both process (Beat) and OpenTelemetry-translated components since both flow through the Coordinator. Ports the approach from elastic/beats#49511 into elastic-agent per maintainer guidance so it also covers non-Beats components and the OTel collector runtime.
Oddly
force-pushed
the
suppress-health-degradation
branch
from
b47f34c to
24edd75
Compare
|
/test |
This comment has been minimized.
This comment has been minimized.
TL;DR
Remediation
Investigation detailsRoot CauseThe failure occurs in The PR changes Evidence
The later Verification
Follow-upThe summary says What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
| // opt-out end to end: a unit with the matching report_* flag does not degrade | ||
| // the agent's aggregate health (while still reporting its own state), and the | ||
| // non-matching flag does not suppress. | ||
| func TestFakeComponentStatusReportingSuppressesHealth(t *testing.T) { |
There was a problem hiding this comment.
I think you need a test where only the status_reporting value changes and nothing else. Not even the message.
We need a test that shows that the health status will change if only the status_reporting value changes and nothing else. Testing locally it looks like this will fail.
There was a problem hiding this comment.
I had an LLM throw together a quick reproduction based on your existing test which I'll include below. This causes the test to time out when it fails which is not ideal.
+// configOnlyChangeHealthConfig is byte-for-byte identical to wrongFlagHealthConfig
+// except the status_reporting flags now match each unit's state, so both units
+// become suppressed. The unit "state"/"message" fields are deliberately left
+// unchanged from wrongFlagHealthConfig: the fake component will therefore not
+// report any new state/message for either unit, so this transition exercises a
+// config-only change with no accompanying unit state transition.
+var configOnlyChangeHealthConfig = `
+outputs:
+ default:
+ type: fake-output
+inputs:
+ - id: degraded
+ type: fake
+ state: 3
+ message: Degraded, wrong flag
+ status_reporting:
+ report_degraded: false
+ - id: failed
+ type: fake
+ state: 4
+ message: Failed, wrong flag
+ status_reporting:
+ report_failed: false
+`
+
// TestFakeComponentStatusReportingSuppressesHealth verifies the status_reporting
// opt-out end to end: a unit with the matching report_* flag does not degrade
// the agent's aggregate health (while still reporting its own state), and the
// non-matching flag does not suppress.
+//
+// It also verifies that flipping status_reporting alone -- with no accompanying
+// change to a unit's own reported state or message -- is picked up by the
+// agent's aggregate health without requiring an unrelated state transition to
+// "jog" the rollup into refreshing.
func TestFakeComponentStatusReportingSuppressesHealth(t *testing.T) {
define.Require(t, define.Requirements{
Group: integration.Default,
@@ -125,6 +155,30 @@ func TestFakeComponentStatusReportingSuppressesHealth(t *testing.T) {
},
},
},
+ // Config-only change: status_reporting now matches each unit's state
+ // (both should become suppressed), but the unit "state"/"message" values
+ // are unchanged from the previous step, so the fake component has no new
+ // state to report. The agent must still pick up the config change and
+ // become Healthy -- if the aggregate rollup only refreshes on a unit's own
+ // state/message changing, this step will never be reached and the test
+ // will time out waiting for it.
+ atesting.State{
+ Configure: configOnlyChangeHealthConfig,
+ AgentState: atesting.NewClientState(client.Healthy),
+ Components: map[string]atesting.ComponentState{
+ "fake-default": {
+ State: atesting.NewClientState(client.Healthy),
+ Units: map[atesting.ComponentUnitKey]atesting.ComponentUnitState{
+ atesting.ComponentUnitKey{UnitType: client.UnitTypeInput, UnitID: "fake-default-degraded"}: {
+ State: atesting.NewClientState(client.Degraded),
+ },
+ atesting.ComponentUnitKey{UnitType: client.UnitTypeInput, UnitID: "fake-default-failed"}: {
+ State: atesting.NewClientState(client.Failed),
+ },
+ },3 participants
The Coordinator health rollup (
generateReportableState/hasState) now reads a per-unitstatus_reportingconfig from the unit's expected config. Settingreport_degraded: falseand/orreport_failed: falseexcludes that unit from the agent's aggregate health, so broad policies with inputs that are not running on every enrolled host (or that fail) no longer drag the whole agent into a degraded state. The unit's own state is still reported per component; only the aggregate rollup ignores it. Works for both process (Beat) and OpenTelemetry-translated components since both flow through the Coordinator.Ports the approach from elastic/beats#49511 into elastic-agent per maintainer guidance so it also covers non-Beats components and the OTel collector runtime.
What does this PR do?
Adds an optional per-unit
status_reportingconfig that lets an input opt out of contributing its state to the agent's aggregate health:hasState(used bygenerateReportableState) skips units that opt out of the queried state. The two flags are independent and state-specific:report_failedonly affects a Failed unit,report_degradedonly a Degraded one. A component-level state with no contributing unit still counts. An absent key ortruekeeps the current behavior.Scope: covers
s.Components(process/Beat and OpenTelemetry-translated components, which carry unit config). The raw OTel collector aggregate (s.Collector, EDOT without the agent supervisor) and the Kubernetes liveness probe (which has its ownfailonopt-out) are intentionally left unchanged.Where to set it
status_reportingis read from the top level of a unit's expected config, which maps to the input level of a policy. The rollup operates per unit, so it is set once per input rather than per stream (unlike the stream-level flag in elastic/beats#49511).Standalone
elastic-agent.yml:Fleet-managed: in the integration policy editor, expand the input's Advanced options and add the block to its custom/advanced YAML field. Keys set there are merged into the input's generated config (the input unit config the rollup reads):
Why is it important?
Agent health is a worst-state-wins rollup: a single failing or not-applicable unit degrades the whole agent. On broad/shared policies where not every input runs on every host, this produces persistently degraded agents with no supported way to opt a unit out. This adds that opt-out per unit while still reporting the unit's real state per component.
Checklist
./changelog/fragmentsusing the changelog toolDisruptive User Impact
None. The config is opt-in; an absent key or
truekeeps the current behavior.How to test this PR locally
Unit tests:
Integration test (
Local, builds and runs the agent with thefakecomponent):TestFakeComponentStatusReportingSuppressesHealthintesting/integration/ess, run through the integration harness (mage integration:...). It asserts the agent stays Healthy while the units report Degraded/Failed.Standalone: run an agent with the
fakeinput in a Degraded/Failed state, with and withoutstatus_reporting.report_degraded/report_failed: false, and checkelastic-agent status. A suppressed unit keeps the agent Healthy while still showing its own state; the wrong flag (e.g.report_degradedon a Failed unit) does not suppress.Related issues