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

scylla_post_start.py and scylla_configure.py should not exit with 0 when HTTP Error occurs #505

Closed
syuu1228 opened this issue Feb 5, 2024 · 2 comments · Fixed by #511
Closed
Assignees

Comments

@syuu1228
Copy link
Contributor

syuu1228 commented Feb 5, 2024

Related with #498

On current curl() implementation, all exceptions are catched to retrying access and never re-raised.
Because of this, we currently not able to get any HTTP Error related exception.
We should re-raise the exception after all retries are failed.

Or, maybe we don't need to retry for all HTTP Error, since the metadata server may returns specific error code when it's not available.

@syuu1228
Copy link
Contributor Author

I was wrong, curl() is actually re-raise exception when HTTP Error received.
However, scylla_post_start.py and scylla_configure.py are catching exceptions and log it as ERROR but it will continue program and always exit with 0.
I think it should at least exit with 1 if Exception catched, or even stop catching Exception and cause Traceback.

@syuu1228 syuu1228 changed the title curl() should raise the exception when getting HTTP Error scylla_post_start.py and scylla_configure.py should not exit with 0 when HTTP Error occurs Mar 16, 2024
@syuu1228
Copy link
Contributor Author

BTW, curl() also has problem.
It unconditionally retries HTTP request when error occurs, even HTTP error was 404 Not Found - it unlikely fix by retrying.
I think we should stop retrying when HTTP Error was 400-499 (Client error responses, like 404 Not found or 403 Forbidden).

syuu1228 added a commit to syuu1228/scylla-machine-image that referenced this issue Mar 16, 2024
On scylladb#498, status of scylla-image-post-start.service is NOT "failed" even
the script causing error, because scylla_post_start.py catches all
exceptions and just logging it, so the script finishes without error.

To getting notify users the script failed operation during running, we
should sys.exit(1) when the exception catched.

fixes scylladb#505
syuu1228 added a commit to syuu1228/scylla-machine-image that referenced this issue Mar 16, 2024
When curl catched HTTP error 4xx = client error such as "404 Not Found"
or "403 Forbidden", it likely our script bug not server side issue, we
should re-raise exception immediately instead of retrying.

related scylladb#505
syuu1228 added a commit to syuu1228/scylla-machine-image that referenced this issue May 22, 2024
On scylladb#498, status of scylla-image-post-start.service is NOT "failed" even
the script causing error, because scylla_post_start.py catches all
exceptions and just logging it, so the script finishes without error.

To getting notify users the script failed operation during running, we
should sys.exit(1) when the exception catched.

fixes scylladb#505
syuu1228 added a commit to syuu1228/scylla-machine-image that referenced this issue May 22, 2024
When curl catched HTTP error 4xx = client error such as "404 Not Found"
or "403 Forbidden", it likely our script bug not server side issue, we
should re-raise exception immediately instead of retrying.

related scylladb#505
syuu1228 added a commit to syuu1228/scylla-machine-image that referenced this issue May 27, 2024
When curl catched HTTP error 4xx = client error such as "404 Not Found"
or "403 Forbidden", it likely our script bug not server side issue, we
should re-raise exception immediately instead of retrying.

related scylladb#505
syuu1228 added a commit to syuu1228/scylla-machine-image that referenced this issue May 27, 2024
On scylladb#498, status of scylla-image-post-start.service is NOT "failed" even
the script causing error, because scylla_post_start.py catches all
exceptions and just logging it, so the script finishes without error.

To getting notify users the script failed operation during running, we
should sys.exit(1) when the exception catched.

fixes scylladb#505

Signed-off-by: Takuya ASADA <syuu@scylladb.com>
yaronkaikov pushed a commit that referenced this issue May 29, 2024
On #498, status of scylla-image-post-start.service is NOT "failed" even
the script causing error, because scylla_post_start.py catches all
exceptions and just logging it, so the script finishes without error.

To getting notify users the script failed operation during running, we
should sys.exit(1) when the exception catched.

fixes #505
yaronkaikov pushed a commit that referenced this issue May 29, 2024
When curl catched HTTP error 4xx = client error such as "404 Not Found"
or "403 Forbidden", it likely our script bug not server side issue, we
should re-raise exception immediately instead of retrying.

related #505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant