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

ENH: Add install_from_ppa pillar setting and PPA pkgrepo #42

Merged
merged 1 commit into from
Aug 10, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions nginx/init.sls
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
{%- set nginx=pillar.get('nginx', {}) %}

include:
- nginx.common
{% if pillar.get('nginx', {}).get('user_auth_enabled', true) %}
{% if nginx.get('user_auth_enabled', true) %}
- nginx.users
{% endif %}
{% if pillar.get('nginx', {}).get('install_from_source') %}
{% if nginx.get('install_from_source') %}
- nginx.source
{% else %}
- nginx.package
Expand Down
4 changes: 3 additions & 1 deletion nginx/map.jinja
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{% set nginx = salt['grains.filter_by']({
'Debian': {
'package': 'nginx',
'apache_utils': 'apache2-utils',
},
'RedHat': {
'package': 'nginx',
'apache_utils': 'httpd-tools',
},
}, merge=salt['pillar.get']('nginx:lookup')) %}
}, merge=salt['pillar.get']('nginx:lookup')) %}
17 changes: 16 additions & 1 deletion nginx/ng/install.sls
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,25 @@
{% from 'nginx/ng/map.jinja' import nginx, sls_block with context %}

nginx_install:
{% if nginx.from_source %}
{% if nginx.install_from_source %}
## add source compilation here
{% else %}
pkg.installed:
{{ sls_block(nginx.package.opts) }}
- name: {{ nginx.lookup.package }}
{% endif %}

{% if salt['grains.get']('os_family') == 'Debian' %}
{% if nginx.install_from_ppa %}
nginx_ppa_repo:
pkgrepo.managed:
- humanname: nginx-ppa-{{ grains['oscodename'] }}
- name: deb http://ppa.launchpad.net/nginx/stable/ubuntu {{ grains['oscodename'] }} main
- file: /etc/apt/sources.list.d/nginx-stable-{{ grains['oscodename'] }}.list
- dist: {{ grains['oscodename'] }}
- keyid: C300EE8C
- keyserver: keyserver.ubuntu.com
- require_in:
- pkg: nginx_install
{% endif %}
{% endif %}
17 changes: 16 additions & 1 deletion nginx/package.sls
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{% from "nginx/map.jinja" import nginx with context %}
{% set use_upstart = pillar.get('nginx', {}).get('use_upstart', true) %}
{% if use_upstart %}
nginx-old-init:
Expand Down Expand Up @@ -35,9 +36,23 @@ nginx-old-init-disable:
- file: nginx-old-init
{% endif %}

{% if salt['grains.get']('os_family') == 'Debian' %}
Copy link

Choose a reason for hiding this comment

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

Won't this always add the PPA even if install_from_ppa is False? Is that desirable? It probably doesn't hurt anything, but would it be better to only add the PPA if planning to install from it?

Copy link
Member

Choose a reason for hiding this comment

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

@westurner Can you create a pull request with a check that @gtback suggests? I saw you did a check in nginx/ng/install.sls but probably forgot to do it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without enabled = False, the pkgrepo would remain enabled in the event that the boolean changes from True to False.

Should I add the 'enabled' attr from Line 42 (package) to Line ~20 (ng.install)?:

enabled: {{ salt['pillar.get']('nginx:install_from_ppa', False) }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On further review, this is more complex to implement than I considered. I apologize.

Does this look correct (nginx.install_from_ppa: True/False)?:

  • False: pkg.install
  • True: pkgrepo.managed, pkg.install
  • False -> True: pkgrepo.managed, pkg.reinstall
  • True -> False: pkgrepo.absent, pkg.reinstall

Is it reasonable to expect an implementer to call pkg.reinstall (install, reinstall=True)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nginx-ppa-repo:
pkgrepo.managed:
- enabled: {{ salt['pillar.get']('nginx:install_from_ppa', False) }}
- humanname: nginx-ppa-{{ grains['oscodename'] }}
- name: deb http://ppa.launchpad.net/nginx/stable/ubuntu {{ grains['oscodename'] }} main
- file: /etc/apt/sources.list.d/nginx-stable-{{ grains['oscodename'] }}.list
- dist: {{ grains['oscodename'] }}
- keyid: C300EE8C
- keyserver: keyserver.ubuntu.com
- require_in:
- pkg: nginx
{% endif %}

nginx:
pkg.installed:
- name: nginx
- name: {{ nginx.package }}
{% if use_upstart %}
file:
- managed
Expand Down
9 changes: 8 additions & 1 deletion pillar.example
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
nginx:
install_from_source: True
install_from_ppa: False
use_upstart: True
user_auth_enabled: True
with_luajit: False
Expand All @@ -13,6 +14,9 @@ nginx:
source: http://github.com/agentzh/headers-more-nginx-module/tarball/v0.21
source_hash: sha1=dbf914cbf3f7b6cb7e033fa7b7c49e2f8879113b

lookup:
package: nginx

# ========
# nginx.ng
# ========
Expand All @@ -30,7 +34,10 @@ nginx:
vhost_use_symlink: True

# Source compilation is not currently a part of nginx.ng
from_source: False
install_from_source: False

# Install nginx.ng.lookup.package from the PPA repository
install_from_ppa: True

package:
opts: {} # this partially exposes parameters of pkg.installed
Expand Down