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

Implement basic version negotiation with Fleet Server #3383

Merged
merged 8 commits into from
Sep 12, 2023

Conversation

pchila
Copy link
Member

@pchila pchila commented Sep 8, 2023

What does this PR do?

We add an Elastic-Api-Version header to all requests to Fleet.
We already have support for detecting and logging warnings and downgrade requests (those are not yet implemented though)

The Warning header coming from Fleet is logged at fleet client level and propagated to coordinator via the specific error type WarningError. When the coordinator receives a WarningError will set fleet status to Degraded (as opposed to Failed that was used for any error up to this point) and set fleet status message to the warning it received.

Why is it important?

We aim for Elastic Agent to support connecting to multiple versions of Fleet server supporting multiple api versions.

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/fragments using the changelog tool
  • [ ] I have added an integration test or an E2E test

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Questions to ask yourself

  • How are we going to support this in production?
  • How are we going to measure its adoption?
  • How are we going to debug this?
  • What are the metrics I should take care of?
  • ...

@pchila pchila added enhancement New feature or request Team:Elastic-Agent Label for the Agent team labels Sep 8, 2023
@pchila pchila self-assigned this Sep 8, 2023
@mergify
Copy link
Contributor

mergify bot commented Sep 8, 2023

This pull request does not have a backport label. Could you fix it @pchila? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip label Sep 8, 2023
@elasticmachine
Copy link
Contributor

elasticmachine commented Sep 8, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-09-11T14:54:02.079+0000

  • Duration: 28 min 25 sec

Test stats 🧪

Test Results
Failed 0
Passed 6301
Skipped 59
Total 6360

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Contributor

elasticmachine commented Sep 8, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 98.78% (81/82) 👍
Files 66.327% (195/294) 👍 0.115
Classes 65.751% (359/546) 👍 0.189
Methods 52.784% (1128/2137) 👍 0.158
Lines 38.287% (12815/33471) 👍 0.044
Conditionals 100.0% (0/0) 💚

@pchila pchila force-pushed the negotiate-fleet-protocol-version branch from 2a5a605 to 2f06a5c Compare September 11, 2023 09:58
@pchila pchila force-pushed the negotiate-fleet-protocol-version branch from 5655e06 to fb2d157 Compare September 11, 2023 14:53
@pchila pchila marked this pull request as ready for review September 11, 2023 14:54
@pchila pchila requested a review from a team as a code owner September 11, 2023 14:54
@pchila pchila requested review from ycombinator and faec September 11, 2023 14:54
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

@elastic-sonarqube
Copy link

@pchila pchila requested review from joshdover and cmacknz September 12, 2023 11:34
@joshdover
Copy link
Contributor

@pchila Ideally we could backport this to the next 8.10.x patch. Any concerns with that?

@pchila
Copy link
Member Author

pchila commented Sep 12, 2023

@joshdover It shouldn't be too much of a risk since we only add a header, a couple of logs and an additional state to the fleet status in case of warning: nothing that should impact the core functionality of elastic-agent.
Just for completeness: should we backport #3394 as well ?

@joshdover
Copy link
Contributor

Just for completeness: should we backport #3394 as well ?

See my comment here: #3362 (comment)

I actually think we might want to revert this as this scenario is not supported (users deploying their own Fleet Server against Serverless projects)

@pchila pchila added the backport-v8.10.0 Automated backport with mergify label Sep 12, 2023
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

@pchila pchila merged commit fe0d427 into elastic:main Sep 12, 2023
@pchila pchila deleted the negotiate-fleet-protocol-version branch September 12, 2023 18:31
mergify bot pushed a commit that referenced this pull request Sep 12, 2023
* Initial implementation of elastic api version in roundtrip and fleet client
* signal Fleet warning on elastic-agent status

(cherry picked from commit fe0d427)
pchila added a commit that referenced this pull request Sep 14, 2023
* Initial implementation of elastic api version in roundtrip and fleet client
* signal Fleet warning on elastic-agent status

(cherry picked from commit fe0d427)
pchila added a commit that referenced this pull request Sep 25, 2023
* Initial implementation of elastic api version in roundtrip and fleet client
* signal Fleet warning on elastic-agent status

(cherry picked from commit fe0d427)

Co-authored-by: Paolo Chilà <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.10.0 Automated backport with mergify enhancement New feature or request skip-changelog Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants