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

Wrap autossh in an infinite loop #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Rjevski
Copy link
Owner

@Rjevski Rjevski commented May 15, 2024

Addresses ThomDietrich#17.

Not sure how good this is and haven't tested it (I am traveling and don't have local access to a Home Assistant system) but here's my attempt. I think we can go further and remove the need for autossh completely but I wanted the change to be minimal.

Feedback/improvements welcome.

Autossh will exit if the first invocation of SSH fails which can happen if the host restarts (because of power failure?) and network isn't yet available (or your are unlucky and your tunnel target host is unavailable during this critical period). This causes the whole add-on to terminate which isn't great if you are relying on it to maintain access to a remote system. This commit makes autossh run in an infinite loop to ensure it never exits by itself.
@ThomDietrich
Copy link

Hey! Thanks for this! Shouldn't the loop start at line 60ish? I'm thinking about the situation when both HA and the SSH server are restarted or the connection between them is wacky...

@Rjevski
Copy link
Owner Author

Rjevski commented May 15, 2024

You are right, though I wonder if the "test command" is actually useful? Personally I would remove it. Either the add-on works and it connects, or it keeps spitting out errors in the logs and you know something is wrong.

@ThomDietrich
Copy link

ThomDietrich commented May 15, 2024

Great remark. I am convinced it is useful, as for the reason you gave: "something is wrong". We have two major sources of wrong: 1. the connection and authentication, 2. the port forwarding process. The test command takes care of 1, the main command fails for the reasons behind 2. It's all about usability and ease of troubleshooting, otherwise this whole script could have been a two-liner :)

@Rjevski
Copy link
Owner Author

Rjevski commented May 16, 2024

Fair enough - I don't see how you would want to integrate the test command with this.

To me the test command is there to exit early and provide useful feedback to the user when setting up the add-on in the first place. However, since we now want to avoid exiting no matter what, it means we can no longer do exit 1 - or rather, we wouldn't want to (even if the target server has a hiccup and temporarily rejects our key, I still don't want to add-on to exit), thus I'm not sure what the test command adds in this new scenario over just running autossh directly.

Ideally we'd be able to only invoke the test on first run or explicitly on user request. "First run" could be detected by hashing all the config variables into a temp file upon a successful connection and then checking whether this file exists. File exists == we've already successfully tested, so run in unattended mode and loop indefinitely. File not exists (yet) == run in one-shot test mode, and upon success write this marker file for the next run to signify the config was tested successfully.

Alternatively, test mode could just be renamed "debug mode" and be an explicit config flag (defaulting to True)? Then users can set up their add-on with it (including the printing of SSH host keys, etc), and when it all works, they can disable it which will skip test mode and enable the "loop forever" functionality?

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.

2 participants