-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add parameters for configuring package and service #31
Conversation
Updated the global Package resource ensure attribute to look at the $autofs::package_ensure parameter. This is to add customizability, as well as, prepare for the class change to private.
Now that all the params are placed in the init.pp and the package.pp and service.pp files look to the params in the init.pp, the old tests were no longer passing. I removed the package_spec and service_spec. Any tests in those files not already covered in the autofs_spec have been added to the autofs_spec.
Removed the hasstatus and hasrestart parameters from init as they are really unnecessary to the autofs service, which by installing the package will already have a hasstatus and hasrestart set to true.
Updated the osfamily and operatingsystem fact calls to use the facts hash instead of direct calls.
The osfamily fact lookup has been replaced by the lookup for facts['os']['family'].
The autofs_spec tests now use on_supported_os.each do from the rspec-puppet-facts gem. This was required to prevent errors related to not mocking osfamily or operatingsystem. It also improves the scope for which the tests run as the validate against all supported OS facts instead of just a manually entered subset
If the $package_ensure is set to 'absent', then it will not include the service class.
Added commit to fix Rubocop failures |
instead of removing and loosing knowledge about there is the possibility to add a skip tag: https://relishapp.com/rspec/rspec-core/v/3-4/docs/pending-and-skipped-examples/skip-examples |
…fs into 21-pkg-svc-params
@vinzent That's a much better idea. |
Service disable test fails on Ubuntu 14.04 (ONLY on ubuntu 14.04). skipping the test in rspec until we can figure out why it is only failing on Ubuntu 14.04.
Syncing my macbook with the work I did last night at home reverted some of the rubocop fixes I made. This puts those fixes back in
Add and skip the service enable check until we can figure out why it fails on Ubuntu 14.04. It doesn't appear to be a Puppet issue as the command line tool shows that puppet sees it as disabled. Looking at Serverspec and SpecInfra for the culprit
Ok - status update on service enable check: It is currently skipped until a PR/fix is provided to the specinfra gem. The Puppet Upstart provider disables a service by creating a specinfra, however, tests Upstart for service enable by checking https://github.com/mizzy/specinfra/blob/master/lib/specinfra/command/debian/base/service.rb#L5 Thanks to @vinzent for the help! |
@dhollinger if Solaris support is definitely dropped, shouldn't we add a line on the README.md, moving it to unsupported and maybe explaining why? |
@yastupin That's still in flux and will be decided later. Right now, I think the conversation that I had with @alexjfisher and brief input from @rnelson0 - we should be able to maintain it as a "self-support" option. The unit tests have been built around checking for Solaris, our only pain point is that we don't have any beaker tests for Solaris. Either way, this PR is dealing more with a couple of new features for managing the autofs package and service. This is just one of several issues slated for the |
manifests/package.pp
Outdated
default: { | ||
fail("${::operatingsystem} not supported.") | ||
fail("${facts['operatingsystem']} not supported.") |
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.
@dhollinger maybe keep the empty Solaris block here then? otherwise the immediate fail() seems somewhat incompatible with self-support
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.
Fixed. Nice catch
|
||
opsys.each do |os| | ||
context 'main init tests' do | ||
on_supported_os.select { |_, f| f[:os]['family'] != 'Solaris' }.each do |os, facts| |
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.
_, f
what kind of magic is this
let(:facts) do | ||
facts.merge(concat_basedir: '/etc') | ||
end | ||
it { is_expected.to compile } |
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.
can we do compile.with_all_deps?
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.
eep, sorry @bastelfreak, saw that it was approved and @dhollinger had asked for a merge earlier in the day; didn't see that these comments were so recent.
Also, the commits weren't squashed, if that's a problem @ mention me, and I can revert and either have @dhollinger squash them or just squash-merge. |
Added parameters that will allow for more granular control of the autofs package and service state.
closes #21