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

Fixes #12437, refs #12288 - better handling of unexpected states #73

Merged
merged 2 commits into from Nov 12, 2015

Conversation

iNecas
Copy link
Member

@iNecas iNecas commented Nov 11, 2015

Don't raise cryptic errors when there is no reasonable output comming from the
proxy.

Also, improve the error message a proxy in missing (and proxy command fails in planning
phase). There is still work to be done to get to the erors from the job hosts list,
but this requires some changes on dynflow side that I don't want to rush to get
into stable relese. But at least, when going to the Monitor -> Tasks and showing
the failed RuhHostJob task, one doesn't see:

undefined method `live_output' for nil:NilClass (NoMethodError)

@iNecas
Copy link
Member Author

iNecas commented Nov 11, 2015

I put the two commits into one PR, as there would be conflicts when merging otherwise

@stbenjam
Copy link
Member

@iNecas There seems to be some regression here, if I try to run a command where /tmp/foreman-proxy is unreadable by the ssh user, the task returns successful instead of failing

@iNecas
Copy link
Member Author

iNecas commented Nov 12, 2015

This is not a regression introduced by this changes IMO, but I see issues with handling encoding on proxy side, that is causing that… I will open a PR against smart_proxy_remote_execution_ssh

@stbenjam
Copy link
Member

Ah ok, this PR does work and handles the case with no output so ACK

@iNecas
Copy link
Member Author

iNecas commented Nov 12, 2015

Here is a fix for the issue you've discovered theforeman/smart_proxy_remote_execution_ssh#15 (description at http://projects.theforeman.org/issues/12456), I think, It can also be related to http://projects.theforeman.org/issues/12437

iNecas added a commit that referenced this pull request Nov 12, 2015
Fixes #12437, refs #12288 - better handling of unexpected states
@iNecas iNecas merged commit 2ae6bb6 into theforeman:master Nov 12, 2015
@iNecas
Copy link
Member Author

iNecas commented Nov 12, 2015

Thanks @stbenjam

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