-
Notifications
You must be signed in to change notification settings - Fork 498
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
(FACT-852) Support for systemd and systemd_version #871
Conversation
end | ||
|
||
Facter.add(:systemd_version) do | ||
confine :kernel => :linux |
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.
Should this additionally be confined to :systemd => true
?
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.
@daenney you're right. Fixed.
This commit adds support for systemd and systemd_version facts.
CLA signed by all contributors. |
init_process_name = Facter::Core::Execution.exec('ps -p 1 -o comm=') | ||
if init_process_name.eql? 'systemd' | ||
result = true | ||
end |
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.
I think you just want to do:
setcode do
...
init_process_name == 'systemd'
end
In other words, the value of the fact should be the last value of the setcode block. If you're on a systemd system, then the last value is true
due to evaluating result = true
, e.g.
irb(main):001:0> result = true
=> true
If you're not on systemd, then the value is actually coming from the if
check:
irb(main):002:0> if 1 == 2
irb(main):003:1> end
=> nil
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.
Actually, wouldn't we want this instead?
setcode do
if Facter::Core::Execution.exec('ps -p 1 -o comm=').eql? 'systemd'
true
else
false
end
end
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.
or just
setcode do
Facter::Core::Execution.exec('ps -p 1 -o comm=') == 'systemd'
end
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.
Oh good point!
Thanks for the contribution! However, we likely aren't going to do any more releases from the master branch of facter. This looks like work that should be targeted at Facter 3, our native implementation in C++. @MikaelSmith do you have any input on this? Backstory: https://groups.google.com/forum/#!msg/puppet-dev/tBqHZB9M308/QqdaldMsJlkJ |
Yes, this will need an additional ticket to cover Facter 3. |
We plan to make that more obvious soon by moving the Facter 3 codebase to facter#master. |
Agreed. I'd target this at Facter 3. And yes, the codebase move will make this all more obvious. Coming soon! For backstory: https://groups.google.com/forum/#!msg/puppet-dev/tBqHZB9M308/QqdaldMsJlkJ |
One other thing: I'd add these two facts as a single new structured fact. |
How would you name it then though? One is binary, one is data:
It feels awkward. |
Hmm, well, naming is hard, but:
actually makes sense to me. And then in code you could reference:
Although ... would it be worth generalizing it to
|
|
Per the existing discussion on this ticket and the concensus in the PR triage discussion meeting, I'm going to close this. New features should be targeted at cfacter as we get ready to release facter 3. |
Just an update to this, we moved the CFacter code into the facter/master branch. So this work should be targeted there. |
This commit adds support for systemd and systemd_version facts.