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

test failure #273

Closed
dkwo opened this issue Dec 4, 2023 · 24 comments
Closed

test failure #273

dkwo opened this issue Dec 4, 2023 · 24 comments
Labels
Incomplete Waiting on more information from reporter

Comments

@dkwo
Copy link
Contributor

dkwo commented Dec 4, 2023

Trying to build and package incus for void linux void-linux/void-packages#47573 , I noticed the warnings

objdump: /destdir//incus-0.3.0/usr/bin/sysinfo: .gnu.version_r invalid entry
objdump: warning: private headers incomplete: bad value
objdump: /destdir//incus-0.3.0/usr/bin/incus-migrate: .gnu.version_r invalid entry
objdump: warning: private headers incomplete: bad value
objdump: /destdir//incus-0.3.0/usr/bin/incus-agent: .gnu.version_r invalid entry
objdump: warning: private headers incomplete: bad value

as well as encountered a test failure (after many passing tests)

PASS
ok  	github.com/lxc/incus/shared/validate	(cached)
?   	github.com/lxc/incus/test/dev_incus-client	[no test files]
?   	github.com/lxc/incus/test/syscall/sysinfo	[no test files]
FAIL

Could you help debug this?

@stgraber
Copy link
Member

stgraber commented Dec 4, 2023

Hmm, how exactly are you building those?

Can you try a standard make from the top directory on Void to make sure that things also misbehave in that case?

The sysinfo binary is a test binary used in our testsuite, it definitely should not be built as part of a package.

The incus-agent failure makes it sound like it's not being built statically which would be a big issue.

Similarly, incus-migrate is usually not included in packages as its goal is to migrate a remote system into Incus, so it should be a static binary that can be retrieved by the user on the source system.

@stgraber stgraber added the Incomplete Waiting on more information from reporter label Dec 4, 2023
@dkwo
Copy link
Contributor Author

dkwo commented Dec 4, 2023

I'm building them as in here void-linux/void-packages@5d82020 with

go_import_path=github.com/lxc/incus
go_build_tags=libsqlite3
go_package="${go_import_path}/..."

and

export CGO_LDFLAGS_ALLOW="(-Wl,-wrap,pthread_create)|(-Wl,-z,now)"
export GOFLAGS="-buildmode=pie"

Notice that the objdump warnings are at the end, when the build script generates runtime dependences, not in the build phase, and do not prevent the building of the package. The test failures (in do_check phase) happen before those.

The test fails in the same way if I use make and make check:

PASS
ok  	github.com/lxc/incus/shared/validate	(cached)
?   	github.com/lxc/incus/test/dev_incus-client	[no test files]
?   	github.com/lxc/incus/test/syscall/sysinfo	[no test files]
FAIL

and also here I see the objdump warnings (if i skip tests).
This is with 0.3 version and go 1.21.4
Thanks for helping.

@dkwo
Copy link
Contributor Author

dkwo commented Dec 4, 2023

As for incus-agent, CGO_ENABLED=0 go install -p "${XBPS_MAKEJOBS}" -v -tags agent,netgo "${go_import_path}/cmd/incus-agent" gives the same objtool warning.

@dkwo
Copy link
Contributor Author

dkwo commented Dec 4, 2023

if instead i use
CGO_LDFLAGS="${CGO_LDFLAGS} -static" go install -p "${XBPS_MAKEJOBS}" -v -tags agent,netgo "${go_import_path}/cmd/incus-agent" i get warnings

# github.com/lxc/incus/cmd/incus-agent
/usr/bin/ld: /tmp/go-link-3559558471/000019.o: in function `_cgo_6f668e16310a_Cfunc_mygetgrouplist':
getgrouplist_unix.cgo2.c:(.text+0x1e): warning: Using 'getgrouplist' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3559558471/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetgrgid_r':
cgo_lookup_cgo.cgo2.c:(.text+0x40): warning: Using 'getgrgid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3559558471/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetgrnam_r':
cgo_lookup_cgo.cgo2.c:(.text+0xe1): warning: Using 'getgrnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3559558471/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetpwnam_r':
cgo_lookup_cgo.cgo2.c:(.text+0x186): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3559558471/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetpwuid_r':
cgo_lookup_cgo.cgo2.c:(.text+0x235): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking

but then objtool does not complain, and i get a static executable
Stripped static executable: /usr/bin/incus-agent
Is this acceptable?

@dkwo
Copy link
Contributor Author

dkwo commented Dec 4, 2023

the test failure is still there though.

@stgraber
Copy link
Member

stgraber commented Dec 4, 2023

Pretty sure you don't want go_package="${go_import_path}/..." as that's making it build every single binary in the entire code tree, including those test binaries and other undesirable things.

@stgraber
Copy link
Member

stgraber commented Dec 4, 2023

What's the output of go version on that system?

@dkwo
Copy link
Contributor Author

dkwo commented Dec 4, 2023

go version go1.21.4 linux/amd64

@dkwo
Copy link
Contributor Author

dkwo commented Dec 4, 2023

what do you suggest then, besides /cmd/{incus,fuidshift} ?

@CameronNemo
Copy link

CameronNemo commented Dec 4, 2023

@dkwo the current Void LXD package ships these binaries:

/usr/bin/fuidshift
/usr/bin/lxc
/usr/bin/lxc-to-lxd
/usr/bin/lxd
/usr/bin/lxd-agent
/usr/bin/lxd-benchmark
/usr/bin/lxd-migrate
/usr/bin/lxd-user

Seems like the matching commands are:

  • fuidshift
  • incus
  • lxc-to-incus
  • incusd
  • incus-agent
  • incus-benchmark
  • incus-migrate
  • incus-user

We also probably want: cmd/lxd-to-incus

Edit: Not sure if we can just use go_package="${go_import_path}/cmd/..."

@stgraber
Copy link
Member

stgraber commented Dec 5, 2023

https://linuxcontainers.org/incus/docs/main/packaging/

@dkwo
Copy link
Contributor Author

dkwo commented Dec 5, 2023

Thanks. The building part now looks better:

   Stripped position-independent executable: /usr/bin/incus-migrate
   Stripped position-independent executable: /usr/bin/incus-user
   Stripped static executable: /usr/bin/incus-agent
   Stripped position-independent executable: /usr/bin/incusd
   Stripped position-independent executable: /usr/bin/fuidshift
   Stripped position-independent executable: /usr/bin/lxc-to-incus
   Stripped position-independent executable: /usr/bin/incus-benchmark
   Stripped position-independent executable: /usr/bin/incus

I'm still testing with

go test -v -tags libsqlite3 ./...
cd test && ./main.sh

Is this the wrong way? the test failure happens during go test.

@stgraber
Copy link
Member

stgraber commented Dec 5, 2023

Is this the wrong way? the test failure happens during go test.

You probably don't want to run the cd test && ./main.sh part as that's a very very heavy testsuite which is better run on a dedicated system. That's what we run upstream for all pull requests and releases.

go test -v -tags libsqlite3 ./... should be fine though.

Can you show the full output you're getting? It could be that it's a somewhat unrelated piece of code like lxc-to-incus that's failing due to the system it's being run on.

@dkwo
Copy link
Contributor Author

dkwo commented Dec 5, 2023

Got it. The build indeed is inside a chroot. I attach the full log. incus.log
Is there a way to make it more verbose?
As a side note, I've not yet added cmd/lxd-to-incus to the build, not sure if relevant.

@stgraber
Copy link
Member

stgraber commented Dec 6, 2023

Right, so it's the tests in lxc-to-incus which is failing.
I've managed to reproduce this here though it's not hitting us in CI.
I'll look at what needs updating in that test to have it behave.

@stgraber
Copy link
Member

stgraber commented Dec 6, 2023

I tracked it down, it's because you're missing the lxc-busybox template script.

On most systems, that should be located at /usr/share/lxc/templates/lxc-busybox and be provided by the lxc package or something like it.

You likely are just missing some kind of dependency to bring that in in your case.

Alternatively, you can skip that particular test with:

go test -v -skip TestConvertNetworkConfig ./...

@dkwo
Copy link
Contributor Author

dkwo commented Dec 6, 2023

Thanks a lot! If I skip that test, it passes all checks.
Our lxc has

$ xlocate lxc-busybox 
xlocate: database outdated, please run xlocate -S.
lxc-5.0.1_3  /usr/share/lxc/templates/lxc-busybox

but even if I add lxc to checkdepends, the test still fails. Let's not worry about that though.
I may bother you later again when I look into the runit service for incus.

@dkwo dkwo closed this as completed Dec 6, 2023
@dkwo
Copy link
Contributor Author

dkwo commented Jan 6, 2024

@stgraber I now have a working PR for incus on void linux at void-linux/void-packages#47573
Do you mind taking a look at the build template and runit services, to see if you spot something strange?

All tests pass, except for TestConvertNetworkConfig (even with lxc in checkdepends, which contains the

$ xbps-query -Rf lxc | grep lxc-bus
/usr/share/lxc/templates/lxc-busybox

is this worth investigating?

One last question: since void folks seem to prefer to keep around lxd for a while, I can make it conflict with incus. However, both require raft. Current raft is at 0.16 in void and lxd at 5.9, so my question: What is the minimal raft version required to build incus and cowsql? My pr packages 0.19.1 from the forked repo, but it seems arch linux can make it work with 0.18 from the old repo (they have a more recent lxd though).

@stgraber
Copy link
Member

stgraber commented Jan 6, 2024

is this worth investigating?

Nah, I wouldn't bother. It's a pretty finicky test which validates the behavior of lxc-to-incus. That's unlikely to affect many users and those that do care will have a working LXC setup so won't face any issues.

One last question: since void folks seem to prefer to keep around lxd for a while, I can make it conflict with incus. However, both require raft. Current raft is at 0.16 in void and lxd at 5.9, so my question: What is the minimal raft version required to build incus and cowsql? My pr packages 0.19.1 from the forked repo, but it seems arch linux can make it work with 0.18 from the old repo (they have a more recent lxd though).

I would try to avoid having Incus and LXD conflict with each other as you need both of them installed at the same time to transition the data with lxd-to-incus (once it supports Void).

cowsql/raft is made to remain compatible with LXD's own use of raft, so you should be able to update the Linux distribution as a whole to use cowsql/raft as the source for the raft package and then have both cowsql and dqlite use that (which are then used for both Incus and LXD).
That's the approach taken by Debian and a few other distros. Others went with instead packaging cowsql/raft as a new raft-cowsql package to keep things fully separate.

@dkwo
Copy link
Contributor Author

dkwo commented Jan 6, 2024

btw, a bunch of warning: Using 'getgrouplist' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking or similar are expected, when building statically incus-migrate and -agent, right?

@stgraber
Copy link
Member

stgraber commented Jan 6, 2024

I've never seen those warnings here, but so long as the resulting binary is indeed static, it "should" be fine. You'd likely know pretty quickly if something is wrong by launching a few VMs and testing the incus-agent that way.

@dkwo
Copy link
Contributor Author

dkwo commented Jan 6, 2024

Many thanks.
If I use
CGO_ENABLED=0 go install -p "${XBPS_MAKEJOBS}" -v -tags netgo "${go_import_path}/cmd/incus-migrate"
then it produces

$ file /usr/bin/incus-migrate
/usr/bin/incus-migrate: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, Go BuildID=NULhYQLFcaMJJlQxQ91h/BBZ330aZF_H2iGDRBO3P/2DGG0TJ4fvin5JyuzXSd/Ehfe69fAoTOORCC8uEyH, stripped
$ ldd /usr/bin/incus-migrate 
	statically linked

as well as (possibly due to our outdated binutils 2.39)

objdump: /destdir//incus-tools-0.4.0/usr/bin/incus-migrate: .gnu.version_r invalid entry
objdump: warning: private headers incomplete: bad value

when building. While if i use
CGO_LDFLAGS="-static" go install -p "${XBPS_MAKEJOBS}" -v -tags agent,netgo -ldflags "${go_ldflags}" "${go_import_path}/cmd/incus-agent" then i get

$ ldd /usr/bin/incus-agent 
	not a dynamic executable
$ file /usr/bin/incus-agent
/usr/bin/incus-agent: ELF 64-bit LSB executable, x86-64, version 1 (GNU/Linux), statically linked, BuildID[sha1]=e5e5b16c660e2201b3b4d5d049545e96a7f7111a, for GNU/Linux 3.2.0, stripped

and the warnings I mentioned during build time:

=> incus-0.4.0_1: running post_build ...
# github.com/lxc/incus/cmd/incus-agent
/usr/bin/ld: /tmp/go-link-3158855029/000019.o: in function `_cgo_6f668e16310a_Cfunc_mygetgrouplist':
getgrouplist_unix.cgo2.c:(.text+0x1e): warning: Using 'getgrouplist' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3158855029/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetgrgid_r':
cgo_lookup_cgo.cgo2.c:(.text+0x40): warning: Using 'getgrgid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3158855029/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetgrnam_r':
cgo_lookup_cgo.cgo2.c:(.text+0xe1): warning: Using 'getgrnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3158855029/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetpwnam_r':
cgo_lookup_cgo.cgo2.c:(.text+0x186): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: /tmp/go-link-3158855029/000018.o: in function `_cgo_6f668e16310a_Cfunc_mygetpwuid_r':
cgo_lookup_cgo.cgo2.c:(.text+0x235): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking

Maybe my best option is 2nd one?

@stgraber
Copy link
Member

stgraber commented Jan 6, 2024

Yeah, I guess so, we definitely don't want anything dynamic in there since it needs to run in an unknown environment (the VM).

@dkwo
Copy link
Contributor Author

dkwo commented Jan 7, 2024

Got it. I think in my case I have to be careful that building everything with -buildmode=pie does not spoil static executables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Incomplete Waiting on more information from reporter
Development

No branches or pull requests

3 participants