From 24bf9ab055646055784ac9dc544681cb5379b69e Mon Sep 17 00:00:00 2001 From: David Moreau Simard Date: Wed, 5 Sep 2018 09:58:29 -0400 Subject: [PATCH] Add Stats model and API - Reset database migrations back to initial (database schema is not stable or supported yet, let's not burden ourselves with migrations needlessly) - Move host stats to a Stats model - Tie the hosts back to the playbook instead of the play While hosts are in fact "children" of plays, Ansible doesn't provide stats per-play. From ARA's perspective, it's simpler to keep hosts at the same level as the stats. Depends-On: https://review.openstack.org/600058 Change-Id: I127efd79a5077488ffa084d2784d5a3c6f2da2da --- ara/api/migrations/0001_initial.py | 50 +++++++++++--- ara/api/migrations/0002_add_labels.py | 31 --------- ara/api/migrations/0003_host_alias.py | 18 ----- ara/api/models.py | 38 ++++++++--- ara/api/serializers.py | 14 +++- ara/api/tests/factories.py | 20 ++++-- ara/api/tests/tests_host.py | 33 ++++----- ara/api/tests/tests_stats.py | 96 +++++++++++++++++++++++++++ ara/api/urls.py | 2 + ara/api/views.py | 13 +++- hacking/validate.py | 12 ++++ 11 files changed, 231 insertions(+), 96 deletions(-) delete mode 100644 ara/api/migrations/0002_add_labels.py delete mode 100644 ara/api/migrations/0003_host_alias.py create mode 100644 ara/api/tests/tests_stats.py diff --git a/ara/api/migrations/0001_initial.py b/ara/api/migrations/0001_initial.py index 590ed65..6ce06b5 100644 --- a/ara/api/migrations/0001_initial.py +++ b/ara/api/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 2.0.6 on 2018-06-19 20:27 +# Generated by Django 2.1.1 on 2018-09-05 13:37 from django.db import migrations, models import django.db.models.deletion @@ -46,16 +46,25 @@ class Migration(migrations.Migration): ('updated', models.DateTimeField(auto_now=True)), ('name', models.CharField(max_length=255)), ('facts', models.BinaryField(max_length=4294967295)), - ('changed', models.IntegerField(default=0)), - ('failed', models.IntegerField(default=0)), - ('ok', models.IntegerField(default=0)), - ('skipped', models.IntegerField(default=0)), - ('unreachable', models.IntegerField(default=0)), + ('alias', models.CharField(max_length=255, null=True)), ], options={ 'db_table': 'hosts', }, ), + migrations.CreateModel( + name='Label', + fields=[ + ('id', models.BigAutoField(editable=False, primary_key=True, serialize=False)), + ('created', models.DateTimeField(auto_now_add=True)), + ('updated', models.DateTimeField(auto_now=True)), + ('name', models.CharField(max_length=255)), + ('description', models.BinaryField(max_length=4294967295)), + ], + options={ + 'db_table': 'labels', + }, + ), migrations.CreateModel( name='Play', fields=[ @@ -84,6 +93,7 @@ class Migration(migrations.Migration): ('parameters', models.BinaryField(max_length=4294967295)), ('file', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='playbooks', to='api.File')), ('files', models.ManyToManyField(to='api.File')), + ('labels', models.ManyToManyField(to='api.Label')), ], options={ 'db_table': 'playbooks', @@ -120,6 +130,24 @@ class Migration(migrations.Migration): 'db_table': 'results', }, ), + migrations.CreateModel( + name='Stats', + fields=[ + ('id', models.BigAutoField(editable=False, primary_key=True, serialize=False)), + ('created', models.DateTimeField(auto_now_add=True)), + ('updated', models.DateTimeField(auto_now=True)), + ('changed', models.IntegerField(default=0)), + ('failed', models.IntegerField(default=0)), + ('ok', models.IntegerField(default=0)), + ('skipped', models.IntegerField(default=0)), + ('unreachable', models.IntegerField(default=0)), + ('host', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='stats', to='api.Host')), + ('playbook', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='stats', to='api.Playbook')), + ], + options={ + 'db_table': 'stats', + }, + ), migrations.CreateModel( name='Task', fields=[ @@ -153,20 +181,24 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name='host', - name='play', - field=models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, related_name='hosts', to='api.Play'), + name='playbook', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='hosts', to='api.Playbook'), ), migrations.AddField( model_name='file', name='content', field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='files', to='api.FileContent'), ), + migrations.AlterUniqueTogether( + name='stats', + unique_together={('host', 'playbook')}, + ), migrations.AlterUniqueTogether( name='record', unique_together={('key', 'playbook')}, ), migrations.AlterUniqueTogether( name='host', - unique_together={('name', 'play')}, + unique_together={('name', 'playbook')}, ), ] diff --git a/ara/api/migrations/0002_add_labels.py b/ara/api/migrations/0002_add_labels.py deleted file mode 100644 index ed38599..0000000 --- a/ara/api/migrations/0002_add_labels.py +++ /dev/null @@ -1,31 +0,0 @@ -# Generated by Django 2.1.1 on 2018-09-03 23:33 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0001_initial'), - ] - - operations = [ - migrations.CreateModel( - name='Label', - fields=[ - ('id', models.BigAutoField(editable=False, primary_key=True, serialize=False)), - ('created', models.DateTimeField(auto_now_add=True)), - ('updated', models.DateTimeField(auto_now=True)), - ('name', models.CharField(max_length=255)), - ('description', models.BinaryField(max_length=4294967295)), - ], - options={ - 'db_table': 'labels', - }, - ), - migrations.AddField( - model_name='playbook', - name='labels', - field=models.ManyToManyField(to='api.Label'), - ), - ] diff --git a/ara/api/migrations/0003_host_alias.py b/ara/api/migrations/0003_host_alias.py deleted file mode 100644 index d30ca43..0000000 --- a/ara/api/migrations/0003_host_alias.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 2.1.1 on 2018-09-04 00:14 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0002_add_labels'), - ] - - operations = [ - migrations.AddField( - model_name='host', - name='alias', - field=models.CharField(max_length=255, null=True), - ), - ] diff --git a/ara/api/models.py b/ara/api/models.py index 3289853..2a3cc90 100644 --- a/ara/api/models.py +++ b/ara/api/models.py @@ -179,21 +179,14 @@ class Task(Duration): class Host(Base): """ Data about Ansible hosts. - Contains compressed host facts and statistics about the host for the - playbook. """ class Meta: db_table = 'hosts' - unique_together = ('name', 'play',) + unique_together = ('name', 'playbook',) name = models.CharField(max_length=255) facts = models.BinaryField(max_length=(2 ** 32) - 1) - changed = models.IntegerField(default=0) - failed = models.IntegerField(default=0) - ok = models.IntegerField(default=0) - skipped = models.IntegerField(default=0) - unreachable = models.IntegerField(default=0) # Ansible doesn't supply a mechanism to uniquely identify a host out of # the box. # ARA can attempt to reconcile what it believes are the results same hosts @@ -203,13 +196,38 @@ class Host(Base): # The logic for supplying aliases does not live here, it's provided by the # clients and consumers. alias = models.CharField(max_length=255, null=True) - - play = models.ForeignKey(Play, on_delete=models.DO_NOTHING, related_name='hosts') + playbook = models.ForeignKey(Playbook, on_delete=models.CASCADE, related_name='hosts') def __str__(self): return '' % (self.id, self.name) +class Stats(Base): + """ + Stats for a host for a playbook. + """ + + class Meta: + db_table = 'stats' + unique_together = ('host', 'playbook',) + + playbook = models.ForeignKey(Playbook, on_delete=models.CASCADE, related_name='stats') + host = models.ForeignKey(Host, on_delete=models.CASCADE, related_name='stats') + changed = models.IntegerField(default=0) + failed = models.IntegerField(default=0) + ok = models.IntegerField(default=0) + skipped = models.IntegerField(default=0) + unreachable = models.IntegerField(default=0) + + def __str__(self): + # Verbose because it's otherwise kind of useless + return ''.format( + host=self.host.name, + id=self.host.id, + playbook=self.playbook.id + ) + + class Result(Duration): """ Data about Ansible results. diff --git a/ara/api/serializers.py b/ara/api/serializers.py index efec13b..143f682 100644 --- a/ara/api/serializers.py +++ b/ara/api/serializers.py @@ -124,7 +124,7 @@ class HostSerializer(serializers.ModelSerializer): def create(self, validated_data): host, created = models.Host.objects.get_or_create( name=validated_data['name'], - play=validated_data['play'], + playbook=validated_data['playbook'], defaults=validated_data ) return host @@ -157,6 +157,7 @@ class PlaybookSerializer(DurationSerializer): parameters = CompressedObjectField(default=zlib.compress(json.dumps({}).encode('utf8'))) file = FileSerializer() files = FileSerializer(many=True, default=[]) + hosts = HostSerializer(many=True, default=[]) labels = LabelSerializer(many=True, default=[]) def create(self, validated_data): @@ -166,12 +167,15 @@ class PlaybookSerializer(DurationSerializer): # Create the playbook without the file and label references for now files = validated_data.pop('files') + hosts = validated_data.pop('hosts') labels = validated_data.pop('labels') playbook = models.Playbook.objects.create(**validated_data) - # Add the files and the labels in + # Add the files, hosts and the labels in for file in files: playbook.files.add(models.File.objects.create(**file)) + for host in hosts: + playbook.hosts.add(models.Host.objects.create(**host)) for label in labels: playbook.labels.add(models.Label.objects.create(**label)) @@ -196,3 +200,9 @@ class TaskSerializer(DurationSerializer): default=zlib.compress(json.dumps([]).encode('utf8')), help_text='A JSON list containing Ansible tags' ) + + +class StatsSerializer(serializers.ModelSerializer): + class Meta: + model = models.Stats + fields = '__all__' diff --git a/ara/api/tests/factories.py b/ara/api/tests/factories.py index 42c6331..df40356 100644 --- a/ara/api/tests/factories.py +++ b/ara/api/tests/factories.py @@ -104,13 +104,8 @@ class HostFactory(factory.DjangoModelFactory): facts = utils.compressed_obj(HOST_FACTS) name = 'hostname' - changed = 1 - failed = 0 - ok = 2 - skipped = 1 - unreachable = 0 alias = "9f5d3ba7-e43d-4f3b-ab17-f90c39e43d07" - play = factory.SubFactory(PlayFactory) + playbook = factory.SubFactory(PlaybookFactory) class ResultFactory(factory.DjangoModelFactory): @@ -121,3 +116,16 @@ class ResultFactory(factory.DjangoModelFactory): status = 'ok' host = factory.SubFactory(HostFactory) task = factory.SubFactory(TaskFactory) + + +class StatsFactory(factory.DjangoModelFactory): + class Meta: + model = models.Stats + + changed = 1 + failed = 0 + ok = 2 + skipped = 1 + unreachable = 0 + playbook = factory.SubFactory(PlaybookFactory) + host = factory.SubFactory(HostFactory) diff --git a/ara/api/tests/tests_host.py b/ara/api/tests/tests_host.py index fdeaf61..9617df8 100644 --- a/ara/api/tests/tests_host.py +++ b/ara/api/tests/tests_host.py @@ -28,23 +28,23 @@ class HostTestCase(APITestCase): self.assertEqual(host.name, 'testhost') def test_host_serializer(self): - play = factories.PlayFactory() + playbook = factories.PlaybookFactory() serializer = serializers.HostSerializer(data={ 'name': 'serializer', - 'play': play.id + 'playbook': playbook.id }) serializer.is_valid() host = serializer.save() host.refresh_from_db() self.assertEqual(host.name, 'serializer') - self.assertEqual(host.play.id, play.id) + self.assertEqual(host.playbook.id, playbook.id) def test_host_serializer_compress_facts(self): - play = factories.PlayFactory() + playbook = factories.PlaybookFactory() serializer = serializers.HostSerializer(data={ 'name': 'compress', 'facts': factories.HOST_FACTS, - 'play': play.id, + 'playbook': playbook.id, }) serializer.is_valid() host = serializer.save() @@ -76,46 +76,41 @@ class HostTestCase(APITestCase): self.assertEqual(0, models.Host.objects.all().count()) def test_create_host(self): - play = factories.PlayFactory() + playbook = factories.PlaybookFactory() self.assertEqual(0, models.Host.objects.count()) request = self.client.post('/api/v1/hosts', { 'name': 'create', - 'play': play.id + 'playbook': playbook.id }) self.assertEqual(201, request.status_code) self.assertEqual(1, models.Host.objects.count()) - def test_post_same_host_for_a_play(self): - play = factories.PlayFactory() + def test_post_same_host_for_a_playbook(self): + playbook = factories.PlaybookFactory() self.assertEqual(0, models.Host.objects.count()) request = self.client.post('/api/v1/hosts', { 'name': 'create', - 'play': play.id, - 'ok': 1 + 'playbook': playbook.id }) self.assertEqual(201, request.status_code) self.assertEqual(1, models.Host.objects.count()) - self.assertEqual(1, request.data['ok']) request = self.client.post('/api/v1/hosts', { 'name': 'create', - 'play': play.id, - 'ok': 2 + 'playbook': playbook.id }) self.assertEqual(201, request.status_code) self.assertEqual(1, models.Host.objects.count()) - # This isn't expected to update the count for 'ok', it's not a patch - self.assertEqual(1, request.data['ok']) def test_partial_update_host(self): host = factories.HostFactory() - self.assertNotEqual(1, host.ok) + self.assertNotEqual('foo', host.name) request = self.client.patch('/api/v1/hosts/%s' % host.id, { - 'ok': 1 + 'name': 'foo' }) self.assertEqual(200, request.status_code) host_updated = models.Host.objects.get(id=host.id) - self.assertEqual(1, host_updated.ok) + self.assertEqual('foo', host_updated.name) def test_get_host(self): host = factories.HostFactory() diff --git a/ara/api/tests/tests_stats.py b/ara/api/tests/tests_stats.py new file mode 100644 index 0000000..4989e07 --- /dev/null +++ b/ara/api/tests/tests_stats.py @@ -0,0 +1,96 @@ +# Copyright (c) 2018 Red Hat, Inc. +# +# This file is part of ARA Records Ansible. +# +# ARA is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# ARA is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with ARA. If not, see . + +from rest_framework.test import APITestCase + +from ara.api import models, serializers +from ara.api.tests import factories + + +class StatsTestCase(APITestCase): + def test_stats_factory(self): + stats = factories.StatsFactory( + changed=2, + failed=1, + ok=3, + skipped=2, + unreachable=1 + ) + self.assertEqual(stats.changed, 2) + self.assertEqual(stats.failed, 1) + self.assertEqual(stats.ok, 3) + self.assertEqual(stats.skipped, 2) + self.assertEqual(stats.unreachable, 1) + + def test_stats_serializer(self): + playbook = factories.PlaybookFactory() + host = factories.HostFactory() + serializer = serializers.StatsSerializer(data=dict( + playbook=playbook.id, + host=host.id, + ok=9001 + )) + serializer.is_valid() + stats = serializer.save() + stats.refresh_from_db() + self.assertEqual(stats.playbook.id, playbook.id) + self.assertEqual(stats.host.id, host.id) + self.assertEqual(stats.ok, 9001) + + def test_create_stats(self): + playbook = factories.PlaybookFactory() + host = factories.HostFactory() + self.assertEqual(0, models.Stats.objects.count()) + request = self.client.post('/api/v1/stats', dict( + playbook=playbook.id, + host=host.id, + ok=9001 + )) + self.assertEqual(201, request.status_code) + self.assertEqual(1, models.Stats.objects.count()) + + def test_get_no_stats(self): + request = self.client.get('/api/v1/stats') + self.assertEqual(0, len(request.data['results'])) + + def test_get_stats(self): + stats = factories.StatsFactory() + request = self.client.get('/api/v1/stats') + self.assertEqual(1, len(request.data['results'])) + self.assertEqual(stats.ok, request.data['results'][0]['ok']) + + def test_get_stats_id(self): + stats = factories.StatsFactory() + request = self.client.get('/api/v1/stats/%s' % stats.id) + self.assertEqual(stats.ok, request.data['ok']) + + def test_partial_update_stats(self): + stats = factories.StatsFactory() + self.assertNotEqual(9001, stats.ok) + request = self.client.patch('/api/v1/stats/%s' % stats.id, dict( + ok=9001 + )) + self.assertEqual(200, request.status_code) + stats_updated = models.Stats.objects.get(id=stats.id) + self.assertEqual(9001, stats_updated.ok) + + def test_delete_stats(self): + stats = factories.StatsFactory() + self.assertEqual(1, models.Stats.objects.all().count()) + request = self.client.delete('/api/v1/stats/%s' % stats.id) + self.assertEqual(204, request.status_code) + self.assertEqual(0, models.Stats.objects.all().count()) diff --git a/ara/api/urls.py b/ara/api/urls.py index 87755e3..b51aa1b 100644 --- a/ara/api/urls.py +++ b/ara/api/urls.py @@ -36,6 +36,8 @@ urlpatterns = [ url(r'^/results/(?P[0-9]+)$', views.ResultDetail.as_view(), name='result-detail'), url(r'^/files$', views.FileList.as_view(), name='file-list'), url(r'^/files/(?P[0-9]+)$', views.FileDetail.as_view(), name='file-detail'), + url(r'^/stats$', views.StatsList.as_view(), name='stats-list'), + url(r'^/stats/(?P[0-9]+)$', views.StatsDetail.as_view(), name='stats-detail'), ] urlpatterns = format_suffix_patterns(urlpatterns) diff --git a/ara/api/views.py b/ara/api/views.py index b10a29a..b3928bb 100644 --- a/ara/api/views.py +++ b/ara/api/views.py @@ -32,7 +32,8 @@ def api_root(request, format=None): 'tasks': reverse('task-list', request=request, format=format), 'files': reverse('file-list', request=request, format=format), 'hosts': reverse('host-list', request=request, format=format), - 'results': reverse('result-list', request=request, format=format) + 'results': reverse('result-list', request=request, format=format), + 'stats': reverse('stats-list', request=request, format=format) }) @@ -118,3 +119,13 @@ class FileList(generics.ListCreateAPIView): class FileDetail(generics.RetrieveUpdateDestroyAPIView): queryset = models.File.objects.all() serializer_class = serializers.FileSerializer + + +class StatsList(generics.ListCreateAPIView): + queryset = models.Stats.objects.all() + serializer_class = serializers.StatsSerializer + + +class StatsDetail(generics.RetrieveUpdateDestroyAPIView): + queryset = models.Stats.objects.all() + serializer_class = serializers.StatsSerializer diff --git a/hacking/validate.py b/hacking/validate.py index 2743e77..e19987c 100644 --- a/hacking/validate.py +++ b/hacking/validate.py @@ -49,6 +49,13 @@ def validate_result(result): assert result['status'] == 'ok' +def validate_stats(stats): + assert 'failed' in stats + assert stats['failed'] == 0 + assert 'playbook' in stats + assert 'host' in stats + + def main(): client = AraOfflineClient() @@ -100,6 +107,11 @@ def main(): result = client.get('/api/v1/results/1') validate_result(result) + stats = client.get('/api/v1/stats') + assert len(stats['results']) == 1 + assert len(stats['count']) == 1 + validate_stats(stats['results'][0]) + client.log.info('All assertions passed.')