Update dependencies #328

mestery · 2018-08-13T15:33:27Z

This commit updates all of the dependencies for cn-infra and get the
entire package compiling (as shown by running make from the
toplevel).

Note this does upgrade etcd to version 3.3.9, because version 3.2.0
no longer built with updated dependencies.

Please note this is an attempt to bring the dependencies up to date outside of the work being down around adding in requisite dependencies for the custom resource definition work being done.

Signed-off-by: Kyle Mestery [email protected]

mestery · 2018-08-13T15:34:18Z

@ondrej-fabry @jmedved @jhnbrns FYI, this seems to update all cn-infra dependencies and get things compiling. Is there a reason these are not kept up to date? See #327 for an easy way to keep this up to date automatically.

mestery · 2018-08-13T15:45:24Z

I am unsure why make check-links fails, seems unrelated to my change. The make test-cover failure is due to the etcd update, though it's not clear to me why that fails either.

mestery · 2018-08-13T17:37:31Z

This is passing CI now, I'd like to have this on merged and then I can move on to updating the dependencies in vpp-agent and finally contiv-vpp.

mestery · 2018-08-13T17:37:48Z

/cc @jmedved @ondrej-fabry @edwarnicke @jhnbrns

lukasmacko · 2018-08-14T06:58:42Z

I would recommand to try it with vpp-agent and contiv/vpp first. It can be tested using source rule in gopkg.toml that will point to your fork with updated dependencies.

Once we know that the new versions work for the projects this could be merged.

ondrej-fabry · 2018-08-14T09:09:43Z

As @lukasmacko already wrote. You should try that these version are compatible with projects that use cn-infra, mainly vpp-agent and contiv. The specific versions in Gopkg.toml were chosen by @lukasmacko due some dependency issues when using cn-infra in contiv.

mestery · 2018-08-14T15:30:04Z

Thanks @lukasmacko and @ondrej-fabry, I was testing with the source rule, will keep at it. BTW, I could only test this with vpp-agent with the contiv-1807 branch there, is there plans to merge that branch into master anytime soon?

This updates all the dependencies to the very latest, and is part of the work to add CRD support to contiv-vpp. Note that this is dependent on an unmerged patch for cn-infra, which can be found here [1]. [1] ligato/cn-infra#328 Signed-off-by: Kyle Mestery <[email protected]>

ondrej-fabry · 2018-08-14T20:49:14Z

@mestery For vpp-agent, we do main development on pantheon-dev branch. Tomorrow planning to merge that into contiv-1807, which is special branch just for contiv/vpp project. Master is used for official releases into which we merge from pantheon-dev.

ondrej-fabry · 2018-08-17T06:05:55Z

Gopkg.toml

 ]

-[[constraint]]
+[[override]]


Why is this override? It's direct dependency for cn-infra: https://github.com/ligato/cn-infra/blob/master/rpc/grpc/plugin_impl_grpc.go#L25

I recall @edwarnicke indicating there as a problem with cn-infra and newer grpc versions. The override here allows this to bump grpc up to version 1.14.0 (the latest). If this will be a problem, let me know.

Overrides are used in the projects to override transitive dependency. You can use override in the project where it makes problem for you to override it to your custom version. What you did here does not help at all with projects that use cn-infra.

See documentation for detailed information: https://golang.github.io/dep/docs/Gopkg.toml.html#override

only overrides from the current project's Gopkg.toml are incorporated.

Overrides should be used cautiously and temporarily, when possible.

ondrej-fabry · 2018-08-17T06:08:35Z

Gopkg.toml

+  version = "=3.3.9"
+
+[[constraint]]
+  name = "github.com/coreos/bbolt"


When did we switch to bbolt and why?

Something else was pulling bbolt and I had to add this constraint, otherwise a dep ensure -update resulted in a tree which failed to compile.

Could you share the failure error? We use Bolt and not BBolt so I'm really curious what was pulling this dependency. There is no mention of bbolt anywhere in cn-infra.

I just pulled this out and re-ran dep ensure -update, and dep completed. So now I'm confused. I'l push a commit to remove it explicitly from Gopkg.toml. However, note that bbolt continues to remain in vendor, I believe because etcd is using it:

machine:cn-infra userid$ grep -R bbolt vendor/ vendor//github.com/boltdb/bolt/README.md:> you look at the CoreOS fork called [bbolt](https://github.com/coreos/bbolt). grep: vendor//github.com/hashicorp/go-rootcerts/test-fixtures/capath-with-symlinks/securetrust.pem: No such file or directory grep: vendor//github.com/hashicorp/go-rootcerts/test-fixtures/capath-with-symlinks/thawte.pem: No such file or directory grep: vendor//github.com/coreos/etcd/cmd/tools: No such file or directory grep: vendor//github.com/coreos/etcd/cmd/etcdctl: No such file or directory grep: warning: vendor//github.com/coreos/etcd/cmd/etcd: recursive directory loop grep: vendor//github.com/coreos/etcd/cmd/functional: No such file or directory grep: vendor//github.com/coreos/etcd/Documentation/README.md: No such file or directory vendor//github.com/coreos/etcd/mvcc/backend/backend.go: bolt "github.com/coreos/bbolt" vendor//github.com/coreos/etcd/mvcc/backend/batch_tx.go: bolt "github.com/coreos/bbolt" vendor//github.com/coreos/etcd/mvcc/backend/config_windows.go:import bolt "github.com/coreos/bbolt" vendor//github.com/coreos/etcd/mvcc/backend/read_tx.go: bolt "github.com/coreos/bbolt" vendor//github.com/coreos/etcd/mvcc/backend/config_default.go:import bolt "github.com/coreos/bbolt" vendor//github.com/coreos/etcd/mvcc/backend/config_linux.go: bolt "github.com/coreos/bbolt" machine:cn-infra userid$

The error probably wasn't occurring during dep, but when compiling tests. Just like it's happening in Travis now.
I believe that the reason is that etcd project uses glide and dep might have issue figuring out bbolt dependency correctly.

You selected etcd v3.3.9 which uses some specific option from bbolt introduced in bbolt v1.3.1, although the dep chose v1.3.0 causing tests to fail to compile.

This means you should actually keep the bbolt in Gopkg.toml. However use [[override]] since we don't actually use bbolt directly, only as transitive dependency from etcd.

ondrej-fabry · 2018-08-20T05:33:12Z

Gopkg.toml

@@ -28,13 +28,13 @@ required = [
    "github.com/grpc-ecosystem/grpc-gateway/runtime"
 ]

-[[constraint]]
+[[override]]


This is still relevant to my comment: #328 (comment)

ondrej-fabry · 2018-08-20T05:35:50Z

Gopkg.toml

