From 81e06dace2db58b6d9625c5edf73e2298a00cf15 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Mon, 7 Aug 2023 23:43:49 -0400 Subject: [PATCH] Add listener_port to provision_instance API changes - cannot change peers or enable peers_from_control_nodes on VM deployments - allow setting ip_address - use ip_address over hostname in the generated group_vars/all.yml - Drop api/v2/peers endpoint DB changes - add ip_address unique constraint, but ignore "" entries Other changes - provision_instance should take listener_port option Tests - test that new controls doesn't disturb other peers relationships - test ip_address over hostname --- awx/api/serializers.py | 35 ++++++++++++---- awx/api/urls/peers.py | 14 ------- awx/api/urls/urls.py | 2 - awx/api/views/__init__.py | 2 - awx/api/views/root.py | 1 - .../management/commands/provision_instance.py | 11 +++-- awx/main/managers.py | 7 +++- awx/main/migrations/0187_hop_nodes.py | 38 ++++++++++++------ awx/main/models/ha.py | 22 +++++----- awx/main/tasks/receptor.py | 2 +- .../functional/api/test_instance_peers.py | 40 +++++++++++++++---- .../InstanceDetail/InstanceDetail.js | 2 +- tools/docker-compose/bootstrap_development.sh | 4 +- 13 files changed, 115 insertions(+), 65 deletions(-) delete mode 100644 awx/api/urls/peers.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index d8db405f51..61cff4838f 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -5484,13 +5484,11 @@ class InstanceSerializer(BaseSerializer): if not self.instance and not settings.IS_K8S: raise serializers.ValidationError(_("Can only create instances on Kubernetes or OpenShift.")) + node_type = get_field_from_model_or_attrs("node_type") peers_from_control_nodes = get_field_from_model_or_attrs("peers_from_control_nodes") listener_port = get_field_from_model_or_attrs("listener_port") - ip_address = get_field_from_model_or_attrs("ip_address") - hostname = get_field_from_model_or_attrs("hostname") - if not ip_address: - attrs["ip_address"] = hostname + peers = attrs.get('peers', []) if peers_from_control_nodes and node_type not in (Instance.Types.EXECUTION, Instance.Types.HOP): raise serializers.ValidationError(_("peers_from_control_nodes can only be enabled for execution or hop nodes.")) @@ -5507,15 +5505,19 @@ class InstanceSerializer(BaseSerializer): if not listener_port and self.instance and self.instance.peers_from.exists(): raise serializers.ValidationError(_("Field listener_port must be a valid integer when other nodes peer to it.")) - for peer in attrs.get('peers', []): + for peer in peers: if peer.listener_port is None: raise serializers.ValidationError(_("Field listener_port must be set on peer ") + peer.hostname + ".") + if not settings.IS_K8S: + if self.instance and set(self.instance.peers.all()) != set(peers): + raise serializers.ValidationError(_("Cannot change peers.")) + return super().validate(attrs) def validate_node_type(self, value): if not self.instance and value not in [Instance.Types.HOP, Instance.Types.EXECUTION]: - raise serializers.ValidationError("Can only create execution nodes.") + raise serializers.ValidationError(_("Can only create execution or hop nodes.")) if self.instance and self.instance.node_type != value: raise serializers.ValidationError(_("Cannot change node type.")) @@ -5539,14 +5541,22 @@ class InstanceSerializer(BaseSerializer): def validate_hostname(self, value): """ - - Hostname cannot be "localhost" - but can be something like localhost.domain - - Cannot change the hostname of an-already instantiated & initialized Instance object + Cannot change the hostname """ if self.instance and self.instance.hostname != value: raise serializers.ValidationError(_("Cannot change hostname.")) return value + def validate_ip_address(self, value): + """ + Cannot change ip address + """ + if self.instance and self.instance.ip_address != value: + raise serializers.ValidationError(_("Cannot change ip_address.")) + + return value + def validate_listener_port(self, value): """ Cannot change listener port, unless going from none to integer, and vice versa @@ -5556,6 +5566,15 @@ class InstanceSerializer(BaseSerializer): return value + def validate_peers_from_control_nodes(self, value): + """ + Can only enable for K8S based deployments + """ + if value and not settings.IS_K8S: + raise serializers.ValidationError(_("Can only be enabled on Kubernetes or Openshift.")) + + return value + class InstanceHealthCheckSerializer(BaseSerializer): class Meta: diff --git a/awx/api/urls/peers.py b/awx/api/urls/peers.py deleted file mode 100644 index 35f2262a83..0000000000 --- a/awx/api/urls/peers.py +++ /dev/null @@ -1,14 +0,0 @@ -# Copyright (c) 2017 Ansible, Inc. -# All Rights Reserved. - -from django.urls import re_path - -from awx.api.views import PeersList, PeersDetail - - -urls = [ - re_path(r'^$', PeersList.as_view(), name='peers_list'), - re_path(r'^(?P[0-9]+)/$', PeersDetail.as_view(), name='peers_detail'), -] - -__all__ = ['urls'] diff --git a/awx/api/urls/urls.py b/awx/api/urls/urls.py index 73afe7be26..c74f9f97e6 100644 --- a/awx/api/urls/urls.py +++ b/awx/api/urls/urls.py @@ -84,7 +84,6 @@ from .oauth2_root import urls as oauth2_root_urls from .workflow_approval_template import urls as workflow_approval_template_urls from .workflow_approval import urls as workflow_approval_urls from .analytics import urls as analytics_urls -from .peers import urls as peers_urls v2_urls = [ re_path(r'^$', ApiV2RootView.as_view(), name='api_v2_root_view'), @@ -154,7 +153,6 @@ v2_urls = [ re_path(r'^bulk/$', BulkView.as_view(), name='bulk'), re_path(r'^bulk/host_create/$', BulkHostCreateView.as_view(), name='bulk_host_create'), re_path(r'^bulk/job_launch/$', BulkJobLaunchView.as_view(), name='bulk_job_launch'), - re_path(r'^peers/', include(peers_urls)), ] diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index b42d51b5d3..b62be5ed39 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -341,10 +341,8 @@ class InstanceDetail(RetrieveUpdateAPIView): def update_raw_data(self, data): # these fields are only valid on creation of an instance, so they unwanted on detail view - data.pop('listener_port', None) data.pop('node_type', None) data.pop('hostname', None) - data.pop('ip_address', None) return super(InstanceDetail, self).update_raw_data(data) def update(self, request, *args, **kwargs): diff --git a/awx/api/views/root.py b/awx/api/views/root.py index 9eaddfbbf4..3a9a910e1c 100644 --- a/awx/api/views/root.py +++ b/awx/api/views/root.py @@ -129,7 +129,6 @@ class ApiVersionRootView(APIView): data['mesh_visualizer'] = reverse('api:mesh_visualizer_view', request=request) data['bulk'] = reverse('api:bulk', request=request) data['analytics'] = reverse('api:analytics_root_view', request=request) - data['peers'] = reverse('api:peers_list', request=request) return Response(data) diff --git a/awx/main/management/commands/provision_instance.py b/awx/main/management/commands/provision_instance.py index 7e1d01b4ba..e7f8063d61 100644 --- a/awx/main/management/commands/provision_instance.py +++ b/awx/main/management/commands/provision_instance.py @@ -25,17 +25,20 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument('--hostname', dest='hostname', type=str, help="Hostname used during provisioning") + parser.add_argument('--listener_port', dest='listener_port', type=int, help="Receptor listener port") parser.add_argument('--node_type', type=str, default='hybrid', choices=['control', 'execution', 'hop', 'hybrid'], help="Instance Node type") parser.add_argument('--uuid', type=str, help="Instance UUID") - def _register_hostname(self, hostname, node_type, uuid): + def _register_hostname(self, hostname, node_type, uuid, listener_port): if not hostname: if not settings.AWX_AUTO_DEPROVISION_INSTANCES: raise CommandError('Registering with values from settings only intended for use in K8s installs') from awx.main.management.commands.register_queue import RegisterQueue - (changed, instance) = Instance.objects.register(ip_address=os.environ.get('MY_POD_IP'), node_type='control', node_uuid=settings.SYSTEM_UUID) + (changed, instance) = Instance.objects.register( + ip_address=os.environ.get('MY_POD_IP'), listener_port=listener_port, node_type='control', node_uuid=settings.SYSTEM_UUID + ) RegisterQueue(settings.DEFAULT_CONTROL_PLANE_QUEUE_NAME, 100, 0, [], is_container_group=False).register() RegisterQueue( settings.DEFAULT_EXECUTION_QUEUE_NAME, @@ -48,7 +51,7 @@ class Command(BaseCommand): max_concurrent_jobs=settings.DEFAULT_EXECUTION_QUEUE_MAX_CONCURRENT_JOBS, ).register() else: - (changed, instance) = Instance.objects.register(hostname=hostname, node_type=node_type, node_uuid=uuid) + (changed, instance) = Instance.objects.register(hostname=hostname, node_type=node_type, node_uuid=uuid, listener_port=listener_port) if changed: print("Successfully registered instance {}".format(hostname)) else: @@ -58,6 +61,6 @@ class Command(BaseCommand): @transaction.atomic def handle(self, **options): self.changed = False - self._register_hostname(options.get('hostname'), options.get('node_type'), options.get('uuid')) + self._register_hostname(options.get('hostname'), options.get('node_type'), options.get('uuid'), options.get('listener_port')) if self.changed: print("(changed: True)") diff --git a/awx/main/managers.py b/awx/main/managers.py index 41096ef3fe..f8c2a76997 100644 --- a/awx/main/managers.py +++ b/awx/main/managers.py @@ -115,7 +115,7 @@ class InstanceManager(models.Manager): return node[0] raise RuntimeError("No instance found with the current cluster host id") - def register(self, node_uuid=None, hostname=None, ip_address=None, node_type='hybrid', defaults=None): + def register(self, node_uuid=None, hostname=None, ip_address=None, listener_port=None, node_type='hybrid', defaults=None): if not hostname: hostname = settings.CLUSTER_HOST_ID @@ -157,6 +157,9 @@ class InstanceManager(models.Manager): if instance.node_type != node_type: instance.node_type = node_type update_fields.append('node_type') + if instance.listener_port != listener_port: + instance.listener_port = listener_port + update_fields.append('listener_port') if update_fields: instance.save(update_fields=update_fields) return (True, instance) @@ -173,5 +176,5 @@ class InstanceManager(models.Manager): uuid_option = {'uuid': node_uuid if node_uuid is not None else uuid.uuid4()} if node_type == 'execution' and 'version' not in create_defaults: create_defaults['version'] = RECEPTOR_PENDING - instance = self.create(hostname=hostname, ip_address=ip_address, node_type=node_type, **create_defaults, **uuid_option) + instance = self.create(hostname=hostname, ip_address=ip_address, listener_port=listener_port, node_type=node_type, **create_defaults, **uuid_option) return (True, instance) diff --git a/awx/main/migrations/0187_hop_nodes.py b/awx/main/migrations/0187_hop_nodes.py index 99674b5a9a..07806e30e9 100644 --- a/awx/main/migrations/0187_hop_nodes.py +++ b/awx/main/migrations/0187_hop_nodes.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2 on 2023-05-17 18:31 +# Generated by Django 4.2.3 on 2023-08-04 20:50 import django.core.validators from django.db import migrations, models @@ -27,6 +27,22 @@ class Migration(migrations.Migration): name='peers_from_control_nodes', field=models.BooleanField(default=False, help_text='If True, control plane cluster nodes should automatically peer to it.'), ), + migrations.AlterField( + model_name='instance', + name='ip_address', + field=models.CharField(blank=True, default=None, max_length=50, null=True), + ), + migrations.AlterField( + model_name='instance', + name='listener_port', + field=models.PositiveIntegerField( + blank=True, + default=None, + help_text='Port that Receptor will listen for incoming connections on.', + null=True, + validators=[django.core.validators.MinValueValidator(1024), django.core.validators.MaxValueValidator(65535)], + ), + ), migrations.AlterField( model_name='instance', name='peers', @@ -42,20 +58,18 @@ class Migration(migrations.Migration): max_length=16, ), ), + migrations.AddConstraint( + model_name='instance', + constraint=models.UniqueConstraint( + condition=models.Q(('ip_address', ''), _negated=True), + fields=('ip_address',), + name='unique_ip_address_not_empty', + violation_error_message='Field ip_address must be unique.', + ), + ), migrations.AddConstraint( model_name='instancelink', constraint=models.CheckConstraint(check=models.Q(('source', models.F('target')), _negated=True), name='source_and_target_can_not_be_equal'), ), - migrations.AlterField( - model_name='instance', - name='listener_port', - field=models.PositiveIntegerField( - blank=True, - default=None, - help_text='Port that Receptor will listen for incoming connections on.', - null=True, - validators=[django.core.validators.MinValueValidator(1024), django.core.validators.MaxValueValidator(65535)], - ), - ), migrations.RunPython(automatically_peer_from_control_plane), ] diff --git a/awx/main/models/ha.py b/awx/main/models/ha.py index 66d42d939e..34f2e3dd46 100644 --- a/awx/main/models/ha.py +++ b/awx/main/models/ha.py @@ -12,7 +12,7 @@ from django.dispatch import receiver from django.utils.translation import gettext_lazy as _ from django.conf import settings from django.utils.timezone import now, timedelta -from django.db.models import Sum +from django.db.models import Sum, Q import redis from solo.models import SingletonModel @@ -79,13 +79,22 @@ class InstanceLink(BaseModel): ordering = ("id",) constraints = [models.CheckConstraint(check=~models.Q(source=models.F('target')), name='source_and_target_can_not_be_equal')] - def get_absolute_url(self, request=None): - return reverse('api:peers_detail', kwargs={'pk': self.pk}, request=request) - class Instance(HasPolicyEditsMixin, BaseModel): """A model representing an AWX instance running against this database.""" + class Meta: + app_label = 'main' + ordering = ("hostname",) + constraints = [ + models.UniqueConstraint( + fields=["ip_address"], + condition=~Q(ip_address=""), # don't apply to constraint to empty entries + name="unique_ip_address_not_empty", + violation_error_message=_("Field ip_address must be unique."), + ) + ] + def __str__(self): return self.hostname @@ -99,7 +108,6 @@ class Instance(HasPolicyEditsMixin, BaseModel): null=True, default=None, max_length=50, - unique=True, ) # Auto-fields, implementation is different from BaseModel created = models.DateTimeField(auto_now_add=True) @@ -187,10 +195,6 @@ class Instance(HasPolicyEditsMixin, BaseModel): peers = models.ManyToManyField('self', symmetrical=False, through=InstanceLink, through_fields=('source', 'target'), related_name='peers_from') peers_from_control_nodes = models.BooleanField(default=False, help_text=_("If True, control plane cluster nodes should automatically peer to it.")) - class Meta: - app_label = 'main' - ordering = ("hostname",) - POLICY_FIELDS = frozenset(('managed_by_policy', 'hostname', 'capacity_adjustment')) def get_absolute_url(self, request=None): diff --git a/awx/main/tasks/receptor.py b/awx/main/tasks/receptor.py index 3bb4b2ec39..073d47bd81 100644 --- a/awx/main/tasks/receptor.py +++ b/awx/main/tasks/receptor.py @@ -699,7 +699,7 @@ def generate_config_data(): # returns two values # receptor config - based on current database peers # should_update - If True, receptor_config differs from the receptor conf file on disk - instances = Instance.objects.me().peers.all() + instances = Instance.objects.filter(node_type__in=(Instance.Types.EXECUTION, Instance.Types.HOP), peers_from_control_nodes=True) receptor_config = list(RECEPTOR_CONFIG_STARTER) for instance in instances: diff --git a/awx/main/tests/functional/api/test_instance_peers.py b/awx/main/tests/functional/api/test_instance_peers.py index 1ae5abf369..37601ee9e4 100644 --- a/awx/main/tests/functional/api/test_instance_peers.py +++ b/awx/main/tests/functional/api/test_instance_peers.py @@ -9,6 +9,14 @@ from awx.main.models import Instance from awx.api.views.instance_install_bundle import generate_group_vars_all_yml +def has_peer(group_vars, peer): + peers = group_vars.get('receptor_peers', []) + for p in peers: + if f"{p['host']}:{p['port']}" == peer: + return True + return False + + @pytest.mark.django_db class TestPeers: @pytest.fixture(autouse=True) @@ -190,6 +198,19 @@ class TestPeers: hop.delete() assert not control.peers.exists() + def test_control_node_retains_other_peers(self): + ''' + if a new node comes online, other peer relationships should + remain intact + ''' + hop1 = Instance.objects.create(hostname='hop1', ip_address="10.10.10.10", node_type='hop', listener_port=6789, peers_from_control_nodes=True) + hop2 = Instance.objects.create(hostname='hop2', node_type='hop', listener_port=6789, peers_from_control_nodes=False) + hop1.peers.add(hop2) + + control = Instance.objects.create(hostname='control', node_type='control', listener_port=None) + + assert hop1.peers.exists() + def test_group_vars(self, get, admin_user): ''' control > hop1 > hop2 < execution @@ -207,13 +228,6 @@ class TestPeers: hop2_vars = yaml.safe_load(generate_group_vars_all_yml(hop2)) execution_vars = yaml.safe_load(generate_group_vars_all_yml(execution)) - def has_peer(group_vars, peer): - peers = group_vars.get('receptor_peers', []) - for p in peers: - if f"{p['host']}:{p['port']}" == peer: - return True - return False - # control group vars assertions assert control_vars.get('receptor_host_identifier', '') == 'control' assert has_peer(control_vars, 'hop1:6789') @@ -240,6 +254,18 @@ class TestPeers: assert not has_peer(execution_vars, 'hop1:6789') assert execution_vars.get('receptor_listener', False) + def test_group_vars_ip_address_over_hostname(self): + ''' + test that ip_address has precedence over hostname in group_vars all.yml + ''' + hop1 = Instance.objects.create(hostname='hop1', node_type='hop', listener_port=6789, peers_from_control_nodes=True) + hop2 = Instance.objects.create(hostname='hop2', ip_address="10.10.10.10", node_type='hop', listener_port=6789, peers_from_control_nodes=False) + hop1.peers.add(hop2) + + hop1_vars = yaml.safe_load(generate_group_vars_all_yml(hop1)) + + assert has_peer(hop1_vars, "10.10.10.10:6789") + def test_write_receptor_config_called(self): ''' Assert that write_receptor_config is called diff --git a/awx/ui/src/screens/Instances/InstanceDetail/InstanceDetail.js b/awx/ui/src/screens/Instances/InstanceDetail/InstanceDetail.js index 0965b55557..8e60ff5d68 100644 --- a/awx/ui/src/screens/Instances/InstanceDetail/InstanceDetail.js +++ b/awx/ui/src/screens/Instances/InstanceDetail/InstanceDetail.js @@ -207,7 +207,7 @@ function InstanceDetail({ setBreadcrumb, isK8s }) { /> - + {(isExecutionNode || isHopNode) && ( <> {instance.related?.install_bundle && ( diff --git a/tools/docker-compose/bootstrap_development.sh b/tools/docker-compose/bootstrap_development.sh index 87c3f81ee7..28f9bf88ce 100755 --- a/tools/docker-compose/bootstrap_development.sh +++ b/tools/docker-compose/bootstrap_development.sh @@ -40,7 +40,7 @@ echo "Admin password: ${DJANGO_SUPERUSER_PASSWORD}" awx-manage create_preload_data awx-manage register_default_execution_environments -awx-manage provision_instance --hostname="$(hostname)" --node_type="$MAIN_NODE_TYPE" +awx-manage provision_instance --hostname="$(hostname)" --node_type="$MAIN_NODE_TYPE" --listener_port=2222 awx-manage register_queue --queuename=controlplane --instance_percent=100 awx-manage register_queue --queuename=default --instance_percent=100 @@ -52,7 +52,7 @@ if [[ -n "$RUN_MIGRATIONS" ]]; then done if [[ $EXECUTION_NODE_COUNT > 0 ]]; then - awx-manage provision_instance --hostname="receptor-hop" --node_type="hop" + awx-manage provision_instance --hostname="receptor-hop" --node_type="hop" --listener_port=5555 awx-manage register_peers "receptor-hop" --peers "awx_1" for (( e=1; e<=$EXECUTION_NODE_COUNT; e++ )); do awx-manage provision_instance --hostname="receptor-$e" --node_type="execution"