Update ILM message for out of order phases error

[Esduard]

Closes #70336. This new message is much more readable. One small inconvenience is that the frozen and delete phases are not metioned even though they are present in the tests. I mentioned only hot, warm and cold because they are the main page names for the "Hot-warm-cold" architecture.

[elasticmachine]

Pinging @elastic/es-core-features (Team:Core/Features)

[jakelandis]

@elasticmachine ok to test

[Esduard]

I just realized I forgot to change a test in "org.elasticsearch.xpack.ilm.IndexLifecycleRestIT.test". Will correct it soon...

[Esduard]

@cjcenizal I am having a bit of trouble making the test case for the yml file x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml line 284

To be honest i have no idea how to write yml test files. Would you help me out by giving a few hints or links?

I am trying to write the correct catch element but i am not able to do so. Right now the statement looks like this:

catch: /Your policy is configured to run the delete phase (min_age\:\ 5s) before the warm phase (min_age\:\ 10s). You should change the phase timing so that the phases will execute in the order of hot, warm then cold./

[danhermann]

@elasticmachine update branch

[danhermann]

@Esduard, you were close. That line needs to be:

catch: /Your policy is configured to run the delete phase \(min_age\:\ 5s\) before the warm phase \(min_age\:\ 10s\). You should change the phase timing so that the phases will execute in the order of hot, warm then cold./

It's a regex expression so the parentheses also need to be escaped.

[danhermann]

@elasticmachine ok to test

[cjcenizal]

This is so awesome! Thanks for working on this @Esduard!

[cjcenizal]

Minor nit: Can we add another comma after warm?

hot, warm, then cold

[Esduard]

Of course. It's a simple replace

[Esduard]

Thanks @danhermann and @cjcenizal for the feedback! I managed to update the test with the correct regex. I also put the comma at the end of the error message.

[danhermann]

@elasticmachine update branch

[Esduard]

@danhermann I'm puzzled by this new error. Is it happening because the test executes code from another branch?

[danhermann]

@danhermann I'm puzzled by this new error. Is it happening because the test executes code from another branch?

Close, it's running with some nodes that are from a previous version. You'll need to temporarily disable that test by adding:

tasks.named("yamlRestCompatTest").configure {
  systemProperty 'tests.rest.blacklist', [
    //TODO: remove once #75099 is back ported to 7.x
    'ilm/10_basic/Test increasing phase timings validated'
  ].join(',')
}

to the bottom of the x-pack/plugin/ilm/qa/rest/build.gradle file.

[danhermann]

@elasticmachine update branch

[danhermann]

@Esduard, this change looks pretty good. One thing I would suggest is a test case that has 3 or more phases with bad ages since it appears that all the current test cases have only 0, 1, or 2 phases with bad ages and there's different handling for the error message in the case of 3+.

[Esduard]

@danhermann, that is a good idea. I will add this case test with 3+ bad ages as soon as I can.

[Esduard]

@danhermann I decided to add 2 test cases: One for 3 bad phases and another for 4. That way we have test cases for every possible ammount of bad phases. Maybe Its overkill. Feel free to remove one If you like.

[danhermann]

Thanks, @Esduard! This looks good. I'll get it merged and backported to the appropriate branches.