Feature: Distribution Packages #97

drewstinnett · 2024-08-28T12:02:21Z

This PR adds RPM and Deb packaging from goreleaser, as well as a little systemd file to start and stop the service.

resolves #96

Adding packaging pieces

…metheus-slurm-exporter into feature-distro-packages

drewstinnett · 2024-08-28T12:04:36Z

Oops, need to fix reuse workflow..

NucciTheBoss · 2024-08-28T17:24:38Z

Hah, looks like we had the same idea this week (#96) - this is great! 😆

I published a PPA for Noble and Jammy on Launchpad for the exporter. I've reached out to the Debian Go team about getting the exporter uploaded to Debian. It would be great to have latest builds published to GitHub and a git buildpackage version on Salsa for inclusion into Debian 😄

drewstinnett · 2024-08-28T17:29:23Z

Hah, looks like we had the same idea this week (#96) - this is great! 😆

I published a PPA for Noble and Jammy on Launchpad for the exporter. I've reached out to the Debian Go team about getting the exporter uploaded to Debian. It would be great to have latest builds published to GitHub and a git buildpackage version on Salsa for inclusion into Debian 😄

Oh I totally missed that...good timing!!

abhinavDhulipala · 2024-08-28T19:58:58Z

Howdy! First of all thanks a ton for your contributions. Give me a day or 2 wrap my mind around this MR and get back to you. In the meanwhile, I don't think tracing functionality is supported the way it is packaged right now. Please checkout my latest MR #95 for how to package and resolve the html files to support tracing. In short, we need to package the ./exporter/templates/* html files. In the systemd file and pkgs, we need to support pointing to the proper templated directory using the TRACE_ROOT_PATH env var.

drewstinnett · 2024-08-28T20:07:22Z

Ah yes that makes sense I'll add that in shortly... Thank you!

…ages

drewstinnett · 2024-08-29T13:42:14Z

Ok @abhinavDhulipala, I think the packaging is correct now if you'd like to take a look when you can (No rush here btw, and thank you so much for this awesome project! 🙏)

Just a thought, but how would you feel if I changed the TRACE_ROOT_PATH logic in the code to look in the order:

$TRACE_ROOT_PATH path
. (cwd)
/usr/share/prometheus-slurm-exporter/templates/

I don't think that would break any current workflows, but it would allow users of the distro packages to use tracing without having to specify an additional env variable. If that is something you would be interested in, I'd be happy to make that change as well as part of this PR. No worries if you'd like to keep it the same though too.

Thank you again!

…ages

abhinavDhulipala · 2024-09-02T02:50:47Z

Howdy @drewstinnett that trace path feature looks like a great feature, but we could also configure the environment variable of the systemd file to fill in the $TRACE_PATH to /usr/share/prometheus-slurm-exporter/templates. If we were to implement the scheme above, I think we should implement it in the following way:

If TRACE_ROOT_PATH is specified, search that directory. If we don't find a templates dir, let's panic and crash the program.
If TRACE_ROOT_PATH isn't specified, we can search cwd and /usr/share/prometheus-slurm-exporter.

Now, I tried to install debian found on your forks release page. I did the following in case you want to repro:

docker run -ti --rm --platform linux/amd64 ubuntu:22.04 bash
$ apt update -y && apt-get install wget -y && mkdir /workspace
$ cd /workspace
$ wget https://github.com/drewstinnett/prometheus-slurm-exporter/releases/download/v0.0.3/prometheus-slurm-exporter_0.0.3_linux_amd64.deb
$  apt-get install -f prometheus-slurm-exporter_0.0.3_linux_amd64.deb
# unfortunately got the following
$  stat /usr/share/prometheus-slurm-exporter/         
stat: cannot statx '/usr/share/prometheus-slurm-exporter/': No such file or directory

It doesn't look like the templates were packaged. Is this stale perhaps?

Thanks again for all your hard work!

drewstinnett · 2024-09-03T12:03:13Z

Thank you @abhinavDhulipala, I will work on these fixes/changes!

* archive static templates with binary * update docs

…metheus-slurm-exporter into feature-distro-packages

Adding packaging pieces Testing goreleaser and re-adding license Fixing whitespace Adding in trace path detection logic Testing goreleaser Adding license bits Oops forgot conflict Adding trace templates to distro package Fixing spacing

…metheus-slurm-exporter into feature-distro-packages

drewstinnett · 2024-09-03T14:46:53Z

Working on the trace logic...quick question for you @abhinavDhulipala, should the app do a 'deep' search of the TRACE_ROOT_PATH for the 'templates' directory, or do we always expect it to be right under the top path? For example:

if TRACE_ROOT_PATH is set to "/srv", should "/srv/subdir/templates" be considered valid, or would we only want to look at "/srv/templates"? Either is fine with me, just wanted to check the intended behavior. Thank you!

abhinavDhulipala · 2024-09-03T15:02:54Z

I have it now as a recursive search, but I think that could be a potential bottleneck if configured wrong. Let's modify it to simply checking for the existence of the one html file we have now.

drewstinnett · 2024-09-03T20:04:11Z

Ok, whenever you get a chance, can you give it another review? I think I was mis-using the github-release command, which appears to have generated the packages based off of an older tag. I think this is fixed now. I verified with:

$ goreleaser release --clean --snapshot
...
$ dpkg-deb -c dist/prometheus-slurm-exporter_1.6.5-SNAPSHOT-0969bb7_linux_amd64.deb
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/bin/
-rwxr-xr-x root/root  10014872 2024-09-03 10:02 ./usr/bin/prometheus-slurm-exporter
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/lib/
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/lib/systemd/
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/lib/systemd/system/
-rw-r--r-- root/root       250 2024-09-03 09:35 ./usr/lib/systemd/system/prometheus-slurm-exporter.service
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/share/
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/share/prometheus-slurm-exporter/
drwxr-xr-x root/root         0 2024-09-03 10:02 ./usr/share/prometheus-slurm-exporter/templates/
-rw-r--r-- root/root       927 2024-08-27 10:47 ./usr/share/prometheus-slurm-exporter/templates/proc_traces.html

The logic for the detection is also in place now

$ LOGLEVEL=debug prometheus-slurm-exporter -trace.enabled=true
time=2024-09-03T12:22:40.135-04:00 level=INFO msg="trace path enabled at path: :9092/trace"
time=2024-09-03T12:22:40.136-04:00 level=DEBUG msg="using trace template path: /usr/share/prometheus-slurm-exporter/templates"
time=2024-09-03T12:22:40.136-04:00 level=INFO msg="serving metrics at :9092/metrics"
…
$ LOGLEVEL=debug TRACE_ROOT_PATH=/tmp/empty-dir prometheus-slurm-exporter -trace.enabled=true
time=2024-09-03T12:23:46.371-04:00 level=INFO msg="trace path enabled at path: :9092/trace"
panic: TRACE_ROOT_PATH must include a directory called: templates
…

Happy to make whatever tweaks/changes you suggest in addition to these as well. Thank you!

abhinavDhulipala

The biggest takeaway here is that I've found it cumbersome to install this in containers which I often use for deployment. Let me know what you think!

EDIT: I've also validated this within a docker container to ensure that your branch works :).

.goreleaser.yaml

abhinavDhulipala · 2024-09-05T23:55:16Z

packaging/postinstall.sh

+systemctl daemon-reload
+systemctl enable --now prometheus-slurm-exporter.service


I'm conflicted on whether we should install a systemctl function by default. I personally won't be using this method of deployment and when I try to develop and test out of containers this gets in the way quite a bit. I'd suggest remove this entirely. We could guard with the following: [[ -d /run/systemd/system ]] && ..., but I still would prefer to drop this and just put this as documentation. Perhaps a curlable snippet of some kind.

I understand what you mean...I'll make this update removing the systemd components from the packaging and include a snippet in the documentation that can be copy/pasted. Thank you for that feedback!

Ok, systemd bits have been removed from the packaging, and I added a little copy/pastable snippet to the README for folks that would like to enable it as a systemd service. Look ok? Thank you!

abhinavDhulipala · 2024-09-06T07:07:08Z

exporter/trace_test.go

+func TestDetectTraceRootPath_Env(t *testing.T) {
+	os.Clearenv()
+	testDir := t.TempDir()
+	t.Setenv("TRACE_ROOT_PATH", testDir)
+	// Ensure that the function panics if given a TRACE_ROOT_PATh with no 'templates' subdirectory
+	assert.PanicsWithValue(t, "TRACE_ROOT_PATH must include a directory called: templates", func() { detectTraceTemplatePath() })
+	require.NoError(t, os.Mkdir(filepath.Join(testDir, templateDirName), 0o700))
+
+	// Now that we have a 'templates' subdir, it should no longer panic
+	assert.Equal(t, filepath.Join(testDir, templateDirName), detectTraceTemplatePath())
+}


This is an awesome test! Great work.

Fixing bad copy/paste, oops! Co-authored-by: abhinavDhulipala <[email protected]>

…metheus-slurm-exporter into feature-distro-packages

Fixing bad copy/paste, oops! Co-authored-by: abhinavDhulipala <[email protected]> make systemctl edit copy/pastable

…metheus-slurm-exporter into feature-distro-packages

abhinavDhulipala

This looks awesome, thanks a ton for all your hard work once again. I'll cut a release on main shortly

drewstinnett added 4 commits August 27, 2024 12:05

Testing goreleaser

a9d2e92

Adding packaging pieces

0f38d02

Testing goreleaser

ecd9692

Adding packaging pieces

Merge branch 'feature-distro-packages' of github.com:drewstinnett/pro…

9a5ca56

…metheus-slurm-exporter into feature-distro-packages

drewstinnett marked this pull request as draft August 28, 2024 12:04

drewstinnett added 3 commits August 28, 2024 08:11

Adding license bits

f0dfa28

Fixing newline

309c302

Fixing whitespace

43b8473

drewstinnett marked this pull request as ready for review August 28, 2024 12:18

drewstinnett added 4 commits August 29, 2024 09:03

Merge remote-tracking branch 'upstream/main' into feature-distro-pack…

c2db4f7

…ages

Oops forgot conflict

aac8ae0

Adding trace templates to distro package

5d43746

Fixing spacing

2c9e0ee

abhinavDhulipala requested a review from tbroch-rv August 30, 2024 00:48

Merge remote-tracking branch 'upstream/main' into feature-distro-pack…

dd8fc17

…ages

drewstinnett added 7 commits September 3, 2024 09:18

Testing goreleaser and re-adding license

7d46236

Fixing whitespace

b6c27ff

[ci] archive static templates with binary (rivosinc#95)

fec2288

* archive static templates with binary * update docs

Adding in trace path detection logic

09270ee

Merge branch 'feature-distro-packages' of github.com:drewstinnett/pro…

f62d0b2

…metheus-slurm-exporter into feature-distro-packages

Testing goreleaser

5a66f9c

Adding packaging pieces Testing goreleaser and re-adding license Fixing whitespace Adding in trace path detection logic Testing goreleaser Adding license bits Oops forgot conflict Adding trace templates to distro package Fixing spacing

Merge branch 'feature-distro-packages' of github.com:drewstinnett/pro…

0969bb7

…metheus-slurm-exporter into feature-distro-packages

trace logic

f4db489

abhinavDhulipala requested changes Sep 6, 2024

View reviewed changes

drewstinnett and others added 5 commits September 6, 2024 11:33

Update .goreleaser.yaml

389d62a

Fixing bad copy/paste, oops! Co-authored-by: abhinavDhulipala <[email protected]>

Removing systemd files for dist packages

969b36d

Merge branch 'feature-distro-packages' of github.com:drewstinnett/pro…

c4208b2

…metheus-slurm-exporter into feature-distro-packages

Update .goreleaser.yaml

5319759

Fixing bad copy/paste, oops! Co-authored-by: abhinavDhulipala <[email protected]> make systemctl edit copy/pastable

Merge branch 'feature-distro-packages' of github.com:drewstinnett/pro…

ca0577c

…metheus-slurm-exporter into feature-distro-packages

drewstinnett requested a review from abhinavDhulipala September 9, 2024 14:43

abhinavDhulipala approved these changes Sep 10, 2024

View reviewed changes

abhinavDhulipala merged commit e285f3e into rivosinc:main Sep 10, 2024
1 check passed

magnuslarsen mentioned this pull request Sep 23, 2024

.deb package does not contain systemd unit file #102

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Distribution Packages #97

Feature: Distribution Packages #97

drewstinnett commented Aug 28, 2024 •

edited by abhinavDhulipala

Loading

drewstinnett commented Aug 28, 2024

NucciTheBoss commented Aug 28, 2024

drewstinnett commented Aug 28, 2024

abhinavDhulipala commented Aug 28, 2024

drewstinnett commented Aug 28, 2024

drewstinnett commented Aug 29, 2024

abhinavDhulipala commented Sep 2, 2024 •

edited

Loading

drewstinnett commented Sep 3, 2024

drewstinnett commented Sep 3, 2024

abhinavDhulipala commented Sep 3, 2024

drewstinnett commented Sep 3, 2024

abhinavDhulipala left a comment •

edited

Loading

abhinavDhulipala Sep 5, 2024

drewstinnett Sep 6, 2024

drewstinnett Sep 9, 2024

abhinavDhulipala Sep 6, 2024

abhinavDhulipala left a comment •

edited

Loading

		systemctl daemon-reload
		systemctl enable --now prometheus-slurm-exporter.service

Feature: Distribution Packages #97

Feature: Distribution Packages #97

Conversation

drewstinnett commented Aug 28, 2024 • edited by abhinavDhulipala Loading

drewstinnett commented Aug 28, 2024

NucciTheBoss commented Aug 28, 2024

drewstinnett commented Aug 28, 2024

abhinavDhulipala commented Aug 28, 2024

drewstinnett commented Aug 28, 2024

drewstinnett commented Aug 29, 2024

abhinavDhulipala commented Sep 2, 2024 • edited Loading

drewstinnett commented Sep 3, 2024

drewstinnett commented Sep 3, 2024

abhinavDhulipala commented Sep 3, 2024

drewstinnett commented Sep 3, 2024

abhinavDhulipala left a comment • edited Loading

Choose a reason for hiding this comment

abhinavDhulipala Sep 5, 2024

Choose a reason for hiding this comment

drewstinnett Sep 6, 2024

Choose a reason for hiding this comment

drewstinnett Sep 9, 2024

Choose a reason for hiding this comment

abhinavDhulipala Sep 6, 2024

Choose a reason for hiding this comment

abhinavDhulipala left a comment • edited Loading

Choose a reason for hiding this comment

drewstinnett commented Aug 28, 2024 •

edited by abhinavDhulipala

Loading

abhinavDhulipala commented Sep 2, 2024 •

edited

Loading

abhinavDhulipala left a comment •

edited

Loading

abhinavDhulipala left a comment •

edited

Loading