diff options
author | Andreas Eriksen <andreer@yahooinc.com> | 2023-10-10 13:18:07 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-10-10 13:18:07 +0200 |
commit | cb0937a18a18f5874f3376750bbf2477f4f94547 (patch) | |
tree | e857fdd735ad483bfb4701a5de7322e6a5f99635 /routing-generator | |
parent | 69263495d5a827cd230e4ffd832747334f15a037 (diff) |
andreer/add routing config diff to vespa log (#28838)
Diffstat (limited to 'routing-generator')
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"; |