Update the cluster model on destination changes (#1568)

microsoft · Feb 11, 2022 · 4d2867b · 4d2867b
1 parent 50c96eb
commit 4d2867b
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 5 deletions.
diff --git a/src/ReverseProxy/Management/ProxyConfigManager.cs b/src/ReverseProxy/Management/ProxyConfigManager.cs
@@ -458,13 +458,13 @@ private void UpdateRuntimeClusters(IEnumerable<ClusterConfig> incomingClusters)
                 {
                     currentCluster.Revision++;
                     Log.ClusterChanged(_logger, incomingCluster.ClusterId);
-
-                    // Config changed, so update runtime cluster
-                    currentCluster.Model = newClusterModel;
                 }
 
                 if (destinationsChanged || configChanged)
                 {
+                    // Config changed, so update runtime cluster
+                    currentCluster.Model = newClusterModel;
+
                     _clusterDestinationsUpdater.UpdateAllDestinations(currentCluster);
 
                     foreach (var listener in _clusterChangeListeners)

diff --git a/src/ReverseProxy/Model/ClusterState.cs b/src/ReverseProxy/Model/ClusterState.cs
@@ -61,7 +61,7 @@ public ClusterDestinationsState DestinationsState
     internal AtomicCounter ConcurrencyCounter { get; } = new AtomicCounter();
 
     /// <summary>
-    /// Tracks changes to the cluster configuration for use with rebuilding dependent endpoints.
+    /// Tracks changes to the cluster configuration for use with rebuilding dependent endpoints. Destination changes do not affect this property.
     /// </summary>
     internal int Revision { get; set; }
 }
diff --git a/src/ReverseProxy/Model/RouteModel.cs b/src/ReverseProxy/Model/RouteModel.cs
@@ -33,7 +33,7 @@ public RouteModel(
 
     // May not be populated if the cluster config is missing. https://github.com/microsoft/reverse-proxy/issues/797
     /// <summary>
-    /// The ClusterInfo instance associated with this route.
+    /// The <see cref="ClusterState"/> instance associated with this route.
     /// </summary>
     public ClusterState? Cluster { get; }
 

diff --git a/test/ReverseProxy.Tests/Management/ProxyConfigManagerTests.cs b/test/ReverseProxy.Tests/Management/ProxyConfigManagerTests.cs
@@ -561,6 +561,55 @@ public async Task ChangeConfig_ActiveHealthCheckIsEnabled_RunInitialCheck()
         Assert.True(activeMonitor.Scheduler.IsScheduled(clusterState));
     }
 
+    [Fact]
+    public async Task ChangeConfig_DestinationChange_IsReflectedOnRouteConfiguration()
+    {
+        var endpoints = new List<RouteConfig>() { new RouteConfig() { RouteId = "r1", ClusterId = "c1", Match = new RouteMatch { Path = "/" } } };
+        var clusters = new List<ClusterConfig>()
+        {
+            new ClusterConfig
+            {
+                ClusterId = "c1",
+                Destinations = new Dictionary<string, DestinationConfig>(StringComparer.OrdinalIgnoreCase)
+                {
+                    { "d1", new DestinationConfig() { Address = "http://d1" } }
+                }
+            }
+        };
+        var services = CreateServices(endpoints, clusters);
+        var inMemoryConfig = (InMemoryConfigProvider)services.GetRequiredService<IProxyConfigProvider>();
+        var configManager = services.GetRequiredService<ProxyConfigManager>();
+        var dataSource = await configManager.InitialLoadAsync();
+
+        var endpoint = Assert.Single(dataSource.Endpoints);
+        var routeConfig = endpoint.Metadata.GetMetadata<RouteModel>();
+        Assert.Equal("http://d1", Assert.Single(routeConfig.Cluster.Destinations).Value.Model.Config.Address);
+        Assert.Equal(1, routeConfig.Cluster.Revision);
+
+        inMemoryConfig.Update(
+            endpoints,
+            new List<ClusterConfig>()
+            {
+                new ClusterConfig
+                {
+                    ClusterId = "c1",
+                    Destinations = new Dictionary<string, DestinationConfig>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        { "d1", new DestinationConfig() { Address = "http://d1-v2" } }
+                    }
+                }
+            });
+
+        var destinationConfig = Assert.Single(routeConfig.Cluster.Destinations).Value.Model.Config;
+        Assert.Equal("http://d1-v2", destinationConfig.Address);
+
+        Assert.Same(destinationConfig, Assert.Single(routeConfig.Cluster.DestinationsState.AllDestinations).Model.Config);
+        Assert.Same(destinationConfig, Assert.Single(routeConfig.Cluster.Model.Config.Destinations).Value);
+
+        // Destination changes do not affect this property
+        Assert.Equal(1, routeConfig.Cluster.Revision);
+    }
+
     [Fact]
     public async Task LoadAsync_RequestVersionValidationError_Throws()
     {