Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cmd/opampsupervisor] fix: Clear config timeout timer & send APPLIED remote config status when running nop config #36733

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

dpaasman00
Copy link
Contributor

Description

When sending a nop config to the supervisor, it does not start the collector. However there is a config timeout that would report an error back to the OpAmp server in this case, despite the agent technically being healthy.

This change clears the config timeout when we aren't starting the agent due to a nop config. It also reports an 'Applied' remote config status, since to the OpAmp server it should appear as if it the collector is successfully running the config.

Link to tracking issue

Fixes #36682

Testing

Updated an e2e test to apply an empty config and verify an 'Applied' status is returned by the supervisor.

@dpaasman00 dpaasman00 force-pushed the fix/supervisor-reports-error-nop-config branch 3 times, most recently from fb6dba4 to 72cc91c Compare December 10, 2024 12:16
@dpaasman00 dpaasman00 marked this pull request as ready for review December 10, 2024 14:13
@dpaasman00 dpaasman00 requested a review from a team as a code owner December 10, 2024 14:13
component: opampsupervisor

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Supervisor clears config timeout and reports 'Applied' remote config status when a nop config is received.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
note: Supervisor clears config timeout and reports 'Applied' remote config status when a nop config is received.
note: Report an 'Applied' remote config status when an empty config is received.

I think the timeout is largely an internal detail. Also, here a no-op config constitutes sending an empty config to the Supervisor, right? As opposed to sending a specific config from the server that is also a no-op.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, the timeout is internal so I'll remove the reference. And yes, this is in regards to an empty config file.

@dpaasman00 dpaasman00 force-pushed the fix/supervisor-reports-error-nop-config branch from 9d07203 to 5c5e255 Compare December 18, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cmd/opampsupervisor] Supervisor reports a failed OpAmp status when it receives a remote nop config
3 participants