Add classes to manage supported SCM packages

[jcpunk]

There are a dozen or so trivial classes on the forge that just install git or similar so folks can use vcsrepo. This is an attempt to help standardize the process of loading these packages, but in an optional way for folks that have more complex needs (such as custom configs).

It is a sort of "meet in the middle" attempt to avoid building a separate class just to load a package resource.

[puppet-community-rangefinder]

vcsrepo::manage::bzr is a class

that may have no external impact to Forge modules.

vcsrepo::manage::cvs is a class

that may have no external impact to Forge modules.

vcsrepo::manage::git is a class

that may have no external impact to Forge modules.

vcsrepo::manage::hg is a class

that may have no external impact to Forge modules.

vcsrepo::manage::p4 is a class

that may have no external impact to Forge modules.

vcsrepo::manage::svn is a class

that may have no external impact to Forge modules.

This module is declared in 111 of 580 indexed public Puppetfiles.

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[ekohl]

I think there's a good chance of conflicts so perhaps ensure_packages() from stdlib is a good idea? And I'd like an acceptance test that just includes all classes.

[jcpunk]

I was concerned about adding a dependency for this module. In practice, I'd expect stdlib to be safe, but currently this module has no listed dependencies.

[jcpunk]

And I'd like an acceptance test that just includes all classes.

Is there an example somewhere I can look at for that... I fear I create my test files via pdk and nothing is jumping out at me there.

[ekohl]

I was concerned about adding a dependency for this module. In practice, I'd expect stdlib to be safe, but currently this module has no listed dependencies.

Ah, fair point. I don't have access on this module so I'll let the maintainers speak.

Is there an example somewhere I can look at for that... I fear I create my test files via pdk and nothing is jumping out at me there.

I never use pdk, but it should be fairly straight forward. Something like spec/acceptance/manage/git_spec.rb would look like:

# frozen_string_literal: true

require 'spec_helper_acceptance'

describe 'vcsrepo::manage::git' do
  let(:pp) { 'include vcsrepo::manage::git' }

  it 'applies idempotently' do
    idempotent_apply(pp)
  end

  it 'installs git' do
    expect(package('git')).to be_installed
  end
end

You could also opt for a one-for-all-spec, but individual files works just as well and gives a bit more control in case you want to skip certain platforms.

[jcpunk]

In theory I've got the acceptance tests up.

On the ensure_packages front, I'll do whatever the code owners want.

[chelnak]

@ekohl Is ensure_packages a standard that we use in other modules?

In theory I'm fine with this as it makes sense but part of me worries about people that rely on specific versions of stdlib.. including the latest version as a dep here (also sensible) might create problems for them.

[jcpunk]

For the integration tests, I think the Red Hat nodes need EPEL for cvs and hg. I'm not actually sure where to get a p4 rpm or bzr for EL8/9...

For p4 the instructions in the wild refer to some sort of helix something or other....

[jcpunk]

Rebased off main

[ekohl]

don't have access here so I can't merge, but looks good to me.

[jcpunk]

Any way to re-trigger the workflows?

[jcpunk]

It seems I don't have epel-release installing correctly for these tests... ideas?

[jcpunk]

Any suggestions on how to get these tests to pass?

[ekohl]

I think it should be done in the setup, where there actually already is a lot:

puppetlabs-vcsrepo/spec/spec_helper_acceptance_local.rb

Lines 29 to 62 in b82414b

	# Configure all nodes in nodeset
	c.before :suite do
	case os[:family]
	when 'redhat'
	if %r{5}.match?(os[:release][0])
	LitmusHelper.instance.run_shell('which git', expect_failures: true)
	LitmusHelper.instance.run_shell('rpm -ivh http://repository.it4i.cz/mirrors/repoforge/redhat/el5/en/x86_64/rpmforge/RPMS/rpmforge-release-0.5.3-1.el5.rf.x86_64.rpm', expect_failures: true)
	LitmusHelper.instance.run_shell('yum install -y git')
	end
	pp = <<-PP
	package { 'git': ensure => present, }
	package { 'subversion': ensure => present, }
	PP
	LitmusHelper.instance.apply_manifest(pp)
	when %r{(ubuntu|[dD]ebian|sles)}
	pp = <<-PP
	package { 'git-core': ensure => present, }
	package { 'subversion': ensure => present, }
	PP
	LitmusHelper.instance.apply_manifest(pp)
	else
	unless run_bolt_task('package', 'action' => 'status', 'name' => 'git')
	puts 'Git package is required for this module'
	exit
	end
	unless run_bolt_task('package', 'action' => 'status', 'name' => 'subversion')
	puts 'Subversion package is required for this module'
	exit
	end
	end
	LitmusHelper.instance.run_shell('git config --global user.email "root@localhost"')
	LitmusHelper.instance.run_shell('git config --global user.name "root"')
	end
	end

I would actually suggest to rewrite tests to include your new class instead of doing it in a setup phase, though the git config may be a bit tricky then.

Now what I'd personally do is copy the pattern that we have in voxpupuli-acceptance. That's use spec/setup_acceptance_node.pp if it's available. So use this in spec/spec_helper_acceptance_local.rb:

RSpec.configure do |c|
  c.before :suite do
    LitmusHelper.instance.apply_manifest(File.read(File.join(__dir__, 'setup_acceptance_node.pp')))
  end
end

Then ensure epel-release is installed for EL 8 & EL 9 using native Puppet.

[jcpunk]

I've taken a stab at the suggested include method... the more ruby I write the less I think I understand this language...

[jcpunk]

Does this look workable?

[jcpunk]

Anything else needed from me?

[CLAassistant]

All committers have signed the CLA.

[jcpunk]

Rebased off head

[GSPatton]

this looks good. But it looks like some of the new Rubocop checks that were introduced with the Puppet 8 support work are complaining.

Looks like it's mostly spec stuff it's complaining about. Shouldn't be too tough to fix