better developer experience: ability to run tests inside docker containers#126
better developer experience: ability to run tests inside docker containers#126vjayaramrh wants to merge 1 commit intoeBay:masterfrom
Conversation
hzhou8
left a comment
hzhou8
left a comment
There was a problem hiding this comment.
Thanks @vjayaramrh
It seems the #121 is addressing the problem in a similar way. Could you take a look?
Please also see my comments.
| @@ -0,0 +1,21 @@ | |||
| FROM openvswitch/ovn:2.12_e60f2f2_debian_master | |||
There was a problem hiding this comment.
2.12 is rather old version, before OVS and OVN split.
There was a problem hiding this comment.
I used that image as the starting point as it was available in dockerhub at https://hub.docker.com/search?q=openvswitch%2Fovn&type=image, I am hoping that OVN images get regulary pushed as per the request at ovn-org/ovn#67 This dockerfile can be updated once the OVN release images gets published regularly, what do you think?
|
|
||
| ARG GOVERPKG=go1.13.9.linux-amd64.tar.gz | ||
| RUN apt-get update | ||
| RUN apt-get install -y wget git gcc python3-pip python3-dev build-essential autoconf automake libtool |
There was a problem hiding this comment.
what's the purpose of installing the tools such as gcc, python, build-essential, etc.?
There was a problem hiding this comment.
The test.sh script in the .travis folder needs them when executed inside the container
There was a problem hiding this comment.
but according to the steps you put in README, the test.sh script is not used. Your approach is supposed to run "go test" directly.
There was a problem hiding this comment.
The README captures the native go way of running tests. Do you think that the README should also capture the alternate way i.e executing the .travis/test.sh? If you think that would be useful to capture as well, I can certainly update the README
| command=/bin/start-ovn ovn-sb-tcp | ||
|
|
||
| [program:ovn-northd-tcp] | ||
| command=/bin/start-ovn ovn-northd-tcp No newline at end of file |
There was a problem hiding this comment.
It seems we don't need northd for go-ovn testing.
There was a problem hiding this comment.
correct, but convenient to have a running container built from this image that has all the relevant services running
@hzhou8 appreciate you pointing this out, I have reached out to the PR author to have an initial discussion and look at ways for combining our efforts |
|
@vjayaramrh I think there are some interesting ideas here. For now, the container does build ovn, only ovs (for the ovsdb-server). The compatibility matrix with ovsdb should be relatively straight forward. But that might change as a result of the discussions in #29. So, when those discussions clear out the go-ovn -> ovn compatibility strategy looking forward, we could improve the current system by using pre-built images. wdyt? |
|
cc @vpickard |
4 participants
The goal of this PR is to allow for developers (running on non linux systems such as MacBook and who do not have ovs/ovn installed) to be be able to run the tests in containers instead.
The PR creates a Dockerfile based off the
openvswitch/ovn:2.12_e60f2f2_debian_masterbase image and adds go binaries. It also installssupervisordto help with running ovn-nb, ovn-sb and ovn-northd in the same container. (Refer http://supervisord.org/introduction.html)Note: I have an enhancement request in the ovn-org/ovn repo requesting that official docker images for each of the OVN release to quay/docker repo (refer ovn-org/ovn#67)