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

Possible solution for Issue #71 #89

Closed
wants to merge 10 commits into from

Conversation

hingstarne
Copy link

Hi,

i added a mechanism that allows the user to choose the debian package repositories.
At the moment its default to use the nginx upstream packages. With the patch you can choose between nginx, dotdeb and debian default packages.
Its tested on Squeeze and Wheezy with all the 3 providers.
Hope you like it.
Best Regards
Arne

Hingst, Arne-Kristian added 5 commits July 11, 2013 01:12
At the moment nginx.org, dot deb.org and plain debian packages are provided.
* 'master' of github.com:hingstarne/puppet-nginx:
  Reformated
removed proxy_http_version from proxy.conf if needed
@@ -26,7 +26,8 @@
$proxy_cache_inactive = $nginx::params::nx_proxy_cache_inactive,
$proxy_http_version = $nginx::params::nx_proxy_http_version,
$types_hash_max_size = $nginx::params::nx_types_hash_max_size,
$types_hash_bucket_size = $nginx::params::nx_types_hash_bucket_size
$types_hash_bucket_size = $nginx::params::nx_types_hash_bucket_size,
$nx_debian_repository = $nginx::params::nx_debian_repository
Copy link
Contributor

Choose a reason for hiding this comment

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

puppet-lint?

@abraham1901
Copy link
Contributor

Hello,
Thanks for this code.
You check this change on ubuntu?

@hingstarne
Copy link
Author

With Ubuntu you cannot use dotdeb packages they are officialy for debian only.
I added some version strings for ubuntu in my changes but i need to rewrite the proxy.conf.erb.
The old versions of nginx in squeeze or jaunty do not support proxy_http_version as a parameter.
I will fix this and send a pull request.

Hingst, Arne-Kristian added 4 commits July 12, 2013 00:00
- http_version 1.0 is default compiled in
- older version of nginx do not support this parameter
- easier if statement in proxy.conf.erb
@hingstarne
Copy link
Author

I tested it under lucid, precise and squeeze, wheezy
It should allow you to set debian / ubuntu original packages with

class {'nginx':
        nx_debian_repository    => 'debian';
    }

Hope this helps you out

@jfryman
Copy link
Contributor

jfryman commented Jul 21, 2013

I've been waffling on this PR mostly because I am debating whether it is the job of this module to manage package repositories or if that responsibility should be delegated to some site-specific manifests.

This would also mean I rip out the current RH code that sets up the Yum repos as well.

I'd ❤️ input here from interested parties.

@hingstarne
Copy link
Author

Hi,
to be honest this would mean to pull out all of the package repository configuration ( except maybe for the package name), for all distros and have external dependencies like puppetlabs-apt or similar for other distros.
Did i understand you correct with this?

@jfryman
Copy link
Contributor

jfryman commented Jul 23, 2013

@hingstarne Yes, and more. I would certainly like to get rid of this mess (https://github.com/jfryman/puppet-nginx/blob/master/manifests/package/debian.pp#L26-L49) and use puppetlabs-apt to manage the apt repositories.

Even more than that, I think that this module should have the responsibility for managing the package resource (name/version), and we can set intelligent defaults in params for each distro (like we do today), but also allow for a new package name to get passed into nginx::package in the event someone else has rolled their own packages and use a non-standard name (It happens). We could keep the yum and apt declarations, but they'd have to be manually included by a sysadmin somewhere in a node manifest, not as part of Class[nginx].

TL;DR: Make it the sysadmin's responsibility to include what package repositories a package comes from external to this module. Include some defaults, but don't make them mandatory.

@elmerfud
Copy link
Contributor

@jfryman I'll count myself as an interested party. It's my preference that package repository setup be handled outside this module, or have it be something that can be enabled/disabled optionally.

In our organization we run private repository mirrors for almost all packages that our systems use. Our production systems can't directly reach public repositories and must either use our mirror, or go through a proxy.

@vrillusions
Copy link
Contributor

@jfryman 👍 to having it managed outside this. As an example I was going to start looking into setting up a new config parameter that would let you choose between stable and mainline branches. Obviously if that task is just ripped out of this module I can instead have my site module set the repo however I want.

@jfryman
Copy link
Contributor

jfryman commented Oct 3, 2013

/cc #146 #144

@jfryman
Copy link
Contributor

jfryman commented Jun 24, 2014

Way old. Thanks for taking a pass at this... going to revisit when 🕐 allows. Please reopen and rebase if you wanna take another pass at this.

@jfryman jfryman closed this Jun 24, 2014
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