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

Bump passphrase timeout to 30secs, check if process is running #1222

Merged
merged 3 commits into from
May 21, 2017

Conversation

barryvdh
Copy link
Contributor

Q A
Bug fix? Yes
New feature? No
BC breaks? No
Deprecations? No
Fixed tickets #1220

This bumps the wait time for a passphrase to 30 secs. It also checks if the process is running, otherwise throw an immediate exception (for example, after entering the wrong passphrase)

@antonmedv
Copy link
Member

It also checks if the process is running,

What for?

@barryvdh
Copy link
Contributor Author

The ssh multiplex command is supposed to keep running in the background, right?
If it fails (eg. connection can't be made, passphrase is not entered correctly) the process exits. So if we keep waiting, it will fail after the timeout (30 secs), instead of directly.

@antonmedv
Copy link
Member

Master connection detaches, doesn't it?

@barryvdh
Copy link
Contributor Author

Not sure, doesn't seem to in my case. When I run the command manually, it just sets up an SSH connection with exiting. Or is that not intended?
I thought you were using start instead of run to run it in the background until the deployment finishes.

Should I remove the check then?

@antonmedv
Copy link
Member

Should I remove the check then?

I'll test it.

I found two cases: in homestad master stick to process, but on my macos it doesn't.

Can you check in homestad?

@barryvdh
Copy link
Contributor Author

With sticks to process, you mean it keeps the process running? That is the same as in CentOS. I can give it a try on the mac, but if there is not way to detect if it's pending, we should just let it fail. Or perhaps we could still get some output of the failing.

@barryvdh
Copy link
Contributor Author

On my Mac is seems to have the same behavior.

One thing that I'm encountering is that is run the deploy:failed task after. Depending on the hooks (eg. after(‘deploy:failed’, ‘deploy:unlock’);), it runs more commands, which in turn ask for passphrases on the failed task. The exception is swallowed untill all tasks are run.

I think the flow should probably be something like this:

  • First initialize SSH (multiplexing etc)
  • If working, continue with the tasks (eg. prepare etc), if then failing, run the failed task
  • If not working, abort before running any commands on the server to avoid multiple passphrase requests / swallowing the init message.

@antonmedv
Copy link
Member

Nice, i think it will be good solution.

@barryvdh
Copy link
Contributor Author

Is that something you are going to do?

@antonmedv
Copy link
Member

No

@antonmedv
Copy link
Member

Maybe you can do it?

@barryvdh
Copy link
Contributor Author

I'll see. Do you still want to merge this first?

@barryvdh
Copy link
Contributor Author

Ah, you already had something like this. I created an InitializationException whichs extends the GracefulShutdownException, which skips the failed task. This shuts down immediatly. Better?

@jconroy
Copy link

jconroy commented May 20, 2017

This bumps the wait time for a passphrase to 30 secs

FWIW this is working well combined with #1220, thanks for the fix @barryvdh (I was hitting the 5sec timeout)

@antonmedv antonmedv merged commit 1f9c74d into deployphp:master May 21, 2017
@barryvdh barryvdh deleted the patch-3 branch May 21, 2017 05:45
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.

None yet

3 participants