[bug][storage] Fixed the ES-Rollover Idempotency

[Manik2708]

Which problem is this PR solving?

Fixes: #6203

Description of the changes

• Currently es-rollover checks for index existence through errors, it is mainly expecting the error:

{"error":{"root_cause":[{"type":"resource_already_exists_exception","reason":"]"}],"type":"resource_already_exists_exception","reason":"request [/jaeger-*] contains unrecognized parameter: [help]"},"status":400}

But it can lead to inconsistent results as found in the issue, where init was failing due to the error:

Error: failed to create index: jaeger-span-000001, request failed, status code: 400, body: {"error":{"root_cause":[{"type":"invalid_index_name_exception","reason":"Invalid index name [jaeger-span-000001], already exists as alias","index_uuid":"_na_","index":"jaeger-span-000001"}],"type":"invalid_index_name_exception","reason":"Invalid index name [jaeger-span-000001], already exists as alias","index_uuid":"_na_","index":"jaeger-span-000001"},"status":400}

Here if we see carefully the error is coming due to existence of index but the reason is different. es-rollover is ready only for resource_already_exists_exception but there are other errors also which can be generated due to this (like the above).

The current way of marshalling error is unsafe, the safe way is: Check if index exists -> Create if not exists

How was this change tested?

• Unit and E2E Tests

Checklist

• I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
• I have signed all commits
• I have added unit tests for the new functionality
• I have run lint and test steps successfully
  • for jaeger: make lint test
  • for jaeger-ui: npm run lint and npm run test

[codecov]

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.93%. Comparing base (6d8c70e) to head (4415f6e).

Files with missing lines	Patch %	Lines
pkg/es/client/index_client.go	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6638      +/-   ##
==========================================
- Coverage   95.94%   95.93%   -0.02%     
==========================================
  Files         365      365              
  Lines       20609    20615       +6     
==========================================
+ Hits        19774    19776       +2     
- Misses        636      639       +3     
- Partials      199      200       +1     

Flag	Coverage Δ
badger_v1	9.91% <ø> (ø)
badger_v2	1.84% <ø> (ø)
cassandra-4.x-v1-manual	14.92% <ø> (ø)
cassandra-4.x-v2-auto	1.83% <ø> (ø)
cassandra-4.x-v2-manual	1.83% <ø> (ø)
cassandra-5.x-v1-manual	14.92% <ø> (ø)
cassandra-5.x-v2-auto	1.83% <ø> (ø)
cassandra-5.x-v2-manual	1.83% <ø> (ø)
elasticsearch-6.x-v1	19.30% <ø> (ø)
elasticsearch-7.x-v1	19.38% <ø> (ø)
elasticsearch-8.x-v1	19.55% <ø> (ø)
elasticsearch-8.x-v2	1.84% <ø> (ø)
grpc_v1	10.89% <ø> (ø)
grpc_v2	7.88% <ø> (ø)
kafka-3.x-v1	10.21% <ø> (ø)
kafka-3.x-v2	1.84% <ø> (ø)
memory_v2	1.84% <ø> (ø)
opensearch-1.x-v1	19.43% <ø> (ø)
opensearch-2.x-v1	19.43% <ø> (ø)
opensearch-2.x-v2	1.84% <ø> (ø)
tailsampling-processor	0.48% <ø> (ø)
unittests	94.81% <95.45%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Manik2708]

@yurishkuro I think, even this approach can lead to the error described in the issue. An index might not exist but still pointed by an alias. Should we ignore that error in init (as that means rollover has taken place already)? I intially thought that index existence could fix this but I may be wrong. So I want to take your views over ignoring this error in init.