Wrapper to gracefully avoid unsupported OS's

[salderma]

Hi, I have attempted to add a suitable means for avoiding unsupported platforms, but allowing the module to be applied in a do-nothing state instead of failing the puppet run. My case is that I have many VMs which run puppet, do not run a supported OS, and belong in a host group in my ENC which applies you module.

I have added a supported boolean to params.pp and changed the instances of using the Fail function to using notify with the unsupported message. Inside your case logic for virtual = vmware, I have put an if $supported { ... } wrapper.

I hope you find this method to your liking, if not I would appreciate your suggestions and will gladly modify my implementation accordingly.

[salderma]

I'm not quite familiar enough with rspec to change the tests to expect the notify instead of error, but that seems to be the hangup in travis.

I'll try and catch up on how to update the spec tests and add another commit to fix them

[razorsedge]

@salderma

This looks good. I have one leading question: where does $supported come from in class vmwaretools?

[razorsedge]

Also, I would change the notify to a notice otherwise every agent run will produce a change event.

[salderma]

Doh! nice catch on $supported. I need to include it in the params passed into class vmwaretools, don't I? :)

I'll fix that up, and make the adjustment from notify to notice.

[razorsedge]

I wouldn't expose $supported to end users via the API. Just use $supported = $vmwaretools::params::supported in the vmwaretools class.

[salderma]

This should be what you're looking for. I'm still quite uneducated with respect to the rspec stuff. Sorry for the awful pun, but I don' t know how to go about updating the spec tests in a useful way. I suppose we could just remove the expect failure types of messages, but avoiding the problem is not really a solution in most cases.

[razorsedge]

I am considering simply dropping all the notices and removing the it 'should fail' do rspec tests. Every time a notice fires, the client shows in the puppet-dashboard as having changed something. The would make dashboard rather useless for tracking changes.

Of course, part of me is wondering why something like the following wouldn't work?

if $::osfamily == 'RedHat' and $::operatingsystem != 'Fedora' {
  include '::vmwaretools'
}

[salderma]

In my case, I'm using an ENC (TheForeman), so it's nice to define the vmwaretools class at the top of my node organization structure (Foreman HostGroups). To do what you suggest, I'd have to write a class to wrap vmwaretools. If you want to reject my suggestion that's what I'll do. If that's your methodology, it does beg the question why test to see if we're vmware, let the end user be responsible for that...

if $::osfamily == 'RedHat' and $::virtual == 'vmware' and $::operatingsystem != 'Fedora' {
  include '::vmwaretools'
}

IMHO its more elegant to say, "Hey, I'm a module to install VMWareTools, someday I might support more platforms than just RedHat, but in the meantime, I'm safe to deploy on unsupported platforms...even real hardware."

[razorsedge]

Yeah, I already have the if not vmware, then do nothing bit in there. I might as well take it all the way...

[razorsedge]

@salderma : Sorry for this taking forever. I've managed to sort out the rspec bits. Unfortunately, I noticed that your pull request is against the master branch and not develop. If you would change that, I will get this merged in.

[salderma]

Sorry, I seem to have missed your updates here. Forgive my git ignorance, do I need to rebase my changes against your develop branch, and then submit the PR? I don't immediately see a means of changing this PR's target branch in your repo.

[razorsedge]

There is probably a fancy, git/GitHub way of doing this, but my git-foo is not that strong. I would probably do the following, which will result in a loss of history, but will also give us a compact commit, and then send another PR:

cd somePlaceOutsideOfTheCurrentRepo
git clone https://github.com/salderma/puppet-vmwaretools.git vmwaretools-new
cd vmwaretools.new
git checkout develop
git checkout -b wrapper  # so that you have a feature branch to work on
cp -p changedFilesFromOldRepo .

I also have the fixes to the rspec tests that I can throw in a branch for you to include to make Travis happy.

[salderma]

Closing and replacing with https://github.com/razorsedge/puppet-vmwaretools/pull/18