Add puppet::package_install_options variable to allow you to pass flags to package resource

[vollmerk]

We needed this internally to add the --diasableexclude=all flag to the yum install, as puppet* is in the base "exclude=" statement to prevent unintentional upgrade of the puppet agent, by overzealous sysadmins who yum -y update I would expect this could be useful in any number of other situations where installation of the package requires specific options being passed to the installer.

As far as I can tell all supported platforms default package management system supports the install_options flag.

[wbclark]

I'm looking for a way to add a simple test that no additional command line arguments are passed to the package manager with default options, to ensure we maintain that behavior going forward.

Looking at the spec tests I see for the agent we have a test for the package class at https://github.com/theforeman/puppet-puppet/blob/master/spec/classes/puppet_agent_spec.rb#L59-L64

I think we could add .with_install_options(nil) on a newline to that test. Could you try it out please?

[wbclark]

For the server there is also a spec test for the package class at https://github.com/theforeman/puppet-puppet/blob/master/spec/classes/puppet_server_spec.rb#L69

In that case extending the same line so that it reads it { should contain_package(puppetserver_pkg).with_install_options(nil) } should suffice

[wbclark]

Finally there is the main init spec with its package test for default params at https://github.com/theforeman/puppet-puppet/blob/master/spec/classes/puppet_init_spec.rb#L38

We can add coverage here by replacing that line with:

        it { should contain_package(puppet_package)
          .with_ensure('present')
          .with_install_options(nil)
        }

[wbclark]

Suggested change

	ensure => $package_version,
	provider => $package_provider,
	install_options => $package_install_options,
	source => $package_source,
	ensure => $package_version,
	provider => $package_provider,
	install_options => $package_install_options,
	source => $package_source,

To fix this test breakage:

manifests/agent/install.pp:13:arrow_alignment:WARNING:indentation of => is not properly aligned (expected in column 23, but found it in column 25)
manifests/agent/install.pp:14:arrow_alignment:WARNING:indentation of => is not properly aligned (expected in column 23, but found it in column 25)
manifests/agent/install.pp:15:arrow_alignment:WARNING:indentation of => is not properly aligned (expected in column 23, but found it in column 25)
manifests/agent/install.pp:16:arrow_alignment:WARNING:indentation of => is not properly aligned (expected in column 23, but found it in column 25)

[wbclark]

Thanks @vollmerk for the submission!

I gave a bit of feedback about simple tests to ensure this remains undefined by default going forward.

Please take a look and let me know if you have any question or comment.

Looking forward to hearing from you,

[vollmerk]

Thanks @vollmerk for the submission!

I gave a bit of feedback about simple tests to ensure this remains undefined by default going forward.

Please take a look and let me know if you have any question or comment.

Looking forward to hearing from you,

Implemented your suggestions, thanks. Let me know if I missed anything.

[wbclark]

Thanks @vollmerk

Overall it looks good to me. I left some minor feedback on the indentation, I think we're good to move forward once that is fixed up

The failures on the EL6 acceptance tests look to me like they are unrelated to this PR. I will investigate what could be causing that on our end so that we can be confident everything should be nice and 🍏

[vollmerk]

Thanks @vollmerk

Overall it looks good to me. I left some minor feedback on the indentation, I think we're good to move forward once that is fixed up

The failures on the EL6 acceptance tests look to me like they are unrelated to this PR. I will investigate what could be causing that on our end so that we can be confident everything should be nice and 🍏

Added the white-space fixes, and re-squashed the commits...

[wbclark]

The reason for the remaining test failure is the unsurprising fact that there no longer exists any centos6 image at https://vagrantcloud.com/centos

[vollmerk]

The reason for the remaining test failure is the unsurprising fact that there no longer exists any centos6 image at https://vagrantcloud.com/centos

Let me know if you need me to make any more adjustments on this relating to those checks, or if ya'll will take care of them in a different pull req

[wbclark]

Hi @vollmerk , I don't think anything further is required on your end.

I opened voxpupuli/beaker-hostgenerator#203 just now to resolve the upstream issue which was breaking the EL6 acceptance tests.

I'll use that to test against your branch locally in a moment.

[wbclark]

As expected, EL6 acceptance tests are passing when I run them locally with a working EL6 image

[wbclark]

Thanks @vollmerk !