andreer/add routing config diff to vespa log (#28838)

3 files changed, 37 insertions, 7 deletions

diff --git a/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/RoutingGenerator.java b/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/RoutingGenerator.java
index df646ab5525..f86ccc3df18 100644
--- a/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/RoutingGenerator.java
+++ b/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/RoutingGenerator.java
@@ -1,6 +1,8 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.routing;
 
+import ai.vespa.cloud.Environment;
+import ai.vespa.cloud.SystemInfo;
 import com.yahoo.cloud.config.LbServicesConfig;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.component.annotation.Inject;
@@ -29,7 +31,6 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiConsumer;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -61,13 +62,14 @@ public class RoutingGenerator extends AbstractComponent {
     private volatile RoutingTable routingTable = null;
 
     @Inject
-    public RoutingGenerator(ZoneConfig zoneConfig, RoutingStatus routingStatus, Metric metric) {
+    public RoutingGenerator(ZoneConfig zoneConfig, RoutingStatus routingStatus, Metric metric, SystemInfo systemInfo) {
         this(new ConfigSourceSet(zoneConfig.configserver()), new Nginx(FileSystems.getDefault(),
                                                                        new ProcessExecuter(),
                                                                        Sleeper.DEFAULT,
                                                                        Clock.systemUTC(),
                                                                        routingStatus,
-                                                                       metric),
+                                                                       metric,
+                                                                       systemInfo.zone().environment() == Environment.prod),
              Clock.systemUTC());
     }
 
diff --git a/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/Nginx.java b/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/Nginx.java
index c8dc3638f5b..b6a4cc98038 100644
--- a/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/Nginx.java
+++ b/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/Nginx.java
@@ -48,16 +48,18 @@ public class Nginx implements Router {
     private final Clock clock;
     private final RoutingStatus routingStatus;
     private final Metric metric;
+    private final boolean outputRoutingDiff;
 
     private final Object monitor = new Object();
 
-    public Nginx(FileSystem fileSystem, ProcessExecuter processExecuter, Sleeper sleeper, Clock clock, RoutingStatus routingStatus, Metric metric) {
+    public Nginx(FileSystem fileSystem, ProcessExecuter processExecuter, Sleeper sleeper, Clock clock, RoutingStatus routingStatus, Metric metric, boolean outputRoutingDiff) {
         this.fileSystem = Objects.requireNonNull(fileSystem);
         this.processExecuter = Objects.requireNonNull(processExecuter);
         this.sleeper = Objects.requireNonNull(sleeper);
         this.clock = Objects.requireNonNull(clock);
         this.routingStatus = Objects.requireNonNull(routingStatus);
         this.metric = Objects.requireNonNull(metric);
+        this.outputRoutingDiff = outputRoutingDiff;
     }
 
     @Override
@@ -89,6 +91,7 @@ public class Nginx implements Router {
     private void loadConfig(int upstreamCount) throws IOException {
         Path configPath = NginxPath.config.in(fileSystem);
         Path tempConfigPath = NginxPath.temporaryConfig.in(fileSystem);
+        String routingDiff = "";
         try {
             String currentConfig = Files.readString(configPath);
             String newConfig = Files.readString(tempConfigPath);
@@ -96,6 +99,8 @@ public class Nginx implements Router {
                 Files.deleteIfExists(tempConfigPath);
                 return;
             }
+            if(outputRoutingDiff)
+                routingDiff = " with diff:\n" + getDiff(configPath, tempConfigPath);
             Path rotatedConfig = NginxPath.config.rotatedIn(fileSystem, clock.instant());
             atomicCopy(configPath, rotatedConfig);
         } catch (NoSuchFileException ignored) {
@@ -104,12 +109,17 @@ public class Nginx implements Router {
         Files.move(tempConfigPath, configPath, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
         metric.add(CONFIG_RELOADS_METRIC, 1, null);
         // Retry reload. Same rationale for retrying as in testConfig()
-        LOG.info("Loading new configuration file from " + configPath);
+        LOG.info("Loading new configuration file from " + configPath + routingDiff);
         retryingExec("/usr/bin/sudo /opt/vespa/bin/vespa-reload-nginx");
         metric.add(OK_CONFIG_RELOADS_METRIC, 1, null);
         metric.set(GENERATED_UPSTREAMS_METRIC, upstreamCount, null);
     }
 
+    private String getDiff(Path configPath, Path tempConfigPath) throws IOException {
+        Pair<Integer, String> executed = processExecuter.exec("diff -U1 " + configPath + " " + tempConfigPath);
+        return executed.getSecond();
+    }
+
     /** Remove old config files */
     private void gcConfig() throws IOException {
         Instant oneWeekAgo = clock.instant().minus(Duration.ofDays(7));
diff --git a/routing-generator/src/test/java/com/yahoo/vespa/hosted/routing/nginx/NginxTest.java b/routing-generator/src/test/java/com/yahoo/vespa/hosted/routing/nginx/NginxTest.java
index b84bec10d58..3e88fecd222 100644
--- a/routing-generator/src/test/java/com/yahoo/vespa/hosted/routing/nginx/NginxTest.java
+++ b/routing-generator/src/test/java/com/yahoo/vespa/hosted/routing/nginx/NginxTest.java
@@ -27,7 +27,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -38,6 +37,8 @@ import static org.junit.Assert.fail;
  */
 public class NginxTest {
 
+    private static final String diffCommand = "diff -U1 /opt/vespa/var/vespa-hosted/routing/nginxl4.conf /opt/vespa/var/vespa-hosted/routing/nginxl4.conf.tmp";
+
     @Test
     public void load_routing_table() {
         NginxTester tester = new NginxTester();
@@ -93,6 +94,7 @@ public class NginxTest {
               .assertLoadedConfig(true)
               .assertConfigContents("nginx-updated.conf")
               .assertTemporaryConfigRemoved(true)
+              .assertProducedDiff()
               .assertRotatedFiles("nginxl4.conf-2022-01-01-15:00:00.000")
               .assertMetric(Nginx.CONFIG_RELOADS_METRIC, 2)
               .assertMetric(Nginx.OK_CONFIG_RELOADS_METRIC, 2);
@@ -102,6 +104,7 @@ public class NginxTest {
         tester.load(table0);
         tester.clock.advance(Duration.ofDays(4).plusSeconds(1));
         tester.load(table1)
+              .assertProducedDiff()
               .assertRotatedFiles("nginxl4.conf-2022-01-04-15:00:00.000",
                                   "nginxl4.conf-2022-01-08-15:00:01.000");
         tester.clock.advance(Duration.ofDays(4));
@@ -116,7 +119,7 @@ public class NginxTest {
         private final RoutingStatusMock routingStatus = new RoutingStatusMock();
         private final ProcessExecuterMock processExecuter = new ProcessExecuterMock();
         private final MockMetric metric = new MockMetric();
-        private final Nginx nginx = new Nginx(fileSystem, processExecuter, Sleeper.NOOP, clock, routingStatus, metric);
+        private final Nginx nginx = new Nginx(fileSystem, processExecuter, Sleeper.NOOP, clock, routingStatus, metric, true);
 
         public NginxTester load(RoutingTable table) {
             processExecuter.clearHistory();
@@ -158,6 +161,12 @@ public class NginxTest {
             return this;
         }
 
+
+        public NginxTester assertProducedDiff() {
+            assertTrue(processExecuter.history.contains(diffCommand));
+            return this;
+        }
+
         public NginxTester assertLoadedConfig(boolean loaded) {
             String reloadCommand = "/usr/bin/sudo /opt/vespa/bin/vespa-reload-nginx";
             if (loaded) {
@@ -198,6 +207,15 @@ public class NginxTest {
             history.add(command);
             int exitCode = 0;
             String out = "";
+            if(command.equals(diffCommand))
+                return new Pair<>(1, """
+                        --- /opt/vespa/var/vespa-hosted/routing/nginxl4.conf	2023-10-09 05:28:09.815315000 +0000
+                        +++ /opt/vespa/var/vespa-hosted/routing/nginxl4.conf.tmp	2023-10-09 05:28:27.223030000 +0000
+                        @@ -45,7 +45,6 @@
+                              server 123.example.com:4443;
+                           -  server 456.example.com:4443;
+                              server 789.example.com:4443;""");
+
             if (++currentFailCount <= wantedFailCount) {
                 exitCode = 1;
                 out = "failing to unit test";