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

Remove management of unzip package #189

Merged
merged 1 commit into from
Sep 25, 2015

Conversation

danieldreier
Copy link
Contributor

The unzip package is already managed by most people's puppet infrastructure, so installing it here introduces resource conflicts.

Even with the ensure_resource function, there are parse-order dependent conflicts if you have the same packages declared elsewhere.

I made this change because I get a resource conflict without it. I'd be open to another approach but wanted to contribute the change back upstream in case other people were running into the same problem.

The unzip package is already managed by most people's puppet
infrastructure, so installing it here introduces resource conflicts.

Even with the ensure_resource function, there are parse-order dependent
conflicts if you have the same packages declared elsewhere.
@solarkennedy
Copy link
Contributor

I agree that ensure_resources isn't great. Other solutions for this are #185 and #123.

/me sighs....

@aj-jester what do you think? Even with other solutions where we do ! defined we still can get conflicts with other modules declaring unzip after consul is included. Should we leave this just up as an exercise to the reader and document it?

@hopperd
Copy link
Contributor

hopperd commented Sep 24, 2015

This is one of those annoying situations with puppet and causes constant issues. Not sure there is really a great workaround for this. Another possibility solution would be (and I'm not a huge fan) but to add an option flag to on wether to manage that package or not, as well not ideal but workable.

@solarkennedy
Copy link
Contributor

Alright, I'm just going to make the call and say this is out of scope of the module, there have been enough pull requests to adjust this. I'll merge this and adjust the docs.

solarkennedy added a commit that referenced this pull request Sep 25, 2015
Remove management of unzip package
@solarkennedy solarkennedy merged commit 7bbf0d5 into voxpupuli:master Sep 25, 2015
@danieldreier
Copy link
Contributor Author

thanks @solarkennedy - one solution I've seen elsewhere is to provide a sample profile that includes extra packages, in a namespace like consul::profile::server / consul::profile::agent. That way somebody who doesn't really know what they're doing can include it verbatim, and somebody who wants more control can copy parts into their own site control repo.

@aj-jester
Copy link

I looked through nanliu/staging module and it looks like they are assuming the unzip package would be installed separately as well.

@solarkennedy So I think we can take that same approach and just document the fact unzip needs to be installed separately and remove it from our module.

If we want, as an added step, we can do require => Package['unzip'] but I would like to hear why this might be a bad idea, cause it sounds like a bad idea to me 😝

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.

4 participants