-
Notifications
You must be signed in to change notification settings - Fork 249
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
Docker Service for runtime tests #2193
Conversation
d2a9ad6
to
dc25d6a
Compare
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
dc25d6a
to
125b750
Compare
How would you feel about using different template delimiters, like |
let (template_key, template_value) = template.split_once('=').with_context(|| { | ||
format!("invalid template '{template}'(template should be in the form $KEY=$VALUE)") |
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.
Am I reading this right that it would preclude the use of spin variables in a test manifest?
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.
I think the point you brought up above about using a different template delimiter applies here. I just completely forgot that {{}}
already has semantic meaning. Perhaps ${{}}
would work?
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.
${{}}
might be confusing as it could be read as "$
+ {{variable}}
".
} | ||
} | ||
|
||
impl Service for DockerService { |
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.
Fwiw you may want to look at https://github.com/testcontainers/testcontainers-rs - at least in Go they've got some useful helpers for things like "waiting for a service to actually be up"
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.
Took a look at the Rust library, and I'm not sure it's worth taking on the dependency especially since the API they expose is sort of directly at odds with how we want to do things. We could make it work, but the code we have now isn't too bad, and I think it ultimately will be easier to maintain.
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.
I implemented health checking now too which was only a few lines of code, so it seems like we're fine without a library for now.
process::{Command, Stdio}, | ||
}; | ||
|
||
pub struct PythonService { |
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.
Not about the code but more about the preference for Python - it's relatively hard to write cross platform Python and also have reliable dependency management that will be consistently available - we probably need some guidelines to go along with this (I may have missed some?)
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.
I think the issue is that we don't have a great alternative. Ideally whatever we pick has these properties:
- Relatively widely known or easy to write by cargo culting
- e.g., Lua might tick some of these other boxes but is a bit of a strange pick
- Can start running relatively quickly
- this is why I didn't chose Rust despite the rest of the project being written in it
- Is possible to run cross platform
- e.g., I'd like to avoid bash scripts so that we can run this easily on Windows
- Can be used consistently
- this is why I didn't pursue using WebAssembly for this since I'd rather not have each service written in a different language
That all being said, I'll definitely try to write some best practice guidelines so we can make the best out of Python.
} | ||
|
||
fn python() -> Command { | ||
Command::new("python3") |
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.
Python 3 is not consistently aliases as python3 across systems and distributions - especially ones that are removing Python 2
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 needs to be improved in other ways too. For example, we should ensure sooner rather than later that the Python we try to use is actually there.
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
This PR introduces Docker based services for runtime tests to compliant Python based services introduced in #2180. With this, we can now run all runtime tests as long as the user has Python and Docker installed and running.
Features
The docker based services are run for each test and will be spun down after the test is run.
Ports
In addition to these services, new support for templating ports in the runtime test's spin.toml manifest was added along with changes to the templating language.
The interpolation now happens through
{{key=value}}
annotations wherekey
is one of a limited number of keys the test runner supports. The supported keys are:source
: This support already existed and allows the manifest to reference pre-built Spin compliant WebAssembly modules that can be found in thetest-components
folder in the Spin repo. The value is substituted for the name of the test component to be used. For example{{source=sqlite}}
will use the test-component named "sqlite" found in thetest-components
directory.port
: The manifest can reference a port that has been exposed by a service. For example, if the test runner sees{{port=1234}}
it will look for a service that exposes the guest port 1234 on some randomly assigned host port and substitute{{port=1234}}
for that randomly assigned port.This ensures that the service does not need to hardcode a port which might already be taken on the host.
Drawbacks
This approach has a few drawbacks:
FAQ
How does one add a Docker service?
Just like a Python service, you just need to add a service configuration file to the
services
directory. For Docker services this configuration file is just a Dockerfile.Why both Python and Docker based services?
When looking to add a new service, always prefer the Python based service as it's generally much quicker and lighter weight to run a Python script than a Docker container. Only use Docker when the service you require is not possible to achieve in cross platform way as a Python script.
For example, we need to be able to run against a PostgreSQL database for some runtime tests. This is not possible to achieve with Python in a cross platform way, do we use Docker. However, the TCP server based tests can easily be achieved with a Python script.