[release/1.7] Fix compile from version control system (source) use case

[austinvazquez]

Issue

Resolves #9969

The module github.com/mitchellh/osext is no longer available via source. The issue is it is currently a indirect dependency via containerd/aufs@v1.0.0 (see containerd/aufs#49 (comment)) and Microsoft/hcsshim@v0.9.10.

Description

This change excludes the github.com/mitchellh/osext indirect dependency to resolve compile from source use case issue due to dependency no longer being available.

Testing

go clean -modcache
export GOPROXY=direct
go mod tidy
go mod vendor

Additional context

• Go mod exclude directive - https://go.dev/doc/modules/gomod-ref#exclude

[k8s-ci-robot]

Hi @austinvazquez. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[estesp]

This seems like a good solution; @thaJeztah you know of any downsides or unintended consequences here?

[thaJeztah]

I honestly had to look up the exclude directive, as I never used it! Here goes; https://go.dev/ref/mod#go-mod-file-exclude

exclude directive

An exclude directive prevents a module version from being loaded by the go command.

Since Go 1.16, if a version referenced by a require directive in any go.mod file is excluded by an exclude directive in the main module’s go.mod file, the requirement is ignored. This may cause commands like go get and go mod tidy to add new requirements on higher versions to go.mod, with an // indirect comment if appropriate.

Before Go 1.16, if an excluded version was referenced by a require directive, the go command listed available versions for the module (as shown with go list -m -versions) and loaded the next higher non-excluded version instead. This could result in non-deterministic version selection, since the next higher version could change over time. Both release and pre-release versions were considered for this purpose, but pseudo-versions were not. If there were no higher versions, the go command reported an error.

exclude directives only apply in the main module’s go.mod file and are ignored in other modules. See Minimal version selection for details.

This StackOverflow post had some explanation as well; https://stackoverflow.com/a/53824026/1811501

By excluding the properly-versioned (but not working) code it causes go mod to fetch from master instead (which is not properly versioned, but has the working code).

Which .. seems to indicate the exclude could affect resolution by upgrading to a newer version than is specified in the go.mod (but with the module going AWOL, I guess that won't be possible).

I'm still confused why go modules is even looking for this dependency; for sure, it may be "somewhere" in the dependency tree, but with both vendor/ being present, the module not being used (see the vendor/ directory, which doesn't contain it) and the "flattened" go.mod dependencies already resolved (// indirect), it confuses me ... why it's even still needed?

I tried reproducing the issue, inside a container, but ... wasn't able to?

git checkout v1.6.30
Note: switching to 'v1.6.30'.
# ...

docker build -t containerd-test -f contrib/Dockerfile.test .

docker run -it --rm containerd-test
root@9412af045bd2:/go/src/github.com/containerd/containerd# make binaries GOPROXY=direct
+ bin/ctr
+ bin/containerd
+ bin/containerd-stress
+ bin/containerd-shim
+ bin/containerd-shim-runc-v1
+ bin/containerd-shim-runc-v2
+ binaries

In the above, source is stored in /go/src/github.com/containerd/containerd, so I was curious if it was GOPATH mode that played a role, so I tried repeating but with the source at a different location, but it still built ok 🤔

docker run -it --rm containerd-test
cd /
root@320cff3c429e:/# mv /go/src/github.com/containerd/containerd /containerd
root@320cff3c429e:/# cd containerd/
root@320cff3c429e:/containerd# make binaries GOPROXY=direct
+ bin/ctr
+ bin/containerd
+ bin/containerd-stress
+ bin/containerd-shim
+ bin/containerd-shim-runc-v1
+ bin/containerd-shim-runc-v2
+ binaries

[thaJeztah]

That said, it's definitely possible something would happen if it were to resolve modules, so I dug a bit deeper, and tried make vendor with GOPROXY=direct and in that case, indeed, it reproduced;

docker run -it --rm containerd-test

make GOPROXY=direct vendor
...
go: downloading github.com/blang/semver v3.5.1+incompatible
go: github.com/containerd/aufs@v1.0.0 requires
	github.com/containerd/containerd@v1.5.0-beta.3 requires
	github.com/Microsoft/hcsshim@v0.8.15 requires
	github.com/containerd/containerd@v1.5.0-beta.1 requires
	github.com/Microsoft/hcsshim/test@v0.0.0-20201218223536-d3e5debf77da requires
	github.com/docker/distribution@v0.0.0-20190905152932-14b96e55d84c requires
	github.com/mitchellh/osext@v0.0.0-20151018003038-5e2d6d41470f: invalid version: git ls-remote -q origin in /go/pkg/mod/cache/vcs/94ed57c5b21c953d93b47487113db43a5c9b69fd990329ec70dc77348c4dd443: exit status 128:
	fatal: could not read Username for 'https://github.com': terminal prompts disabled
Confirm the import path was entered correctly.
If this is a private repository, see https://golang.org/doc/faq#git_https for additional information.
make: *** [Makefile:447: vendor] Error 1

So the issue there is that it (again; I recall we had that before) discovers an old version of containerd (1.5), and now starts traversing those dependencies. ISTR Go maintainers mentioned "that issue is solved, and we now (automatically) use the local (main) module", but ... maybe that's not the case? It's somewhat odd that it goes looking for dependencies for an older version of the main module we're in, to resolve dependencies for that, knowing that those dependencies won't be used for the current version 🤔

All of that doesn't make a lot of sense; if it's following SemVer, then containerd 1.6 (current version of the main module) is compatible with any of the 1.x versions it may discover; it should know here that the minimum required version is now v1.6.30 (version of the main module). What I suspect here is that this is a side effect of the local code never being considered a module (it's only a module if it's downloaded, not when built from source control). So as a result the local version of containerd is (devel);

make binaries GOPROXY=direct
+ bin/ctr
+ bin/containerd
...

go version -m bin/containerd
bin/containerd: go1.21.8
	path	github.com/containerd/containerd/cmd/containerd
	mod	github.com/containerd/containerd	(devel)
	dep	dario.cat/mergo	v1.0.0	h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk=

So... would that mean that .. the main module doesn't have a version and is ignored in version resolution, and therefore the minimum required version of github.com/containerd/containerd needs to be resolved based on the dependency tree?

So, curious about that, I tried adding back the "replace" hack; and ... yes, that works!

docker run -it --rm containerd-test

go mod edit -replace github.com/containerd/containerd=./

make GOPROXY=direct vendor
...
go: found github.com/satori/go.uuid in github.com/satori/go.uuid v1.2.0
all modules verified

So, as the "missing module" is (per above) a consequence of go modules using an old version of the containerd module; but not limited to just that module; it also goes looking for other indirect modules (any of which could go AWOL); perhaps we should exclude that module (i.e., don't use containerd 1.5, and therefore not any of its dependencies);

But unfortunately that doesn't work, because that ignored v1.5.0-beta.3, but likely found another version of containerd v1.5 somewhere in the dependency tree, which now became the "minimum", so now it went looking for v1.5.7;

docker run -it --rm containerd-test

go mod edit -exclude github.com/containerd/containerd@v1.5.0-beta.3

make GOPROXY=direct vendor
...
go: github.com/Microsoft/hcsshim@v0.9.10 requires
	github.com/containerd/containerd@v1.5.7 requires
	github.com/containerd/imgcrypt@v1.1.1 requires
	github.com/containerd/containerd@v1.5.0-rc.0 requires
	github.com/Microsoft/hcsshim@v0.8.15 requires
	github.com/containerd/containerd@v1.5.0-beta.1 requires
	github.com/Microsoft/hcsshim/test@v0.0.0-20201218223536-d3e5debf77da requires
	github.com/docker/distribution@v0.0.0-20190905152932-14b96e55d84c requires
	github.com/mitchellh/osext@v0.0.0-20151018003038-5e2d6d41470f: invalid version: git ls-remote -q origin in /go/pkg/mod/cache/vcs/94ed57c5b21c953d93b47487113db43a5c9b69fd990329ec70dc77348c4dd443: exit status 128:
	fatal: could not read Username for 'https://github.com': terminal prompts disabled
Confirm the import path was entered correctly.
If this is a private repository, see https://golang.org/doc/faq#git_https for additional information.
make: *** [Makefile:447: vendor] Error 1
ro

And exclude requires a version (can't ignore "any" version);

go mod edit -exclude github.com/containerd/container
go: -exclude=github.com/containerd/container: need path@version

Excluding all versions found .. isn't a realistic option here either;

go mod graph | grep ' github.com/containerd/containerd' | wc -l
33

[thaJeztah]

So ...

• This looks to be a similar case as we fixed through a "replace" hack in go.mod: cut circular dependency on github.com/containerd/containerd #5441
• That hack was removed in Build with Go 1.18 #6605 (comment) as it no longer reproduced
• ☝️ ISTR there was a mention of this being because Go fixed this situation (main module automatically leading and determining the "minimum required version", so older versions being pruned during version resolution); but maybe that's not the case?
• ☝️ related to that could be that the main module isn't considered a "module" because it wasn't downloaded as a module, and therefore doesn't have a version .. it's (devel), and only gets a module version when using (e.g.) go install github.com/containerd/containerd ...
• Adding back a replace hack works (but definitely is ugly); benefits of it is that it may be able to skip resolution of all the old version, and therefore transitive dependencies that "once were"
• Adding an exclude for all containerd versions found .. not very realistic as an option.
• ❗ For the github.com/mitchellh/osext dependency, we happen to be lucky, as there's only a single occurence in the dependency tree, and only a single version (otherwise we'd have the same fate)

[thaJeztah]

LGTM

Sorry for the long comments; I was really curious "whyyy go? whyyyy? why again??"

[austinvazquez]

LGTM

Sorry for the long comments; I was really curious "whyyy go? whyyyy? why again??"

Love it. Thanks for the deep dive. Certainly a few TILs for me there. 🤭

From golang/go#51285 (comment), the dependencies need to be go >= 1.17 for module graph pruning. In the case of both containerd/aufs@v1.0.0 and Microsoft/hcsshim@v0.9.10 have go version 1.13. So perhaps that is why the older versions of containerd are not pruned from the dependency graph.