The 0.7.6 version does not throw an exception when keepAliveOnce failed

#1254
etcd-io · Oct 18, 2023 · ba451f3 · ba451f3
1 parent 71315d7
commit ba451f3
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 17 deletions.
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
@@ -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<LeaseKeepAliveResponse> 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
@@ -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
@@ -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
@@ -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<String> 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<String> 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
@@ -40,6 +40,7 @@ public EtcdClusterImpl(
         String prefix,
         int nodes,
         boolean ssl,
+        boolean debug,
         Collection<String> 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
@@ -24,16 +24,16 @@
 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;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 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<String> additionalArgs) {
             builder.withAdditionalArgs(additionalArgs);
             return this;