PLUTON-19231 | Implement @awmackowiak's changes

Draft pull request: #519

This changes the iteration order during prepare_redirects (changing the loop from:
```for redirect in self.input.redirects:
   ...
       for mapped_domain in redirect.src_domain.mapped_domains(self.varnish.cluster):
```
to:
```for related_domain in related_domains:
   ...
       for redirect in self.input.redirects.get(related_domain):
       ...
          for mapped_domain in redirect.src_domain.mapped_domains(self.varnish.cluster):

```
In testing this caused a speedup in the time to prepare redirects from ~0.7 to ~0.1

Author: mgolski

vaas/vaas/router/models.py

@@ -1,10 +1,9 @@
 # -*- coding: utf-8 -*-
 import uuid
-from typing import Dict, Tuple, List
+from typing import Dict
 from django.db import models
 from django.conf import settings
 from django.core.validators import MinValueValidator, MaxValueValidator
-from urllib.parse import urlsplit
 
 from vaas.cluster.models import DomainMapping, LogicalCluster
 from vaas.manager.models import Director
@@ -37,16 +36,6 @@ class ResponseStatusCode(models.IntegerChoices):
     def get_hashed_assertions_pks(self) -> Dict[int, int]:
         return {hash((a.given_url, a.expected_location)): a.pk for a in self.assertions.all()}
 
-    def fetch_all_destinations_mappings(self, cluster: LogicalCluster) -> Tuple[str, List[str]]:
-        """
-        Fetch tuple containing domain parsed from destination url and all found mappings for input cluster
-        """
-        all_mappings = set()
-        destination_domain = urlsplit(self.destination).netloc
-        for domain_mapping in DomainMapping.objects.filter(domain=destination_domain):
-            all_mappings = all_mappings.union(set(domain_mapping.mapped_domains(cluster)))
-        return destination_domain, list(all_mappings)
-
     @property
     def final_condition(self):
         if self.required_custom_header:

vaas/vaas/vcl/renderer.py

@@ -5,7 +5,6 @@
 import hashlib
 import time
 import functools
-from typing import Dict, List
 from urllib.parse import urlsplit
 
 from django.conf import settings
@@ -187,50 +186,36 @@ def __init__(self, varnish, input_data):
             'mesh_routing': varnish.cluster.service_mesh_routing
         }
 
-    def filter_cluster_domain_mappings(
-        self, cluster: LogicalCluster
-    ) -> dict[str, list[str]]:
-        """Returns a dictionary where keys are domains, and values are mapped to by the key"""
-        # we can PROBABLY assume that each domain has only one mapping
-        # in an experiment trying to add a new mapping for a source domain removed the old one
-        destination_dict = {}
-        for domain_mapping in self.input.domain_mappings:
-            domain = domain_mapping.domain
-            destination_dict[domain] = domain_mapping.mapped_domains(cluster)
-        return destination_dict
-
-    def provide_related_domains(self, cluster: LogicalCluster) -> list[str]:
-        """Returns a list of all domains related to a cluster - for static mappings,
-        the list is specified within the mapping, for dynamic - clusters are associated using
-        labels (mapping targets can contain labels, and each cluster can have multiple labels, but
-        each label belongs to exactly one cluster)"""
-        result = {m.domain for m in self.input.static_domain_mappings_by_cluster[cluster.name]}
-        for m in self.input.mapping_provider.mappings["dynamic"]:
-            if m.is_cluster_related_by_labels(cluster):
-                result.add(m.domain)
-        return sorted(list(result))
+    def fetch_all_destinations_mappings(self, cluster: LogicalCluster, redirect: str,
+                                        domain_mappings: list[DomainMapping]) -> tuple[str, list[str]]:
+        """Fetch tuple containing domain parsed from destination url and all found mappings for input cluster
+        """
+        all_mappings = set()
+        destination_domain = urlsplit(redirect).netloc
+        for domain_mapping in domain_mappings:
+            all_mappings = all_mappings.union(set(domain_mapping.mapped_domains(cluster)))
+        return destination_domain, list(all_mappings)
 
     @collect_processing
-    def prepare_redirects(self) -> Dict[str, List[VclRedirect]]:
-        redirects = {}
-        related_domains = self.provide_related_domains(self.varnish.cluster)
-        destinations_dict = self.filter_cluster_domain_mappings(self.varnish.cluster)
-        for redirect in self.input.redirects:
-            if str(redirect.src_domain) not in related_domains:
+    def prepare_redirects(self) -> dict[str, list[VclRedirect]]:
+        redirects = dict()
+        related_domains = self.input.mapping_provider.provide_related_domains(self.varnish.cluster)
+        for related_domain in related_domains:
+            if related_domain not in self.input.redirects.keys():
                 continue
