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 friendly #3031

Closed
16 tasks done
rcgoodfellow opened this issue Feb 19, 2019 · 31 comments
Closed
16 tasks done

go mod friendly #3031

rcgoodfellow opened this issue Feb 19, 2019 · 31 comments

Comments

@rcgoodfellow
Copy link

rcgoodfellow commented Feb 19, 2019

https://github.com/containerd Subproject checklist (EDIT: added by @AkihiroSuda )


Description

Containerd currently causes issues for downstream projects using go mod. Since this is the official Go dependency management tool now, support would be nice.

Steps to reproduce the issue:

  1. Import containerd from a project using go mod for dependency management.
  2. Build the project

Describe the results you received:
Does not build. Specifically due to the following.

# github.com/containerd/containerd/images/archive
/home/ry/go/pkg/mod/github.com/containerd/containerd@v1.2.4/images/archive/reference.go:72:21: undefined: reference.ParseDockerRef

Module based builds were actually working fine until this commit ed756ff which introduced a dependency on code that is only in the master branch of docker/distribution.

As a workaround we currently use

replace github.com/docker/distribution v2.7.1+incompatible => github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible

in our go.mod file. But this is not really maintainable and also slows down built time significantly as it requires re-download of certain modules on every build.

Describe the results you expected:

Project builds.

Output of containerd --version:

containerd github.com/containerd/containerd v1.2.2 9754871865f7fe2f4e74d43e2fc7ccd237edcbce

Thanks

@AkihiroSuda
Copy link
Member

