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

[Elastic-Agent] Make monitoring settings configurable by fleet #17855

Merged
merged 8 commits into from
Apr 22, 2020

Conversation

michalpristas
Copy link
Contributor

@michalpristas michalpristas commented Apr 21, 2020

What does this PR do?

This PR performs (a bit more complicated to review) refactor where it goes from monitor per app to central monitor as initial idea of having option for each app to be monitorable or not is not planned. Now we have central setting saying monitor or not.

This central monitoring component holds information about whether monitor or not or which part do we need to monitor (logs/metrics).
On config change state of this component is updated first and then applications will pick them up and act accordingly.

Reload is also happening when config is coming from fleet. Up until now it as static setting on load which operator was fed at the start time.

Why is it important?

To enable setting this config from fleet and change the value during runtime.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

By default monitoring beats are feeding metricbeat-{version}-{date} or filebeat-{version}-{date} indices. When testing in standalone (using default credentials) this works correctly with fleet we lack permissions on these and publishing fails:

reason":"action [indices:admin/create] is unauthorized for API key id [tPA_mHEBM0-54aIPYncP] of user [fleet_enroll]"

^^ cc @nchaulet

I tested few scenarios:

Test 1:

Standalone, monitoring enabled
Result: starts FB and MB, indices created and with data

Test 2:

Standalone, monitoring disabled
Result: monitoring not started, indices not created

Test 3

Standalone, monitoring disabled to enabled (reload test)
Result: initially nothing started, after config change starts FB and MB, indices created and with data

Test 4:

Fleet, monitoring disabled
Result: monitoring not started, indices not created

Test 5:

Fleet, monitoring enabled
Result: starts FB and MB, indices NOT created
FAILS on permissions

edit:
after modifying index changes are

Test 5:

Fleet, monitoring enabled
Result: starts FB and MB, indices created with data

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest-management (Project:fleet)

@ph ph self-requested a review April 21, 2020 13:08
@ph ph changed the title [Agent] Make monitoring settings configurable by fleet [Elastic-Agent] Make monitoring settings configurable by fleet Apr 21, 2020
@ph ph added the needs_backport PR is waiting to be backported to other branches. label Apr 21, 2020
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM, I've looked through the code and the changes LGTM, One thing missing is we need to be able to unit tests theses scenarios. We can do it in another PR.

@michalpristas michalpristas merged commit b6b8d74 into elastic:master Apr 22, 2020
michalpristas added a commit to michalpristas/beats that referenced this pull request Apr 22, 2020
…ic#17855)

[Elastic-Agent] Make monitoring settings configurable by fleet (elastic#17855)
ph pushed a commit that referenced this pull request Apr 22, 2020
… (#17891)

[Elastic-Agent] Make monitoring settings configurable by fleet (#17855)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs_backport PR is waiting to be backported to other branches. review [zube]: In Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants