Skip to content
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

go mod not working with master #11850

Closed
jordansissel opened this issue Apr 17, 2019 · 14 comments
Closed

go mod not working with master #11850

jordansissel opened this issue Apr 17, 2019 · 14 comments
Assignees
Labels

Comments

@jordansissel
Copy link
Contributor

I saw #11330 merged and am trying against master, but it still fails with go mod:

% go get
...
go: github.com/Sirupsen/logrus@v1.4.1: parsing go.mod: unexpected module path "github.com/sirupsen/logrus"

I'm not sure why logrus is still attempted. I've checked beats master and my beater code and neither import logrus.

While fiddling, I tried go mod init in beats master:

% go mod init github.com/elastic/beats
go: creating new go.mod: module github.com/elastic/beats
go: copying requirements from vendor/vendor.json
...

vendor.json, eh?

% jq '.package[] | select(.path | test ("logrus"))'  vendor/vendor.json | less
{
  "checksumSHA1": "1KA6FgHyBKaIqBbOMFYt5wkMap8=",
  "path": "github.com/sirupsen/logrus",
  "revision": "a6c0064cfaf982707445a1c90368f956421ebcf0",
  "revisionTime": "2019-03-31T13:19:41Z"
}
{
  "checksumSHA1": "1Z9bN43Wk647bMXb/yMDdVEYTHo=",
  "path": "github.com/sirupsen/logrus/terminal",
  "revision": "a6c0064cfaf982707445a1c90368f956421ebcf0",
  "revisionTime": "2019-03-31T13:19:41Z"
}

logrus is still mentioned here. I don't know if this is the problem, though. The go tooling for debugging dependency problems like this seem to be pretty scarce because go fails while trying to build the dependency graph, so things like go graph and go why don't work for this.

@kvch
Copy link
Contributor

kvch commented Apr 17, 2019

Two of our dependencies need github.com/sirupsen/logrus thus it shows up in our vendor.json:

  • github.com/docker/docker/pkg/fileutils
  • github.com/docker/docker/pkg/system

Same issue mentioned here: #10791 (comment)

While looking at our dependencies, I found an incorrect vendor entry:

{
        "path": "plugin",
        "revision": ""
}

I am opening a PR to remove it.

PR: #11853

@ph
Copy link
Contributor

ph commented Apr 26, 2019

I ran into the same issues, looking at the path I do not see anything with the capital S.

@ph
Copy link
Contributor

ph commented Apr 26, 2019

Well this seems a rabbit hole golang/go#28489

@ph
Copy link
Contributor

ph commented Apr 26, 2019

looking at docker it seems fixed in master https://github.com/moby/moby/blob/master/pkg/fileutils/fileutils.go

@ph ph self-assigned this Apr 26, 2019
@ph
Copy link
Contributor

ph commented Apr 26, 2019

Not a fun dependencies experience.

@jordansissel I have a workaround for you, If you do the following.

go get github.com/docker/docker/@master
go get github.com/elastic/ecs/code/go/ecs/@master
go get github.com/elastic/beats/libbeat

You should get a warning but that should work, I need to upgrade the docker vendor of beats and the versioning that docker uses doesn't play well with go mod...

@ph ph added the libbeat label Apr 26, 2019
@ph
Copy link
Contributor

ph commented Apr 26, 2019

Well, the goods new I was able to migrate the code to the latest version of docker from https://github.com/moby/moby the bad news is.. when looking at our vendor.json I found out that we have actually forked the project to add support to netbsd in http://github.com/exekias/moby/. So I was like simple enough I will just try to apply that patch to the latest code base and I've discovered that they actually dropped solaris support.

At the moment we do not actually support solaris or even netbsd.

@kvch
Copy link
Contributor

kvch commented Apr 30, 2019

What about sending our patch to the upstream repo?

@andrewkroh
Copy link
Member

andrewkroh commented Jun 19, 2019

Since Docker doesn't support netbsd or solaris can we put some build tags in place or stubs to make docker features conditional on the GOOS? We need to address this soon to be able to start using go mod.

I think we need to move at a minimum to moby/moby@8af4db6#diff-8a5da52ed126447d359e70c05721a8aa (it's about 2 months later than our current docker/docker version). But if we can do it without too much disruption then we can go to an even newer version.

We use a weird mix of sources and commits for Docker in vendor.json. Going forward we should not mix and match upstream sources and commits for a single repo (go mod does not support this AFAICT and this make migrating very difficult).

BTW from a third-part repo that includes beats as a dep this is the error I get that shows the tree of how github.com/Sirupsen/logrus is creeping in.

$ go mod tidy
...
        github.com/elastic/beats/libbeat/processors/add_docker_metadata imports
        github.com/elastic/beats/libbeat/common/docker imports
        github.com/docker/docker/api imports
        github.com/Sirupsen/logrus: github.com/Sirupsen/logrus@v1.4.2: parsing go.mod: unexpected module path "github.com/sirupsen/logrus"