-            destination_domain = urlsplit(redirect.destination).netloc
-            destination_mappings = destinations_dict.get(destination_domain)
-            for mapped_domain in redirect.src_domain.mapped_domains(self.varnish.cluster):
-                destination = str(redirect.destination)
-                if destination_domain == redirect.src_domain.domain:
-                    destination = destination.replace(destination_domain, mapped_domain)
-                elif destination_domain and len(destination_mappings) == 1:
-                    destination = destination.replace(destination_domain, destination_mappings[0])
-
-                if entries := redirects.get(mapped_domain, []):
-                    entries.append(VclRedirect(redirect, mapped_domain, destination))
-                else:
-                    redirects[mapped_domain] = [VclRedirect(redirect, mapped_domain, destination)]
+            for redirect in self.input.redirects.get(related_domain):
+                destination_domain, destination_mappings = self.fetch_all_destinations_mappings(
+                    self.varnish.cluster, redirect.destination, self.input.domain_mappings)
+                for mapped_domain in redirect.src_domain.mapped_domains(self.varnish.cluster):
+                    destination = str(redirect.destination)
+                    if destination_domain == redirect.src_domain.domain:
+                        destination = destination.replace(destination_domain, mapped_domain)
+                    elif all((destination_domain, len(destination_mappings) == 1)):
+                        destination = destination.replace(destination_domain, destination_mappings[0])
+                    if entries := redirects.get(mapped_domain, []):
+                        entries.append(VclRedirect(redirect, mapped_domain, destination))
+                    else:
+                        redirects[mapped_domain] = [VclRedirect(redirect, mapped_domain, destination)]
         return redirects
 
     @collect_processing
@@ -410,7 +395,7 @@ def fetch_render_data(self):
             Prefetch('clusters', queryset=LogicalCluster.objects.only('pk'), to_attr='cluster_ids'),
         ))
         self.routes.sort(key=lambda route: "{:03d}-{}".format(route.priority, route.director.name))
-        self.redirects = list(Redirect.objects.all().order_by('src_domain', 'priority'))
+        self.redirects = self.assemble_redirects()
         self.dcs = list(Dc.objects.all())
         self.template_blocks = list(VclTemplateBlock.objects.all().prefetch_related('template'))
         self.vcl_variables = list(VclVariable.objects.all())
@@ -422,16 +407,15 @@ def fetch_render_data(self):
         self.distributed_canary_backends = self.prepare_canary_backends(canary_backend_ids, backends)
         self.domain_mappings = DomainMapping.objects.all()
         self.mapping_provider = MappingProvider(self.domain_mappings)
-        self.static_domain_mappings_by_cluster = self.cluster_static_domain_mappings()
-
-    def cluster_static_domain_mappings(self) -> dict[str, list[DomainMapping]]:
-        """Returns a dict cluster.name: [associated domain mappings]"""
-        # NTH: get the same output with only one database query, perhaps with prefetch_related()
-        result_dict = dict()
-        all_clusters = LogicalCluster.objects.all()
-        for cluster in all_clusters:
-            result_dict[cluster.name] = cluster.domainmapping_set.filter(type="static")
-        return result_dict
+
+    @collect_processing
+    def assemble_redirects(self) -> dict[str, list[Redirect]]:
+        redirects = {}
+        for redirect in Redirect.objects.all().order_by('src_domain', 'priority'):
+            if redirect.src_domain.domain not in redirects.keys():
+                redirects[redirect.src_domain.domain] = []
+            redirects[redirect.src_domain.domain].append(redirect)
+        return redirects
 
     @collect_processing
     def distribute_backends(self, backends):

vaas/vaas/vcl/tests/expected-vcl-4.0-canary.vcl

@@ -339,7 +339,7 @@ sub vcl_synth {
     if (resp.status == 989) {
         set resp.status = 200;
         set resp.http.Content-Type = "application/json";
-        synthetic ( {"{ "vcl_version" : "0bbef", "varnish_status": "disabled" }"} );
+        synthetic ( {"{ "vcl_version" : "ce716", "varnish_status": "disabled" }"} );
         return (deliver);
     }
 }
@@ -424,12 +424,6 @@ sub vcl_recv {
         set req.http.x-response-code = "301";
         set req.http.x-action = "redirect";
     }
-    else if (req.url ~ "/external") {
-        set req.http.x-redirect = "3";
-        set req.http.x-destination = "http://example-external.com/external_destination";
-        set req.http.x-response-code = "301";
-        set req.http.x-action = "redirect";
-    }
     }
     if (req.http.host == "example.prod.org") {
     if (req.url ~ "/source") {
@@ -444,12 +438,6 @@ sub vcl_recv {
         set req.http.x-response-code = "301";
         set req.http.x-action = "redirect";
     }
-    else if (req.url ~ "/external") {
-        set req.http.x-redirect = "3";
-        set req.http.x-destination = "http://example-external.com/external_destination";
-        set req.http.x-response-code = "301";
-        set req.http.x-action = "redirect";
-    }
     }
 
 # Test ROUTER

vaas/vaas/vcl/tests/expected-vcl-4.0.vcl

@@ -366,7 +366,7 @@ sub vcl_synth {
     if (resp.status == 989) {
         set resp.status = 200;
         set resp.http.Content-Type = "application/json";
-        synthetic ( {"{ "vcl_version" : "d584a", "varnish_status": "disabled" }"} );
+        synthetic ( {"{ "vcl_version" : "721d3", "varnish_status": "disabled" }"} );
         return (deliver);
     }
 }
@@ -451,12 +451,6 @@ sub vcl_recv {
         set req.http.x-response-code = "301";
         set req.http.x-action = "redirect";
     }
-    else if (req.url ~ "/external") {
-        set req.http.x-redirect = "3";
-        set req.http.x-destination = "http://example-external.com/external_destination";
-        set req.http.x-response-code = "301";
-        set req.http.x-action = "redirect";
-    }
     }
     if (req.http.host == "example.prod.org") {
     if (req.url ~ "/source") {
@@ -471,12 +465,6 @@ sub vcl_recv {
         set req.http.x-response-code = "301";
         set req.http.x-action = "redirect";
     }
-    else if (req.url ~ "/external") {
-        set req.http.x-redirect = "3";
-        set req.http.x-destination = "http://example-external.com/external_destination";
-        set req.http.x-response-code = "301";
-        set req.http.x-action = "redirect";
-    }
     }
 
 # Test ROUTER

vaas/vaas/vcl/tests/test_renderer.py

@@ -280,7 +280,7 @@ def setUp(self):
         )
 
         Redirect.objects.create(
-            src_domain=self.example_domain_mapping,
+            src_domain=self.external_domain_mapping,
             condition='req.url ~ "/external"',
             destination='http://external.com/external_destination',
             action=301,
@@ -436,23 +436,26 @@ def test_should_decorate_set_backend_tag_with_fallback_service_in_dc1(self):
     def test_should_decorate_flexible_router_tag_with_properly_mapped_destination_domain(self):
         vcl_tag_builder = VclTagBuilder(self.varnish, VclRendererInput())
         tag = vcl_tag_builder.get_expanded_tags('FLEXIBLE_ROUTER').pop()
-        assert_set_equal({'example.prod.com', 'example.prod.org'}, set(tag.parameters['redirects'].keys()))
+        assert_set_equal({'example.prod.com', 'example-external.com', 'example.prod.org'},
+                            set(tag.parameters['redirects'].keys()))
         assert_equals('example.com', tag.parameters['redirects']['example.prod.com'][1].src_domain.domain)
         assert_equals('example.com', tag.parameters['redirects']['example.prod.org'][1].src_domain.domain)
         assert_equals('http://example.prod.com/destination',
                       tag.parameters['redirects']['example.prod.com'][1].destination)
         assert_equals('http://example.prod.org/destination',
                       tag.parameters['redirects']['example.prod.org'][1].destination)
         assert_equals('http://example-external.com/external_destination',
-                      tag.parameters['redirects']['example.prod.com'][2].destination)
+                      tag.parameters['redirects']['example-external.com'][0].destination)
 
     def test_should_sort_redirects_by_priority(self):
         vcl_tag_builder = VclTagBuilder(self.varnish, VclRendererInput())
         tag = vcl_tag_builder.get_expanded_tags('FLEXIBLE_ROUTER').pop()
-        assert_set_equal({'example.prod.com', 'example.prod.org'}, set(tag.parameters['redirects'].keys()))
+        assert_set_equal({'example.prod.com', 'example-external.com', 'example.prod.org'},
+                         set(tag.parameters['redirects'].keys()))
         assert_equals('2/example.prod.com', tag.parameters['redirects']['example.prod.com'][0].id)
         assert_equals('1/example.prod.com', tag.parameters['redirects']['example.prod.com'][1].id)
-        assert_equals('3/example.prod.com', tag.parameters['redirects']['example.prod.com'][2].id)
+        assert_equals('2/example.prod.org', tag.parameters['redirects']['example.prod.org'][0].id)
+        assert_equals('3/example-external.com', tag.parameters['redirects']['example-external.com'][0].id)
 
 
 class VclRendererInputTest(TestCase):