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

testing/prometheus-node-exporter: new aport #4105

Closed
wants to merge 1 commit into from

Conversation

kpcyrd
Copy link
Contributor

@kpcyrd kpcyrd commented Apr 24, 2018

https://github.com/prometheus/node_exporter
Prometheus exporter for hardware and OS metrics exposed by *NIX kernels, written in Go with pluggable metric collectors.

Copy link
Contributor

@andypost andypost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found somehow strange this kind of building golang apps but checked other abuilds and that's a common way

needs work for check() implementation which should be make test

mkdir -p ${builddir%/*}
mv "$srcdir/node_exporter-$pkgver" "$builddir"
cd "$builddir"
default_prepare
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks really strange

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see response on the other comment.

builddir="$srcdir/src/github.com/prometheus/node_exporter"

prepare() {
mkdir -p ${builddir%/*}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When building go, the code needs to be located at a specific location relative to $GOPATH, which is set to $srcdir. That command is creating the parent directory of $builddir and then moving the extracted source to $builddir itself. I don't like this either, but I'm afraid that's the golang way. An alternative could be:

mkdir -p "`dirname "$builddir"`"

which is only slightly less ugly, in my opinion.


check() {
cd "$builddir"
./node_exporter --help 2> /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those tests use features in go that are broken on musl: golang/go#14481

./ttar -C collector/fixtures -x -f collector/fixtures/sys.ttar
touch collector/fixtures/sys/.unpacked
>> running tests
GO15VENDOREXPERIMENT=1 go test -short -race github.com/prometheus/node_exporter github.com/prometheus/node_exporter/collector github.com/prometheus/node_exporter/collector/ganglia
# runtime/race
race_linux_amd64.syso: In function `__sanitizer::InternalAlloc(unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*, unsigned long)':
gotsan.cc:(.text+0x16e1): undefined reference to `__libc_malloc'
race_linux_amd64.syso: In function `__sanitizer::GetArgv()':
gotsan.cc:(.text+0x4543): undefined reference to `__libc_stack_end'
race_linux_amd64.syso: In function `__sanitizer::InternalRealloc(void*, unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
gotsan.cc:(.text+0x7dd0): undefined reference to `__libc_realloc'
race_linux_amd64.syso: In function `__sanitizer::ReExec()':
gotsan.cc:(.text+0xed47): undefined reference to `__libc_stack_end'
race_linux_amd64.syso: In function `__sanitizer::InternalFree(void*, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
gotsan.cc:(.text+0x7438): undefined reference to `__libc_free'
collect2: error: ld returned 1 exit status
FAIL	github.com/prometheus/node_exporter [build failed]
FAIL	github.com/prometheus/node_exporter/collector [build failed]
make: *** [Makefile:72: test] Error 2
>>> ERROR: prometheus-node-exporter*: check failed
>>> ERROR: prometheus-node-exporter: all failed

@andypost andypost added S-changes-requested Some changes have been requested, waiting for contributor A-new Adds a new aport package labels Apr 28, 2018
@tcely
Copy link
Collaborator

tcely commented Jan 30, 2019

This can be closed now that #5516 was merged, correct?

@kpcyrd
Copy link
Contributor Author

kpcyrd commented Jan 30, 2019

Yes :)

@kpcyrd kpcyrd closed this Jan 30, 2019
@kpcyrd kpcyrd deleted the add-prometheus branch January 30, 2019 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-new Adds a new aport package S-changes-requested Some changes have been requested, waiting for contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants