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

Get error from kubernetes response #51

Merged
merged 1 commit into from
Nov 21, 2019
Merged

Get error from kubernetes response #51

merged 1 commit into from
Nov 21, 2019

Conversation

aliksend
Copy link
Contributor

@aliksend aliksend commented Nov 1, 2019

Expected behavior

Fail step when exit code != 0

Actual behavior

Step marked as OK even if exit code != 0

Solution

To check error, returned by kubernetes

Error checking code stealed from latest version of kubernetes-client/python-base:
https://github.com/kubernetes-client/python-base/blob/6b6546131217a2a9fdcf431a286c346619d2923a/stream/ws_client.py#L215

I'm not a python developer so code can be not perfect
May be it will be useful to add code like this in other connect_get_namespaced_pod_exec calls (like in run_command) but I not sure how to do it correctly

@aliksend
Copy link
Contributor Author

aliksend commented Nov 4, 2019

@ltamaster What do you think about it?

@ltamaster ltamaster self-requested a review November 6, 2019 11:51
@ltamaster
Copy link
Contributor

Hi @aliksend

Sorry, I am busy this week, I will review it next week

Thanks
Luis

@aliksend
Copy link
Contributor Author

aliksend commented Nov 6, 2019

@ltamaster Thank you for response.

Look when it will be convenient for you
Thanks

@aliksend
Copy link
Contributor Author

@ltamaster Will you be able to look at the weekend?

Copy link
Contributor

@ltamaster ltamaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it works great!!!!

@ltamaster ltamaster merged commit 19bf9d1 into rundeck-plugins:master Nov 21, 2019
@aliksend
Copy link
Contributor Author

Please, can you make new release?

@ltamaster
Copy link
Contributor

done

@aliksend
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants