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

Patch service to default to systemd on Debian 9 #720

Closed
wants to merge 1 commit into from

Conversation

hashar
Copy link
Contributor

@hashar hashar commented Sep 6, 2018

Puppet 4.8.x systemd provider does not recognize Debian 9 (Stretch), it
thus fall back to runit and the spec fails with:

Could not find the daemon directory (tested [/etc/sv,/var/lib/service])

Tested using:

PUPPET_GEM_VERSION='4.8.2' bundle install
bundle exec rspec ./spec/classes/relationship__titles_spec.rb

References:
puppetlabs/puppet@d5a69fb
https://phabricator.wikimedia.org/T203645

Puppet 4.8.x systemd provider does not recognize Debian 9 (Stretch), it
thus fall back to runit and the spec fails with:

  Could not find the daemon directory (tested [/etc/sv,/var/lib/service])

Tested using:

 PUPPET_GEM_VERSION='4.8.2' bundle install
 bundle exec rspec ./spec/classes/relationship__titles_spec.rb

References:
puppetlabs/puppet@d5a69fb
https://phabricator.wikimedia.org/T203645
@hashar
Copy link
Contributor Author

hashar commented Sep 6, 2018

Debian Stretch comes with puppet 4.8.2, the Debian package has been patched but the gem from rubygems.org is not.

A potential fix is to add to every spec_helpers:

service_type = Puppet::Type.type(:service)
service_type.provider_hash[:systemd].defaultfor :operatingsystem => :debian

I would rather have it handled by rspec-puppet :]

# https://github.com/puppetlabs/puppet/commit/d5a69fb57c15683873b422cb5e17ef06ca13cea5
if Puppet::Util::Package.versioncmp(Puppet.version, '4.8.0') >= 0 && Puppet::Util::Package.versioncmp(Puppet.version, '4.10.0') < 0
Puppet::Type.type(:service).provide(:systemd).instance_eval do
defaultfor :operatingsystem => :debian

Choose a reason for hiding this comment

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

This condition is incorrect, as it will make tests for Debian < 8.0 fail or be inaccurate (those systems don't use systemd).

Also, the condition set in puppet 4.10.x is still wrong and overridden in the debian pacakges to this day.

The correct condition would probably be:

  • Monkey-patch all versions that include a systemd provider and offer the defaultfor method for providers
  • Add a specific defaultfor to the "debian" provider for debian versions up to 7 (you can limit yourself to 5,6,7)
  • Add this defaultfor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And on further discussion, @lavagetto pointed that the patch modifies the behavior of the puppet interpreter. That could potentially hide an issue when running puppet from Gem, but I guess everyone just use the patched up puppet debian package.

@TheMeier
Copy link

We see the same problem is there an open issue to this PR?

@DavidS
Copy link
Collaborator

DavidS commented Nov 9, 2020

I'm very uncomfortable with this change as it is modifying the underlying puppet's behaviour. Since these test results seem to reflect reality (wrong behaviour on debian), please do patch your puppet version to not exhibit buggy behaviour, or upgrade to a version that is actually supported and maintained.

@DavidS DavidS closed this Nov 9, 2020
@DavidS DavidS self-assigned this Nov 9, 2020
@hashar
Copy link
Contributor Author

hashar commented Nov 9, 2020

The Puppet version shipped by Debian is patched. The issue lies in the Puppet 4.8 / 4.10 rubygems which, being no more supported at the time, lacked the bit to flag systemd as the default. When running tests with a Gemfile to install Puppet, that leads to the failure. If I remember properly, we ended up using monkey patching and this pull request is merely to upstream our monkey patch.

Then Stretch / Puppet 4.x are old, so probably we should not bother with this PR anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants