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

Fix agent download timeout values in yaml files #29039

Merged
merged 1 commit into from
Nov 25, 2021
Merged

Fix agent download timeout values in yaml files #29039

merged 1 commit into from
Nov 25, 2021

Conversation

michel-laterman
Copy link
Contributor

What does this PR do?

Change the agent.download.timeout value to 120s to match the code

beats/x-pack/elastic-agent/pkg/artifact/config.go

Lines 47 to 48 in 5ae799c

// binaries are a getting bit larger it might take >30s to download them
transport.Timeout = 120 * time.Second

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

@michel-laterman michel-laterman added docs Team:Elastic-Agent Label for the Agent team backport-v8.0.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Nov 18, 2021
@elasticmachine
Copy link
Collaborator

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

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 18, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 18, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 18, 2021 •
edited by jenkins-beats-ci bot
Loading

💚 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: 2021-11-23T17:52:55.175+0000

  • Duration: 93 min 45 sec

  • Commit: 7de9b79

Test stats 🧪

Test Results
Failed 0
Passed 7128
Skipped 16
Total 7144

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@michel-laterman
Copy link
Contributor Author

/package

@michel-laterman
Copy link
Contributor Author

/test

@nimarezainia
Copy link
Contributor

@michel-laterman thanks for taking this on. I am wondering how the 120s was arrived at?
I don't want to make this complicated but seems to me that this should be a function of the network speed/bandwidth and any number of seconds we pick for time out could be arbitrary. As in 120s may not even be enough of a time out in some scenarios. Also agents (as part of the same fleet) get installed in different environments.

Do we have any other counters at our disposal? like bytes transferred or something like that to allow us to determine the timeout more dynamically? again I don;t want to unnecessarily complicate things. What would happen if the timeout was 10 mins?

@ruflin @michalpristas your thoughts?

@ruflin
Copy link
Contributor

ruflin commented Nov 23, 2021

This here is only a fix of the wrong value shown as it is already 120s. But I agree with @nimarezainia we should have a deeper discussion on what we expect when downloading binaries. What is the timeout? What are the retries? What happens on slow / broken downloads? Etc.

@nimarezainia Could we take this a separate issue?

ruflin
ruflin approved these changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Change LGTM but seem CI is not happy?

@michel-laterman
Copy link
Contributor Author

/package

@michel-laterman
Copy link
Contributor Author

/test

@michel-laterman
Copy link
Contributor Author

/package

@michel-laterman michel-laterman added the backport-v7.15.0 Automated backport with mergify label Nov 25, 2021
@michel-laterman michel-laterman merged commit 692ee17 into elastic:master Nov 25, 2021
@michel-laterman michel-laterman deleted the fix-download-ref branch November 25, 2021 23:35
mergify bot pushed a commit that referenced this pull request Nov 25, 2021
@michel-laterman
Fix agent download timeout values in yaml files (#29039)
9c0f0e3 
(cherry picked from commit 692ee17)
@mergify mergify bot mentioned this pull request Nov 25, 2021
Merged
mergify bot pushed a commit that referenced this pull request Nov 25, 2021
@michel-laterman
Fix agent download timeout values in yaml files (#29039)
f8cf5c9 
(cherry picked from commit 692ee17)
@mergify mergify bot mentioned this pull request Nov 25, 2021
Merged
mergify bot pushed a commit that referenced this pull request Nov 25, 2021
@michel-laterman
Fix agent download timeout values in yaml files (#29039)
7979e36 
(cherry picked from commit 692ee17)
@mergify mergify bot mentioned this pull request Nov 25, 2021
Merged
michel-laterman added a commit that referenced this pull request Nov 29, 2021
@mergify @michel-laterman
Fix agent download timeout values in yaml files (#29039) (#29160)
fb63c15 
(cherry picked from commit 692ee17)

Co-authored-by: Michel Laterman <[email protected]>
michel-laterman added a commit that referenced this pull request Nov 29, 2021
@mergify @michel-laterman
Fix agent download timeout values in yaml files (#29039) (#29158)
1965e56 
(cherry picked from commit 692ee17)

Co-authored-by: Michel Laterman <[email protected]>
michel-laterman added a commit that referenced this pull request Nov 29, 2021
@mergify @michel-laterman
Fix agent download timeout values in yaml files (#29039) (#29159)
0b67440 
(cherry picked from commit 692ee17)

Co-authored-by: Michel Laterman <[email protected]>
v1v added a commit to v1v/beats that referenced this pull request Nov 30, 2021
@v1v
Merge remote-tracking branch 'upstream/master' into feature/use-debian-9
53d5763 
* upstream/master: (577 commits)
  Remove Journalbeat (elastic#29131)
  Add note that there is no warranty or support for generator code (elastic#28797)
  packetbeat: preparation for npcap addition (elastic#29017)
  Use the generic helper for opening file to read in filestream (elastic#29180)
  Workflow for macos (elastic#29174)
  Fix `decode_json_fields` processor to always add error key (elastic#29107)
  Workflow for macos (elastic#29156)
  Fix agent download timeout values in yaml files (elastic#29039)
  Added workflow file for builds with macos (elastic#29148)
  CI: enable AWS cloud testing on a PR basis if changes in certain files (elastic#29047)
  Remove links to Journalbeat (elastic#29134)
  Fix rds metadata in cloudwatch metricset (elastic#29106)
  [mergify]: notify conflicts in PRs that are still open (elastic#29122)
  Use NamedWatcher in Agent's k8s provider (elastic#29095)
  override host on statsd metricset (elastic#29103)
  Skip config check in autodiscover for duplicated configurations (elastic#29048)
  Change "filebeat.config.modules.enabled" to "true" (elastic#28769)
  Remove deprecated spool queue from Beats (elastic#28869)
  Add `beat` field back to beat.stats (elastic#29094)
  Revert "Move labels and annotations under kubernetes.namespace. (elastic#27917)" (elastic#29069)
  ...
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@mergify @michel-laterman
Fix agent download timeout values in yaml files (elastic#29039) (elas…
624f11c 
…tic#29158)

(cherry picked from commit e749fea)

Co-authored-by: Michel Laterman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

Assignees
No one assigned
Labels
backport-v7.15.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify docs Team:Elastic-Agent Label for the Agent team Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

incorrect default value of agent.download.timeout in Elastic Agent reference yaml
4 participants
@michel-laterman @elasticmachine @nimarezainia @ruflin