Make rollover init step idempotent #1964
Conversation
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
pavolloffay
requested review from
black-adder,
jpkrohling,
objectiser,
tiffon,
vprithvi and
yurishkuro
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1964 +/- ##
=======================================
Coverage 96.99% 96.99%
=======================================
Files 203 203
Lines 10061 10061
=======================================
Hits 9759 9759
Misses 264 264
Partials 38 38Continue to review full report at Codecov.
|
plugin/storage/es/esRollover.py
Outdated
| @@ -69,6 +69,10 @@ def main(): | |||
|
|
|||
| def perform_action(action, client, write_alias, read_alias, index_to_rollover, template_name): | |||
| if action == 'init': | |||
| if not is_alias_empty(client, read_alias) or not is_alias_empty(client, write_alias): | |||
| print("Alias {} or {} is not empty aborting init step".format(read_alias, write_alias)) | |||
There was a problem hiding this comment.
Shouldn't instead the creation of the index be made conditional? If this script fails after creating a read alias but before write alias there's no way to fix the situation by rerunning it.
There was a problem hiding this comment.
+1. I think that was the initial goal. I have reverted this and fixed catching the exception in the create index func
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
plugin/storage/es/esRollover.py
Outdated
| @@ -112,8 +112,10 @@ def create_index(client, name): | |||
| try: | |||
| create.do_action() | |||
| except curator.exceptions.FailedExecution as e: | |||
| if "index_already_exists_exception" not in str(e) and "resource_already_exists_exception" not in str(e): | |||
| if ("index_already_exists_exception" or "resource_already_exists_exception") in str(e): | |||
There was a problem hiding this comment.
Doesn't OR of two strings simply return the first one?
There was a problem hiding this comment.
ohh this would not work. My python skills...
Actually str(e) never returns this string. There is a parameter ignore_existing=True I would use that instead. If the index exists it logs it.
Index jaeger-span-000001 already exists.
plugin/storage/es/esRollover.py
Outdated
| raise e | ||
| else: | ||
| print("Index {} already exists".format(name)) |
There was a problem hiding this comment.
Aside from a log entry, how is it functionally different from before, when the exception was simply ignored?
There was a problem hiding this comment.
This just logs that the index was not created because it already exists
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
|
@yurishkuro could you please re-review? |
Resolves jaegertracing/jaeger-operator#757