@@ -130,7 +130,7 @@ required = [

 [[override]]
  name = "github.com/ugorji/go"
-  revision = "ded73eae5db7e7a0ef6f55aace87a2873c5d2b74"
+  revision = "v1.1.1"


This should most likely be version = "1.1.1".

It appears not. Look at networkservicemesh and a dep status, you can see that grpc has the v in front of the version:

servernetworkservicemesh username$ dep status|grep grpc google.golang.org/grpc v1.14.0 (override) v1.14.0 32fb0ac v1.14.0 26 server:networkservicemesh username$

You can't really compare output of a dep status with syntax used in Gopkg.toml file.

My initial comment was coming from official documentation for Gopkg.toml and it's version property: https://golang.github.io/dep/docs/Gopkg.toml.html#version
Where you will find version without any prefix.

And even if it allows having both cases, why go for inconsistency if there is no other version property with v prefix in the current Gopkg.toml? Inconsistency creates confusion.

Either put this without prefix or put all others with prefix, so we have same format for all version properties in Gopkg.toml file.

That's a good explanation and I'll fix this one. Thanks @ondrej-fabry!

OK, something weird happened. When I changes this to 1.1.1 instead of v1.1.1, a run of dep ensure -update failed because it could no longer match the version of github.com/ugorji/go it needed for etcd-3.3.9. I wonder if this is related to their usage of glide.

ondrej-fabry · 2018-08-20T06:46:53Z

Gopkg.lock

-  branch = "master"
-  name = "github.com/cockroachdb/cmux"
+  digest = "1:c28625428387b63dd7154eb857f51e700465cfbf7c06f619e71f2da33cefe47e"
+  name = "github.com/coreos/bbolt"


See: #328 (comment)

mestery

Replying to review comments.

ondrej-fabry · 2018-08-21T18:33:36Z

Rebase after #331

Also, remove github.com/coreos/bbolt from required and github.com/ugorji/go from Gopkg.toml that helped me when trying locally and running dep ensure.

This commit updates all of the dependencies for cn-infra and get the entire package compiling (as shown by running `make` from the toplevel). Note this does upgrade etcd to version 3.3.9, because version 3.2.0 no longer built with updated dependencies. Signed-off-by: Kyle Mestery <[email protected]>

Add github.com/cores/bbolt dependency, and update the dependency version of github.com/ugorji/go. Signed-off-by: Kyle Mestery <[email protected]>

Signed-off-by: Kyle Mestery <[email protected]>

This is still being pulled in, but we don't explicitely require it. Signed-off-by: Kyle Mestery <[email protected]>

Signed-off-by: Kyle Mestery <[email protected]>

While tinkering with changing the version of github.com/ugorji/go and running `dep ensure -update`, the following new dependencies were added to the project. This was done with dep at the following version: dep: version : v0.5.0 build date : 2018-07-26 git hash : 224a564 go version : go1.10.3 go compiler : gc platform : darwin/amd64 features : ImportDuringSolve=false Signed-off-by: Kyle Mestery <[email protected]>

Signed-off-by: Kyle Mestery <[email protected]>

This updates all the dependencies to the very latest, and is part of the work to add CRD support to contiv-vpp. Note that this is dependent on an unmerged patch for cn-infra, which can be found here [1]. [1] ligato/cn-infra#328 Signed-off-by: Kyle Mestery <[email protected]>

the work to add CRD support to contiv-vpp. Note that this is dependent on an unmerged patch for cn-infra, which can be found here [1]. [1] ligato/cn-infra#328 Signed-off-by: Kyle Mestery <[email protected]>

jmedved requested review from rastislavs, ondrej-fabry and VladoLavor August 13, 2018 22:52

jmedved assigned ondrej-fabry Aug 13, 2018

jmedved requested a review from lukasmacko August 13, 2018 22:53

This was referenced Aug 14, 2018

WIP: DO NOT MERGE YET: Update dependencies ligato/vpp-agent#786

Closed

WIP, DO NOT MERGE YET: Update dependencies ligato/vpp-agent#787

Closed

ondrej-fabry reviewed Aug 17, 2018

View reviewed changes

ondrej-fabry reviewed Aug 20, 2018

View reviewed changes

mestery commented Aug 20, 2018

View reviewed changes

mestery added 7 commits August 21, 2018 13:39

Update more dependencies

87989e7

Add github.com/cores/bbolt dependency, and update the dependency version of github.com/ugorji/go. Signed-off-by: Kyle Mestery <[email protected]>

Update github.com/grpc-ecosystem/grpc-gateway to version 1.4.1

0de7934

Signed-off-by: Kyle Mestery <[email protected]>

Remove explicit bbolt dependency

78f5184

This is still being pulled in, but we don't explicitely require it. Signed-off-by: Kyle Mestery <[email protected]>

Re-add bbolt and address review comments

bdc4c9f

Signed-off-by: Kyle Mestery <[email protected]>

Addressing review comments

e1f9617

Signed-off-by: Kyle Mestery <[email protected]>

Rebased and re-ran dep ensure -update

dc1b4d1

Signed-off-by: Kyle Mestery <[email protected]>

mestery force-pushed the pr-fix-deps branch from a4f4101 to dc1b4d1 Compare August 22, 2018 14:51

mestery mentioned this pull request Aug 22, 2018

Update dependencies ligato/vpp-agent#816

Merged

ondrej-fabry approved these changes Aug 22, 2018

View reviewed changes

ondrej-fabry requested review from lukasmacko and removed request for lukasmacko August 22, 2018 20:34

lukasmacko approved these changes Aug 24, 2018

View reviewed changes

ondrej-fabry merged commit 13ca0c1 into ligato:master Aug 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies #328

Update dependencies #328

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

lukasmacko commented Aug 14, 2018

ondrej-fabry commented Aug 14, 2018

mestery commented Aug 14, 2018

ondrej-fabry commented Aug 14, 2018

ondrej-fabry Aug 17, 2018

mestery Aug 17, 2018

ondrej-fabry Aug 17, 2018 •

edited

Loading

mestery Aug 20, 2018

ondrej-fabry Aug 17, 2018

mestery Aug 17, 2018

ondrej-fabry Aug 17, 2018

mestery Aug 17, 2018

ondrej-fabry Aug 20, 2018 •

edited

Loading

mestery Aug 20, 2018

ondrej-fabry Aug 20, 2018

ondrej-fabry Aug 20, 2018

mestery Aug 20, 2018

ondrej-fabry Aug 21, 2018 •

edited

Loading

mestery Aug 21, 2018

mestery Aug 21, 2018

ondrej-fabry Aug 20, 2018

mestery left a comment

ondrej-fabry commented Aug 21, 2018

Update dependencies #328

Update dependencies #328

Conversation

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

mestery commented Aug 13, 2018

lukasmacko commented Aug 14, 2018

ondrej-fabry commented Aug 14, 2018

mestery commented Aug 14, 2018

ondrej-fabry commented Aug 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ondrej-fabry Aug 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ondrej-fabry Aug 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ondrej-fabry Aug 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mestery left a comment

Choose a reason for hiding this comment

ondrej-fabry commented Aug 21, 2018

ondrej-fabry Aug 17, 2018 •

edited

Loading

ondrej-fabry Aug 20, 2018 •

edited

Loading

ondrej-fabry Aug 21, 2018 •

edited

Loading