-
-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add post script functionality #45
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few housekeeping tasks before approval and some clarification.
@@ -62,5 +62,22 @@ COMMAND="autossh "\ | |||
|
|||
echo "[INFO ] # ${COMMAND}" | |||
|
|||
# Run command | |||
exec ${COMMAND} | |||
# Run autossh command in background and save pid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please write a test for this in a new container, you may call it the local-post-script
container if you'd like.
The test container should exit with 0
when you have proven the post_script ran successfully and return non-zero when the script failed to run.
This is useful to send some details to the server about the autossh client. | ||
For example, if a random port is used, then it's useful for the server to know who is on this port. | ||
SSH_TUNNEL_PORT can be used in the post script like so: | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add another space, and don't forget to call out the specific language in the markdown. (e.g. '''sh
, note the incorrect use of quotes because I couldn't easily figure out how to add in backticks). See line 112 of this file for an example.
FILE=/tmp/autossh.${SSH_CLIENT_ID} | ||
echo "${SSH_TUNNEL_PORT}" > ${FILE} | ||
scp ${FILE} ${SSH_REMOTE_USER}@${SSH_REMOTE_HOST}:/tmp | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add one blank line below codeblock.
sleep 1 | ||
|
||
# Export the Tunnel Port for post script to use | ||
export SSH_TUNNEL_PORT=${SSH_TUNNEL_PORT} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any other interesting variables we can/should expose?
# Run autossh command in background and save pid | ||
exec ${COMMAND} & | ||
pid=$! | ||
sleep 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we sleeping here? If we're just sleeping here to give time for the tunnel to instantiate, we should find a better way to confirm that.
Also, if that fails, we should exit()
in some fashion.
|
||
When set, SSH_POST_SCRIPT will be the location of a shell script to execute after starting autossh. | ||
This is useful to send some details to the server about the autossh client. | ||
For example, if a random port is used, then it's useful for the server to know who is on this port. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's useful for the server to know who is on this port.
This does not make sense just reading it.
Do you mean to say you can associate a specific container/instance of autossh is using a specific port remotely? I'm a bit confused as to the usecase.
When set, SSH_POST_SCRIPT will be the location of a shell script to execute after starting autossh. This is useful to send some details to the server about the autossh client. For example, if a random port is used, then it's useful for the server to know who is on this port.