mirror of
https://github.com/ansible/awx.git
synced 2026-01-11 18:09:57 -03:30
enforce a stable list order when attaching/detaching instance groups
This commit is contained in:
parent
6908558acd
commit
e4a50f3595
@ -11,6 +11,7 @@ from jinja2 import Environment, StrictUndefined
|
||||
from jinja2.exceptions import UndefinedError, TemplateSyntaxError
|
||||
|
||||
# Django
|
||||
import django
|
||||
from django.core import exceptions as django_exceptions
|
||||
from django.db.models.signals import (
|
||||
post_save,
|
||||
@ -24,8 +25,10 @@ from django.db.models.fields.related_descriptors import (
|
||||
ForwardManyToOneDescriptor,
|
||||
ManyToManyDescriptor,
|
||||
ReverseManyToOneDescriptor,
|
||||
create_forward_many_to_many_manager
|
||||
)
|
||||
from django.utils.encoding import smart_text
|
||||
from django.utils.functional import cached_property
|
||||
from django.utils.translation import ugettext_lazy as _
|
||||
|
||||
# jsonschema
|
||||
@ -52,8 +55,8 @@ from awx.main import utils
|
||||
|
||||
|
||||
__all__ = ['AutoOneToOneField', 'ImplicitRoleField', 'JSONField',
|
||||
'SmartFilterField', 'update_role_parentage_for_instance',
|
||||
'is_implicit_parent']
|
||||
'SmartFilterField', 'OrderedManyToManyField',
|
||||
'update_role_parentage_for_instance', 'is_implicit_parent']
|
||||
|
||||
|
||||
# Provide a (better) custom error message for enum jsonschema validation
|
||||
@ -987,3 +990,115 @@ class OAuth2ClientSecretField(models.CharField):
|
||||
if value and value.startswith('$encrypted$'):
|
||||
return decrypt_value(get_encryption_key('value', pk=None), value)
|
||||
return value
|
||||
|
||||
|
||||
class OrderedManyToManyDescriptor(ManyToManyDescriptor):
|
||||
"""
|
||||
Django doesn't seem to support:
|
||||
|
||||
class Meta:
|
||||
ordering = [...]
|
||||
|
||||
...on custom through= relations for ManyToMany fields.
|
||||
|
||||
Meaning, queries made _through_ the intermediary table will _not_ apply an
|
||||
ORDER_BY clause based on the `Meta.ordering` of the intermediary M2M class
|
||||
(which is the behavior we want for "ordered" many to many relations):
|
||||
|
||||
https://github.com/django/django/blob/stable/1.11.x/django/db/models/fields/related_descriptors.py#L593
|
||||
|
||||
This descriptor automatically sorts all queries through this relation
|
||||
using the `position` column on the M2M table.
|
||||
"""
|
||||
|
||||
@cached_property
|
||||
def related_manager_cls(self):
|
||||
model = self.rel.related_model if self.reverse else self.rel.model
|
||||
|
||||
def add_custom_queryset_to_many_related_manager(many_related_manage_cls):
|
||||
class OrderedManyRelatedManager(many_related_manage_cls):
|
||||
def get_queryset(self):
|
||||
return super(OrderedManyRelatedManager, self).get_queryset().order_by(
|
||||
'%s__position' % self.through._meta.model_name
|
||||
)
|
||||
|
||||
def add(self, *objs):
|
||||
# Django < 2 doesn't support this method on
|
||||
# ManyToManyFields w/ an intermediary model
|
||||
# We should be able to remove this code snippet when we
|
||||
# upgrade Django.
|
||||
# see: https://github.com/django/django/blob/stable/1.11.x/django/db/models/fields/related_descriptors.py#L926
|
||||
if not django.__version__.startswith('1.'):
|
||||
raise RuntimeError(
|
||||
'This method is no longer necessary in Django>=2'
|
||||
)
|
||||
try:
|
||||
self.through._meta.auto_created = True
|
||||
super(OrderedManyRelatedManager, self).add(*objs)
|
||||
finally:
|
||||
self.through._meta.auto_created = False
|
||||
|
||||
def remove(self, *objs):
|
||||
# Django < 2 doesn't support this method on
|
||||
# ManyToManyFields w/ an intermediary model
|
||||
# We should be able to remove this code snippet when we
|
||||
# upgrade Django.
|
||||
# see: https://github.com/django/django/blob/stable/1.11.x/django/db/models/fields/related_descriptors.py#L944
|
||||
if not django.__version__.startswith('1.'):
|
||||
raise RuntimeError(
|
||||
'This method is no longer necessary in Django>=2'
|
||||
)
|
||||
try:
|
||||
self.through._meta.auto_created = True
|
||||
super(OrderedManyRelatedManager, self).remove(*objs)
|
||||
finally:
|
||||
self.through._meta.auto_created = False
|
||||
|
||||
return OrderedManyRelatedManager
|
||||
|
||||
return add_custom_queryset_to_many_related_manager(
|
||||
create_forward_many_to_many_manager(
|
||||
model._default_manager.__class__,
|
||||
self.rel,
|
||||
reverse=self.reverse,
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
class OrderedManyToManyField(models.ManyToManyField):
|
||||
"""
|
||||
A ManyToManyField that automatically sorts all querysets
|
||||
by a special `position` column on the M2M table
|
||||
"""
|
||||
|
||||
def _update_m2m_position(self, sender, **kwargs):
|
||||
if kwargs.get('action') in ('post_add', 'post_remove'):
|
||||
order_with_respect_to = None
|
||||
for field in sender._meta.local_fields:
|
||||
if (
|
||||
isinstance(field, models.ForeignKey) and
|
||||
isinstance(kwargs['instance'], field.related_model)
|
||||
):
|
||||
order_with_respect_to = field.name
|
||||
for i, ig in enumerate(sender.objects.filter(**{
|
||||
order_with_respect_to: kwargs['instance'].pk}
|
||||
)):
|
||||
if ig.position != i:
|
||||
ig.position = i
|
||||
ig.save()
|
||||
|
||||
def contribute_to_class(self, cls, name, **kwargs):
|
||||
super(OrderedManyToManyField, self).contribute_to_class(cls, name, **kwargs)
|
||||
setattr(
|
||||
cls, name,
|
||||
OrderedManyToManyDescriptor(self.remote_field, reverse=False)
|
||||
)
|
||||
|
||||
through = getattr(cls, name).through
|
||||
if isinstance(through, str) and "." not in through:
|
||||
# support lazy loading of string model names
|
||||
through = '.'.join([cls._meta.app_label, through])
|
||||
m2m_changed.connect(
|
||||
self._update_m2m_position,
|
||||
sender=through
|
||||
)
|
||||
|
||||
43
awx/main/migrations/0073_v360_create_instance_group_m2m.py
Normal file
43
awx/main/migrations/0073_v360_create_instance_group_m2m.py
Normal file
@ -0,0 +1,43 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.11.20 on 2019-05-06 13:48
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from django.db import migrations, models
|
||||
import django.db.models.deletion
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('main', '0072_v350_deprecate_fields'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.CreateModel(
|
||||
name='InventoryInstanceGroupMembership',
|
||||
fields=[
|
||||
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
|
||||
('position', models.PositiveIntegerField(default=None, null=True, db_index=True)),
|
||||
('instancegroup', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.InstanceGroup')),
|
||||
('inventory', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.Inventory')),
|
||||
],
|
||||
),
|
||||
migrations.CreateModel(
|
||||
name='OrganizationInstanceGroupMembership',
|
||||
fields=[
|
||||
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
|
||||
('position', models.PositiveIntegerField(default=None, null=True, db_index=True)),
|
||||
('instancegroup', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.InstanceGroup')),
|
||||
('organization', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.Organization')),
|
||||
],
|
||||
),
|
||||
migrations.CreateModel(
|
||||
name='UnifiedJobTemplateInstanceGroupMembership',
|
||||
fields=[
|
||||
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
|
||||
('position', models.PositiveIntegerField(default=None, null=True, db_index=True)),
|
||||
('instancegroup', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.InstanceGroup')),
|
||||
('unifiedjobtemplate', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.UnifiedJobTemplate')),
|
||||
],
|
||||
),
|
||||
]
|
||||
@ -0,0 +1,42 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.11.20 on 2019-05-06 13:49
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from django.db import migrations
|
||||
|
||||
|
||||
def create_through_relations(apps, schema_editor):
|
||||
relations = [
|
||||
[
|
||||
'UnifiedJobTemplate',
|
||||
apps.get_model('main', 'UnifiedJobTemplateInstanceGroupMembership'),
|
||||
],
|
||||
[
|
||||
'Organization',
|
||||
apps.get_model('main', 'OrganizationInstanceGroupMembership'),
|
||||
],
|
||||
[
|
||||
'Inventory',
|
||||
apps.get_model('main', 'InventoryInstanceGroupMembership'),
|
||||
],
|
||||
]
|
||||
for cls, Membership in relations:
|
||||
Target = apps.get_model('main', cls)
|
||||
for x in Target.objects.iterator():
|
||||
for i, instance_group in enumerate(x.instance_groups.all()):
|
||||
Membership(
|
||||
instancegroup=instance_group,
|
||||
position=i,
|
||||
**{'%s' % cls.lower(): x}
|
||||
).save()
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('main', '0073_v360_create_instance_group_m2m'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.RunPython(create_through_relations)
|
||||
]
|
||||
@ -0,0 +1,27 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.11.20 on 2019-05-06 15:15
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from django.db import migrations
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('main', '0074_v360_migrate_instance_group_relations'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.RemoveField(
|
||||
model_name='inventory',
|
||||
name='instance_groups',
|
||||
),
|
||||
migrations.RemoveField(
|
||||
model_name='organization',
|
||||
name='instance_groups',
|
||||
),
|
||||
migrations.RemoveField(
|
||||
model_name='unifiedjobtemplate',
|
||||
name='instance_groups',
|
||||
),
|
||||
]
|
||||
@ -0,0 +1,31 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.11.20 on 2019-05-06 15:20
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from django.db import migrations, models
|
||||
from awx.main.fields import OrderedManyToManyField
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('main', '0075_v360_remove_old_instance_group_relations'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='inventory',
|
||||
name='instance_groups',
|
||||
field=OrderedManyToManyField(blank=True, through='main.InventoryInstanceGroupMembership', to='main.InstanceGroup'),
|
||||
),
|
||||
migrations.AddField(
|
||||
model_name='organization',
|
||||
name='instance_groups',
|
||||
field=OrderedManyToManyField(blank=True, through='main.OrganizationInstanceGroupMembership', to='main.InstanceGroup'),
|
||||
),
|
||||
migrations.AddField(
|
||||
model_name='unifiedjobtemplate',
|
||||
name='instance_groups',
|
||||
field=OrderedManyToManyField(blank=True, through='main.UnifiedJobTemplateInstanceGroupMembership', to='main.InstanceGroup'),
|
||||
),
|
||||
]
|
||||
@ -332,3 +332,54 @@ def on_job_create(sender, instance, created=False, raw=False, **kwargs):
|
||||
instance=Instance.objects.me(),
|
||||
unified_job=instance,
|
||||
)
|
||||
|
||||
|
||||
class UnifiedJobTemplateInstanceGroupMembership(models.Model):
|
||||
|
||||
unifiedjobtemplate = models.ForeignKey(
|
||||
'UnifiedJobTemplate',
|
||||
on_delete=models.CASCADE
|
||||
)
|
||||
instancegroup = models.ForeignKey(
|
||||
'InstanceGroup',
|
||||
on_delete=models.CASCADE
|
||||
)
|
||||
position = models.PositiveIntegerField(
|
||||
null=True,
|
||||
default=None,
|
||||
db_index=True,
|
||||
)
|
||||
|
||||
|
||||
class OrganizationInstanceGroupMembership(models.Model):
|
||||
|
||||
organization = models.ForeignKey(
|
||||
'Organization',
|
||||
on_delete=models.CASCADE
|
||||
)
|
||||
instancegroup = models.ForeignKey(
|
||||
'InstanceGroup',
|
||||
on_delete=models.CASCADE
|
||||
)
|
||||
position = models.PositiveIntegerField(
|
||||
null=True,
|
||||
default=None,
|
||||
db_index=True,
|
||||
)
|
||||
|
||||
|
||||
class InventoryInstanceGroupMembership(models.Model):
|
||||
|
||||
inventory = models.ForeignKey(
|
||||
'Inventory',
|
||||
on_delete=models.CASCADE
|
||||
)
|
||||
instancegroup = models.ForeignKey(
|
||||
'InstanceGroup',
|
||||
on_delete=models.CASCADE
|
||||
)
|
||||
position = models.PositiveIntegerField(
|
||||
null=True,
|
||||
default=None,
|
||||
db_index=True,
|
||||
)
|
||||
|
||||
@ -38,6 +38,7 @@ from awx.main.fields import (
|
||||
ImplicitRoleField,
|
||||
JSONBField,
|
||||
SmartFilterField,
|
||||
OrderedManyToManyField,
|
||||
)
|
||||
from awx.main.managers import HostManager
|
||||
from awx.main.models.base import (
|
||||
@ -157,9 +158,10 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin):
|
||||
default=None,
|
||||
help_text=_('Filter that will be applied to the hosts of this inventory.'),
|
||||
)
|
||||
instance_groups = models.ManyToManyField(
|
||||
instance_groups = OrderedManyToManyField(
|
||||
'InstanceGroup',
|
||||
blank=True,
|
||||
through='InventoryInstanceGroupMembership',
|
||||
)
|
||||
admin_role = ImplicitRoleField(
|
||||
parent_role='organization.inventory_admin_role',
|
||||
|
||||
@ -15,7 +15,9 @@ from django.utils.translation import ugettext_lazy as _
|
||||
|
||||
# AWX
|
||||
from awx.api.versioning import reverse
|
||||
from awx.main.fields import AutoOneToOneField, ImplicitRoleField
|
||||
from awx.main.fields import (
|
||||
AutoOneToOneField, ImplicitRoleField, OrderedManyToManyField
|
||||
)
|
||||
from awx.main.models.base import (
|
||||
BaseModel, CommonModel, CommonModelNameNotUnique, CreatedModifiedModel,
|
||||
NotificationFieldsModel
|
||||
@ -39,9 +41,10 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi
|
||||
app_label = 'main'
|
||||
ordering = ('name',)
|
||||
|
||||
instance_groups = models.ManyToManyField(
|
||||
instance_groups = OrderedManyToManyField(
|
||||
'InstanceGroup',
|
||||
blank=True,
|
||||
through='OrganizationInstanceGroupMembership'
|
||||
)
|
||||
max_hosts = models.PositiveIntegerField(
|
||||
blank=True,
|
||||
|
||||
@ -48,7 +48,7 @@ from awx.main.utils import polymorphic, schedule_task_manager
|
||||
from awx.main.constants import ACTIVE_STATES, CAN_CANCEL
|
||||
from awx.main.redact import UriCleaner, REPLACE_STR
|
||||
from awx.main.consumers import emit_channel_notification
|
||||
from awx.main.fields import JSONField, AskForField
|
||||
from awx.main.fields import JSONField, AskForField, OrderedManyToManyField
|
||||
|
||||
__all__ = ['UnifiedJobTemplate', 'UnifiedJob', 'StdoutMaxBytesExceeded']
|
||||
|
||||
@ -164,9 +164,10 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio
|
||||
blank=True,
|
||||
related_name='%(class)s_labels'
|
||||
)
|
||||
instance_groups = models.ManyToManyField(
|
||||
instance_groups = OrderedManyToManyField(
|
||||
'InstanceGroup',
|
||||
blank=True,
|
||||
through='UnifiedJobTemplateInstanceGroupMembership'
|
||||
)
|
||||
|
||||
def get_absolute_url(self, request=None):
|
||||
|
||||
102
awx/main/tests/functional/test_instance_group_ordering.py
Normal file
102
awx/main/tests/functional/test_instance_group_ordering.py
Normal file
@ -0,0 +1,102 @@
|
||||
import pytest
|
||||
|
||||
from awx.main.models import InstanceGroup
|
||||
|
||||
|
||||
@pytest.fixture(scope='function')
|
||||
def source_model(request):
|
||||
return request.getfixturevalue(request.param)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@pytest.mark.parametrize(
|
||||
'source_model', ['job_template', 'inventory', 'organization'], indirect=True
|
||||
)
|
||||
def test_instance_group_ordering(source_model):
|
||||
groups = [
|
||||
InstanceGroup.objects.create(name='host-%d' % i)
|
||||
for i in range(5)
|
||||
]
|
||||
groups.reverse()
|
||||
for group in groups:
|
||||
source_model.instance_groups.add(group)
|
||||
|
||||
assert [g.name for g in source_model.instance_groups.all()] == [
|
||||
'host-4', 'host-3', 'host-2', 'host-1', 'host-0'
|
||||
]
|
||||
assert [
|
||||
(row.position, row.instancegroup.name)
|
||||
for row in source_model.instance_groups.through.objects.all()
|
||||
] == [
|
||||
(0, 'host-4'),
|
||||
(1, 'host-3'),
|
||||
(2, 'host-2'),
|
||||
(3, 'host-1'),
|
||||
(4, 'host-0'),
|
||||
]
|
||||
|
||||
source_model.instance_groups.remove(groups[0])
|
||||
assert [g.name for g in source_model.instance_groups.all()] == [
|
||||
'host-3', 'host-2', 'host-1', 'host-0'
|
||||
]
|
||||
assert [
|
||||
(row.position, row.instancegroup.name)
|
||||
for row in source_model.instance_groups.through.objects.all()
|
||||
] == [
|
||||
(0, 'host-3'),
|
||||
(1, 'host-2'),
|
||||
(2, 'host-1'),
|
||||
(3, 'host-0'),
|
||||
]
|
||||
|
||||
source_model.instance_groups.clear()
|
||||
assert source_model.instance_groups.through.objects.count() == 0
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@pytest.mark.parametrize(
|
||||
'source_model', ['job_template', 'inventory', 'organization'], indirect=True
|
||||
)
|
||||
def test_instance_group_middle_deletion(source_model):
|
||||
groups = [
|
||||
InstanceGroup.objects.create(name='host-%d' % i)
|
||||
for i in range(5)
|
||||
]
|
||||
groups.reverse()
|
||||
for group in groups:
|
||||
source_model.instance_groups.add(group)
|
||||
|
||||
source_model.instance_groups.remove(groups[2])
|
||||
assert [g.name for g in source_model.instance_groups.all()] == [
|
||||
'host-4', 'host-3', 'host-1', 'host-0'
|
||||
]
|
||||
assert [
|
||||
(row.position, row.instancegroup.name)
|
||||
for row in source_model.instance_groups.through.objects.all()
|
||||
] == [
|
||||
(0, 'host-4'),
|
||||
(1, 'host-3'),
|
||||
(2, 'host-1'),
|
||||
(3, 'host-0'),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@pytest.mark.parametrize(
|
||||
'source_model', ['job_template', 'inventory', 'organization'], indirect=True
|
||||
)
|
||||
def test_explicit_ordering(source_model):
|
||||
groups = [
|
||||
InstanceGroup.objects.create(name='host-%d' % i)
|
||||
for i in range(5)
|
||||
]
|
||||
groups.reverse()
|
||||
for group in groups:
|
||||
source_model.instance_groups.add(group)
|
||||
|
||||
assert [g.name for g in source_model.instance_groups.all()] == [
|
||||
'host-4', 'host-3', 'host-2', 'host-1', 'host-0'
|
||||
]
|
||||
assert [g.name for g in source_model.instance_groups.order_by('name').all()] == [
|
||||
'host-0', 'host-1', 'host-2', 'host-3', 'host-4'
|
||||
]
|
||||
Loading…
x
Reference in New Issue
Block a user