Fixes #24548 - make SSH timeout configurable #5926
Conversation
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
2 similar comments
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
|
Issues: #24548 |
app/models/setting/provisioning.rb
Outdated
| @@ -52,6 +52,7 @@ def self.default_settings | |||
| self.set('query_local_nameservers', N_("Foreman will query the locally configured resolver instead of the SOA/NS authorities"), false, N_('Query local nameservers')), | |||
| self.set('remote_addr', N_("If Foreman is running behind Passenger or a remote load balancer, the IP should be set here. This is a regular expression, so it can support several load balancers, i.e: (10.0.0.1|127.0.0.1)"), "127.0.0.1", N_('Remote address')), | |||
| self.set('token_duration', N_("Time in minutes installation tokens should be valid for, 0 to disable token generation"), 60 * 6, N_('Token duration')), | |||
| self.set('ssh_timeout', N_("Time in seconds before SSH provisioning times out"), 60 * 6, N_('SSH timeout')), | |||
There was a problem hiding this comment.
I would add the explanation of the unit that is used here.
For example "microseconds" for better understanding the calculation, and the value to place when setting it up.
There was a problem hiding this comment.
@ik5 I was assuming users would enter a time in seconds (Timeout default), which is why I made the help text say "Time in seconds ... ". I tried to follow how token_duration does it. Is there something more that you think I should add?
There was a problem hiding this comment.
Can you actually make the default value of 2 minutes? Our old default was way to big, browser will kick you out already. Since it's configurable now users can change this?
There was a problem hiding this comment.
Sure, I'll change it to 2 minutes.
| @@ -72,7 +72,7 @@ def logger | |||
| end | |||
|
|
|||
| def initiate_connection! | |||
| Timeout.timeout(360) do | |||
| Timeout.timeout(Setting[:ssh_timeout]) do | |||
There was a problem hiding this comment.
I would add .to_i, that is:
Timeout.timeout(Setting[:ssh_timeout].to_i) doin order to make sure that it is actually gets an integer rather then unknown type.
There was a problem hiding this comment.
Sure, I'll add that
theforeman-bot
added
Waiting on contributor
Needs re-review
Needs testing
and removed
Not yet reviewed
Waiting on contributor
labels
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
ianballou
force-pushed
the
configure-ssh-timeout
branch
from
1a5ed3e
to
886b431
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
I think [test] will fail because this needs to be added to |
I found that I needed to increase this value when SSH provisioning a very slow machine. Making this configurable could help others who have the same problem.