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

Rework service logic #354

Closed
wants to merge 11 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@sciurus
Copy link
Contributor

sciurus commented Sep 17, 2014

I noticed a number of bugs around service configuration. This PR attempts to do a few things

  • separate the code for choosing an init system and configuring the service
  • choose the correct init system
  • generate correct service configurations
  • remove unused templates and better organize the remaining ones
  • improve test coverage

ServerSpec tests are passing on Ubuntu, Fedora, and Debian. CentOS requires more work.

$ kitchen list
Instance            Driver   Provisioner  Last Action
server-ubuntu-1204  Vagrant  ChefSolo     Verified
server-centos-510   Vagrant  ChefSolo     <Not Created>
server-centos-65    Vagrant  ChefSolo     <Not Created>
server-centos-70    Vagrant  ChefSolo     <Not Created>
server-fedora-20    Vagrant  ChefSolo     Verified
server-debian-76    Vagrant  ChefSolo     Verified
@sciurus

This comment has been minimized.

Copy link
Contributor Author

sciurus commented Sep 19, 2014

Looks like I made travis unhappy. Hopefully the last commit is the fix.

$ bundle exec rake style spec
FC024: Consider adding platform equivalents: /Users/brianp/Code/chef-logstash/libraries/logstash_util.rb:47
Running RuboCop...
Inspecting 47 files
...............................................

47 files inspected, no offenses detected
/opt/chefdk/embedded/bin/ruby -S rspec test/unit/spec
....***..........*......*.............

Pending:
  logstash::beaver ubuntu writes some chefspec code
    # todo
    # ./test/unit/spec/beaver_spec.rb:14
  logstash::default ubuntu writes some chefspec code
    # todo
    # ./test/unit/spec/default_spec.rb:14
  logstash::haproxy ubuntu writes some chefspec code
    # todo
    # ./test/unit/spec/haproxy_spec.rb:14
  logstash::pyshipper ubuntu writes some chefspec code
    # todo
    # ./test/unit/spec/pyshipper_spec.rb:14
  logstash::source ubuntu writes some chefspec code
    # todo
    # ./test/unit/spec/source_spec.rb:14

Finished in 2 minutes 59.3 seconds
38 examples, 0 failures, 5 pending

ChefSpec Coverage report generated...

  Total Resources:   14
  Touched Resources: 14
  Touch Coverage:    100.0%

You are awesome and so is your test coverage! Have a fantastic day!
@sciurus

This comment has been minimized.

Copy link
Contributor Author

sciurus commented Sep 23, 2014

Does this look good, or are any changes needed?

@mjuarez

This comment has been minimized.

Copy link

mjuarez commented Sep 26, 2014

Just want to chime in that this branch helped solve an issue with me deploying this cookbook out to fedora-19 boxes. Thanks @sciurus for the work!

Brian Pitts added some commits Sep 27, 2014

Brian Pitts Brian Pitts
Support early upstart versions
If upstart does not natively haev the ability to set the user and group
a service will run as, use sudo instead.
@sciurus

This comment has been minimized.

Copy link
Contributor Author

sciurus commented Sep 27, 2014

I'm glad it was helpful @mjuarez mjuarez.

I've pushed a fix for CentOS 6.5.

@sciurus

This comment has been minimized.

Copy link
Contributor Author

sciurus commented Sep 28, 2014

As an aside, I think the need for all this init logic is a shame and that ohai should do a better job of providing it to us.

I had not heard of pleaserun when I started working on this. I could be convinced that it is the answer if someone puts in the effort to test that it and chef-pleaserun work reliably on the many platforms chef-logstash aims to support.

I'd still hope that this could be merged to improve the current situation.

@paulczar

This comment has been minimized.

Copy link
Collaborator

paulczar commented Sep 28, 2014

I really want to merge this in, just stuck in a bad few weeks for doing anything at all. Hang in there and I'll do my best to review/merge ASAP.

@sciurus

This comment has been minimized.

Copy link
Contributor Author

sciurus commented Sep 28, 2014

I totally understand @paulczar , no rush on my end.

@paulczar paulczar closed this Sep 30, 2014

@paulczar

This comment has been minimized.

Copy link
Collaborator

paulczar commented Sep 30, 2014

merged manually into master! <3

@sciurus

This comment has been minimized.

Copy link
Contributor Author

sciurus commented Sep 30, 2014

🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment