-
Notifications
You must be signed in to change notification settings - Fork 33
Fixes #36973 - Use dnf needs-restarting to collect tracer information #149
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
Conversation
e928ff2
to
1f722a5
Compare
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.
Looks good to me
@parthaa @ianballou could one of y'all take a look? |
Can we get a redmine for this please? |
@ehelms I think https://yum.theforeman.org/client/nightly/el7/x86_64/foreman-client-release.rpm might be leading to the wrong repository:
|
Thanks for the heads up! |
The fix is now deployed and the tests are passing for EL7. |
src/katello/tracer/dnf.py
Outdated
from katello.tracer.SystemdDbus import SystemdDbus | ||
|
||
STATIC_SERVICES = [ | ||
"systemd", |
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.
a060d5b
to
086dc5a
Compare
As an aside, I had to hack my system by copying the |
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.
Based on my comments above, I'd like to know if this issue with session traces is a surprise or not. Perhaps it was tested with a different client package upgrade?
I have tested this some time ago and session traces were recognized by https://github.com/Katello/katello-host-tools/pull/149/files#diff-21052bd00993aa53f643b56c70d49991ad54e23c97da7378b12a0b5b31197e28R36 The is_session is similar to https://github.com/FrostyX/tracer/blob/master/tracer/resources/processes.py#L268 See also this comment: #149 (comment) Ah, maybe I just missed this one to grab: https://github.com/FrostyX/tracer/blob/ff8fc924fcbe2f638dd88b50549813dab2b8595b/tracer/resources/applications.py#L208 |
Wow, I am surprised there is such specialized logic in there for ssh sessions. Nice catch! |
@sbernhard after your update, I'm still seeing the same issue with ssh. The packages I'm playing with are:
Tracer is still showing:
but I'm not seeing the session info yet. Is there any debugging / log statements you'd like me to try to help figure out what's going wrong? |
Added some more handling for ssh sessions https://github.com/Katello/katello-host-tools/pull/149/files#diff-21052bd00993aa53f643b56c70d49991ad54e23c97da7378b12a0b5b31197e28R29 Similar to https://github.com/FrostyX/tracer/blob/master/tracer/resources/processes.py#L79 |
Awesome, I'll give it a re-test |
Just need to check the static type and also test on EL7 to make sure the legacy stuff isn't broken. |
I'm a little confused why Here is the tracer -a output:
Here is the
Also (sd-pam) seems to be an outlier since it's not a service. The old tracer reports it as needing "manual restart". Here's it in systemctl status
|
I'm thinking |
It looks like with old tracer, vs
This is on EL7. |
I think anything that the old tracer puts under the "manual restart" category can be ignored, like (sd-pam) and sudo. |
Co-Authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
I have added a workaround @ianballou |
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.
Looking pretty much perfect! Just one small addition:
Co-authored-by: Ian Ballou <[email protected]>
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.
Okay, looking good to me!
@sbernhard can you please fix the Redmine issue and/or squash? :) |
I took care of it during the squash merge 👍 |
oh right, of course squash fixes that 🙈 |
Discussion, see: https://community.theforeman.org/t/tracer-for-dnf/35633?u=bernhard_suttner