Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Improve the SSH UX on errors #123

Closed
luxas opened this issue Jul 10, 2019 · 6 comments · Fixed by #149
Closed

Improve the SSH UX on errors #123

luxas opened this issue Jul 10, 2019 · 6 comments · Fixed by #149
Assignees
Milestone

Comments

@luxas
Copy link
Contributor

luxas commented Jul 10, 2019

Ref #121
Make so that:

  • SSH retries a couple of times (e.g. 10 or similar), notifying the user every n seconds.
  • If the container is down, say that
  • If the IP isn't reachable (i.e. can't be pinged or similar), say that

probably more...

@luxas luxas added this to the v0.4.1 milestone Jul 10, 2019
@twelho
Copy link
Contributor

twelho commented Jul 10, 2019

Also if the user shuts down the VM by issuing reboot, ignite ssh will throw an error. This might be difficult to fix properly though.

@luxas luxas mentioned this issue Jul 12, 2019
@twelho
Copy link
Contributor

twelho commented Jul 12, 2019

If the container is down, say that

To this Ignite responds: VM "<uid>" is not running.

If the IP isn't reachable (i.e. can't be pinged or similar), say that

If the VM hasn't got an IP accessible from outside (its network config is invalid),
Ignite responds: VM "<uid>" has no usable IP addresses.

Do these fulfill the requirements?

SSH retries a couple of times (e.g. 10 or similar), notifying the user every n seconds.

This is hard to implement, as for each "remote command failure" SSH reports exit code 255 and both "connection failed" and "the user rebooted the VM" return that code and the same error message.

@twelho
Copy link
Contributor

twelho commented Jul 12, 2019

Leading to Ignite trying to reconnect to a rebooted VM, which is not what we want.

@twelho
Copy link
Contributor

twelho commented Jul 12, 2019

I fixed a bunch of other issues though regarding authentication and warnings/errors for a seamless experience, will open a PR soon.

If you want, I can make the connection timeout customizable via a flag (instead of the retries, which cannot be implemented due to restart being synonymous with connection failure)

@luxas
Copy link
Contributor Author

luxas commented Jul 12, 2019

Do these fulfill the requirements?

For now, yes. If we find more stuff we want to improve later, then we'll do that.

If you want, I can make the connection timeout customizable via a flag (instead of the retries, which cannot be implemented due to restart being synonymous with connection failure)

Sounds good 👍

Leading to Ignite trying to reconnect to a rebooted VM, which is not what we want.

Can't we know that as long as the VM object's status is Running it's meant to be running and ignite-spawn hasn't stopped it? Or check if the docker container is running? I'm just thinking out loud here, we should not do that kind of stuff for v0.4.1.

@twelho
Copy link
Contributor

twelho commented Jul 12, 2019

Can't we know that as long as the VM object's status is Running it's meant to be running and ignite-spawn hasn't stopped it? Or check if the docker container is running?

The problem is that the VM is still Running while it's shutting down (rebooting), so ignite ssh will not know it's supposed to be shutting down. We also have the issue of ignite-spawn and the CLI trying to read/write to the same file at the same time. Either we need some field indicating state transitions or just wait until have the communication in place between ignite-spawn and the CLI.

Either way, that's not for v0.4.1 like you said. All the other rework is now in #149.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants