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

libnetwork: Exclude unsupported architectures #9547

Merged
merged 1 commit into from
Aug 10, 2019

Conversation

G-M0N3Y-2503
Copy link
Contributor

@G-M0N3Y-2503 G-M0N3Y-2503 commented Jul 23, 2019

Maintainer: @G-M0N3Y-2503
Compile tested: x86_x64, Hyper-V, OpenWrt Master
Run tested: x86_x64, Hyper-V, OpenWrt Master

Description:
As brought up in #9546, libnetwork was failing to compile for MIPS. Given libnetwork's dependencies on docker-ce libnetwork is unsupported on MIPS and have added @TARGET_x86_64.

@neheb
Copy link
Contributor

neheb commented Jul 23, 2019

This will create a recursive dependency. Remove it from docker-ce.

utils/libnetwork/Makefile Outdated Show resolved Hide resolved
@neheb
Copy link
Contributor

neheb commented Jul 23, 2019

I think one of the reasons that this is failing is because musl defines SIGSTKFLT as 16 for all architectures, except MIPS where it is 7. In other words, this doesn't really fix the issue so much as working around it.

This is probably a go bug. Ping @jefferyto

@jefferyto
Copy link
Member

There are a number of errors in the log (mostly due to old vendor packages):

# runtime/cgo
gcc_mipsx.S: Assembler messages:
gcc_mipsx.S:25: Error: invalid operands `sw $18,12($29)'
...

This would be fixed by adding PKG_USE_MIPS16:=0.

# github.com/docker/libnetwork/vendor/github.com/docker/docker/pkg/signal
src/github.com/docker/libnetwork/vendor/github.com/docker/docker/pkg/signal/signal_linux.go:35:14: undefined: unix.SIGSTKFLT

This was fixed in moby/moby#37491 then moby/libnetwork#2361.

# github.com/docker/libnetwork/vendor/github.com/boltdb/bolt
src/github.com/docker/libnetwork/vendor/github.com/boltdb/bolt/db.go:101:13: undefined: maxMapSize

This was fixed in moby/libnetwork#2268, by switching from boltdb/bolt (which is archived) to etcd-io/bbolt (a fork which has mips support).

# github.com/docker/libnetwork/vendor/github.com/vishvananda/netns
src/github.com/docker/libnetwork/vendor/github.com/vishvananda/netns/netns_linux.go:29:30: undefined: SYS_SETNS

This should be fixed in vishvananda/netns#21 then moby/libnetwork#2119 (for mips and mipsel, mips64el support was added later but not mips64).

# github.com/docker/libnetwork/vendor/github.com/docker/docker/pkg/system
src/github.com/docker/libnetwork/vendor/github.com/docker/docker/pkg/system/stat_linux.go:11:3: cannot use s.Rdev (type uint32) as type uint64 in field value

This they are working on in moby/moby#37490.


Almost all of these errors have already been fixed because the version we are packaging (0.8.0-dev.2) is from 2016. (This was their last "official" release so I understand why we are packaging it.)

I believe the proxy binary bundled with docker is built from a newer commit. Perhaps we should consider packaging the version of libnetwork that is pinned for the version of docker-ce we are targeting?

@G-M0N3Y-2503
Copy link
Contributor Author

Almost all of these errors have already been fixed because the version we are packaging (0.8.0-dev.2) is from 2016. (This was their last "official" release so I understand why we are packaging it.)

I believe the proxy binary bundled with docker is built from a newer commit. Perhaps we should consider packaging the version of libnetwork that is pinned for the version of docker-ce we are targeting?

This is actually packaging the commit you mentioned, I chose 0.8.0-dev.2 as the version as it's the only version they have and the dev implies it's in a partial state anyway, Perhaps a better version, especially for docker-ce v19.03.0 would just be to use the commit hash or append it given no other apparent versions.

The next version I'd package is for would be moby/libnetwork@fc5a7d9 for docker-ce v19.03.0 and the issues that @jefferyto thoroughly documented are still present aside from github.com/boltdb/bolt.

Are we suggesting that we leave the issue open until the upstream fixes are eventually packaged and merge the workaround for now?

@neheb
Copy link
Contributor

neheb commented Jul 24, 2019

If the package can be fixed as @jefferyto stated, I'd prefer that instead of making it unavailable.

utils/libnetwork/Makefile Outdated Show resolved Hide resolved
utils/libnetwork/Makefile Outdated Show resolved Hide resolved
utils/libnetwork/Makefile Outdated Show resolved Hide resolved
@neheb
Copy link
Contributor

neheb commented Aug 4, 2019

This should be taken care of before the docket pull request.

@yousong yousong self-assigned this Aug 6, 2019
@yousong
Copy link
Member

yousong commented Aug 6, 2019

How about we just let libnetwork depends on target x86_64 the same way like dockerd does. It's my understanding that binaries from libnetwork do not have much use when not in the context of dockerd containers.

@jefferyto
Copy link
Member

I'm ok with limiting this to x86-64 for now.

@G-M0N3Y-2503
Copy link
Contributor Author

From what I know of libnetwork and the executables provided by this package they would have limited use outside of docker which is currently unable to be compiled for mips due to another of its dependencies also not currently able to be compiled for mips (containerd).
Due to containerd's usefulness on its own, I'd first focus what little benefit I could bring to that, not having access to mips hardware, then #9546.
If limiting libnetwork to x86-64 brings no positive change we should close this pull request.

@G-M0N3Y-2503 G-M0N3Y-2503 force-pushed the feature_libnetwork branch 2 times, most recently from 9bc3a5e to 674b11b Compare August 10, 2019 04:49
Signed-off-by: Gerard Ryan <G.M0N3Y.2503@gmail.com>