SGTM (but let's wait for Go 1.12 GA)

@SvenDowideit
Copy link

SvenDowideit commented Mar 7, 2019

yup, using go v1.12 go mod with v1.2.4 breaks:

# github.com/containerd/containerd/images/archive
../../go/pkg/mod/github.com/containerd/containerd@v1.2.4/images/archive/reference.go:72:21: undefined: reference.ParseDockerRef

setting the go.mod requirements to use containerd v1.2.3 fixes it for me - @thaJeztah sorry, but #2986 , like the rename from Sirupsen to sirupsen has side effects.

nick96 pushed a commit to nick96/flask-monitoring that referenced this issue Mar 30, 2019
@CpuID
Copy link

CpuID commented Aug 14, 2019

SGTM (but let's wait for Go 1.12 GA)

Go 1.12.x has been GA for a while. Can we get a resolution to this? Causing some dependency hell here, thx.

@kjanshair
Copy link

Also breaks with 1.2.8.

@scottshotgg
Copy link

Still broken with 1.2.9 - can confirm that changing to 1.2.3 (heh) fixes it.

@imiric
Copy link

imiric commented Oct 14, 2019

Seems to still be broken with 1.3.0, if this error is related:

$ go get github.com/moby/buildkit
go get: github.com/moby/buildkit@v0.6.2 requires
        github.com/containerd/containerd@v1.3.0-0.20190507210959-7c1e88399ec0: invalid pseudo-version: version before v1.3.0 would have negative patch number

go version go1.13.1 linux/amd64

If it helps anyone, I was able to workaround it with the following in go.mod:

replace (
	github.com/containerd/containerd v1.3.0-0.20190507210959-7c1e88399ec0 => github.com/containerd/containerd v1.3.0-beta.2.0.20190823190603-4a2f61c4f2b4
	github.com/docker/docker => github.com/moby/moby v0.7.3-0.20190826074503-38ab9da00309
	golang.org/x/crypto v0.0.0-20190129210102-0709b304e793 => golang.org/x/crypto v0.0.0-20180904163835-0709b304e793
)

So looks like a beta 1.3.0 works fine.

@rossedman
Copy link

rossedman commented Oct 15, 2019

@imiric Not sure how you got this to work, but def broken for me. replace did not solve my problems.

@scottshotgg can confirm this worked for me

@thaJeztah
Copy link
Member

Go mod doesn't work well with repositories that use release branches; 1.3.0-beta.2 was the last beta tagged from master, after which the release/1.3 branch got created, from which 1.3.0-rc.0 and up are released. I think there's a regression in go mod in go1.13, where that got worse

@edmorley
Copy link

edmorley commented Oct 24, 2019

I think there's a regression in go mod in go1.13, where that got worse

@thaJeztah It looks like this was an intentional change:
golang/go@6bf2767

Edit: Actually that commit seems unrelated; I guess it's another one of the commits seen in pingbacks in golang/go#27173

@adamcohen
Copy link

Any update on this? I'm still affected using go 1.13.4.

@AkihiroSuda
Copy link
Member

Added check list to the top comment of this issue for tracking the progress across github.com/containerd repos. Help wanted.

@thaJeztah
Copy link
Member

Still wondering how we'll deal with versioning. While this repository has SemVer tags, those tags apply to the binary releases, not to the Go API. The Go client API is explicitly called out as "unstable", and therefore many projects use a specific version from master (which could be problematic if Go mod prefers tags over master).

While there is an intent "to stabilize the API in a future release", it's possible that changes to that API (interfaces/signatures) may follow a different cadence than the releases; should separate tags be used for Go, or should they be in lock-step with the containerd releases themselves?

@AkihiroSuda
Copy link
Member

I think we have never broken Go API in patch releases (correct me if wrong).

@thaJeztah
Copy link
Member

I think we have never broken Go API in patch releases (correct me if wrong).

Agreed, I don't think patch releases (vX.Y.Z) broke compatibility, but minor releases may definitely not have been minor from the API perspective (at least, I'm not sure if that has been carefully looked at with each change).
With Go (afaik) requiring major releases to be included in the import path, any breaking change in the Go client API should then have its major version bumped and (again, if I'm not mistaken) import paths updated.

@lumjjb
Copy link
Contributor

lumjjb commented Dec 14, 2019

Agreed. Seems like it may be good to agree of a certain major & minor version of golang we should use to be consistent.

@AkihiroSuda AkihiroSuda mentioned this issue Dec 14, 2019
4 tasks
@AkihiroSuda
Copy link
Member

AkihiroSuda commented Dec 14, 2019

With Go (afaik) requiring major releases to be included in the import path, any breaking change in the Go client API should then have its major version bumped and (again, if I'm not mistaken) import paths updated.

It is "recommended strategy", but we don't need strictly adhere to?
IIUC, a bunch of projects including Kubernetes adopted go.mod without strictly obeying that rule.

Anyway, we are trying to do our best to establish Go API stability in 1.4 (#3554 ).

@alexellis
Copy link
Contributor

alexellis commented Dec 20, 2019

I seem to be getting an error with dep:

alex@nightshade:~/go/src/github.com/alexellis/faasd$ go version
go version go1.12.14 linux/amd64
alex@nightshade:~/go/src/github.com/alexellis/faasd$ export GO111MODULE=off
alex@nightshade:~/go/src/github.com/alexellis/faasd$ go build
# github.com/alexellis/faasd/vendor/github.com/containerd/containerd/images/archive
vendor/github.com/containerd/containerd/images/archive/reference.go:73:21: undefined: "github.com/alexellis/faasd/vendor/github.com/docker/distribution/reference".ParseDockerRef
alex@nightshade:~/go/src/github.com/alexellis/faasd$ 
[[constraint]]
  name = "github.com/containerd/containerd"
  version = "1.2.0"

[prune]
  go-tests = true
  unused-packages = true

I get the same if I bump to the latest version - https://github.com/containerd/containerd/releases/tag/v1.3.2

@thaJeztah
Copy link
Member

@alexellis it currently depends on a version of docker/distribution from master having this commit: distribution/distribution@0ac367f (the 1.3.2 branch is using this commit: https://github.com/containerd/containerd/blob/v1.3.2/vendor.conf#L59).

dep probably picks the latest tag (v2.7.1), which does not have that patch. I opened a backport for the 2.7.x release branch (distribution/distribution#3002) but it isn't merged yet, and will require a new tag/release so that dep (and go mod) will pick it up automatically.

For now, you can add an override/constraint to your Gopkg.toml to pin the dependency to the same commit as the 1.3.2 version (distribution/distribution@0d3efad)

@AkihiroSuda
Copy link
Member

Mixing up vndr and go mod is becoming harder (containerd/cgroups#139).

Can we soon finish migrating the entire stuff to go mod?
Is there any blocker?

@alexellis
Copy link
Contributor

Thanks @thaJeztah

@estesp
Copy link
Member

estesp commented Jan 9, 2020

Based on this tweet from @ibuildthecloud, it looks like once CRI starts to vendor in k8s 1.17, it gets a lot less messy to use go mod. Maybe @Random-Liu can say when they are ready to move up the CRI project deps and then we can vendor that back in and move to 1.17 and be ready to fully switch to go mod?

Edit: A bit more detail here

@TBBle
Copy link
Contributor

TBBle commented Jan 28, 2020

For the release-branches issue, how about tagging 7f9530d (I think that's the first commit after release/1.3 branched) as v1.4.0-alpha.0? If I understand correctly, that would mean "go modules" in 1.13 would generate psueudo-versions for subsequent commits on master as v1.4.0-alpha.0.<date>-<commitref>, which is SemVer-newer than any tag on the release/1.3 branch could be, and SemVer-older than any v1.4.X tag.

Then transitive dependency resolution would pick the correct newer (master) version, over older (branch) versions.

This is assuming I'm correct, and the problem affecting consumers like buildkit is not that they want to tie to a tag, but that when multiple versions are visible, it's resolving a commit with most-recent-ancestor tag of v1.3.2 (i.e. release-branch), instead of a commit that's from master but has a SemVer-older most-recent-ancestor tag of 1.3.0-beta.2.

It would also prevent users reading their go.mod, seeing 1.3.0-beta.2 (for a master commit) and trying to "upgrade" that to v1.3.2, and wondering why APIs have disappeared.

@g00nix
Copy link

g00nix commented Mar 25, 2020

Running a basic app still fails:

package main

import (
	"log"

	"github.com/containerd/containerd"
)

func main() {
	client, err := containerd.New("/run/containerd/containerd.sock")
	defer client.Close()
}

with the following error:

# github.com/containerd/containerd/images/archive
../../.go/pkg/mod/github.com/containerd/containerd@v1.3.3/images/archive/reference.go:73:21: undefined: "github.com/docker/distribution/reference".ParseDockerRef

@dims
Copy link
Member

dims commented Mar 25, 2020

@gun1x try the following require(s) in your go.mod

module example.com/m

go 1.13

require (
	github.com/containerd/containerd v1.3.3
	github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible // indirect
)

@g00nix
Copy link

g00nix commented Mar 25, 2020

That worked. it would help to have stuff like this in the docs here: https://github.com/containerd/containerd/blob/master/docs/getting-started.md

@ibuildthecloud
Copy link
Contributor

@Random-Liu One of the significant issues of moving to go.mod is that CRI uses the streaming server from k8s.io/kubernetes. Is there any effort to move the streaming server to k8s.io/streaming-server or something similar? Until this happens go.mod in containerd will be a real headache.

@AkihiroSuda AkihiroSuda added this to the 1.5 milestone Aug 4, 2020
@marwan-at-work
Copy link

Any updates here?

Thanks!

@mikebrow
Copy link
Member

dims moved the streaming code... and we are merging cri into containerd... and containerd/cri#1377 (comment)

@zhsj
Copy link
Contributor

zhsj commented Nov 14, 2020

It looks like go mod init github.com/containerd/containerd is happy to run now.

go.mod
module github.com/containerd/containerd

go 1.15

require (
        github.com/BurntSushi/toml v0.3.1
        github.com/Microsoft/go-winio v0.4.15-0.20200908182639-5b44b70ab3ab
        github.com/Microsoft/hcsshim v0.8.10
        github.com/Microsoft/hcsshim/test v0.0.0-20201111181659-f14fc666e78f
        github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4
        github.com/containerd/btrfs v0.0.0-20201111183144-404b9149801e
        github.com/containerd/cgroups v0.0.0-20200824123100-0b889c03f102
        github.com/containerd/console v1.0.1
        github.com/containerd/continuity v0.0.0-20200710164510-efbc4488d8fe
        github.com/containerd/fifo v0.0.0-20201026212402-0724c46b320c
        github.com/containerd/go-cni v1.0.1
        github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328
        github.com/containerd/imgcrypt v1.0.1
        github.com/containerd/nri v0.0.0-20201007170849-eb1350a75164
        github.com/containerd/ttrpc v1.0.2
        github.com/containerd/typeurl v1.0.1
        github.com/containerd/zfs v0.0.0-20200918131355-0a33824f23a2
        github.com/containernetworking/plugins v0.8.6
        github.com/coreos/go-systemd/v22 v22.1.0
        github.com/davecgh/go-spew v1.1.1
        github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c
        github.com/docker/go-metrics v0.0.1
        github.com/docker/go-units v0.4.0
        github.com/emicklei/go-restful v2.9.5+incompatible
        github.com/fsnotify/fsnotify v1.4.9
        github.com/gogo/googleapis v1.4.0
        github.com/gogo/protobuf v1.3.1
        github.com/golang/protobuf v1.4.2
        github.com/google/go-cmp v0.5.1
        github.com/google/uuid v1.1.1
        github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
        github.com/hashicorp/go-multierror v1.0.0
        github.com/imdario/mergo v0.3.10
        github.com/moby/sys/symlink v0.1.0
        github.com/opencontainers/go-digest v1.0.0
        github.com/opencontainers/image-spec v1.0.1
        github.com/opencontainers/runc v1.0.0-rc92
        github.com/opencontainers/runtime-spec v1.0.3-0.20200728170252-4d89ac9fbff6
        github.com/opencontainers/selinux v1.6.0
        github.com/pkg/errors v0.9.1
        github.com/prometheus/client_golang v1.7.1
        github.com/sirupsen/logrus v1.7.0
        github.com/stretchr/testify v1.4.0
        github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2
        github.com/tchap/go-patricia v2.2.6+incompatible
        github.com/urfave/cli v1.22.2
        github.com/willf/bitset v1.1.11 // indirect
        go.etcd.io/bbolt v1.3.5
        golang.org/x/net v0.0.0-20200707034311-ab3426394381
        golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208
        golang.org/x/sys v0.0.0-20201013081832-0aaa2718063a
        google.golang.org/grpc v1.30.0
        gotest.tools/v3 v3.0.2
        k8s.io/api v0.19.2
        k8s.io/apimachinery v0.19.2
        k8s.io/apiserver v0.19.2
        k8s.io/client-go v0.19.2
        k8s.io/component-base v0.19.2
        k8s.io/cri-api v0.19.2
        k8s.io/klog/v2 v2.2.0
        k8s.io/utils v0.0.0-20200729134348-d5654de09c73
)

But I notice some dependencies are bumped. And it seems that the release-tool needs update too.

@dmcgowan
Copy link
Member

@zhsj thanks for testing out. I have tried to make updates in release-tool to better handle the difference representation in dependencies. If you are seeing issues please file it over there. I want to get those fixed early so we don't have that as a headache if we pull go mod into the next release.

@AkihiroSuda
Copy link
Member

Completed in #4760

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

No branches or pull requests