-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Promtail: Update debian image and use a newer libsystemd #2957
Conversation
…f the newer OS distros
Apologies if I gave the impression that this was a fully worked out solution @slim-bean. A key question here, and why I held off submitting a pull request myself, is whether this change will break others who are on older systemd versions and I think unfortunately there is a pretty good chance it will. This is why I asked a question in the original issue regarding how we think we should be solving for this - should we be aiming for, for example, a version that builds to a specific docker tag aimed at newer systemd versions? I will test the new version against an older pre-fedora 33 machine now, and get back to you if it crashes out, but even if it doesn't I'm not sure this is going to give me loads of confidence there won't be a regression somewhere. |
Per comment on issue I have tested against older and newer systemd successfully which is good news. Still not 100% confident that the newer systemd API is backwardly compatible and will work on all old system configurations - but that is probably more of a comment on my lack of understanding of the details here. |
I'm less worried about this, the changes indicate that the journals created with newer files can't be read with older code (which we experienced) however they do not indicate any other kind of backward compatibility which I suspect they would have if this were expected. Also some testing on our end and yours at least sanity checks that it still works. I am comfortable merging this and if someone upgrades and experiences problems at least the issues is quickly remedied by rolling back to the previous promtail version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment or two around this for future us? LGTM
Codecov Report
@@ Coverage Diff @@
## master #2957 +/- ##
==========================================
- Coverage 61.88% 61.77% -0.12%
==========================================
Files 182 182
Lines 14867 14867
==========================================
- Hits 9201 9184 -17
- Misses 4820 4837 +17
Partials 846 846
|
Corresponds to grafana/loki#2957
Corresponds to grafana/loki#2957
Corresponds to grafana/loki#2957
* Use a newer libsystemd in Docker containers Corresponds to grafana/loki#2957 * changelog * merge apt-get commands to create less layers
* Use a newer libsystemd in Docker containers Corresponds to grafana/loki#2957 * changelog * merge apt-get commands to create less layers
Fixes #2792, but really thanks to @fifofonix and others in that thread for doing all the hard work!