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

Add SSH Support #510

Merged
merged 333 commits into from
Oct 26, 2023
Merged

Add SSH Support #510

merged 333 commits into from
Oct 26, 2023

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented Sep 18, 2023

  • Adds SSH support for remote exec and file copy operations via asyncssy library. Closes ssh environment support #379

  • This requires an event loop thread to operate the async calls in the background without blocking the rest of our main thread operations or changing our APIs dramatically. Instead we interact with all operations as futures after that.

  • Add json schema config support for SSH

  • Adds test infrastructure for running SSH servers inside containers via pytest-docker which uses docker compose to start/stop them as fixtures. This should work in all dev environments (Linux host, WSL host, Windows host), but is quite sensitive to networking setups.

    • test the background thread cleanup logic
    • test error handling of a broken ssh endpoint
    • test basic exec commands
    • test host operations (e.g., reboot)
    • test basic copy commands
    • test for single connection reuse across services
    • test multi-server support
    • use random ports to avoid conflicts
    • provide some config examples for using this
    • add remote_env exec tests with this (increase code coverage)

bpkroth added a commit that referenced this pull request Oct 25, 2023
Work towards making the setup/teardown work for the SshService managed
by a Context instead of global.

To do this, we introspect all of the methods registered for a Service
mixin (which we also renamed from `self._services` to
`self._service_methods`), and pull out their unique Service instance
references from the `__self__` attribute in the bound method that was
registered, then store them as `self._services`.
This is to handle the case where a single method name was overridden
multiple times.
At this point, we have enough information to invoke a sort of
`__enter__` context for each of those Service instances.
However, since we use `__enter__` to invoke all of them, we instead need
to implement a separate `_enter_context()` method that does the actual
work, and can then be overridden by the subclasses.
 
Unfortunately, this technique make tamper with the order the Services
were layered on one another, so any logic that required relying on that
order for context setup might be problematic. I don't know if that will
actually be an issue, so for now, I'm ignoring the issue and just
documenting it.

This PR does not include explicit new tests for this, though does have
to fixup a few mocks in order to make sure that we're registered bound
methods and not just functions.

#510 will add additional tests after this is merged.
Copy link
Member

@motus motus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

motus pushed a commit that referenced this pull request Oct 26, 2023
Extracting out from another refactor of SSH support PR #510
@bpkroth bpkroth enabled auto-merge (squash) October 26, 2023 18:26
@bpkroth bpkroth merged commit 1f0f8ee into microsoft:main Oct 26, 2023
11 checks passed
@bpkroth bpkroth deleted the add-ssh-support branch October 26, 2023 18:49
bpkroth added a commit that referenced this pull request Oct 26, 2023
Per comments in #510 

- [x] Convert test classes to `MockService`
- [x] Add json schema support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ssh environment support
2 participants