diff options
35 files changed, 420 insertions, 682 deletions
diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index 2e8685887c6..5df7b1fc021 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -258,11 +258,10 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { private static TimingValues createTimingValues() { // Proxy should time out before clients upon subscription. double timingValuesRatio = 0.8; - TimingValues tv = new TimingValues(); - tv.setFixedDelay((long) (tv.getFixedDelay() * timingValuesRatio)). - setSubscribeTimeout((long) (tv.getSubscribeTimeout() * timingValuesRatio)). - setConfiguredErrorTimeout(-1); // Never cache errors - return tv; + + return new TimingValues() + .setFixedDelay((long) (new TimingValues().getFixedDelay() * timingValuesRatio)) + .setSubscribeTimeout((long) (new TimingValues().getSubscribeTimeout() * timingValuesRatio)); } } diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigDefinition.java b/config/src/main/java/com/yahoo/vespa/config/ConfigDefinition.java index 216fa84cd5b..4b5ba652235 100644 --- a/config/src/main/java/com/yahoo/vespa/config/ConfigDefinition.java +++ b/config/src/main/java/com/yahoo/vespa/config/ConfigDefinition.java @@ -15,7 +15,7 @@ import java.util.regex.Pattern; /** * Represents one legal def file, or (internally) one array or inner array definition in a def file. - * @author vegardh + * @author Vegard Balgaard Havdal * */ public class ConfigDefinition { diff --git a/config/src/main/java/com/yahoo/vespa/config/ErrorType.java b/config/src/main/java/com/yahoo/vespa/config/ErrorType.java deleted file mode 100644 index 24af997367e..00000000000 --- a/config/src/main/java/com/yahoo/vespa/config/ErrorType.java +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config; - -/** - * @author hmusum - */ -public enum ErrorType { - TRANSIENT, FATAL; - - public static ErrorType getErrorType(int errorCode) { - switch (errorCode) { - case com.yahoo.jrt.ErrorCode.CONNECTION: - case com.yahoo.jrt.ErrorCode.TIMEOUT: - return ErrorType.TRANSIENT; - case ErrorCode.UNKNOWN_CONFIG: - case ErrorCode.UNKNOWN_DEFINITION: - case ErrorCode.UNKNOWN_DEF_MD5: - case ErrorCode.ILLEGAL_NAME: - case ErrorCode.ILLEGAL_VERSION: - case ErrorCode.ILLEGAL_CONFIGID: - case ErrorCode.ILLEGAL_DEF_MD5: - case ErrorCode.ILLEGAL_CONFIG_MD5: - case ErrorCode.ILLEGAL_TIMEOUT: - case ErrorCode.OUTDATED_CONFIG: - case ErrorCode.INTERNAL_ERROR: - case ErrorCode.APPLICATION_NOT_LOADED: - case ErrorCode.UNKNOWN_VESPA_VERSION: - case ErrorCode.ILLEGAL_PROTOCOL_VERSION: - case ErrorCode.INCONSISTENT_CONFIG_MD5: - return ErrorType.FATAL; - default: - return ErrorType.FATAL; - } - } -} diff --git a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java index 56f85845aa0..f19be768811 100644 --- a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java +++ b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java @@ -16,7 +16,6 @@ public class TimingValues { private final long errorTimeout; private final long initialTimeout; private long subscribeTimeout = 55000; - private long configuredErrorTimeout = -1; // Don't ever timeout (and do not use error response) when we are already configured private long fixedDelay = 5000; private final Random rand; @@ -97,11 +96,6 @@ public class TimingValues { return this; } - public TimingValues setConfiguredErrorTimeout(long t) { - configuredErrorTimeout = t; - return this; - } - /** * Returns fixed delay that is used when retrying getting config no matter if it was a success or an error * and independent of number of retries. @@ -134,7 +128,6 @@ public class TimingValues { + ", errorTimeout=" + errorTimeout + ", initialTimeout=" + initialTimeout + ", subscribeTimeout=" + subscribeTimeout - + ", configuredErrorTimeout=" + configuredErrorTimeout + ", fixedDelay=" + fixedDelay + ", rand=" + rand + "]"; } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java b/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java index 47d3203b32e..771068aa67a 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java @@ -24,8 +24,6 @@ public interface ConfigResponse { boolean applyOnRestart(); - String getConfigMd5(); - void serialize(OutputStream os, CompressionType uncompressed) throws IOException; default boolean hasEqualConfig(JRTServerConfigRequest request) { diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java index 68a9601cb02..a657d92647a 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java @@ -273,11 +273,6 @@ public class JRTClientConfigRequestV3 implements JRTClientConfigRequest { } @Override - public String getRequestConfigMd5() { - return requestData.getRequestConfigMd5(); - } - - @Override public String getRequestDefMd5() { return requestData.getRequestDefMd5(); } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTConfigRequest.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTConfigRequest.java index 0fc751dc49f..45ee82b3e6e 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTConfigRequest.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTConfigRequest.java @@ -30,13 +30,6 @@ public interface JRTConfigRequest { boolean validateParameters(); /** - * Returns the config md5 of the config request. Return an empty string if no response has been returned. - * - * @return a config md5. - */ - String getRequestConfigMd5(); - - /** * Returns the md5 of the config definition in the request. * * @return an md5 of config definition in request. diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java index e52a159cf15..e62d8018323 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java @@ -40,13 +40,6 @@ public interface JRTServerConfigRequest extends JRTConfigRequest, GetConfigReque void addOkResponse(Payload payload, long generation, boolean applyOnRestart, PayloadChecksums payloadChecksums); /** - * Get the current config md5 of the client config. - * - * @return a config md5. - */ - String getRequestConfigMd5(); - - /** * Returns the md5 of the config definition in the request. * * @return an md5 of config definition in request. diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java index e6c9bc2175b..c255b9b7ed9 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java @@ -70,26 +70,23 @@ public class JRTServerConfigRequestV3 implements JRTServerConfigRequest { } @Override - public void addOkResponse(Payload payload, long generation, boolean applyOnRestart, PayloadChecksums payloadChecksums) { + public void addOkResponse(Payload payload, long generation, boolean applyOnRestart, PayloadChecksums checksums) { this.applyOnRestart = applyOnRestart; Payload responsePayload = payload.withCompression(getCompressionType()); - ByteArrayOutputStream byteArrayOutputStream = new NoCopyByteArrayOutputStream(4096); + if (responsePayload == null) + throw new RuntimeException("Payload is null for ' " + this + ", not able to create response"); + + ByteArrayOutputStream outputStream = new NoCopyByteArrayOutputStream(4096); try { - JsonGenerator jsonGenerator = createJsonGenerator(byteArrayOutputStream); + JsonGenerator jsonGenerator = createJsonGenerator(outputStream); jsonGenerator.writeStartObject(); + addCommonReturnValues(jsonGenerator); - if (payloadChecksums.getForType(MD5) != null) - setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_MD5, payloadChecksums.getForType(MD5).asString()); - if (payloadChecksums.getForType(XXHASH64) != null) - setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_XXHASH64, payloadChecksums.getForType(XXHASH64).asString()); + addPayloadCheckSums(jsonGenerator, checksums); setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_GENERATION, generation); setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_APPLY_ON_RESTART, applyOnRestart); jsonGenerator.writeObjectFieldStart(SlimeResponseData.RESPONSE_COMPRESSION_INFO); - if (responsePayload == null) { - throw new RuntimeException("Payload is null for ' " + this + ", not able to create response"); - } - CompressionInfo compressionInfo = responsePayload.getCompressionInfo(); - compressionInfo.serialize(jsonGenerator); + responsePayload.getCompressionInfo().serialize(jsonGenerator); jsonGenerator.writeEndObject(); jsonGenerator.writeEndObject(); @@ -97,15 +94,7 @@ public class JRTServerConfigRequestV3 implements JRTServerConfigRequest { } catch (IOException e) { throw new IllegalArgumentException("Could not add OK response for " + this); } - request.returnValues().add(createResponseValue(byteArrayOutputStream)); - ByteBuffer buf = responsePayload.getData().wrap(); - if (buf.hasArray() && buf.remaining() == buf.array().length) { - request.returnValues().add(new DataValue(buf.array())); - } else { - byte[] dst = new byte[buf.remaining()]; - buf.get(dst); - request.returnValues().add(new DataValue(dst)); - } + addPayload(responsePayload, outputStream); } @Override @@ -182,11 +171,6 @@ public class JRTServerConfigRequestV3 implements JRTServerConfigRequest { } @Override - public String getRequestConfigMd5() { - return requestData.getRequestConfigMd5(); - } - - @Override public String getRequestDefMd5() { return requestData.getRequestDefMd5(); } public PayloadChecksums getRequestConfigChecksums() { return requestData.getRequestConfigChecksums(); } @@ -228,6 +212,25 @@ public class JRTServerConfigRequestV3 implements JRTServerConfigRequest { jsonGenerator.writeRawValue(getRequestTrace().toString(true)); } + private void addPayloadCheckSums(JsonGenerator jsonGenerator, PayloadChecksums checksums) throws IOException { + if (checksums.getForType(MD5) != null) + setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_MD5, checksums.getForType(MD5).asString()); + if (checksums.getForType(XXHASH64) != null) + setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_XXHASH64, checksums.getForType(XXHASH64).asString()); + } + + private void addPayload(Payload responsePayload, ByteArrayOutputStream byteArrayOutputStream) { + request.returnValues().add(createResponseValue(byteArrayOutputStream)); + ByteBuffer buf = responsePayload.getData().wrap(); + if (buf.hasArray() && buf.remaining() == buf.array().length) { + request.returnValues().add(new DataValue(buf.array())); + } else { + byte[] dst = new byte[buf.remaining()]; + buf.get(dst); + request.returnValues().add(new DataValue(dst)); + } + } + @Override public long getRequestGeneration() { return requestData.getRequestGeneration(); diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java index 018b1340b5d..ab96081a16e 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java @@ -1,16 +1,14 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.protocol; -import com.yahoo.vespa.config.PayloadChecksums; import com.yahoo.text.AbstractUtf8Array; import com.yahoo.vespa.config.ConfigPayload; +import com.yahoo.vespa.config.PayloadChecksums; import java.io.IOException; import java.io.OutputStream; import java.nio.ByteBuffer; -import static com.yahoo.vespa.config.PayloadChecksum.Type.MD5; - /** * Class for serializing config responses based on {@link com.yahoo.slime.Slime} implementing the {@link ConfigResponse} interface. * @@ -62,11 +60,6 @@ public class SlimeConfigResponse implements ConfigResponse { public boolean applyOnRestart() { return applyOnRestart; } @Override - public String getConfigMd5() { - return payloadChecksums.getForType(MD5).asString(); - } - - @Override public void serialize(OutputStream os, CompressionType type) throws IOException { ByteBuffer buf = Payload.from(payload, compressionInfo).withCompression(type).getData().wrap(); os.write(buf.array(), buf.arrayOffset()+buf.position(), buf.remaining()); diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java index e60c84df887..62a25fadf25 100644 --- a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java @@ -8,14 +8,11 @@ import com.yahoo.jrt.Request; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.ConnectionPool; import com.yahoo.vespa.config.ErrorCode; -import com.yahoo.vespa.config.ErrorType; import com.yahoo.vespa.config.PayloadChecksums; import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.JRTServerConfigRequestV3; import org.junit.Test; -import java.util.Arrays; -import java.util.List; import java.util.Random; import static com.yahoo.config.subscription.impl.JRTConfigRequester.calculateFailedRequestDelay; @@ -53,21 +50,6 @@ public class JRTConfigRequesterTest { } @Test - public void testErrorTypes() { - List<Integer> transientErrors = Arrays.asList(com.yahoo.jrt.ErrorCode.CONNECTION, com.yahoo.jrt.ErrorCode.TIMEOUT); - List<Integer> fatalErrors = Arrays.asList(ErrorCode.UNKNOWN_CONFIG, ErrorCode.UNKNOWN_DEFINITION, ErrorCode.OUTDATED_CONFIG, - ErrorCode.UNKNOWN_DEF_MD5, ErrorCode.ILLEGAL_NAME, ErrorCode.ILLEGAL_VERSION, ErrorCode.ILLEGAL_CONFIGID, - ErrorCode.ILLEGAL_DEF_MD5, ErrorCode.ILLEGAL_CONFIG_MD5, ErrorCode.ILLEGAL_TIMEOUT, ErrorCode.INTERNAL_ERROR, - 9999); // unknown should also be fatal - for (Integer i : transientErrors) { - assertEquals(ErrorType.TRANSIENT, ErrorType.getErrorType(i)); - } - for (Integer i : fatalErrors) { - assertEquals(ErrorType.FATAL, ErrorType.getErrorType(i)); - } - } - - @Test public void testFirstRequestAfterSubscribing() { ConfigSubscriber subscriber = new ConfigSubscriber(); final TimingValues timingValues = getTestTimingValues(); diff --git a/config/src/test/java/com/yahoo/vespa/config/ErrorTypeTest.java b/config/src/test/java/com/yahoo/vespa/config/ErrorTypeTest.java deleted file mode 100644 index 66fa5440fe7..00000000000 --- a/config/src/test/java/com/yahoo/vespa/config/ErrorTypeTest.java +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config; - -import org.junit.Test; - -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; - -/** - * @author Ulf Lilleengen - */ -public class ErrorTypeTest { - - @Test - public void testErrorType() { - assertThat(ErrorType.getErrorType(com.yahoo.jrt.ErrorCode.CONNECTION), is(ErrorType.TRANSIENT)); - assertThat(ErrorType.getErrorType(com.yahoo.jrt.ErrorCode.TIMEOUT), is(ErrorType.TRANSIENT)); - assertThat(ErrorType.getErrorType(ErrorCode.UNKNOWN_CONFIG), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.UNKNOWN_DEFINITION), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.UNKNOWN_DEF_MD5), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_NAME), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_VERSION), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_CONFIGID), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_DEF_MD5), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_CONFIG_MD5), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_TIMEOUT), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_GENERATION), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_SUB_FLAG), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.OUTDATED_CONFIG), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.INTERNAL_ERROR), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(ErrorCode.ILLEGAL_SUB_FLAG), is(ErrorType.FATAL)); - assertThat(ErrorType.getErrorType(0xdeadc0de), is(ErrorType.FATAL)); - } - -} diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java index f7216058cf5..5a29e494d81 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java @@ -216,7 +216,6 @@ public class JRTConfigRequestV3Test { assertTrue(sub.nextConfig(120_0000)); sub.close(); JRTClientConfigRequest nextReq = createReq(sub, Trace.createNew()); - assertEquals(nextReq.getRequestConfigMd5(), sub.getConfigState().getChecksums().getForType(MD5).asString()); assertEquals(nextReq.getRequestConfigChecksums().getForType(MD5).asString(), sub.getConfigState().getChecksums().getForType(MD5).asString()); assertEquals(nextReq.getRequestConfigChecksums().getForType(XXHASH64).asString(), sub.getConfigState().getChecksums().getForType(XXHASH64).asString()); assertEquals(nextReq.getRequestGeneration(), currentGeneration); diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java index 5ba5985db37..99f49f10526 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java @@ -26,6 +26,7 @@ class ExecutorServiceWrapper extends ForwardingExecutorService { private final long maxThreadExecutionTimeMillis; private final int queueCapacity; private final Thread metricReporter; + private final boolean threadPoolIsOnlyQ; private final AtomicBoolean closed = new AtomicBoolean(false); ExecutorServiceWrapper(WorkerCompletionTimingThreadPoolExecutor wrapped, @@ -37,21 +38,32 @@ class ExecutorServiceWrapper extends ForwardingExecutorService { this.metric = metric; this.processTerminator = processTerminator; this.maxThreadExecutionTimeMillis = maxThreadExecutionTimeMillis; - this.queueCapacity = wrapped.getMaximumPoolSize() + wrapped.getQueue().remainingCapacity() + wrapped.getQueue().size(); + int maxQueueCapacity = wrapped.getQueue().remainingCapacity() + wrapped.getQueue().size(); + this.threadPoolIsOnlyQ = (maxQueueCapacity == 0); + this.queueCapacity = threadPoolIsOnlyQ + ? wrapped.getMaximumPoolSize() + : maxQueueCapacity; metric.reportThreadPoolSize(wrapped.getPoolSize()); metric.reportActiveThreads(wrapped.getActiveCount()); - metricReporter = new Thread(this::reportMetrics); + reportMetrics(); + metricReporter = new Thread(this::reportMetricsRegularly); metricReporter.setName(name + "-threadpool-metric-reporter"); metricReporter.start(); } private void reportMetrics() { + int activeThreads = wrapped.getActiveCount(); + metric.reportThreadPoolSize(wrapped.getPoolSize()); + metric.reportActiveThreads(activeThreads); + int queueSize = threadPoolIsOnlyQ ? activeThreads : wrapped.getQueue().size(); + metric.reportWorkQueueSize(queueSize); + metric.reportWorkQueueCapacity(queueCapacity); + } + + private void reportMetricsRegularly() { while (timeToReportMetricsAgain(100)) { - metric.reportThreadPoolSize(wrapped.getPoolSize()); - metric.reportActiveThreads(wrapped.getActiveCount()); - metric.reportWorkQueueSize(wrapped.getQueue().size()); - metric.reportWorkQueueCapacity(queueCapacity); + reportMetrics(); } } private boolean timeToReportMetricsAgain(int timeoutMS) { diff --git a/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java b/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java index 2233ea44b2e..b56d89cafb3 100644 --- a/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java +++ b/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java @@ -4,10 +4,10 @@ package com.yahoo.container.handler.threadpool; import com.yahoo.collections.Tuple2; import com.yahoo.concurrent.Receiver; import com.yahoo.container.protect.ProcessTerminator; +import com.yahoo.container.test.MetricMock; import com.yahoo.jdisc.Metric; import org.junit.Ignore; import org.junit.Test; -import org.mockito.Mockito; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; @@ -26,8 +26,9 @@ public class DefaultContainerThreadPoolTest { @Test public final void testThreadPool() throws InterruptedException { + Metric metrics = new MetricMock(); ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder().maxThreads(1)); - ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, Mockito.mock(Metric.class)); + ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, metrics); Executor exec = threadPool.executor(); Tuple2<Receiver.MessageState, Boolean> reply; FlipIt command = new FlipIt(); @@ -58,12 +59,15 @@ public class DefaultContainerThreadPoolTest { } private ThreadPoolExecutor createPool(int maxThreads, int queueSize) { + return createPool(new MetricMock(), maxThreads, queueSize); + } + private ThreadPoolExecutor createPool(Metric metric, int maxThreads, int queueSize) { ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder() .maxThreads(maxThreads) .minThreads(maxThreads) .queueSize(queueSize)); ContainerThreadPool threadPool = new DefaultContainerThreadpool( - config, Mockito.mock(Metric.class), new MockProcessTerminator(), CPUS); + config, metric, new MockProcessTerminator(), CPUS); ExecutorServiceWrapper wrapper = (ExecutorServiceWrapper) threadPool.executor(); WorkerCompletionTimingThreadPoolExecutor executor = (WorkerCompletionTimingThreadPoolExecutor)wrapper.delegate(); return executor; @@ -71,15 +75,27 @@ public class DefaultContainerThreadPoolTest { @Test public void testThatThreadPoolSizeFollowsConfig() { - ThreadPoolExecutor executor = createPool(3, 1200); + MetricMock metrics = new MetricMock(); + ThreadPoolExecutor executor = createPool(metrics, 3, 1200); assertEquals(3, executor.getMaximumPoolSize()); assertEquals(1200, executor.getQueue().remainingCapacity()); + assertEquals(4, metrics.innvocations().size()); + assertEquals(3L, metrics.innvocations().get("serverThreadPoolSize").val); + assertEquals(0L, metrics.innvocations().get("serverActiveThreads").val); + assertEquals(1200L, metrics.innvocations().get("jdisc.thread_pool.work_queue.capacity").val); + assertEquals(0L, metrics.innvocations().get("jdisc.thread_pool.work_queue.size").val); } @Test public void testThatThreadPoolSizeAutoDetected() { - ThreadPoolExecutor executor = createPool(0, 0); + MetricMock metrics = new MetricMock(); + ThreadPoolExecutor executor = createPool(metrics, 0, 0); assertEquals(CPUS*4, executor.getMaximumPoolSize()); assertEquals(0, executor.getQueue().remainingCapacity()); + assertEquals(4, metrics.innvocations().size()); + assertEquals(64L, metrics.innvocations().get("serverThreadPoolSize").val); + assertEquals(0L, metrics.innvocations().get("serverActiveThreads").val); + assertEquals(64L, metrics.innvocations().get("jdisc.thread_pool.work_queue.capacity").val); + assertEquals(0L, metrics.innvocations().get("jdisc.thread_pool.work_queue.size").val); } @Test public void testThatQueueSizeAutoDetected() { @@ -111,7 +127,8 @@ public class DefaultContainerThreadPoolTest { .maxThreads(2) .maxThreadExecutionTimeSeconds(1)); MockProcessTerminator terminator = new MockProcessTerminator(); - ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, Mockito.mock(Metric.class), terminator); + Metric metrics = new MetricMock(); + ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, metrics, terminator); // No dying when threads hang shorter than max thread execution time threadPool.executor().execute(new Hang(500)); diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java index 9207fb34935..2592b587539 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java @@ -1,11 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.jdisc; +import com.yahoo.container.test.MetricMock; import com.yahoo.jdisc.Metric; import org.junit.Test; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import static org.assertj.core.api.Assertions.assertThat; @@ -23,28 +22,12 @@ public class ThreadedHttpRequestHandlerTest { driver.sendRequest("http://localhost/myhandler").readAll(); String expectedMetricName = "jdisc.http.handler.unhandled_exceptions"; - assertThat(metricMock.addInvocations) + assertThat(metricMock.innvocations()) .containsKey(expectedMetricName); - assertThat(metricMock.addInvocations.get(expectedMetricName).dimensions) + assertThat(((MetricMock.SimpleMetricContext)metricMock.innvocations().get(expectedMetricName).ctx).dimensions) .containsEntry("exception", "DummyException"); } - private static class MetricMock implements Metric { - final ConcurrentHashMap<String, SimpleMetricContext> addInvocations = new ConcurrentHashMap<>(); - - @Override public void add(String key, Number val, Context ctx) { - addInvocations.put(key, (SimpleMetricContext)ctx); - } - @Override public void set(String key, Number val, Context ctx) {} - @Override public Context createContext(Map<String, ?> properties) { return new SimpleMetricContext(properties); } - } - - private static class SimpleMetricContext implements Metric.Context { - final Map<String, String> dimensions; - - @SuppressWarnings("unchecked") - SimpleMetricContext(Map<String, ?> dimensions) { this.dimensions = (Map<String, String>)dimensions; } - } private static class ThreadedHttpRequestHandlerThrowingException extends ThreadedHttpRequestHandler { ThreadedHttpRequestHandlerThrowingException(Metric metric) { diff --git a/container-core/src/test/java/com/yahoo/container/test/MetricMock.java b/container-core/src/test/java/com/yahoo/container/test/MetricMock.java new file mode 100644 index 00000000000..8dd023b5605 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/test/MetricMock.java @@ -0,0 +1,51 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.test; + +import com.yahoo.jdisc.Metric; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Simple mock for use whne testing metrics. + * + * @author bjorncs + */ +public class MetricMock implements Metric { + public static class SimpleMetricContext implements Context { + public final Map<String, String> dimensions; + + @SuppressWarnings("unchecked") + SimpleMetricContext(Map<String, ?> dimensions) { + this.dimensions = (Map<String, String>) dimensions; + } + } + + public static class Invocation { + public final Number val; + public final Context ctx; + public Invocation(Number val, Context ctx) { + this.val = val; + this.ctx = ctx; + } + } + + private final Map<String, Invocation> addInvocations = new ConcurrentHashMap<>(); + + public Map<String, Invocation> innvocations() { + return addInvocations; + } + + @Override + public void add(String key, Number val, Context ctx) { + addInvocations.put(key, new Invocation(val, ctx)); + } + + @Override + public void set(String key, Number val, Context ctx) { addInvocations.put(key, new Invocation(val, ctx)); } + + @Override + public Context createContext(Map<String, ?> properties) { + return new SimpleMetricContext(properties); + } +} diff --git a/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java b/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java index 83173782118..c8a5f17fdfb 100644 --- a/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java +++ b/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java @@ -1,18 +1,17 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.metrics.simple; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.util.Collection; import java.util.Map.Entry; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import org.junit.After; import org.junit.Before; import org.junit.Test; -import com.yahoo.metrics.ManagerConfig; import com.yahoo.metrics.simple.jdisc.JdiscMetricsFactory; import com.yahoo.metrics.simple.jdisc.SimpleMetricConsumer; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java index 74a7d23c36d..3e484a5669b 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java @@ -9,13 +9,17 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; /** * @author tokle + * @author andreer */ public class EndpointCertificateMock implements EndpointCertificateProvider { private final Map<ApplicationId, List<String>> dnsNames = new HashMap<>(); + private final Map<String, EndpointCertificateMetadata> providerMetadata = new HashMap<>(); public List<String> dnsNamesOf(ApplicationId application) { return Collections.unmodifiableList(dnsNames.getOrDefault(application, List.of())); @@ -28,18 +32,39 @@ public class EndpointCertificateMock implements EndpointCertificateProvider { applicationId.application(), applicationId.instance()); long epochSecond = Instant.now().getEpochSecond(); long inAnHour = epochSecond + 3600; - return new EndpointCertificateMetadata(endpointCertificatePrefix + "-key", endpointCertificatePrefix + "-cert", 0, 0, - "mock-id-string", dnsNames, "mockCa", Optional.of(inAnHour), Optional.of(epochSecond)); + String requestId = UUID.randomUUID().toString(); + EndpointCertificateMetadata metadata = new EndpointCertificateMetadata(endpointCertificatePrefix + "-key", endpointCertificatePrefix + "-cert", 0, 0, + requestId, dnsNames, "mockCa", Optional.of(inAnHour), Optional.of(epochSecond)); + providerMetadata.put(requestId, metadata); + return metadata; } @Override - public List<EndpointCertificateMetadata> listCertificates() { - return List.of(); + public List<EndpointCertificateRequestMetadata> listCertificates() { + + return providerMetadata.values().stream() + .map(p -> new EndpointCertificateRequestMetadata( + p.requestId(), + "mock", + "mock", + "mock", + p.requestedDnsSans().stream() + .map(san -> new EndpointCertificateRequestMetadata.DnsNameStatus(san, "done")) + .collect(Collectors.toUnmodifiableList()), + 3600, + "ok", + "2021-09-28T00:14:31.946562037Z", + p.expiry().orElseThrow(), + p.issuer(), + "rsa_2048" + )) + .collect(Collectors.toUnmodifiableList()); } @Override - public void deleteCertificate(ApplicationId applicationId, EndpointCertificateMetadata endpointCertificateMetadata) { + public void deleteCertificate(ApplicationId applicationId, String requestId) { dnsNames.remove(applicationId); + providerMetadata.remove(requestId); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java index a4c9d4d8b3a..fbaeb57fec1 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java @@ -15,7 +15,7 @@ public interface EndpointCertificateProvider { EndpointCertificateMetadata requestCaSignedCertificate(ApplicationId applicationId, List<String> dnsNames, Optional<EndpointCertificateMetadata> currentMetadata); - List<EndpointCertificateMetadata> listCertificates(); + List<EndpointCertificateRequestMetadata> listCertificates(); - void deleteCertificate(ApplicationId applicationId, EndpointCertificateMetadata endpointCertificateMetadata); + void deleteCertificate(ApplicationId applicationId, String requestId); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateRequestMetadata.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateRequestMetadata.java new file mode 100644 index 00000000000..81e04190244 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateRequestMetadata.java @@ -0,0 +1,175 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.certificates; + +import java.util.List; +import java.util.Objects; + +/** + * This class is used for metadata about an application's endpoint certificate received from the certificate provider. + * + * @author andreer + */ +public class EndpointCertificateRequestMetadata { + + public EndpointCertificateRequestMetadata(String requestId, + String requestor, + String ticketId, + String athenzDomain, + List<DnsNameStatus> dnsNames, + long durationSec, + String status, + String createTime, + long expiry, + String issuer, + String publicKeyAlgo) { + this.requestId = requestId; + this.requestor = requestor; + this.ticketId = ticketId; + this.athenzDomain = athenzDomain; + this.dnsNames = dnsNames; + this.durationSec = durationSec; + this.status = status; + this.createTime = createTime; + this.expiry = expiry; + this.issuer = issuer; + this.publicKeyAlgo = publicKeyAlgo; + } + + public static class DnsNameStatus { + public final String dnsName; + public final String status; + + public DnsNameStatus(String dnsName, String status) { + this.dnsName = dnsName; + this.status = status; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DnsNameStatus that = (DnsNameStatus) o; + return dnsName.equals(that.dnsName) && status.equals(that.status); + } + + @Override + public int hashCode() { + return Objects.hash(dnsName, status); + } + + @Override + public String toString() { + return "DnsNameStatus{" + + "dnsName='" + dnsName + '\'' + + ", status='" + status + '\'' + + '}'; + } + } + + private final String requestId; + private final String requestor; + private final String ticketId; + private final String athenzDomain; + private final List<DnsNameStatus> dnsNames; + private final long durationSec; + private final String status; + private final String createTime; // ISO 8601 + private final long expiry; + private final String issuer; + private final String publicKeyAlgo; + + public String requestId() { + return requestId; + } + + public String requestor() { + return requestor; + } + + public String ticketId() { + return ticketId; + } + + public String athenzDomain() { + return athenzDomain; + } + + public List<DnsNameStatus> dnsNames() { + return dnsNames; + } + + public long durationSec() { + return durationSec; + } + + public String status() { + return status; + } + + public String createTime() { + return createTime; + } + + public long expiry() { + return expiry; + } + + public String issuer() { + return issuer; + } + + public String publicKeyAlgo() { + return publicKeyAlgo; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + EndpointCertificateRequestMetadata that = (EndpointCertificateRequestMetadata) o; + return durationSec == that.durationSec && + expiry == that.expiry && + requestId.equals(that.requestId) && + requestor.equals(that.requestor) && + ticketId.equals(that.ticketId) && + athenzDomain.equals(that.athenzDomain) && + dnsNames.equals(that.dnsNames) && + status.equals(that.status) && + createTime.equals(that.createTime) && + issuer.equals(that.issuer) && + publicKeyAlgo.equals(that.publicKeyAlgo); + } + + @Override + public int hashCode() { + return Objects.hash( + requestId, + requestor, + ticketId, + athenzDomain, + dnsNames, + durationSec, + status, + createTime, + expiry, + issuer, + publicKeyAlgo); + } + + @Override + public String toString() { + return "EndpointCertificateRequestMetadata{" + + "requestId='" + requestId + '\'' + + ", requestor='" + requestor + '\'' + + ", ticketId='" + ticketId + '\'' + + ", athenzDomain='" + athenzDomain + '\'' + + ", dnsNames=" + dnsNames + + ", durationSec=" + durationSec + + ", status='" + status + '\'' + + ", createTime='" + createTime + '\'' + + ", expiry=" + expiry + + ", issuer='" + issuer + '\'' + + ", publicKeyAlgo='" + publicKeyAlgo + '\'' + + '}'; + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index 40e75cab8aa..6b6c9084e87 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -2,16 +2,20 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.google.common.collect.Sets; +import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.SystemName; import com.yahoo.container.jdisc.secretstore.SecretNotFoundException; import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.log.LogLevel; import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateProvider; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateRequestMetadata; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; @@ -22,6 +26,7 @@ import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.HashSet; +import java.util.List; import java.util.Optional; import java.util.OptionalInt; import java.util.Set; @@ -31,7 +36,7 @@ import java.util.stream.Collectors; /** * Updates refreshed endpoint certificates and triggers redeployment, and deletes unused certificates. - * + * <p> * See also EndpointCertificateManager, which provisions, reprovisions and validates certificates on deploy * * @author andreer @@ -45,7 +50,9 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { private final CuratorDb curator; private final SecretStore secretStore; private final EndpointCertificateProvider endpointCertificateProvider; + private final BooleanFlag deleteUnmaintainedCertificates = Flags.DELETE_UNMAINTAINED_CERTIFICATES.bindTo(controller().flagSource()); + @Inject public EndpointCertificateMaintainer(Controller controller, Duration interval) { super(controller, interval, null, SystemName.all()); this.deploymentTrigger = controller.applications().deploymentTrigger(); @@ -62,7 +69,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { deployRefreshedCertificates(); updateRefreshedCertificates(); deleteUnusedCertificates(); - reportUnmanagedCertificates(); + deleteOrReportUnmanagedCertificates(); } catch (Exception e) { log.log(LogLevel.ERROR, "Exception caught while maintaining endpoint certificates", e); return 0.0; @@ -94,20 +101,20 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { private void deployRefreshedCertificates() { var now = clock.instant(); curator.readAllEndpointCertificateMetadata().forEach((applicationId, endpointCertificateMetadata) -> - endpointCertificateMetadata.lastRefreshed().ifPresent(lastRefreshTime -> { - Instant refreshTime = Instant.ofEpochSecond(lastRefreshTime); - if (now.isAfter(refreshTime.plus(4, ChronoUnit.DAYS))) { - controller().applications().getInstance(applicationId) - .ifPresent(instance -> instance.productionDeployments().forEach((zone, deployment) -> { - if (deployment.at().isBefore(refreshTime)) { - JobType job = JobType.from(controller().system(), zone).get(); - deploymentTrigger.reTrigger(applicationId, job); - log.info("Re-triggering deployment job " + job.jobName() + " for instance " + - applicationId.serializedForm() + " to roll out refreshed endpoint certificate"); - } - })); - } - })); + endpointCertificateMetadata.lastRefreshed().ifPresent(lastRefreshTime -> { + Instant refreshTime = Instant.ofEpochSecond(lastRefreshTime); + if (now.isAfter(refreshTime.plus(4, ChronoUnit.DAYS))) { + controller().applications().getInstance(applicationId) + .ifPresent(instance -> instance.productionDeployments().forEach((zone, deployment) -> { + if (deployment.at().isBefore(refreshTime)) { + JobType job = JobType.from(controller().system(), zone).orElseThrow(); + deploymentTrigger.reTrigger(applicationId, job); + log.info("Re-triggering deployment job " + job.jobName() + " for instance " + + applicationId.serializedForm() + " to roll out refreshed endpoint certificate"); + } + })); + } + })); } private OptionalInt latestVersionInSecretStore(EndpointCertificateMetadata originalCertificateMetadata) { @@ -129,7 +136,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { if (Optional.of(storedMetaData).equals(curator.readEndpointCertificateMetadata(applicationId))) { log.log(Level.INFO, "Cert for app " + applicationId.serializedForm() + " has not been requested in a month and app has no deployments, deleting from provider and ZK"); - endpointCertificateProvider.deleteCertificate(applicationId, storedMetaData); + endpointCertificateProvider.deleteCertificate(applicationId, storedMetaData.requestId()); curator.deleteEndpointCertificateMetadata(applicationId); } } @@ -137,16 +144,6 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { }); } - private void reportUnmanagedCertificates() { - Set<String> managedRequestIds = curator.readAllEndpointCertificateMetadata().values().stream().map(EndpointCertificateMetadata::requestId).collect(Collectors.toSet()); - - for (EndpointCertificateMetadata cameoCertificateMetadata : endpointCertificateProvider.listCertificates()) { - if (!managedRequestIds.contains(cameoCertificateMetadata.requestId())) { - log.info("Certificate metadata exists with provider but is not managed by controller: " + cameoCertificateMetadata.requestId() + ", " + cameoCertificateMetadata.issuer() + ", " + cameoCertificateMetadata.requestedDnsSans()); - } - } - } - private Lock lock(ApplicationId applicationId) { return curator.lock(TenantAndApplicationId.from(applicationId)); } @@ -159,4 +156,22 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { return deployments.isEmpty() || deployments.get().size() == 0; } + private void deleteOrReportUnmanagedCertificates() { + List<EndpointCertificateRequestMetadata> endpointCertificateMetadata = endpointCertificateProvider.listCertificates(); + Set<String> managedRequestIds = curator.readAllEndpointCertificateMetadata().values().stream().map(EndpointCertificateMetadata::requestId).collect(Collectors.toSet()); + + for (var cameoCertificateMetadata : endpointCertificateMetadata) { + if (!managedRequestIds.contains(cameoCertificateMetadata.requestId())) { + if (deleteUnmaintainedCertificates.value()) { + // The certificate is not known - however it could be in the process of being requested by us or another controller. + // So we only delete if it was requested more than 7 days ago. + if (Instant.parse(cameoCertificateMetadata.createTime()).isBefore(Instant.now().minus(7, ChronoUnit.DAYS))) { + endpointCertificateProvider.deleteCertificate(ApplicationId.fromSerializedForm("applicationid:is:unknown"), cameoCertificateMetadata.requestId()); + } + } else { + log.info("Certificate metadata exists with provider but is not managed by controller: " + cameoCertificateMetadata); + } + } + } + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java index 5698aefaa46..24ac473a7b7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java @@ -2,12 +2,16 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMock; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; +import com.yahoo.yolean.concurrent.Sleeper; import org.junit.Test; import java.time.Duration; @@ -29,6 +33,9 @@ public class EndpointCertificateMaintainerTest { private final SecretStoreMock secretStore = (SecretStoreMock) tester.controller().secretStore(); private final EndpointCertificateMaintainer maintainer = new EndpointCertificateMaintainer(tester.controller(), Duration.ofHours(1)); private final EndpointCertificateMetadata exampleMetadata = new EndpointCertificateMetadata("keyName", "certName", 0, 0, "uuid", List.of(), "issuer", Optional.empty(), Optional.empty()); + { + ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.DELETE_UNMAINTAINED_CERTIFICATES.id(), true); + } @Test public void old_and_unused_cert_is_deleted() { @@ -115,4 +122,17 @@ public class EndpointCertificateMaintainerTest { deploymentContext.assertRunning(productionUsWest1); } + + @Test + public void unmaintained_cert_is_deleted() { + EndpointCertificateMock endpointCertificateProvider = (EndpointCertificateMock) tester.controller().serviceRegistry().endpointCertificateProvider(); + + ApplicationId unknown = ApplicationId.fromSerializedForm("applicationid:is:unknown"); + endpointCertificateProvider.requestCaSignedCertificate(unknown, List.of("a", "b", "c"), Optional.empty()); // Unknown to controller! + + assertEquals(1.0, maintainer.maintain(), 0.0000001); + + assertTrue(endpointCertificateProvider.dnsNamesOf(unknown).isEmpty()); + assertTrue(endpointCertificateProvider.listCertificates().isEmpty()); + } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index ff2be07937d..37b6f3481fc 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -143,7 +143,7 @@ public class Flags { ZONE_ID, APPLICATION_ID); public static final UnboundIntFlag DOCSTORE_COMPRESSION_LEVEL = defineIntFlag( - "docstore-compression-level", 9, + "docstore-compression-level", 3, List.of("baldersheim"), "2021-10-08", "2022-01-01", "Default compression level used for document store", "Takes effect at redeployment", @@ -303,6 +303,13 @@ public class Flags { APPLICATION_ID ); + public static final UnboundBooleanFlag DELETE_UNMAINTAINED_CERTIFICATES = defineFeatureFlag( + "delete-unmaintained-certificates", false, + List.of("andreer"), "2021-09-23", "2021-11-11", + "Whether to delete certificates that are known by provider but not by controller", + "Takes effect on next run of EndpointCertificateMaintainer" + ); + public static final UnboundBooleanFlag ENABLE_TENANT_DEVELOPER_ROLE = defineFeatureFlag( "enable-tenant-developer-role", false, List.of("bjorncs"), "2021-09-23", "2021-12-31", diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java index dd3c26fd653..9a110427223 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java @@ -215,18 +215,6 @@ public class NodeList extends AbstractFilteringList<Node, NodeList> { n.allocation().get().membership().cluster().group().equals(Optional.of(ClusterSpec.Group.from(index)))); } - // TODO(mpolden): Remove these when HostEncrypter is removed - /** Returns the subset of nodes which are being encrypted */ - public NodeList encrypting() { - return matching(node -> node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isPresent() && - node.reports().getReport(Report.DISK_ENCRYPTED_ID).isEmpty()); - } - - /** Returns the subset of nodes which are encrypted */ - public NodeList encrypted() { - return matching(node -> node.reports().getReport(Report.DISK_ENCRYPTED_ID).isPresent()); - } - /** Returns the parent node of the given child node */ public Optional<Node> parentOf(Node child) { return child.parentHostname() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java deleted file mode 100644 index 017e30c2d46..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java +++ /dev/null @@ -1,131 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.component.Version; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.NodeType; -import com.yahoo.jdisc.Metric; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.IntFlag; -import com.yahoo.vespa.flags.ListFlag; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.NodeRepository; -import com.yahoo.vespa.hosted.provision.node.Agent; -import com.yahoo.vespa.hosted.provision.node.ClusterId; - -import java.time.Duration; -import java.time.Instant; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.logging.Logger; -import java.util.stream.Collectors; - -/** - * This maintainer triggers encryption of hosts that have unencrypted disk. - * - * A host to be encrypted is retired and marked as want-to-encrypt by storing a report. - * - * This uses the same host selection criteria as {@link com.yahoo.vespa.hosted.provision.os.RebuildingOsUpgrader}. - * - * @author mpolden - */ -// TODO(mpolden): This can be removed once all hosts are encrypted -public class HostEncrypter extends NodeRepositoryMaintainer { - - private static final Logger LOG = Logger.getLogger(HostEncrypter.class.getName()); - - private final IntFlag maxEncryptingHosts; - private final ListFlag<String> deferApplicationEncryption; - - public HostEncrypter(NodeRepository nodeRepository, Duration interval, Metric metric) { - super(nodeRepository, interval, metric); - this.maxEncryptingHosts = Flags.MAX_ENCRYPTING_HOSTS.bindTo(nodeRepository.flagSource()); - this.deferApplicationEncryption = Flags.DEFER_APPLICATION_ENCRYPTION.bindTo(nodeRepository.flagSource()); - } - - @Override - protected double maintain() { - Instant now = nodeRepository().clock().instant(); - NodeList allNodes = nodeRepository().nodes().list(); - for (var nodeType : NodeType.values()) { - if (!nodeType.isHost()) continue; - if (upgradingVespa(allNodes, nodeType)) continue; - unencryptedHosts(allNodes, nodeType).forEach(host -> encrypt(host, now)); - } - return 1.0; - } - - /** Returns whether any node of given type is currently upgrading its Vespa version */ - private boolean upgradingVespa(NodeList allNodes, NodeType hostType) { - return allNodes.state(Node.State.ready, Node.State.active) - .nodeType(hostType) - .changingVersion() - .size() > 0; - } - - /** Returns unencrypted hosts of given type that can be encrypted */ - private List<Node> unencryptedHosts(NodeList allNodes, NodeType hostType) { - if (!hostType.isHost()) throw new IllegalArgumentException("Expected host type, got " + hostType); - NodeList hostsOfTargetType = allNodes.nodeType(hostType); - int hostLimit = hostLimit(hostsOfTargetType, hostType); - - // Find stateful clusters with retiring nodes - NodeList activeNodes = allNodes.state(Node.State.active); - Set<ClusterId> retiringClusters = new HashSet<>(activeNodes.nodeType(hostType.childNodeType()) - .retiring() - .statefulClusters()); - - // Encrypt hosts not containing stateful clusters with retiring nodes, up to limit - List<Node> hostsToEncrypt = new ArrayList<>(hostLimit); - - Set<ApplicationId> deferredApplications = deferApplicationEncryption.value().stream() - .map(ApplicationId::fromSerializedForm) - .collect(Collectors.toSet()); - NodeList candidates = hostsOfTargetType.state(Node.State.active) - .not().encrypted() - .not().encrypting() - .matching(host -> encryptHost(host, allNodes, deferredApplications)) - // Require an OS version supporting encryption - .matching(node -> node.status().osVersion().current() - .orElse(Version.emptyVersion) - .getMajor() >= 8); - - for (Node host : candidates) { - if (hostsToEncrypt.size() == hostLimit) break; - Set<ClusterId> clustersOnHost = activeNodes.childrenOf(host).statefulClusters(); - boolean canEncrypt = Collections.disjoint(retiringClusters, clustersOnHost); - if (canEncrypt) { - hostsToEncrypt.add(host); - retiringClusters.addAll(clustersOnHost); - } - } - return Collections.unmodifiableList(hostsToEncrypt); - - } - - /** Returns the number of hosts that can encrypt concurrently */ - private int hostLimit(NodeList hosts, NodeType hostType) { - if (hosts.stream().anyMatch(host -> host.type() != hostType)) throw new IllegalArgumentException("All hosts must be a " + hostType); - if (maxEncryptingHosts.value() < 1) return 0; // 0 or negative value effectively stops encryption of all hosts - int limit = hostType == NodeType.host ? maxEncryptingHosts.value() : 1; - return Math.max(0, limit - hosts.encrypting().size()); - } - - private boolean encryptHost(Node host, NodeList allNodes, Set<ApplicationId> deferredApplications) { - Set<ApplicationId> applicationsOnHost = allNodes.childrenOf(host).stream() - .filter(node -> node.allocation().isPresent()) - .map(node -> node.allocation().get().owner()) - .collect(Collectors.toSet()); - return Collections.disjoint(applicationsOnHost, deferredApplications); - } - - private void encrypt(Node host, Instant now) { - LOG.info("Retiring and encrypting " + host); - nodeRepository().nodes().encrypt(host.hostname(), Agent.HostEncrypter, now); - } - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index 873d8ceb7b9..2313dfbde0b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -66,7 +66,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { maintainers.add(new AutoscalingMaintainer(nodeRepository, deployer, metric, defaults.autoscalingInterval)); maintainers.add(new ScalingSuggestionsMaintainer(nodeRepository, defaults.scalingSuggestionsInterval, metric)); maintainers.add(new SwitchRebalancer(nodeRepository, defaults.switchRebalancerInterval, metric, deployer)); - maintainers.add(new HostEncrypter(nodeRepository, defaults.hostEncrypterInterval, metric)); provisionServiceProvider.getLoadBalancerService(nodeRepository) .map(lbService -> new LoadBalancerExpirer(nodeRepository, defaults.loadBalancerExpirerInterval, lbService, metric)) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java index 2894e1575ca..5a3e5527a0a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Generation.java @@ -1,8 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.node; -import javax.annotation.concurrent.Immutable; - /** * An immutable generation, with wanted and current generation fields. Wanted generation * is increased when an action (restart services or reboot are the available @@ -10,7 +8,6 @@ import javax.annotation.concurrent.Immutable; * * @author hmusum */ -@Immutable public class Generation { private final long wanted; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index 9ba7f5684ee..480fd72967e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -316,8 +316,7 @@ public class Nodes { public Node deallocate(Node node, Agent agent, String reason, NestedTransaction transaction) { if (parkOnDeallocationOf(node, agent)) { - boolean keepAllocation = node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isPresent(); - return park(node.hostname(), keepAllocation, agent, reason, transaction); + return park(node.hostname(), false, agent, reason, transaction); } else { return db.writeTo(Node.State.dirty, List.of(node), agent, Optional.of(reason), transaction).get(0); } @@ -641,11 +640,6 @@ public class Nodes { return decommission(hostname, DecommissionOperation.rebuild, agent, instant); } - /** Retire and encrypt given host and all of its children */ - public List<Node> encrypt(String hostname, Agent agent, Instant instant) { - return decommission(hostname, DecommissionOperation.encrypt, agent, instant); - } - private List<Node> decommission(String hostname, DecommissionOperation op, Agent agent, Instant instant) { Optional<NodeMutex> nodeMutex = lockAndGet(hostname); if (nodeMutex.isEmpty()) return List.of(); @@ -654,23 +648,14 @@ public class Nodes { List<Node> result; boolean wantToDeprovision = op == DecommissionOperation.deprovision; boolean wantToRebuild = op == DecommissionOperation.rebuild; - Optional<Report> wantToEncryptReport = op == DecommissionOperation.encrypt - ? Optional.of(Report.basicReport(Report.WANT_TO_ENCRYPT_ID, Report.Type.UNSPECIFIED, instant, "")) - : Optional.empty(); try (NodeMutex lock = nodeMutex.get(); Mutex allocationLock = lockUnallocated()) { // This takes allocationLock to prevent any further allocation of nodes on this host host = lock.node(); result = performOn(list(allocationLock).childrenOf(host), (node, nodeLock) -> { Node newNode = node.withWantToRetire(true, wantToDeprovision, wantToRebuild, agent, instant); - if (wantToEncryptReport.isPresent()) { - newNode = newNode.with(newNode.reports().withReport(wantToEncryptReport.get())); - } return write(newNode, nodeLock); }); Node newHost = host.withWantToRetire(true, wantToDeprovision, wantToRebuild, agent, instant); - if (wantToEncryptReport.isPresent()) { - newHost = newHost.with(newHost.reports().withReport(wantToEncryptReport.get())); - } result.add(write(newHost, lock)); } return result; @@ -841,7 +826,6 @@ public class Nodes { .orElse(false); return node.status().wantToDeprovision() || node.status().wantToRebuild() || - node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isPresent() || retirementRequestedByOperator; } @@ -849,7 +833,6 @@ public class Nodes { private enum DecommissionOperation { deprovision, rebuild, - encrypt, } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java index f5c5b9f2857..37141d8f25b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java @@ -27,11 +27,6 @@ public class Report { /** The description of the report. */ public static final String DESCRIPTION_FIELD = "description"; - /** Known report IDs */ - // TODO(mpolden): Remove together with HostEncrypter - public static final String WANT_TO_ENCRYPT_ID = "wantToEncrypt"; - public static final String DISK_ENCRYPTED_ID = "diskEncrypted"; - private final String reportId; private final Type type; private final Instant createdTime; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializer.java deleted file mode 100644 index 058b5a45d8c..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializer.java +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.persistence; - -import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.NodeType; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Inspector; -import com.yahoo.slime.ObjectTraverser; -import com.yahoo.slime.Slime; -import com.yahoo.slime.SlimeUtils; - -import java.io.IOException; -import java.io.UncheckedIOException; -import java.util.Map; -import java.util.TreeMap; - -/** - * Serializer for docker images that are set per node type. - * - * @author freva - */ -public class NodeTypeContainerImagesSerializer { - - private NodeTypeContainerImagesSerializer() {} - - public static byte[] toJson(Map<NodeType, DockerImage> dockerImages) { - Slime slime = new Slime(); - Cursor object = slime.setObject(); - dockerImages.forEach((nodeType, dockerImage) -> - object.setString(NodeSerializer.toString(nodeType), dockerImage.asString())); - try { - return SlimeUtils.toJsonBytes(slime); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - public static Map<NodeType, DockerImage> fromJson(byte[] data) { - Map<NodeType, DockerImage> images = new TreeMap<>(); // Use TreeMap to sort by node type - Inspector inspector = SlimeUtils.jsonToSlime(data).get(); - inspector.traverse((ObjectTraverser) (key, value) -> - images.put(NodeSerializer.nodeTypeFromString(key), DockerImage.fromString(value.asString()))); - return images; - } - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java index 02b426ed6fc..b7b334aaba4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.provision.restapi; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.serialization.NetworkPortsSerializer; @@ -166,7 +165,7 @@ class NodesResponse extends SlimeJsonResponse { if (node.type().isHost()) nodeRepository.firmwareChecks().requiredAfter().ifPresent(after -> object.setLong("wantedFirmwareCheck", after.toEpochMilli())); node.status().vespaVersion().ifPresent(version -> object.setString("vespaVersion", version.toFullString())); - currentDockerImage(node).ifPresent(dockerImage -> object.setString("currentDockerImage", dockerImage.asString())); + currentContainerImage(node).ifPresent(image -> object.setString("currentDockerImage", image.asString())); object.setLong("failCount", node.status().failCount()); object.setBool("wantToRetire", node.status().wantToRetire()); object.setBool("preferToRetire", node.status().preferToRetire()); @@ -206,14 +205,15 @@ class NodesResponse extends SlimeJsonResponse { } } - // Hack: For non-docker nodes, return current docker image as default prefix + current Vespa version - // TODO: Remove current + wanted docker image from response for non-docker types - private Optional<DockerImage> currentDockerImage(Node node) { - return node.status().containerImage() - .or(() -> Optional.of(node) - .filter(n -> n.flavor().getType() != Flavor.Type.DOCKER_CONTAINER) - .flatMap(n -> n.status().vespaVersion() - .map(version -> nodeRepository.containerImages().get(n).withTag(version)))); + private Optional<DockerImage> currentContainerImage(Node node) { + if (node.status().containerImage().isPresent()) { + return node.status().containerImage(); + } + if (node.type().isHost()) { + // Return the image used by children of this host. This is used by host-admin to preload container images. + return node.status().vespaVersion().map(version -> nodeRepository.containerImages().get(node).withTag(version)); + } + return Optional.empty(); } private void ipAddressesToSlime(Set<String> ipAddresses, Cursor array) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java deleted file mode 100644 index 2149badda70..00000000000 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java +++ /dev/null @@ -1,198 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.component.Version; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.HostSpec; -import com.yahoo.config.provision.NodeResources; -import com.yahoo.config.provision.NodeType; -import com.yahoo.jdisc.test.MockMetric; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.node.Agent; -import com.yahoo.vespa.hosted.provision.node.Allocation; -import com.yahoo.vespa.hosted.provision.node.Report; -import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; -import org.junit.Test; - -import java.time.Duration; -import java.time.Instant; -import java.util.Comparator; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.function.Consumer; -import java.util.function.Supplier; -import java.util.stream.Collectors; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; - -/** - * @author mpolden - */ -public class HostEncrypterTest { - - private final ApplicationId infraApplication = ApplicationId.from("hosted-vespa", "infra", "default"); - private final ProvisioningTester tester = new ProvisioningTester.Builder().build(); - private final HostEncrypter encrypter = new HostEncrypter(tester.nodeRepository(), Duration.ofDays(1), new MockMetric()); - - @Test - public void no_hosts_encrypted_with_default_flag_value() { - provisionHosts(1); - encrypter.maintain(); - assertEquals(0, tester.nodeRepository().nodes().list().encrypting().size()); - } - - @Test - public void deferred_hosts_are_not_encrypted() { - int hostCount = 4; - int proxyHostCount = 1; - ApplicationId app1 = ApplicationId.from("t1", "a1", "i1"); - ApplicationId app2 = ApplicationId.from("t2", "a2", "i2"); - provisionHosts(hostCount); - deployApplication(app1); - deployApplication(app2); - - ApplicationId proxyHostApp = ApplicationId.from("hosted-vespa", "proxy-host", "default"); - List<Node> proxyHosts = tester.makeReadyNodes(proxyHostCount, "default", NodeType.proxyhost, 10); - tester.patchNodes(proxyHosts, (host) -> host.with(host.status().withOsVersion(host.status().osVersion().withCurrent(Optional.of(Version.fromString("8.0")))))); - tester.prepareAndActivateInfraApplication(proxyHostApp, NodeType.proxyhost); - - tester.flagSource() - .withIntFlag(Flags.MAX_ENCRYPTING_HOSTS.id(), hostCount + proxyHostCount) - .withListFlag(Flags.DEFER_APPLICATION_ENCRYPTION.id(), List.of(app2.serializedForm()), String.class); - encrypter.maintain(); - NodeList allNodes = tester.nodeRepository().nodes().list(); - NodeList encryptingHosts = allNodes.encrypting().parents(); - - assertEquals(1, encryptingHosts.nodeType(NodeType.proxyhost).size()); - - assertEquals(1, encryptingHosts.nodeType(NodeType.host).size()); - assertEquals("Host of included application is encrypted", Set.of(app1), - allNodes.childrenOf(encryptingHosts.nodeType(NodeType.host).asList().get(0)).stream() - .map(node -> node.allocation().get().owner()) - .collect(Collectors.toSet())); - } - - @Test - public void encrypt_hosts() { - tester.flagSource().withIntFlag(Flags.MAX_ENCRYPTING_HOSTS.id(), 3); - Supplier<NodeList> hosts = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host); - - // Provision hosts and deploy applications - int hostCount = 5; - ApplicationId app1 = ApplicationId.from("t1", "a1", "i1"); - ApplicationId app2 = ApplicationId.from("t2", "a2", "i2"); - provisionHosts(hostCount); - deployApplication(app1); - deployApplication(app2); - - // Encrypts 1 host per stateful cluster and 1 empty host - encrypter.maintain(); - NodeList allNodes = tester.nodeRepository().nodes().list(); - List<Node> hostsEncrypting = allNodes.nodeType(NodeType.host) - .encrypting() - .sortedBy(Comparator.comparing(Node::hostname)) - .asList(); - List<Optional<ApplicationId>> owners = List.of(Optional.of(app1), Optional.of(app2), Optional.empty()); - assertEquals(owners.size(), hostsEncrypting.size()); - for (int i = 0; i < hostsEncrypting.size(); i++) { - Optional<ApplicationId> owner = owners.get(i); - List<Node> retiringChildren = allNodes.childrenOf(hostsEncrypting.get(i)).retiring().encrypting().asList(); - assertEquals(owner.isPresent() ? 1 : 0, retiringChildren.size()); - assertEquals("Encrypting host of " + owner.map(ApplicationId::toString) - .orElse("no application"), - owner, - retiringChildren.stream() - .findFirst() - .flatMap(Node::allocation) - .map(Allocation::owner)); - } - - // Replace any retired nodes - replaceNodes(app1); - replaceNodes(app2); - - // Complete encryption - completeEncryptionOf(hostsEncrypting); - assertEquals(3, hosts.get().encrypted().size()); - - // Both applications have moved their nodes to the remaining unencrypted hosts - allNodes = tester.nodeRepository().nodes().list(); - NodeList unencryptedHosts = allNodes.nodeType(NodeType.host).not().encrypted(); - assertEquals(2, unencryptedHosts.size()); - for (var host : unencryptedHosts) { - assertEquals(1, allNodes.childrenOf(host).owner(app1).size()); - assertEquals(1, allNodes.childrenOf(host).owner(app2).size()); - } - - // Since both applications now occupy all remaining hosts, we can only upgrade 1 at a time - for (int i = 0; i < unencryptedHosts.size(); i++) { - encrypter.maintain(); - hostsEncrypting = hosts.get().encrypting().asList(); - assertEquals(1, hostsEncrypting.size()); - replaceNodes(app1); - replaceNodes(app2); - completeEncryptionOf(hostsEncrypting); - } - - // Resuming encryption has no effect as all hosts are now encrypted - encrypter.maintain(); - NodeList allHosts = hosts.get(); - assertEquals(0, allHosts.encrypting().size()); - assertEquals(allHosts.size(), allHosts.encrypted().size()); - } - - private void provisionHosts(int hostCount) { - List<Node> provisionedHosts = tester.makeReadyNodes(hostCount, new NodeResources(48, 128, 2000, 10), NodeType.host, 10); - // Set OS version supporting encryption - tester.patchNodes(provisionedHosts, (host) -> host.with(host.status().withOsVersion(host.status().osVersion().withCurrent(Optional.of(Version.fromString("8.0")))))); - tester.prepareAndActivateInfraApplication(infraApplication, NodeType.host); - } - - private void completeEncryptionOf(List<Node> nodes) { - Instant now = tester.clock().instant(); - // Redeploy to park retired hosts - replaceNodes(infraApplication, (application) -> tester.prepareAndActivateInfraApplication(application, NodeType.host)); - List<Node> patchedNodes = tester.patchNodes(nodes, (node) -> { - assertSame(Node.State.parked, node.state()); - assertTrue(node + " wants to encrypt", node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isPresent()); - return node.with(node.reports().withReport(Report.basicReport(Report.DISK_ENCRYPTED_ID, - Report.Type.UNSPECIFIED, - now, - "Host is encrypted"))); - }); - patchedNodes = tester.nodeRepository().nodes().deallocate(patchedNodes, Agent.system, getClass().getSimpleName()); - tester.nodeRepository().nodes().setReady(patchedNodes, Agent.system, getClass().getSimpleName()); - tester.activateTenantHosts(); - } - - private void deployApplication(ApplicationId application) { - ClusterSpec contentSpec = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("content1")).vespaVersion("7").build(); - List<HostSpec> hostSpecs = tester.prepare(application, contentSpec, 2, 1, new NodeResources(4, 8, 100, 0.3)); - tester.activate(application, hostSpecs); - } - - private void replaceNodes(ApplicationId application) { - replaceNodes(application, this::deployApplication); - } - - private void replaceNodes(ApplicationId application, Consumer<ApplicationId> deployer) { - // Deploy to retire nodes - deployer.accept(application); - List<Node> retired = tester.nodeRepository().nodes().list().owner(application).retired().asList(); - assertFalse("At least one node is retired", retired.isEmpty()); - tester.nodeRepository().nodes().setRemovable(application, retired); - - // Redeploy to deactivate removable nodes and allocate new ones - deployer.accept(application); - tester.nodeRepository().nodes().list(Node.State.inactive).owner(application) - .forEach(node -> tester.nodeRepository().nodes().removeRecursively(node, true)); - } - -} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializerTest.java deleted file mode 100644 index 4d4669f0b42..00000000000 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeTypeContainerImagesSerializerTest.java +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.persistence; - -import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.NodeType; -import org.junit.Test; - -import java.util.Map; -import java.util.TreeMap; - -import static org.junit.Assert.assertEquals; - -/** - * @author freva - */ -public class NodeTypeContainerImagesSerializerTest { - - @Test - public void test_serialization() { - Map<NodeType, DockerImage> images = new TreeMap<>(); - images.put(NodeType.host, DockerImage.fromString("docker.domain.tld/my/repo:1.2.3")); - images.put(NodeType.confighost, DockerImage.fromString("docker.domain.tld/my/image:2.1")); - - Map<NodeType, DockerImage> serialized = NodeTypeContainerImagesSerializer.fromJson(NodeTypeContainerImagesSerializer.toJson(images)); - assertEquals(images, serialized); - } - -} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json index 72224ef3cba..4c8c5d80018 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json @@ -13,9 +13,6 @@ "name": "FailedExpirer" }, { - "name": "HostEncrypter" - }, - { "name": "InactiveExpirer" }, { |