Giuseppe Lavagetto has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/369385 )
Change subject: Use own differ instead of puppet catalog diff
......................................................................
Use own differ instead of puppet catalog diff
This allows faster execution and better control.
Change-Id: I03d4319b49f3c7a195e076decc6a7b741e13d399
---
A puppet_compiler/differ.py
M puppet_compiler/filter.py
M puppet_compiler/presentation/html.py
A puppet_compiler/templates/diffs.jinja2
M puppet_compiler/templates/hostpage.future.jinja2
M puppet_compiler/templates/hostpage.jinja2
A puppet_compiler/tests/fixtures/catalog-change.pson
A puppet_compiler/tests/fixtures/catalog.pson
M puppet_compiler/tests/test_controller.py
A puppet_compiler/tests/test_differ.py
M puppet_compiler/tests/test_hostworker.py
A puppet_compiler/tests/test_html.py
M puppet_compiler/worker.py
M setup.py
14 files changed, 894 insertions(+), 96 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software/puppet-compiler
refs/changes/85/369385/1
diff --git a/puppet_compiler/differ.py b/puppet_compiler/differ.py
new file mode 100644
index 0000000..9f5cbb6
--- /dev/null
+++ b/puppet_compiler/differ.py
@@ -0,0 +1,113 @@
+import difflib
+import json
+
+import datadiff
+
+
+class PuppetResource(object):
+
+ def __init__(self, data, resource_filter=None):
+ self.resource_type = data['type']
+ self.title = data['title']
+ self.exported = data['exported']
+ if resource_filter is None:
+ self._filter = lambda x: x
+ else:
+ self._filter = resource_filter
+ self._init_params(data.get('parameters', {}))
+
+ def _init_params(self, kwargs):
+ self.parameters = {}
+ self.content = ''
+ self.source = None
+ # Let's first look for special
+ for k, v in kwargs.items():
+ # Filter out null values
+ if v is None:
+ pass
+ elif k == 'content':
+ self.content = v
+ else:
+ self.parameters[k] = self._filter(v)
+
+ def __str__(self):
+ return "{res}[{title}]".format(res=self.resource_type,
+ title=self.title)
+
+ def is_same_of(self, other):
+ """
+ True if it designates the same resource
+ """
+ return (str(self) == str(other))
+
+ def __eq__(self, other):
+ if not self.is_same_of(other):
+ return False
+ return (self.content == other.content and
+ self.source == other.source and
+ self.parameters == other.parameters)
+
+ def __ne__(self, other):
+ return not self.__eq__(other)
+
+ def diff_if_present(self, other):
+ if self == other:
+ return None
+
+ out = {'resource': str(self)}
+ if self.content != other.content and self.resource_type == 'File':
+ other_content = other.content.split('\n')
+ my_content = self.content.split('\n')
+ content_diff = [
+ line for line in difflib.unified_diff(
+ my_content, other_content,
+ fromfile='{}.orig'.format(self.title),
+ tofile=self.title)
+ ]
+ out['content'] = content_diff
+ if self.parameters != other.parameters:
+ out['parameters'] = datadiff.diff(
+ self.parameters, other.parameters,
+ fromfile='{}.orig'.format(str(self)), tofile=str(self)
+ )
+ return out
+
+
+class PuppetCatalog(object):
+ def __init__(self, filename, resource_filter=None):
+ self.resources = {}
+ with open(filename, 'r') as fh:
+ catalog = json.load(fh, 'latin_1')
+ for resource in catalog['data']['resources']:
+ r = PuppetResource(resource, resource_filter)
+ self.resources[str(r)] = r
+ self.all_resources = set(self.resources.keys())
+ self.name = catalog['data']['name']
+
+ def diff_if_present(self, other):
+ diffs = []
+ only_in_other = other.all_resources - self.all_resources
+ only_in_self = self.all_resources - other.all_resources
+
+ for resource in self.all_resources & other.all_resources:
+ mine = self.resources[resource]
+ theirs = other.resources[resource]
+ out = mine.diff_if_present(theirs)
+ if out is not None:
+ diffs.append(out)
+
+ num_changed = len(diffs)
+ num_other = len(only_in_other)
+ num_self = len(only_in_self)
+ total_affected = (num_changed + num_other + num_self)
+ num_resources = len(self.all_resources)
+ perc_changed = '%.2f%%' % (100 * float(total_affected) / num_resources)
+ if (total_affected) == 0:
+ return None
+ return {
+ 'total': len(self.all_resources),
+ 'only_in_self': only_in_self,
+ 'only_in_other': only_in_other,
+ 'resource_diffs': diffs,
+ 'perc_changed': perc_changed
+ }
diff --git a/puppet_compiler/filter.py b/puppet_compiler/filter.py
index e1a6efa..508e243 100644
--- a/puppet_compiler/filter.py
+++ b/puppet_compiler/filter.py
@@ -76,3 +76,23 @@
def setup_filters(self):
return [self.flatten_one_element_arrays,
self.cast_integers_to_string]
+
+
+def itos(value):
+ """Transform all integers in strings"""
+ if type(value) == int:
+ return str(value)
+ elif type(value) == list:
+ return [itos(v) for v in value]
+ elif type(value) == dict:
+ return {k: itos(v) for k, v in value.items()}
+ else:
+ return value
+
+
+def flatten(value):
+ """Flatten one-element arrays"""
+ if type(value) == list and len(value) == 1:
+ return value[0]
+ else:
+ return value
diff --git a/puppet_compiler/presentation/html.py
b/puppet_compiler/presentation/html.py
index 5510cc7..e120b33 100644
--- a/puppet_compiler/presentation/html.py
+++ b/puppet_compiler/presentation/html.py
@@ -15,7 +15,6 @@
self.retcode = retcode
self.hostname = hostname
self.outdir = files.outdir
- self.diff_file = files.file_for('change', 'diff')
def _retcode_to_desc(self):
if self.retcode == 'noop':
@@ -27,18 +26,17 @@
else:
return 'compiler failure'
- def htmlpage(self):
+ def htmlpage(self, diffs=None):
"""
Create the html page
"""
_log.debug("Rendering index page for %s", self.hostname)
- data = {'retcode': self.retcode}
- if self.retcode == 'diff':
- with open(self.diff_file, 'r') as f:
- data['diffs'] = f.read()
+ data = {'retcode': self.retcode, 'host': self.hostname}
+ if self.retcode == 'diff' and diffs is not None:
+ data['diffs'] = diffs
data['desc'] = self._retcode_to_desc()
t = env.get_template(self.tpl)
- page = t.render(host=self.hostname, jid=job_id, chid=change_id, **data)
+ page = t.render(jid=job_id, chid=change_id, **data)
with open(os.path.join(self.outdir, self.page_name), 'w') as f:
f.write(page)
@@ -49,7 +47,6 @@
def __init__(self, hostname, files, retcode):
super(FutureHost, self).__init__(hostname, files, retcode)
- self.diff_file = files.file_for('future', 'diff')
def _retcode_to_desc(self):
if self.retcode == 'break':
diff --git a/puppet_compiler/templates/diffs.jinja2
b/puppet_compiler/templates/diffs.jinja2
new file mode 100644
index 0000000..3e11d7f
--- /dev/null
+++ b/puppet_compiler/templates/diffs.jinja2
@@ -0,0 +1,26 @@
+{% macro bigsep() %}{{ "%-80s"|format("=" * 80) }}{% endmacro %}
+{% macro smallsep() %}{{ "%-80s"|format("-" * 80) }}{% endmacro %}
+{{ bigsep() }}
+{{ "%-70s%10s"|format(host, diffs.perc_changed) }}
+{{ bigsep() }}
+Total Resources: {{ diffs.total }}
+{% if diffs.only_in_self %}Only in old:
+{% for resource in diffs.only_in_self %} {{ resource }}
+{% endfor %}{% endif %}
+
+{% if diffs.only_in_other %}Only in new:
+{% for resource in diffs.only_in_other %} {{ resource }}
+{% endfor %}{% endif %}
+
+{% if diffs.resource_diffs %}Resources changed:{% for diff in
diffs.resource_diffs %}
+{{ smallsep() }}
+Resource: {{ diff.resource}}
+{% if diff.parameters %}
+Parameters differences:
+{{ diff.parameters }}
+{% endif %}
+{% if diff.content %}
+Content differences:
+{{ diff.content }}
+{% endif %}{{smallsep()}}{% endfor %}
+{% endif %}
diff --git a/puppet_compiler/templates/hostpage.future.jinja2
b/puppet_compiler/templates/hostpage.future.jinja2
index 47f731a..d047000 100644
--- a/puppet_compiler/templates/hostpage.future.jinja2
+++ b/puppet_compiler/templates/hostpage.future.jinja2
@@ -28,7 +28,7 @@
<h1>Compilation results for {{ host }}: <span class="{{retcode}}">{{
desc }}</span></h1>
{% if retcode == "diff" %}
<h2>Catalog differences</h2>
- <textarea rows="20" cols="82" class="source">{{ diffs }}</textarea>
+ <textarea rows="20" cols="82" class="source">{% include
"diffs.jinja2" %}</textarea>
{% endif %}
<h2>Relevant files</h2>
<ul>
diff --git a/puppet_compiler/templates/hostpage.jinja2
b/puppet_compiler/templates/hostpage.jinja2
index 1fd4e60..3a3de79 100644
--- a/puppet_compiler/templates/hostpage.jinja2
+++ b/puppet_compiler/templates/hostpage.jinja2
@@ -28,7 +28,7 @@
<h1>Compilation results for {{ host }}: <span class="{{retcode}}">{{
desc }}</span></h1>
{% if retcode == "diff" %}
<h2>Catalog differences</h2>
- <textarea rows="20" cols="82" class="source">{{ diffs }}</textarea>
+ <textarea rows="20" cols="82" class="source">{% include
"diffs.jinja2" %}</textarea>
{% endif %}
<h2>Relevant files</h2>
<ul>
diff --git a/puppet_compiler/tests/fixtures/catalog-change.pson
b/puppet_compiler/tests/fixtures/catalog-change.pson
new file mode 100644
index 0000000..1dd6d19
--- /dev/null
+++ b/puppet_compiler/tests/fixtures/catalog-change.pson
@@ -0,0 +1,261 @@
+{
+ "document_type": "Catalog",
+ "data": {
+ "tags": ["settings","systemd","node","class"],
+ "name": "test123.test",
+ "version": 1500366969,
+ "environment": "production",
+ "resources": [
+ {
+ "type": "Stage",
+ "title": "main",
+ "tags": ["stage"],
+ "exported": false,
+ "parameters": {
+ "name": "main"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Settings",
+ "tags": ["class","settings"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "main",
+ "tags": ["class"],
+ "exported": false,
+ "parameters": {
+ "name": "main"
+ }
+ },
+ {
+ "type": "Node",
+ "title": "test123.test",
+ "tags": ["node","test123.test","class"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "Role::Configcluster",
+ "tags":
["class","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "Standard",
+ "tags":
["class","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false,
+ "parameters": {
+ "has_default_mail_relay": true,
+ "has_admin": true,
+ "has_ganglia": true
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Profile::Base",
+ "tags":
["class","profile::base","profile","base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false,
+ "parameters": {
+ "puppetmaster": "puppet",
+ "dns_alt_names": "foobar.test",
+ "environment": "future",
+ "use_apt_proxy": true,
+ "domain_search": "test",
+ "remote_syslog": [
+ "syslog.eqiad.wmnet",
+ "syslog.test"
+ ],
+ "monitoring": true,
+ "core_dump_pattern": "/var/tmp/core/core.%h.%e.%p.%t",
+ "ssh_server_settings": {
+
+ },
+ "nrpe_allowed_hosts":
"127.0.0.1,208.80.154.14,208.80.153.74,208.80.155.119",
+ "group_contact": "admins",
+ "check_disk_options": "-w 6% -c 3% -l -e -A -i \"/srv/sd[a-b][1-3]\"
--exclude-type=tracefs",
+ "check_disk_critical": false,
+ "check_raid_policy": "",
+ "require": "Class[Profile::Base::Certificates]"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Profile::Base::Certificates",
+ "tags":
["class","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "Sslcert3",
+ "tags":
["class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false
+ },
+ {
+ "type": "Package",
+ "title": "openssl",
+ "tags":
["package","openssl","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 15,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Package",
+ "title": "ssl-cert",
+ "tags":
["package","ssl-cert","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 15,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Package",
+ "title": "ca-certificates",
+ "tags":
["package","ca-certificates","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 15,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Exec",
+ "title": "update-ca-certificates",
+ "tags":
["exec","update-ca-certificates","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 21,
+ "exported": false,
+ "parameters": {
+ "command": "/usr/sbin/update-ca-certificates -a",
+ "refreshonly": true,
+ "require": "Package[ca-certificates]"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/etc/ssl/localcerts",
+ "tags":
["file","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 31,
+ "exported": false,
+ "parameters": {
+ "ensure": "directory",
+ "owner": "root",
+ "group": "ssl-cert",
+ "mode": "0755",
+ "require": "Package[ssl-cert]"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/etc/ssl/private",
+ "tags":
["file","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 41,
+ "exported": false,
+ "parameters": {
+ "ensure": "directory",
+ "owner": "root",
+ "group": "ssl-cert",
+ "mode": "0711",
+ "require": "Package[ssl-cert]"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/usr/local/sbin/x509-bundle",
+ "tags":
["file","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 50,
+ "exported": false,
+ "parameters": {
+ "ensure": "present",
+ "owner": "root",
+ "group": "root",
+ "mode": "0755",
+ "source": "puppet:///modules/sslcert/x509-bundle"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Base::Kernel",
+ "tags":
["class","base::kernel","base","kernel","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/profile/manifests/base.pp",
+ "line": 84,
+ "exported": false
+ },
+ {
+ "type": "File",
+ "title": "/etc/modprobe.d/blacklist-wmf.conf",
+ "tags":
["file","class","base::kernel","base","kernel","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/base/manifests/kernel.pp",
+ "line": 26,
+ "exported": false,
+ "parameters": {
+ "ensure": "present",
+ "owner": "root",
+ "group": "root",
+ "mode": "0444",
+ "source": "puppet:///modules/base/kernel/blacklist-wmf.conf"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/etc/modprobe.d/blacklist-linux44.conf",
+ "tags":
["file","class","base::kernel","base","kernel","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/base/manifests/kernel.pp",
+ "line": 35,
+ "exported": false,
+ "parameters": {
+ "ensure": "present",
+ "owner": "root",
+ "group": "root",
+ "mode": "0444",
+ "source": "puppet:///modules/base/kernel/blacklist-linux44.conf"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Base::Debdeploy",
+ "tags":
["class","base::debdeploy","base","debdeploy","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/profile/manifests/base.pp",
+ "line": 85,
+ "exported": false
+ },
+ {
+ "type": "Package",
+ "title": "debdeploy-minion",
+ "tags":
["package","debdeploy-minion","class","base::debdeploy","base","debdeploy","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/base/manifests/debdeploy.pp",
+ "line": 10,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Salt::Grain",
+ "title": "debdeploy-etcd-codfw",
+ "tags":
["salt::grain","salt","grain","debdeploy-etcd-codfw","class","base::debdeploy","base","debdeploy","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false,
+ "parameters": {
+ "value": "standard",
+ "grain": "debdeploy-etcd-codfw",
+ "ensure": "present",
+ "replace": false
+ }
+ }
+ ]
+ },
+ "metadata": {
+ "api_version": 1
+ }
+}
diff --git a/puppet_compiler/tests/fixtures/catalog.pson
b/puppet_compiler/tests/fixtures/catalog.pson
new file mode 100644
index 0000000..c4c9692
--- /dev/null
+++ b/puppet_compiler/tests/fixtures/catalog.pson
@@ -0,0 +1,261 @@
+{
+ "document_type": "Catalog",
+ "data": {
+ "tags": ["settings","systemd","node","class"],
+ "name": "test123.test",
+ "version": 1500366969,
+ "environment": "production",
+ "resources": [
+ {
+ "type": "Stage",
+ "title": "main",
+ "tags": ["stage"],
+ "exported": false,
+ "parameters": {
+ "name": "main"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Settings",
+ "tags": ["class","settings"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "main",
+ "tags": ["class"],
+ "exported": false,
+ "parameters": {
+ "name": "main"
+ }
+ },
+ {
+ "type": "Node",
+ "title": "test123.test",
+ "tags": ["node","test123.test","class"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "Role::Configcluster",
+ "tags":
["class","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "Standard",
+ "tags":
["class","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false,
+ "parameters": {
+ "has_default_mail_relay": true,
+ "has_admin": true,
+ "has_ganglia": true
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Profile::Base",
+ "tags":
["class","profile::base","profile","base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false,
+ "parameters": {
+ "puppetmaster": "puppet",
+ "dns_alt_names": false,
+ "environment": "future",
+ "use_apt_proxy": true,
+ "domain_search": "test",
+ "remote_syslog": [
+ "syslog.eqiad.wmnet",
+ "syslog.test"
+ ],
+ "monitoring": true,
+ "core_dump_pattern": "/var/tmp/core/core.%h.%e.%p.%t",
+ "ssh_server_settings": {
+
+ },
+ "nrpe_allowed_hosts":
"127.0.0.1,208.80.154.14,208.80.153.74,208.80.155.119",
+ "group_contact": "admins",
+ "check_disk_options": "-w 6% -c 3% -l -e -A -i \"/srv/sd[a-b][1-3]\"
--exclude-type=tracefs",
+ "check_disk_critical": false,
+ "check_raid_policy": "",
+ "require": "Class[Profile::Base::Certificates]"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Profile::Base::Certificates",
+ "tags":
["class","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false
+ },
+ {
+ "type": "Class",
+ "title": "Sslcert",
+ "tags":
["class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false
+ },
+ {
+ "type": "Package",
+ "title": "openssl",
+ "tags":
["package","openssl","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 15,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Package",
+ "title": "ssl-cert",
+ "tags":
["package","ssl-cert","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 15,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Package",
+ "title": "ca-certificates",
+ "tags":
["package","ca-certificates","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 15,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Exec",
+ "title": "update-ca-certificates",
+ "tags":
["exec","update-ca-certificates","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 21,
+ "exported": false,
+ "parameters": {
+ "command": "/usr/sbin/update-ca-certificates",
+ "refreshonly": true,
+ "require": "Package[ca-certificates]"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/etc/ssl/localcerts",
+ "tags":
["file","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 31,
+ "exported": false,
+ "parameters": {
+ "ensure": "directory",
+ "owner": "root",
+ "group": "ssl-cert",
+ "mode": "0755",
+ "require": "Package[ssl-cert]"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/etc/ssl/private",
+ "tags":
["file","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 41,
+ "exported": false,
+ "parameters": {
+ "ensure": "directory",
+ "owner": "root",
+ "group": "ssl-cert",
+ "mode": "0711",
+ "require": "Package[ssl-cert]"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/usr/local/sbin/x509-bundle",
+ "tags":
["file","class","sslcert","profile::base::certificates","profile","base","certificates","profile::base","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/sslcert/manifests/init.pp",
+ "line": 50,
+ "exported": false,
+ "parameters": {
+ "ensure": "present",
+ "owner": "root",
+ "group": "root",
+ "mode": "0755",
+ "source": "puppet:///modules/sslcert/x509-bundle"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Base::Kernel",
+ "tags":
["class","base::kernel","base","kernel","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/profile/manifests/base.pp",
+ "line": 84,
+ "exported": false
+ },
+ {
+ "type": "File",
+ "title": "/etc/modprobe.d/blacklist-wmf.conf",
+ "tags":
["file","class","base::kernel","base","kernel","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/base/manifests/kernel.pp",
+ "line": 26,
+ "exported": false,
+ "parameters": {
+ "ensure": "present",
+ "owner": "root",
+ "group": "root",
+ "mode": "0444",
+ "source": "puppet:///modules/base/kernel/blacklist-wmf.conf"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/etc/modprobe.d/blacklist-linux44.conf",
+ "tags":
["file","class","base::kernel","base","kernel","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/base/manifests/kernel.pp",
+ "line": 35,
+ "exported": false,
+ "parameters": {
+ "ensure": "present",
+ "owner": "root",
+ "group": "root",
+ "mode": "0444",
+ "source": "puppet:///modules/base/kernel/blacklist-linux44.conf"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Base::Debdeploy",
+ "tags":
["class","base::debdeploy","base","debdeploy","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/profile/manifests/base.pp",
+ "line": 85,
+ "exported": false
+ },
+ {
+ "type": "Package",
+ "title": "debdeploy-minion",
+ "tags":
["package","debdeploy-minion","class","base::debdeploy","base","debdeploy","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "file": "/tmp/testspc/change/src/modules/base/manifests/debdeploy.pp",
+ "line": 10,
+ "exported": false,
+ "parameters": {
+ "ensure": "present"
+ }
+ },
+ {
+ "type": "Salt::Grain",
+ "title": "debdeploy-etcd-codfw",
+ "tags":
["salt::grain","salt","grain","debdeploy-etcd-codfw","class","base::debdeploy","base","debdeploy","profile::base","profile","standard","role::configcluster","role","configcluster","node","test123.test"],
+ "exported": false,
+ "parameters": {
+ "value": "standard",
+ "grain": "debdeploy-etcd-codfw",
+ "ensure": "present",
+ "replace": false
+ }
+ }
+ ]
+ },
+ "metadata": {
+ "api_version": 1
+ }
+}
diff --git a/puppet_compiler/tests/test_controller.py
b/puppet_compiler/tests/test_controller.py
index f04c68c..b6d2ffe 100644
--- a/puppet_compiler/tests/test_controller.py
+++ b/puppet_compiler/tests/test_controller.py
@@ -40,7 +40,8 @@
c.m.refresh = mock.MagicMock()
c.m.refresh.return_value = True
- c.run()
+ with mock.patch('time.sleep'):
+ c.run()
c.m.prepare.assert_called_once_with()
c.m.refresh.assert_not_called()
self.assertEquals(c.state.modes['change']['fail'],
set(['test.eqiad.wmnet']))
@@ -48,7 +49,8 @@
c.config['puppet_src'] = '/src'
c.config['puppet_private'] = '/private'
mocker.return_value = (False, False, None)
- c.run()
+ with mock.patch('time.sleep'):
+ c.run()
c.m.refresh.assert_has_calls([mock.call('/src'),
mock.call('/private')])
self.assertEquals(c.state.modes['change']['noop'],
set(['test.eqiad.wmnet']))
diff --git a/puppet_compiler/tests/test_differ.py
b/puppet_compiler/tests/test_differ.py
new file mode 100644
index 0000000..5e1e580
--- /dev/null
+++ b/puppet_compiler/tests/test_differ.py
@@ -0,0 +1,110 @@
+import mock
+import os
+import unittest
+
+from puppet_compiler.differ import PuppetResource, PuppetCatalog
+from puppet_compiler.worker import future_filter
+
+
+class TestPuppetResource(unittest.TestCase):
+
+ def setUp(self):
+ self.raw_resource = {
+ 'type': 'Class',
+ 'title': 'hhvm',
+ 'exported': False,
+ 'parameters': {
+ 'ensure': 'present',
+ 'type': 'fcgi',
+ 'settings': {'foo': 1, 'bar': [2, 3]},
+ 'before': ['Class[apache2]'],
+ 'nope': None,
+ }
+ }
+ self.r = PuppetResource(
+ self.raw_resource,
+ future_filter,
+ )
+
+ def test_init(self):
+ """Test initialization"""
+ self.assertEqual(self.r.resource_type, 'Class')
+ self.assertEqual(self.r.title, 'hhvm')
+ self.assertEqual(self.r._filter, future_filter)
+ self.assertEqual(self.r.content, '')
+ self.assertEqual(self.r.parameters['before'], 'Class[apache2]')
+ self.assertEqual(self.r.parameters['settings'], {'foo': '1', 'bar':
['2', '3']})
+ self.assertNotIn('nope', self.r.parameters)
+
+ def test_str(self):
+ """Test stringification"""
+ self.assertEqual(str(self.r), 'Class[hhvm]')
+
+ def test_is_same(self):
+ """Test function `PuppetResource.is_same_of`"""
+ other = mock.MagicMock()
+ other.__str__.return_value = 'Class[hhvm]'
+ self.assertTrue(self.r.is_same_of(other))
+ other.__str__.return_value = 'Hhvm::monitoring[test]'
+ self.assertFalse(self.r.is_same_of(other))
+
+ def test_equality(self):
+ # Not filtered resource, it should be different
+ other = PuppetResource(self.raw_resource)
+ self.assertNotEqual(self.r, other)
+ other = PuppetResource(self.raw_resource, future_filter)
+ self.assertEqual(self.r, other)
+ other.content = 'lala'
+ self.assertNotEqual(self.r, other)
+
+ @mock.patch('difflib.unified_diff')
+ @mock.patch('datadiff.diff')
+ def test_diff_if_present(self, datadiff_mock, difflib_mock):
+ other = PuppetResource(self.raw_resource, future_filter)
+ self.assertIsNone(self.r.diff_if_present(other))
+ # Now let's add some content
+ self.r.resource_type = 'File'
+ other.resource_type = 'File'
+ other.content = "lala\nalal"
+ diff = self.r.diff_if_present(other)
+ datadiff_mock.assert_not_called()
+ difflib_mock.assert_called_with(
+ [''], ['lala', 'alal'],
+ fromfile='hhvm.orig',
+ tofile='hhvm'
+ )
+ self.r.resource_type = 'Class'
+ datadiff_mock.reset_mock()
+ difflib_mock.reset_mock()
+ # Not filtered resource, it should be different
+ other = PuppetResource(self.raw_resource)
+ diff = self.r.diff_if_present(other)
+ difflib_mock.assert_not_called()
+ datadiff_mock.assert_called_with(
+ self.r.parameters, other.parameters,
+ fromfile='Class[hhvm].orig', tofile='Class[hhvm]'
+ )
+ self.assertNotIn('content', diff)
+ self.assertIn('parameters', diff)
+
+
+class TestPuppetCatalog(unittest.TestCase):
+
+ def setUp(self):
+ fixtures = os.path.join(os.path.dirname(__file__), 'fixtures')
+ self.orig = PuppetCatalog(os.path.join(fixtures, 'catalog.pson'))
+ self.change = PuppetCatalog(os.path.join(fixtures,
'catalog-change.pson'))
+
+ def test_init(self):
+ """Test initialization."""
+ self.assertEqual(self.orig.name, 'test123.test')
+ self.assertIsInstance(self.orig.resources, dict)
+
+ def test_diff_is_present(self):
+ self.assertIsNone(self.orig.diff_if_present(self.orig))
+ diffs = self.orig.diff_if_present(self.change)
+ self.assertEqual(diffs['total'], 22)
+ self.assertEqual(diffs['only_in_self'], set(['Class[Sslcert]']))
+ self.assertEqual(diffs['only_in_other'], set(['Class[Sslcert3]']))
+ self.assertEqual(len(diffs['resource_diffs']), 2)
+ self.assertEqual(diffs['perc_changed'], '18.18%')
diff --git a/puppet_compiler/tests/test_hostworker.py
b/puppet_compiler/tests/test_hostworker.py
index c6f7aaf..013e0e9 100644
--- a/puppet_compiler/tests/test_hostworker.py
+++ b/puppet_compiler/tests/test_hostworker.py
@@ -17,6 +17,8 @@
self.assertEquals(self.hw.hostname, 'test.example.com')
self.assertItemsEqual(['prod', 'change'], self.hw._envs)
+ self.assertIsNone(self.hw.diffs)
+ self.assertEqual(self.hw.resource_filter, worker.future_filter)
@mock.patch('os.path.isfile')
def test_facts_file(self, isfile):
@@ -58,18 +60,28 @@
err = self.hw._compile_all()
self.assertEquals(err, 2)
- @mock.patch('puppet_compiler.puppet.diff')
+ @mock.patch('puppet_compiler.worker.PuppetCatalog')
def test_make_diff(self, mocker):
- mocker.return_value = True
- self.hw._get_diff = mock.Mock(return_value=True)
- retval = self.hw._make_diff()
- mocker.assert_called_with('change', 'test.example.com', base='prod')
- self.assertEquals(retval, True)
- self.hw._get_diff = mock.Mock(return_value=False)
- self.assertEquals(self.hw._make_diff(), None)
- mocker.reset_mock()
- mocker.side_effect = subprocess.CalledProcessError(cmd="ehehe",
returncode=30)
- self.assertEquals(self.hw._make_diff(), False)
+ instance_mock = mocker.return_value
+ instance_mock.diff_if_present.return_value = None
+ self.assertIsNone(self.hw._make_diff())
+ self.assertIsNone(self.hw.diffs)
+
+ mocker.assert_has_calls(
+ [
+ mock.call(
+
'/mnt/jenkins-workspace/19/production/catalogs/test.example.com.pson',
worker.future_filter
+ ),
+ mock.call(
+
'/mnt/jenkins-workspace/19/change/catalogs/test.example.com.pson',
worker.future_filter
+ )
+ ]
+ )
+ instance_mock.diff_if_present.return_value = {'foo': 'bar'}
+ self.assertTrue(self.hw._make_diff())
+ self.assertEqual(self.hw.diffs, {'foo': 'bar'})
+ instance_mock.diff_if_present.side_effect = ValueError("ehehe")
+ self.assertFalse(self.hw._make_diff())
@mock.patch('os.path.isfile')
@mock.patch('os.makedirs')
@@ -80,35 +92,15 @@
mock_makedirs.assert_called_with(self.hw._files.outdir, 0755)
assert not mock_copy.called
mock_isfile.return_value = True
- self.hw._get_diff = mock.MagicMock(return_value=False)
self.hw._make_output()
mock_copy.assert_called_with(
'/mnt/jenkins-workspace/19/change/catalogs/test.example.com.err',
'/mnt/jenkins-workspace/output/19/test.example.com/change.test.example.com.err',
)
- self.hw._get_diff = mock.MagicMock(return_value='test_value')
- self.hw._make_output()
- mock_copy.assert_called_with(
- 'test_value',
- '/mnt/jenkins-workspace/output/19/test.example.com'
- )
-
- @mock.patch('os.path')
- def test_get_diff(self, mock_path):
- mock_path.join.return_value = self.fixtures + '/' + 'exit_nodiff'
- mock_path.isfile.return_value = True
- mock_path.getsize.return_value = 100
- self.assertEquals(self.hw._get_diff(), False)
- filename = self.fixtures + '/' + 'exit_diff'
- mock_path.join.return_value = filename
- self.assertEquals(self.hw._get_diff(), filename)
-
- def test_build_html(self):
- pass
def test_run_host(self):
self.hw.facts_file = mock.Mock(return_value=False)
- self.assertEquals(self.hw.run_host(), 'fail')
+ self.assertEquals(self.hw.run_host(), (True, True, None))
self.hw.facts_file = mock.Mock(return_value=True)
self.hw._compile_all = mock.Mock(return_value=0)
self.hw._make_diff = mock.Mock(return_value=True)
@@ -141,8 +133,6 @@
def test_compile_all(self):
self.hw._compile = mock.Mock(return_value=True)
- self.hw.filter_future.run = mock.Mock()
- self.hw.filter_change.run = mock.Mock()
self.assertEquals(self.hw._compile_all(), 0)
future_parser = [
'--environment=future',
@@ -154,5 +144,3 @@
mock.call('change', []),
mock.call('future', future_parser)
])
- self.hw.filter_future.run.assert_called_with()
- self.hw.filter_change.run.assert_called_with()
diff --git a/puppet_compiler/tests/test_html.py
b/puppet_compiler/tests/test_html.py
new file mode 100644
index 0000000..372850a
--- /dev/null
+++ b/puppet_compiler/tests/test_html.py
@@ -0,0 +1,40 @@
+import os
+import shutil
+import tempfile
+import unittest
+
+from puppet_compiler.presentation import html
+from puppet_compiler.differ import PuppetCatalog
+from puppet_compiler.directories import FHS, HostFiles
+
+
+class TestHost(unittest.TestCase):
+
+ def setUp(self):
+ self.tempdir = tempfile.mkdtemp(prefix='puppet-compiler')
+ fixtures = os.path.join(os.path.dirname(__file__), 'fixtures')
+ orig = PuppetCatalog(os.path.join(fixtures, 'catalog.pson'))
+ change = PuppetCatalog(os.path.join(fixtures, 'catalog-change.pson'))
+ self.diffs = orig.diff_if_present(change)
+ FHS.setup('10', self.tempdir)
+ self.files = HostFiles('test.example.com')
+ os.makedirs(self.files.outdir)
+
+ def tearDown(self):
+ shutil.rmtree(self.tempdir)
+
+ def test_init(self):
+ h = html.Host('test.example.com', self.files, 'diff')
+ self.assertEqual(h.retcode, 'diff')
+ self.assertEqual(h.hostname, 'test.example.com')
+ self.assertEqual(h.outdir, self.files.outdir)
+
+ def test_htmlpage(self):
+ h = html.Host('test.example.com', self.files, 'diff')
+ h.htmlpage(self.diffs)
+ # Test the test file has been produced
+ output = os.path.join(h.outdir, h.page_name)
+ assert os.path.isfile(output)
+ with open(output, 'r') as fh:
+ html_data = fh.read()
+ assert len(html_data) > 0
diff --git a/puppet_compiler/worker.py b/puppet_compiler/worker.py
index 1f5406b..654c4b2 100644
--- a/puppet_compiler/worker.py
+++ b/puppet_compiler/worker.py
@@ -1,13 +1,18 @@
import os
-import re
import shutil
import subprocess
from puppet_compiler import puppet, _log
-from puppet_compiler.filter import FilterFutureParser
+from puppet_compiler.filter import itos, flatten
+from puppet_compiler.differ import PuppetCatalog
from puppet_compiler.directories import HostFiles, FHS
from puppet_compiler.presentation import html
from puppet_compiler.state import ChangeState, FutureState
+
+
+def future_filter(value):
+ """This filter must be used during the transition to the future parser"""
+ return itos(flatten(value))
class HostWorker(object):
@@ -23,6 +28,8 @@
self._files = HostFiles(hostname)
self._envs = ['prod', 'change']
self.hostname = hostname
+ self.diffs = None
+ self.resource_filter = future_filter
def facts_file(self):
""" Finds facts file for the current hostname """
@@ -39,7 +46,7 @@
if not self.facts_file():
_log.error('Unable to find facts for host %s, skipping',
self.hostname)
- return 'fail'
+ return (True, True, None)
errors = self._compile_all()
if errors == self.E_OK:
diff = self._make_diff()
@@ -112,21 +119,26 @@
def _make_diff(self):
"""
Will produce diffs.
+
+ Returns True if there are diffs, None if no diffs are found,
+ False if diffing failed
"""
- # Both nodes compiled correctly
_log.info("Calculating diffs for %s", self.hostname)
try:
- puppet.diff(self._envs[1], self.hostname, base=self._envs[0])
- except subprocess.CalledProcessError as e:
+ original = PuppetCatalog(self._files.file_for(self._envs[0],
'catalog'),
+ self.resource_filter)
+ new = PuppetCatalog(self._files.file_for(self._envs[1], 'catalog'),
+ self.resource_filter)
+ self.diffs = original.diff_if_present(new)
+ except Exception as e:
_log.error("Diffing the catalogs failed: %s", self.hostname)
- _log.info("Diffing exited with code %d", e.returncode)
- _log.debug("Failed command: %s", e.cmd)
+ _log.info("Diffing failed with exception %s", e)
return False
else:
- if self._get_diff():
- return True
- else:
+ if self.diffs is None:
return None
+ else:
+ return True
def _make_output(self):
"""
@@ -142,38 +154,14 @@
newname = self._files.outfile_for(env, label)
shutil.copy(filename, newname)
- diff = self._get_diff()
- if diff:
- shutil.copy(diff, self._files.outdir)
- return True
-
- return False
-
- def _get_diff(self):
- """
- Get diffs name if the file exists
- """
- diff = self._files.file_for(self._envs[1], 'diff')
-
- if os.path.isfile(diff) and os.path.getsize(diff) > 0:
- with open(diff, 'r') as f:
- for line in f:
- # If no changes were detected, puppet catalog diff outputs
- # a line with 'fqdn 0.0%'
- # we use that to detect a noop change since we have no
other
- # means to detect that.
- if re.match('^{}\s+0.0\%'.format(self.hostname), line):
- return False
- return diff
return False
def _build_html(self, retcode):
"""
build the HTML output
"""
- # TODO: implement the actual html parsing
host = self.html_page(self.hostname, self._files, retcode)
- host.htmlpage()
+ host.htmlpage(self.diffs)
class FutureHostWorker(HostWorker):
@@ -196,8 +184,6 @@
def __init__(self, vardir, hostname):
super(FutureHostWorker, self).__init__(vardir, hostname)
self._envs = ['change', 'future']
- self.filter_future = FilterFutureParser(self._files.file_for('future',
'catalog'))
- self.filter_change = FilterFutureParser(self._files.file_for('change',
'catalog'))
def _compile_all(self):
future_args = [
@@ -208,15 +194,9 @@
]
args = []
errors = self.E_OK
- if self._compile(self._envs[0], args):
- _log.info("Filtering the change catalog (%s)", self.hostname)
- self.filter_change.run()
- else:
+ if not self._compile(self._envs[0], args):
errors += self.E_BASE
- if self._compile(self._envs[1], future_args):
- _log.info("Filtering the future catalog (%s)", self.hostname)
- self.filter_future.run()
- else:
+ if not self._compile(self._envs[1], future_args):
errors += self.E_CHANGED
return errors
diff --git a/setup.py b/setup.py
index 3a52ad6..16d771a 100755
--- a/setup.py
+++ b/setup.py
@@ -12,11 +12,11 @@
setup(
name='puppet_compiler',
- version='0.2.3',
+ version='0.3.0',
description='Tools to compile puppet catalogs as a service',
author='Joe',
author_email='glavage...@wikimedia.org',
- install_requires=['jinja2', 'requests', 'pyyaml'],
+ install_requires=['jinja2', 'requests', 'pyyaml', 'datadiff>=2.0.0'],
test_suite='nose.collector',
tests_require=['mock<1.1.0', 'nose'],
zip_safe=True,
--
To view, visit https://gerrit.wikimedia.org/r/369385
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I03d4319b49f3c7a195e076decc6a7b741e13d399
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/puppet-compiler
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
