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

Ilm disable template #3610

Merged
merged 3 commits into from
Apr 1, 2022
Merged

Conversation

rbizos
Copy link
Contributor

@rbizos rbizos commented Apr 1, 2022

clone of #3599 from another source branch to fix CI

Problem

With ILM, index patter overridden without lifecycle policy set by collector if not explicitly disabled with --es.create-index-templates=false
Proposal

Making useILM disable index creation to prevent this issue to happen

rbizos added 3 commits April 1, 2022 11:25
The index template should be created by es-rollover. Without this, it
was mandatory to use --es.create-index-templates=false with
--es.use-ilm, otherwise it would override the index template with an
empty lifecycle.name thus preventing the rollover to work

Signed-off-by: Raphael Bizos <r.bizos@criteo.com>
Signed-off-by: Raphael Bizos <r.bizos@criteo.com>
Signed-off-by: Raphael Bizos <r.bizos@criteo.com>
@rbizos rbizos requested a review from a team as a code owner April 1, 2022 14:33
@rbizos rbizos requested a review from yurishkuro April 1, 2022 14:34
@codecov
Copy link

codecov bot commented Apr 1, 2022

Codecov Report

Merging #3610 (b63574a) into main (2a66711) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #3610   +/-   ##
=======================================
  Coverage   96.54%   96.54%           
=======================================
  Files         264      264           
  Lines       15447    15449    +2     
=======================================
+ Hits        14914    14916    +2     
  Misses        449      449           
  Partials       84       84           
Impacted Files Coverage Δ
plugin/storage/es/factory.go 97.61% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a66711...b63574a. Read the comment docs.

@albertteoh albertteoh merged commit bb042cc into jaegertracing:main Apr 1, 2022
@albertteoh
Copy link
Contributor

Thanks!

@rbizos
Copy link
Contributor Author

rbizos commented Apr 4, 2022

My pleasure, thanks for the review

albertteoh pushed a commit to albertteoh/jaeger that referenced this pull request Jul 13, 2022
* useILM will prevent index template creation

The index template should be created by es-rollover. Without this, it
was mandatory to use --es.create-index-templates=false with
--es.use-ilm, otherwise it would override the index template with an
empty lifecycle.name thus preventing the rollover to work

Signed-off-by: Raphael Bizos <r.bizos@criteo.com>

* fixing ILM test

Signed-off-by: Raphael Bizos <r.bizos@criteo.com>

* Adding a comment for ILM and index creation

Signed-off-by: Raphael Bizos <r.bizos@criteo.com>
Signed-off-by: Albert Teoh <see.kwang.teoh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants