From ba451f322126152cb36df84a7bd039b2bc96b659 Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Wed, 18 Oct 2023 19:31:03 +0200 Subject: [PATCH] The 0.7.6 version does not throw an exception when keepAliveOnce failed #1254 --- .../java/io/etcd/jetcd/impl/LeaseImpl.java | 19 ++++- .../etcd/jetcd/impl/LeaseMemoryLeakTest.java | 18 ++--- .../etcd/jetcd/impl/LeaseOnceErrorTest.java | 78 +++++++++++++++++++ .../java/io/etcd/jetcd/launcher/Etcd.java | 11 ++- .../etcd/jetcd/launcher/EtcdClusterImpl.java | 2 + .../etcd/jetcd/test/EtcdClusterExtension.java | 11 ++- 6 files changed, 122 insertions(+), 17 deletions(-) create mode 100644 jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseOnceErrorTest.java diff --git a/jetcd-core/src/main/java/io/etcd/jetcd/impl/LeaseImpl.java b/jetcd-core/src/main/java/io/etcd/jetcd/impl/LeaseImpl.java index 86384f587..a278f839e 100644 --- a/jetcd-core/src/main/java/io/etcd/jetcd/impl/LeaseImpl.java +++ b/jetcd-core/src/main/java/io/etcd/jetcd/impl/LeaseImpl.java @@ -27,7 +27,11 @@ import java.util.concurrent.atomic.AtomicReference; import io.etcd.jetcd.Lease; -import io.etcd.jetcd.api.*; +import io.etcd.jetcd.api.LeaseGrantRequest; +import io.etcd.jetcd.api.LeaseKeepAliveRequest; +import io.etcd.jetcd.api.LeaseRevokeRequest; +import io.etcd.jetcd.api.LeaseTimeToLiveRequest; +import io.etcd.jetcd.api.VertxLeaseGrpc; import io.etcd.jetcd.common.Service; import io.etcd.jetcd.common.exception.ErrorCode; import io.etcd.jetcd.lease.LeaseGrantResponse; @@ -40,7 +44,9 @@ import io.grpc.stub.StreamObserver; import io.vertx.core.streams.WriteStream; -import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.*; +import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.newClosedLeaseClientException; +import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.newEtcdException; +import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.toEtcdException; import static java.util.Objects.requireNonNull; /** @@ -146,7 +152,14 @@ public CompletableFuture keepAliveOnce(long leaseId) { ref.set(s); s.write(req); }) - .handler(r -> future.complete(new LeaseKeepAliveResponse(r))) + .handler(r -> { + if (r.getTTL() != 0) { + future.complete(new LeaseKeepAliveResponse(r)); + } else { + future.completeExceptionally( + newEtcdException(ErrorCode.NOT_FOUND, "etcdserver: requested lease not found")); + } + }) .exceptionHandler(future::completeExceptionally); return future.whenComplete((r, t) -> ref.get().end(req)); diff --git a/jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseMemoryLeakTest.java b/jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseMemoryLeakTest.java index 901022dc6..6a2c7cbce 100644 --- a/jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseMemoryLeakTest.java +++ b/jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseMemoryLeakTest.java @@ -26,6 +26,14 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import io.etcd.jetcd.ByteSequence; +import io.etcd.jetcd.Client; +import io.etcd.jetcd.KV; +import io.etcd.jetcd.Lease; +import io.etcd.jetcd.options.PutOption; +import io.etcd.jetcd.test.EtcdClusterExtension; +import io.restassured.RestAssured; + import org.assertj.core.data.Percentage; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -36,14 +44,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import io.etcd.jetcd.ByteSequence; -import io.etcd.jetcd.Client; -import io.etcd.jetcd.KV; -import io.etcd.jetcd.Lease; -import io.etcd.jetcd.options.PutOption; -import io.etcd.jetcd.test.EtcdClusterExtension; -import io.restassured.RestAssured; - import static org.assertj.core.api.Assertions.assertThat; @Timeout(value = 45, unit = TimeUnit.SECONDS) @@ -58,7 +58,7 @@ public class LeaseMemoryLeakTest { private KV kvClient; private Lease leaseClient; - private static final Logger LOGGER = LoggerFactory.getLogger(LeaseMemoryLeakTest.class); + private static final Logger LOGGER = LoggerFactory.getLogger(LeaseOnceErrorTest.class); private static final long TTL = 10; private static final int ITERATIONS = 5; private static final Pattern GO_ROUTINES_EXTRACT_PATTERN = Pattern.compile("go_goroutines (\\d+)"); diff --git a/jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseOnceErrorTest.java b/jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseOnceErrorTest.java new file mode 100644 index 000000000..f7b3e8bf6 --- /dev/null +++ b/jetcd-core/src/test/java/io/etcd/jetcd/impl/LeaseOnceErrorTest.java @@ -0,0 +1,78 @@ +/* + * Copyright 2016-2021 The jetcd authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.etcd.jetcd.impl; + +import java.util.concurrent.TimeUnit; + +import io.etcd.jetcd.Client; +import io.etcd.jetcd.Lease; +import io.etcd.jetcd.common.exception.EtcdException; +import io.etcd.jetcd.lease.LeaseKeepAliveResponse; +import io.etcd.jetcd.lease.LeaseRevokeResponse; +import io.etcd.jetcd.test.EtcdClusterExtension; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.extension.RegisterExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +@Timeout(value = 45, unit = TimeUnit.SECONDS) +public class LeaseOnceErrorTest { + + @RegisterExtension + public static final EtcdClusterExtension CLUSTER = EtcdClusterExtension.builder() + .withNodes(1) + .build(); + + private Client client; + private Lease leaseClient; + + private static final long TTL = 2; + + @BeforeEach + public void setUp() { + client = TestUtil.client(CLUSTER).build(); + leaseClient = client.getLeaseClient(); + } + + @AfterEach + public void tearDown() { + if (client != null) { + client.close(); + } + } + + // https://github.com/etcd-io/jetcd/issues/1254 + @Test + public void testKeepAliveError() throws Exception { + final long leaseID = leaseClient.grant(TTL).get(1, TimeUnit.SECONDS).getID(); + + LeaseKeepAliveResponse ka1 = leaseClient.keepAliveOnce(leaseID).get(1, TimeUnit.SECONDS); + assertThat(ka1.getTTL()).isNotZero(); + + LeaseRevokeResponse r1 = leaseClient.revoke(leaseID).get(1, TimeUnit.SECONDS); + assertThat(r1).isNotNull(); + + assertThatThrownBy(() -> leaseClient.keepAliveOnce(leaseID).get(1, TimeUnit.SECONDS)) + .hasCauseInstanceOf(EtcdException.class) + .hasMessageContaining("etcdserver: requested lease not found"); + } +} diff --git a/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/Etcd.java b/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/Etcd.java index af32f0e44..8c6b613f4 100644 --- a/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/Etcd.java +++ b/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/Etcd.java @@ -23,10 +23,10 @@ import java.util.List; import java.util.UUID; -import org.testcontainers.containers.Network; - import com.google.common.base.Strings; +import org.testcontainers.containers.Network; + public final class Etcd { public static final String CONTAINER_IMAGE = "gcr.io/etcd-development/etcd:v3.5.9"; public static final int ETCD_CLIENT_PORT = 2379; @@ -54,6 +54,7 @@ public static class Builder { private String prefix; private int nodes = 1; private boolean ssl = false; + private boolean debug = false; private List additionalArgs; private Network network; private boolean shouldMountDataDirectory = true; @@ -78,6 +79,11 @@ public Builder withSsl(boolean ssl) { return this; } + public Builder withDebug(boolean debug) { + this.debug = debug; + return this; + } + public Builder withAdditionalArgs(Collection additionalArgs) { this.additionalArgs = Collections.unmodifiableList(new ArrayList<>(additionalArgs)); return this; @@ -105,6 +111,7 @@ public EtcdCluster build() { prefix, nodes, ssl, + debug, additionalArgs, network != null ? network : Network.newNetwork(), shouldMountDataDirectory); diff --git a/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/EtcdClusterImpl.java b/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/EtcdClusterImpl.java index 7fb4f4c23..aa109a2bd 100644 --- a/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/EtcdClusterImpl.java +++ b/jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/EtcdClusterImpl.java @@ -40,6 +40,7 @@ public EtcdClusterImpl( String prefix, int nodes, boolean ssl, + boolean debug, Collection additionalArgs, Network network, boolean shouldMountDataDirectory) { @@ -53,6 +54,7 @@ public EtcdClusterImpl( .map(e -> new EtcdContainer(image, e, endpoints) .withClusterToken(clusterName) .withSll(ssl) + .withDebug(debug) .withAdditionalArgs(additionalArgs) .withNetwork(network) .withShouldMountDataDirectory(shouldMountDataDirectory)) diff --git a/jetcd-test/src/main/java/io/etcd/jetcd/test/EtcdClusterExtension.java b/jetcd-test/src/main/java/io/etcd/jetcd/test/EtcdClusterExtension.java index 58fc800e3..accc9d4a1 100644 --- a/jetcd-test/src/main/java/io/etcd/jetcd/test/EtcdClusterExtension.java +++ b/jetcd-test/src/main/java/io/etcd/jetcd/test/EtcdClusterExtension.java @@ -24,6 +24,9 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import io.etcd.jetcd.launcher.Etcd; +import io.etcd.jetcd.launcher.EtcdCluster; + import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; @@ -31,9 +34,6 @@ import org.junit.jupiter.api.extension.ExtensionContext; import org.testcontainers.containers.Network; -import io.etcd.jetcd.launcher.Etcd; -import io.etcd.jetcd.launcher.EtcdCluster; - /** * JUnit5 Extension to have etcd cluster in tests. */ @@ -143,6 +143,11 @@ public Builder withSsl(boolean ssl) { return this; } + public Builder withDebug(boolean debug) { + builder.withDebug(debug); + return this; + } + public Builder withAdditionalArgs(Collection additionalArgs) { builder.withAdditionalArgs(additionalArgs); return this;