Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add ensure=>file to pinning file #475

Merged
merged 2 commits into from
Feb 8, 2016
Merged

Conversation

alexjfisher
Copy link
Member

Hi

This fix for #465 actually seems too simple. It works for me, but I suspect I'm missing something! :)
But without an 'ensure' property, what did the file resource actually do? Chown files if they happened to exist?

Kind Regards,
Alex

@jhoblitt
Copy link
Member

@alexjfisher I agree it is odd that file resource doesn't have an ensure param. I'm not sure if the de#tent was to only set the permissions on the .pinned file, if it exists or to pin all managed plugins. Hmm.

@jenkinsadmin
Copy link

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

@sfhardman
Copy link
Contributor

I think the trouble is this would pin all plugins, not just bundled ones which are also being explicitly added. Not sure how big of a deal that is. It may only be an issue if a plugin is not bundled when puppet is applied, but gets added to the bundle on a later Jenkins release.

I suspect the cleanest thing would be to add a "pinned" parameter and let people manually select whether to pin a plugin. #380, and the result of the fix (32c015e) suggests that it is difficult to detect whether a plugin should be pinned automatically, and have it work on first and subsequent applies.

Defaults to false, but when set to true sets ensure on the .pinned file
resource to 'file'.
@alexjfisher
Copy link
Member Author

@sfhardman I've added a new parameter to control the behaviour as you suggested. I actually called it 'pin' (instead of 'pinned').

@sfhardman
Copy link
Contributor

Works for me :-)

@jhoblitt jhoblitt added bug Something isn't working enhancement New feature or request labels Feb 8, 2016
@jhoblitt
Copy link
Member

jhoblitt commented Feb 8, 2016

@rtyler Any thoughts on this?

rtyler pushed a commit that referenced this pull request Feb 8, 2016
Add ensure=>file to pinning file
@rtyler rtyler merged commit 00ea19d into voxpupuli:master Feb 8, 2016
@rtyler rtyler added this to the 1.7.0 - Oddjob milestone Feb 8, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants