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

Allow to specify package version and revision for ensure parameter #5

Merged
merged 2 commits into from
Mar 27, 2017

Conversation

dnsmichi
Copy link
Contributor

@dnsmichi dnsmichi commented Mar 24, 2017

First of all, thanks for starting your work on an official Puppet module. I've been previously using https://github.com/Nextdoor/puppet-kibana5 but am in progress migrating over here.

One thing I need to ensure is that the Kibana version is pinned. That version is used to determine the correct Elasticsearch API call for storing Kibana config, i.e. the default index (from filebeat, icingabeat, or anything else).

In order to achieve that, I'll also need to specify the exact package revision inside the version string on yum based systems.

Currently the regex checking for valid parameter values does not allow for that. I have modified and tested it working on CentOS7 by using a Vagrant box to install Elastic Stack using the official modules. Puppet version: 3.8.7

CLA is signed.

Pull request acceptance prerequisites:

  • Signed the CLA (if not already signed)
  • Rebased/up-to-date with base branch
  • Updated CHANGELOG.md with patch notes (if necessary)
  • Any relevant docs (README.markdown or inline documentation) updated (if necessary)
  • Updated CONTRIBUTORS (if you would like attribution)
  • Tests pass CI

@tylerjl tylerjl added the enhancement New feature or request label Mar 24, 2017
@tylerjl
Copy link
Contributor

tylerjl commented Mar 24, 2017

Thanks for the contribution @dnsmichi, the change makes sense. It looks like your regex is too restrictive though, as the tests are failing because it's now requiring every package to have a package revision.

Copy link
Contributor

@tylerjl tylerjl left a comment

Choose a reason for hiding this comment

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

The extra *(-...) part of the regex probably should be optional? ?

@dnsmichi
Copy link
Contributor Author

Ah, sorry. Shouldn't fiddle with regex on a Friday. Will push an update soon.

@dnsmichi
Copy link
Contributor Author

So, various test cases evaluated over at https://regex101.com:

/^\d([.]\d+)*(-[\d\w]+)?$/
5.2.2
5.2.2-1
5.2.2-bpo1

Added the last one to tests too, adjust as needed please :)

@tylerjl tylerjl removed the waiting label Mar 27, 2017
@tylerjl
Copy link
Contributor

tylerjl commented Mar 27, 2017

Great work, thanks for the contribution @dnsmichi!

@tylerjl tylerjl removed the tests label Mar 27, 2017
@tylerjl tylerjl added this to the Next Release milestone Mar 27, 2017
@tylerjl tylerjl merged commit bf1aebf into voxpupuli:master Mar 27, 2017
@dnsmichi dnsmichi deleted the fix/package-revisions branch March 28, 2017 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants