-
Notifications
You must be signed in to change notification settings - Fork 24
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
Removes SSH and GCP command builders #1786
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.
Do we still have a use of Host.Kind ?
We do, because it's used to differentiate between local tests and k8s tests, but the rest of the HostInfo config values I think are unused, so I've removed them. |
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.
This looks all good to me !
Not something to do as part of this PR, but given that tests are meant to either run on docker or k8s (at least the way they are put together now), we can probably delegate the decision of what kind of runner to use to the tests themselves, instead of making it configurable and risk someone attempting to run tests on a system they are not meant to work on. If we ever put together a test that is able to run on docker or k8s independently we might need to put some mechanism for this back in place, but I don't think we have plans to do this any time soon. |
Yeah good point. I'll do that in a follow up. |
Description
The SSH and GCP command builders are no longer used. This PR simply removes them. CI testing should be sufficient.