Fix : Rally should not overwrite pre existing templates by default (#…

[Thomas-Belly]

Description

This PR fixes an issue where Rally would unconditionally overwrite existing index templates (specifically the rally-annotations template), entirely ignoring the datastore.overwrite_existing_templates setting defined by the user in rally.ini.

What changed?

I updated the logic in metrics.py (EsRaceStore) to evaluate self._index_handler.overwrite_templates before applying the template update.

Now, if the template already exists and the user has not explicitly opted to overwrite it (which defaults to False), Rally will preserve the existing template and simply log a debug message detailing the diff, honoring the intended default behavior.

Related Issue

Fixes #1900

How was this tested?

• Verified the logic locally.
• Ran make format to ensure code style compliance (Black/isort).
• Ran make lint test successfully (all unit tests passed without errors).

[b-deam]

Thanks for picking this up, much appreciated!

Sorry the surrounding code makes this trickier than it should be. The template handling has grown over time and rally-annotations is a bit of an outlier. Just a couple of things to tighten up and I think we'll be good to merge.

Firstly, the description says Rally will "log a debug message detailing the diff," but the current check silently skips the put_template when preserving, so no diff is computed or logged. The good news is that this behaviour you want already exists.

IndexHandler._should_apply_update handles all the cases (create when missing, skip when identical, keep-and-log-the-diff by default, and overwrite-with-a-warning when overwrite_existing_templates=true). _ensure_date_based_template is the closest example of feeding a single index template through it, so that's the spot to model on. I'd avoid routing this through ensure_index_template, since that one is tied to EsStoreType and annotations aren't a store type.

Take a look at what _ensure_date_based_template actually does step by step. It checks whether the template exists, fetches the existing one if it does (and treats it as absent if not), then passes the old and new versions to _should_apply_update, which decides whether to call put_template.

That middle sequence (exists, fetch, compare, conditionally put) is exactly what annotations need too. So rather than copying it, the idea is to lift those shared steps into one small helper on IndexHandler that takes the template name and body as inputs. Then _ensure_date_based_template calls the new helper with the store's template name, and annotations call it with rally-annotations and annotations_template().

That way add_annotation then just calls that path instead of doing the exists/put logic inline.

Lastly, no good deed goes unpunished, so this needs tests 😄.

A good example is test_open from #1912, which covers each branch (missing, exists but identical, differs and kept, differs and overwritten). Mirroring those four cases for annotations would be ideal.

Happy to help if you get stuck.