Skip to content
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 the ability to override package_source #209

Merged
merged 4 commits into from
Dec 5, 2018

Conversation

beezly
Copy link
Contributor

@beezly beezly commented Nov 28, 2018

Pull Request (PR) description

Allows you to specify manage_package_source to override the modules behaviour when you want to specify your own package_source. The existing functionality in the module is not enough, as sometimes it is necessary to specify an undefined value, (for example, if you are using yum to install from your own repos - the source parameter to the package type needs to be undef) which at the moment gets overridden by the value from $splunk::params::server_pkg_source

@beezly beezly force-pushed the feature/fix_unmanged_repos branch from 0c1ecc2 to eb8790e Compare November 28, 2018 16:37
manifests/forwarder.pp Outdated Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet tests-fail labels Nov 28, 2018
@bastelfreak
Copy link
Member

Hi @beezly. thanks for the PR! Can you take a look at the inline comments I made and the failing spec tests?

@beezly
Copy link
Contributor Author

beezly commented Nov 29, 2018

@bastelfreak I've added datatypes to those two classes and fixed the failing tests. It looks like one job is going to fail (https://travis-ci.org/voxpupuli/puppet-splunk/jobs/461227416) but I believe that is unrelated to the changes I've made (it's failing to restart the ssh service when setting up a debian 8 docker container to run the tests in). I'll give it a nudge and see if it passes a second time.

@beezly beezly closed this Nov 29, 2018
@beezly beezly reopened this Nov 29, 2018
@beezly
Copy link
Contributor Author

beezly commented Nov 29, 2018

Also - apologies for the close/reopen. It was the only way I could get Travis to re-run the tests after the failed run earlier.

manifests/forwarder.pp Outdated Show resolved Hide resolved
@alexjfisher
Copy link
Member

Also - apologies for the close/reopen. It was the only way I could get Travis to re-run the tests after the failed run earlier.

That's fine. I think it's even documented as a way to rerun tests in one of @bastelfreak's presentations.

@bastelfreak
Copy link
Member

@alexjfisher @beezly correct, closing/reopening is totally fine.

@beezly beezly closed this Dec 3, 2018
@beezly beezly reopened this Dec 3, 2018
) inherits splunk::params {

$virtual_service = $splunk::params::forwarder_service
$staging_subdir = $splunk::params::staging_subdir

$path_delimiter = $splunk::params::path_delimiter

$_package_source = $manage_package_source ? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this a simple if and check package_source isn't undef when manage_package_source is false.

Something along the lines of

if $manage_package_source {
  $_package_source = $splunk::params::forwarder_pkg_src
} else {
  unless $package_source { fail('`package_source` parameter must be set when `manage_package_source` is false') }
  $_package_source = $package_source
}

?

Copy link
Contributor Author

@beezly beezly Dec 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexjfisher but that's exactly the behaviour we want! It should be possible to specify manage_package_source => false, package_source => undef so that if they are using the yum provider, the source parameter in the package needs to be undef otherwise yum will try and download from the source provided, rather than search through its locally configured repos.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, ok. Didn't at first spot the if $pkg_provider != undef and $pkg_provider != 'yum' and $pkg_provider != 'apt' and $pkg_provider != 'chocolatey'

@alexjfisher alexjfisher merged commit 4fe1532 into voxpupuli:master Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-work not ready to merge just yet tests-fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants