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

Template generation maintenance + remove number of routing shards from default template #16038

Merged
merged 3 commits into from
Feb 5, 2020
Merged

Template generation maintenance + remove number of routing shards from default template #16038

merged 3 commits into from
Feb 5, 2020

Conversation

urso
Copy link

@urso urso commented Feb 3, 2020 •
edited
Loading

  • Bug

What does this PR do?

  • Remove number_of_routing_shards from default template if the setting is not present and if we target Elasticsearch >= 7.0.
  • Update template generation unit tests adding some more test cases, and updating cases to use the the current Beat version as well.
  • introduce testTemplate and other helpers for writing more concise tests.

Why is it important?

  • The number_of_routing_shards setting has become optional with Elasticsearch 7.0 and should not be set by default anymore.
  • Some of our tests only did check for one particular ES version. The template output has been slightly changed for 7.0 to deal with types removal. The tests haven't been adapted in the past.
  • Helpers hopefully make it easier to see what is tested and encourage devs to add more tests in the future.

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

Author's Checklist

  • [ ]

How to test this PR locally

  • Export template and check number_of_routing_shards setting is missing by default from the template.

  • run Beats with number of routing shards 30 is not divisible by.

Related issues

Use cases

Screenshots

Logs

@urso urso requested a review from a team February 3, 2020 18:24
@urso urso requested review from a team as code owners February 3, 2020 18:24
@urso urso mentioned this pull request Feb 3, 2020
Closed
@urso urso added needs_backport PR is waiting to be backported to other branches. v7.7.0 labels Feb 4, 2020
@urso urso merged commit 9d99b89 into elastic:master Feb 5, 2020
@urso urso deleted the template-no-routing-shards branch February 5, 2020 16:56
@urso urso mentioned this pull request Feb 5, 2020
Merged
5 tasks
@urso urso removed the needs_backport PR is waiting to be backported to other branches. label Feb 5, 2020
urso pushed a commit to urso/beats that referenced this pull request Feb 5, 2020
Template generation maintenance + remove number of routing sha… (elas…
9c94fc7 
…tic#16038)

* remove number of routing shards from default template
* Add more template version tests

(cherry picked from commit 9d99b89)
urso pushed a commit that referenced this pull request Feb 6, 2020
Cherry-pick #16038 to 7.x: Template generation maintenance + r… (#16109)
af01fb7 
* Template generation maintenance + remove number of routing sha… (#16038)

* remove number of routing shards from default template
* Add more template version tests

(cherry picked from commit 9d99b89)
@andresrc andresrc added the Team:Integrations Label for the Integrations team label Mar 6, 2020
@urso urso mentioned this pull request Mar 23, 2020
Closed
@urso urso added the test-plan Add this PR to be manual test plan label Mar 27, 2020
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvch kvch kvch approved these changes

Assignees
No one assigned
Labels
bug libbeat review Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[metricbeat][libbeat] template setup fails with number_of_shards=4
3 participants
@urso @kvch @andresrc