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.

Sign up for GitHub

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

Jump to bottom

[Elastic Agent] Reload fleet.kibana.hosts from policy change #21599

Merged
merged 5 commits into from
Oct 7, 2020
Merged

[Elastic Agent] Reload fleet.kibana.hosts from policy change #21599

merged 5 commits into from
Oct 7, 2020

Conversation

blakerouse
Copy link
Contributor

@blakerouse blakerouse commented Oct 6, 2020 •
edited
Loading

What does this PR do?

When a policy change is sent to the Elastic Agent it will now read the fleet.kibana.hosts values and if its different then the current running values it will re-create the Kibana client with the new hosts list and update the fleet.yml to include the new information.

Why is it important?

Allows an Agent to be updated with the new endpoints to contact back to Kibana, including multiple hosts.

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.

Related issues

@blakerouse blakerouse self-assigned this Oct 6, 2020
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 6, 2020
@blakerouse blakerouse marked this pull request as ready for review October 6, 2020 19:58
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest-management (Team:Ingest Management)

@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 6, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #21599 updated]

  • Start Time: 2020-10-07T13:57:24.424+0000

  • Duration: 36 min 8 sec

Test stats 🧪

Test Results
Failed 0
Passed 1386
Skipped 4
Total 1390

@ph ph requested a review from michalpristas October 6, 2020 20:54
michalpristas
michalpristas approved these changes Oct 7, 2020
View reviewed changes
Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

Small things to look at but it looks good

x-pack/elastic-agent/pkg/agent/application/handler_action_policy_change.go
return nil
}
// only set protocol/hosts as that is all Fleet currently sends
h.config.Fleet.Kibana.Protocol = cfg.Fleet.Kibana.Protocol
Copy link
Contributor

Choose a reason for hiding this comment

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

in case client creation fails we will end up with updated values in memory which does not correspond to currently running client, we should probably do some rollback on error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I have added in rollback of the previous values on failure.

x-pack/elastic-agent/pkg/agent/application/fleet_gateway.go
@@ -253,3 +253,7 @@ func (f *fleetGateway) stop() {
close(f.done)
f.wg.Wait()
}

func (f *fleetGateway) SetClient(client clienter) {
f.client = client
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should here and in acker make sure client is not change while performing execute/ack action.
at least it can be misleading e.g

  • performin a call using client1 - fails
  • client is updated
  • reporting failure from step 1 but client.URI is different now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I think this flow is correct, because each action from Fleet is handled synchronously in the Agent. So when a policy change comes in with an updated hosts, they action will not be Ack'd until this code is able to re-connect back to Kibana using the new hosts information.

fleetapi.NewAuthWithConfig ensures that the created client can communicate with Kibana, so that means the Ack will not happen until the updated client is created and set.

@blakerouse blakerouse merged commit 1891717 into elastic:master Oct 7, 2020
@blakerouse blakerouse deleted the agent-reload-kibana-info branch October 7, 2020 23:53
@blakerouse blakerouse mentioned this pull request Oct 7, 2020
Merged
4 tasks
blakerouse added a commit to blakerouse/beats that referenced this pull request Oct 7, 2020
@blakerouse
[Elastic Agent] Reload fleet.kibana.hosts from policy change (elastic…
2013f07 
…#21599)

* Update the connected client for kibana from policy change.

* Fix vet.

* Add changelog.

* Add protocol compare.

* Rollback protocol and hosts on failure.

(cherry picked from commit 1891717)
blakerouse added a commit to blakerouse/beats that referenced this pull request Oct 7, 2020
@blakerouse
[Elastic Agent] Reload fleet.kibana.hosts from policy change (elastic…
3c5c911 
…#21599)

* Update the connected client for kibana from policy change.

* Fix vet.

* Add changelog.

* Add protocol compare.

* Rollback protocol and hosts on failure.

(cherry picked from commit 1891717)
@blakerouse blakerouse mentioned this pull request Oct 7, 2020
Merged
4 tasks
v1v added a commit to v1v/beats that referenced this pull request Oct 8, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/2.0-skip-…
36ed51a 
…ci-build-label-support

* upstream/master: (60 commits)
  Skip publisher flaky tests (elastic#21657)
  backport: add 7.10 branch (elastic#21635)
  [CI: Packaging] fix: push ubi8 images too (elastic#21621)
  Docker build resiliance with a retry (elastic#21587)
  Fix flaky FSWatch/FSScanner tests (elastic#21625)
  chore: add versions 7.1x (elastic#21670)
  [Elastic Agent] Reload fleet.kibana.hosts from policy change (elastic#21599)
  Fix cyberark/corepas pipeline (elastic#21643)
  Add openstack ssl provider in add_cloud_metadata (elastic#21590)
  Add fips_enabled into all aws filesets (elastic#21626)
  [Filebeat S3] Change log.file.path to be nested object (elastic#21624)
  [CI] Change notification channel (elastic#21559)
  Add `add_observer_metadata` `geo.name` to Quickstart (elastic#21501)
  [E2E Tests] fix: set versions ony for PRs (elastic#21608)
  Fix badger build in 386 (elastic#21613)
  docs: Update timestamp.asciidoc (elastic#20395)
  Remove nil-zero metrics and linux-exclusive metrics from Metricbeat (elastic#21457)
  [Metricbeat] Use timestamp from CloudWatch for events (elastic#21498)
  [Filebeat][S3 Input] Add support for FIPS endpoints (elastic#21585)
  [Ingest Manager] Use new form of fleet API paths (elastic#21478)
  ...
blakerouse added a commit that referenced this pull request Oct 8, 2020
@blakerouse
[Elastic Agent] Reload fleet.kibana.hosts from policy change (#21599) (…
47b32e9 
…#21666)

* Update the connected client for kibana from policy change.

* Fix vet.

* Add changelog.

* Add protocol compare.

* Rollback protocol and hosts on failure.

(cherry picked from commit 1891717)
blakerouse added a commit that referenced this pull request Oct 8, 2020
@blakerouse
[Elastic Agent] Reload fleet.kibana.hosts from policy change (#21599) (…
c528af8 
…#21665)

* Update the connected client for kibana from policy change.

* Fix vet.

* Add changelog.

* Add protocol compare.

* Rollback protocol and hosts on failure.

(cherry picked from commit 1891717)
v1v added a commit to v1v/beats that referenced this pull request Oct 8, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/windows-2…
b3b7cb1 
…012-2.0

* upstream/master: (110 commits)
  Skip publisher flaky tests (elastic#21657)
  backport: add 7.10 branch (elastic#21635)
  [CI: Packaging] fix: push ubi8 images too (elastic#21621)
  Docker build resiliance with a retry (elastic#21587)
  Fix flaky FSWatch/FSScanner tests (elastic#21625)
  chore: add versions 7.1x (elastic#21670)
  [Elastic Agent] Reload fleet.kibana.hosts from policy change (elastic#21599)
  Fix cyberark/corepas pipeline (elastic#21643)
  Add openstack ssl provider in add_cloud_metadata (elastic#21590)
  Add fips_enabled into all aws filesets (elastic#21626)
  [Filebeat S3] Change log.file.path to be nested object (elastic#21624)
  [CI] Change notification channel (elastic#21559)
  Add `add_observer_metadata` `geo.name` to Quickstart (elastic#21501)
  [E2E Tests] fix: set versions ony for PRs (elastic#21608)
  Fix badger build in 386 (elastic#21613)
  docs: Update timestamp.asciidoc (elastic#20395)
  Remove nil-zero metrics and linux-exclusive metrics from Metricbeat (elastic#21457)
  [Metricbeat] Use timestamp from CloudWatch for events (elastic#21498)
  [Filebeat][S3 Input] Add support for FIPS endpoints (elastic#21585)
  [Ingest Manager] Use new form of fleet API paths (elastic#21478)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalpristas michalpristas michalpristas approved these changes

Assignees

@blakerouse blakerouse

Labels
v7.10.0 v7.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Elastic Agent] Allow the elastic agent to receive new Kibana host and reconnect to them.
3 participants
@blakerouse @elasticmachine @michalpristas