migrate to vendor #82
Conversation
|
Changes look fine but the build is failing... |
unclejack
force-pushed
the
move_to_vendor
branch
from
15794dd
to
502b1af
Compare
|
The failures don't seem to be related to the libovsdb, netlink and netns. Reverting those doesn't fix the issue. |
|
@jojimt: Could you take a look at the logs when you have the time, please? I'm not sure why it's failing right now. |
unclejack
force-pushed
the
move_to_vendor
branch
from
502b1af
to
93b6ab3
Compare
|
I've reverted the change which was switching to I've opened an issue to track the TestPolicyAddDelete failure: #87. This PR will allow us to bump the other deps (libovsdb, libOpenflow). |
| sudo -E PATH=$(PATH) /opt/gopath/bin/godep go test -v ./ | ||
| sudo -E PATH=$(PATH) /opt/gopath/bin/godep go test -v ./ofctrl | ||
| sudo -E PATH=$(PATH) /opt/gopath/bin/godep go test -v ./pqueue | ||
| PATH=${PATH} sudo -E /usr/local/go/bin/go test -v ./ |
There was a problem hiding this comment.
If the box isn't being updated, do we still need the host-test path changes in Makefile? IIRC the old box had Go installed under the /opt path
There was a problem hiding this comment.
@dseevr: This makes it easy for us to migrate away from godep. Even though we're not upgrading to Go 1.7, we're on Go 1.6 and we can get rid of godep in some places. The new box has Go in the same place, but it's easier to not rely on the box's gopath.
|
the last time we tried moving ofnet to vendoring (go1.6) there was dependency issue when ofnet was vendored into netplugin. Did you get a chance to test out netplugin with the vendoring ? |
|
@abhinandanpb: I'll try this out and post a reply. The Godeps workspace was broken without this PR. I can imagine that's what was breaking it before. |
|
what was broken ? We have been using godeps forever with bunch of dependency brought in and out. We havent had any issues with godeps in ofnet. |
|
@abhinandanpb: How are you using godeps to update dependencies in ofnet? I get errors. |
|
@abhinandanpb: Please let me know if there's anything else you'd like me to do or look into. |
Signed-off-by: Cristian Staretu <[email protected]>
Signed-off-by: Cristian Staretu <[email protected]>
unclejack
force-pushed
the
move_to_vendor
branch
from
93b6ab3
to
fb758db
Compare
|
This PR is rebased on top of master. I haven't changed the version of the Vagrant box. We have to address some failures with Go 1.7 in a separate PR. The libOpenflow library is also not upgraded or modified in any way in this PR. That's to be done in a separate PR. |
There was a problem hiding this comment.
Please check this against contiv/netplugin. The last I remember we had issues when we migrated to vendoring way of handling dependencies when used with netplugin (Not aware of the specific details of failure). Let us know the systemtest results of contiv/netplugin. If everything looks good there, we can merge this PR.
|
Also, what is the recommended way of moving to vendor when it is used in other repositories? Will it even make a difference? |
|
@DivyaVavili it doesn't make a difference as to how the code is used under |
|
@DivyaVavili: There's nothing to check with netplugin in this case. ofnet itself is functioning properly and it's brought in with all of its dependencies. The difference is that we won't be running into errors any more when we try to bump dependencies. We need to make progress and improve the code, but this is a major roadblock. |
|
The full netplugin CI suite will be run when netplugin's Godeps are updated to use the new version of this code. We need to switch to vendor. It's been the ordained way to manage dependencies in Go projects for almost 1.5 years, and our code no longer compiles with any version of Go which does not support vendoring, and We should merge this ASAP so that we can open the PR to update this dependency in netplugin itself |
|
I've tested this with netplugin. ofnet was bumped properly. There's some work left to be done when we bump this in netplugin itself. This work which needs to be done isn't something which should block this PR, though. It should be safe to proceed at this point. |
|
Thanks for checking this. LGTM with respect to this change. But when we are including it into contiv/netplugin, let's check it rigorously before merging in for the next major release so that we don't run into any dependency issues. |
This PR migrates ofnet to vendor.
This PR makes the following changes:
switches to the centos/73 box to use Go 1.7.5delayed for a future PRMost of these changes were made in one commit because godep was causing some issues with vendor.