Fixes #32203 - Add timeout option to ansible runner #41
Conversation
|
The timeout is respected, run finishes with 0 and is marked as success:
For ssh jobs, timeout is a failure. Any chance we could make it a failure as well when we hit the timeout? |
| @@ -19,6 +20,7 @@ def initialize(input, suspended_action:) | |||
| @check_mode = action_input[:check_mode] | |||
| @tags = action_input[:tags] | |||
| @tags_flag = action_input[:tags_flag] | |||
| @job_invocation_timeout = action_input[:job_invocation_timeout] | |||
There was a problem hiding this comment.
why do we use different naming than ScriptRunner, which use execution_timeout_interval?
I'd love if we could keep the naming matching for the same purpose, or is there a reason for different name?
There was a problem hiding this comment.
@ezr-ondrej From what I understood the execution_timeout_interval is from the template invocation but if the value is nil in the template it does not mean that in the specific job invocation the timeout is nil as well. so I created another variable for the job_invocation timeout.
There was a problem hiding this comment.
well ... if I'm reading the theforeman/foreman_ansible#422 correctly, you're getting it from job_invocation anyway, I don't think template_invocation has timeout. All the user inputs are on job_invocation, so that would be weird.
There was a problem hiding this comment.
@ezr-ondrej you are right there is no need to add another parameter. I will fix this.
|
Needs a rebase now. |
No I haven't fixed it yet, just uploaded another update. sorry for the confusion |
@xprazak2 This should work now. |
| hosts = @inventory['all']['hosts'] | ||
| hosts.each do |hostname| | ||
| @exit_statuses[hostname] = 2 | ||
| publish_data_for(hostname, 'Timeout for execution passed, stopping the job', 'stdout') |
There was a problem hiding this comment.
Would this message work as broadcasted by broadcast_data?
There was a problem hiding this comment.
Yes definitely, why is it better to use the broadcast_data?
| hosts.each do |hostname| | ||
| @exit_statuses[hostname] = 2 | ||
| end | ||
| broadcast_data('Timeout for execution passed, stopping the job', 'stdout') |
There was a problem hiding this comment.
Maybe stderr would be better? After all it is more of an error condition.
| hosts = @inventory['all']['hosts'] | ||
| hosts.each do |hostname| | ||
| @exit_statuses[hostname] = 2 | ||
| end |
There was a problem hiding this comment.
This could probably go on a single line.
|
Works as expected. I see an error in proxy logs when terminating on timeout, could it cause problems? |
I think it is related to this code: https://github.com/theforeman/smart_proxy_ansible/pull/41/files#:~:text=FileUtils.remove_entry(%40root)%20if%20%40tmp_working_dir but I am not sure why this is happening. @ezr-ondrej did you saw this error too? |
Is it possible, than if the |
I checked the timeout does not work without the calling the close method. but this line is necessary: It should work now with no errors. |
|
I no longer see the error, thanks! |
No description provided.