@eloyekunle
Copy link
Contributor

Has there been any discussion around switching the project to use Go modules?

@kvch
Copy link
Contributor

kvch commented Sep 4, 2019

Yes, we are moving to go modules. Related PRs: #13415, #13474

@eloyekunle
Copy link
Contributor

Okay great 👍

andrewkroh added a commit to andrewkroh/beats that referenced this issue Sep 4, 2019
This bumps up the version of all vendored github.com/docker/docker packages to 8af4db6f002ac907b6ef8610b237879dfcaa5b7a. This does impact netbsd, because this version of Docker doesn't compile for netbsd.

This is earliest version of Docker that fixes the github.com/Sirupsen/logrus issue causing problems with go.mod in elastic#11850. I was trying to minimize the amount of changes to the Docker libraries to avoid breaking changes.
andrewkroh added a commit that referenced this issue Sep 6, 2019
…es (#13415)

* Update github/docker/docker to a consistent version across all packages

This bumps up the version of all vendored github.com/docker/docker packages to `github.com/docker/engine@v19.03.2`. This does impact netbsd, because this version of Docker doesn't compile for netbsd. But we will add the appropriate build tags in a follow-up change.

github.com/docker/engine is a fork of github.com/moby/moby but with release tags. (source: moby/moby#39302 (comment))

Ran command:
    govendor fetch github.com/docker/docker/...::github.com/docker/engine@v19.03.2

This fixes the github.com/Sirupsen/logrus issue causing problems with go.mod in #11850. And ensures that all sub-packages are coming from the same repo (another go.mod requirement).

* Add integration build tag to Metricbeat docker/image metricset test

* Run script/clean_vendor.sh
@kvch
Copy link
Contributor

kvch commented Sep 11, 2019

Resolved in #13415

@kvch kvch closed this as completed Sep 11, 2019
@richard-mauri
Copy link

richard-mauri commented Oct 28, 2019

I thought beats was using go modules, but go build does not work.
Why did I have to mod mod init as shown below?

Why vendor dir and no go.module after a master branch clone?
I cloned master branch of beats (commit 5cf0b84)

$ go version
go version go1.13.3 darwin/amd64
/Users/richardmauri/.bash_history:env|grep GO
$ export GOPATH=/Users/richardmauri/gopath
$ export GO111MODULE=on
$ go build
go: cannot find main module, but found vendor/vendor.json in /Users/richardmauri/gopath/src/github.com/elastic/beats
to create a module there, run:
go mod init

So, I ran go mod init (even though it should've been done out of the box; not users responsibility IMHO)

$ go mod init
go: creating new go.mod: module github.com/elastic/beats
go: copying requirements from vendor/vendor.json
go: converting vendor/vendor.json: stat github.com/docker/docker/pkg/homedir@ed20165a37b40ff1cfbe55e218344c5e89f30ee2: unknown revision ed20165a37b40ff1cfbe55e218344c5e89f30ee2
... omitting many similar "go: converting vendor/vendor.json" lines

$ go build
go: github.com/coreos/etcd@v0.5.0-alpha.5.0.20190829190724-4d210173ae0d: parsing go.mod:
module declares its path as: go.etcd.io/etcd
but was required as: github.com/coreos/etcd

So I removed the etcd line from the generated go.module to see if go build would sort itself out but I got:

$ go build
go: github.com/google/certificate-transparency-go@v1.0.22-0.20190829120130-2c006aff63ed requires
github.com/golangci/golangci-lint@v1.17.1 requires
github.com/go-critic/go-critic@v0.0.0-20181204210945-1df300866540: invalid pseudo-version: does not match version-control timestamp (2019-05-26T07:48:19Z)

Next, I removed certificate-transparency-go from go.module (again to see if go build could sort it out)

$ go build
can't load package: package .: build constraints exclude all Go files in /Users/richardmauri/gopath/src/github.com/elastic/beats

Is it or will it ever be possible to get beats to build with native go tooling?
I am interested at the moment on building a Helloworldbeat and hope/expect to be able to do it without having Makefile or magefiles or needing python or virtualenv. It would be great if the new beat development could be done with go module builds; I could live with the complete beats build depending on all the pyton,virtualenv ...

@kvch
Copy link
Contributor

kvch commented Oct 28, 2019

Beats is not yet using go modules, thus we do not provide a go.mod file. By resolving this issue we got one step closer to the support. But we are not there yet. We are working on supporting go modules as it is going to be the default way of handling dependencies.

Regarding your question about building beats with native go tooling, as it was mentioned in the discuss question, it is also in progress. Our goal is to remove all python dependencies when building Beats. But it is not related to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants