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

Handle old kernels where we can't determine the init system #683

Merged
merged 1 commit into from
Dec 14, 2015

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Dec 13, 2015

New kernels will always have /proc/1/comm, but older kernels don't.
Those systems are all init based (debian 6, ubuntu 10.04, centos 5.11).
A good number of these systems are EoL, but some like CentOS 5.11 are
going to be around for several more years. We should probably support
detecting init on them properly.

New kernels will always have /proc/1/comm, but older kernels don't.
Those systems are all init based (debian 6, ubuntu 10.04, centos 5.11).
A good number of these systems are EoL, but some like CentOS 5.11 are
going to be around for several more years.  We should probably support
detecting init on them properly.
end

init_package package_name
init_package File.exists?("/proc/1/comm") ? File.open("/proc/1/comm").gets.chomp : 'init'
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason you can't/shouldn't just set package_name to init up top and leave the rest of the code untouched? I think it's probably a fair chunk more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not a huge fan of default values that you potentially change later on. Without a comment it's often hard to understand why the initial value is set to something like init. When you explicitly say if this file doesn't have content then use init it becomes a bit more clean to me that you chose init due to another value.

@thommay
Copy link
Contributor

thommay commented Dec 14, 2015

one nit, but essentially 👍

@tas50
Copy link
Contributor Author

tas50 commented Dec 14, 2015

ping @chef/client-core for review

@lamont-granquist
Copy link
Contributor

👍

@lamont-granquist
Copy link
Contributor

there should be similar code in the platform helpers in core chef as well...

tas50 added a commit that referenced this pull request Dec 14, 2015
Handle old kernels where we can't determine the init system
@tas50 tas50 merged commit f928375 into chef:master Dec 14, 2015
@thommay thommay added Type: Bug Does not work as expected. and removed Bug labels Jan 24, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants