Skip to content

(#9418) naggen fixes#557

Closed
maxstepanov wants to merge 3 commits intopuppetlabs:masterfrom
maxstepanov:patch-1
Closed

(#9418) naggen fixes#557
maxstepanov wants to merge 3 commits intopuppetlabs:masterfrom
maxstepanov:patch-1

Conversation

@maxstepanov
Copy link
Copy Markdown

Updated naggen to work with all the changes in 2.7.x

Previously naggen did not honor its `[naggen]` section in puppet.conf
due to changes between 2.6 and 2.7 with run_mode.

This commit changes naggen to extend Puppet::Application, which
simplifies how settings are resolved, option handling, and displaying
help information.
This commit only fixes whitespace and indentation issues.
@pcarlisle
Copy link
Copy Markdown
Contributor

Thanks for this. Do you think you'd be able to add some tests for it so we can keep it working in the future?

@cprice404
Copy link
Copy Markdown

Hello max, thanks a ton for the submission. A couple of updated comments:

  1. Have you signed our CLA? http://projects.puppetlabs.com/projects/puppet/wiki/Development_Lifecycle
  2. We are moving nagios out of puppet core and into a module, so we may need to rebase these changes to the new nagios module repo: https://github.com/puppetlabs/puppetlabs-nagios

naggen itself did not get moved yet--not sure if this is intentional or an oversight. I'll find out and update the pull request accordingly.

@cprice404
Copy link
Copy Markdown

Talked to Daniel, he's fine with merging this as long as the code looks ok. We don't have any well-defined patterns on testing stuff in /ext, so it's OK to merge w/o that.

We do need to take care of the CLA issue first, though. Also, I'd like to merge this into Telly instead of 2.7.x.

Ideally we'd merge this soon and then move the merged version of the tool over into the Nagios module, but that depends on whether or not we get the CLA in time. I'll start the process of working with Kelsey and Pieter to ensure that the current module packaging / distribution system will support shipping stuff in an 'ext' folder.

@jeffweiss
Copy link
Copy Markdown
Contributor

@cprice-puppet According to Redmine, Max has signed the CLA.
naggen not getting moved was an oversight. I'm working on looking at what it would take to put into the nagios module.

@jeffweiss
Copy link
Copy Markdown
Contributor

When I try to run puppet naggen from this pull request, it errors out complaining about puppet/lib/puppet/util.rb:211:in 'absolute_path?': undefined method 'features'. There's an existing pull request (611) for 2.7.x from @pcarlisle that should resolve the absolute_path? problem. This should be tested once the dependent pull request is merged.

@kelseyhightower
Copy link
Copy Markdown

Closing this due to lack of response from @maxstepanov. I think we can totally merge this on the https://github.com/puppetlabs/puppetlabs-nagios master branch.

melissa pushed a commit to melissa/puppet that referenced this pull request Mar 30, 2018
(PCP-529) Archive logs from acceptance runs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants