aws / session-manager-plugin

This plugin helps you to use the AWS Command Line Interface (AWS CLI) to start and end sessions to your managed instances
Apache License 2.0
257 stars 69 forks source link

How to control behavior for ChannelClosedMessage #86

Open breml opened 9 months ago

breml commented 9 months ago

We use the session-manager-plugin in a DevOps tool of ours and we would like to have more control over the shutdown / program ending logic. Currently this is not possible, because session-manager-plugin calls os.Exit(0) after a ChanellClosedMessage is received. IMHO, this is pretty rude behavior for a library package, because it event prevents the usage of defer-ed function for more graceful shutdown.

Therefore I wonder, what is the reason for this and what options do exist to take more control over the program shutdown if session-manager-plugin is used.

yuting-fan commented 4 months ago

Hi @breml , thank you for reaching out. This was handled this way to terminate ongoing session in response to a terminate request, and not allowing additional inputs/outputs ASAP. But I agree with you that this logic could be improved to gracefully shut down. I have captured this in our feature request backlog, and it will be prioritized based on +1s and roadmap planning.

rue-nsilverman commented 4 months ago

I have implemented my own graceful shutdown in a fork of this repo. It sets up a session ended attribute on the data channel that can be read by the goroutines. My fork also has a number of other fixes that are unrelated to graceful shutdown (fixes to use newer Go and better raw terminal handling) so we wouldn't be able to PR it and merge it immediately, but we could definitely cherry pick some of it.

The main issue I had with this approach was that for shell sessions, the read keyboard input function was a blocking call, so to address that, I moved that loop in its own goroutine that checks for session ending and gracefully returns when its ended. I have been using this fork for a while now with great success for all session types.

Here are my changes https://github.com/aws/session-manager-plugin/commit/010f529ab85df1d931f34f3bb8cc8d02122d15a3