Merge pull request #1278 from spotify/dxia/periodically-refresh-master-srvs

client: periodically refresh helios master SRV records

Author: davidxia

helios-client/src/main/java/com/spotify/helios/client/HeliosClient.java

@@ -65,7 +65,7 @@
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.spotify.helios.common.HeliosException;
 import com.spotify.helios.common.Json;
-import com.spotify.helios.common.Resolver;
+import com.spotify.helios.common.PeriodicResolver;
 import com.spotify.helios.common.Version;
 import com.spotify.helios.common.VersionCompatibility;
 import com.spotify.helios.common.descriptors.Deployment;
@@ -102,6 +102,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.ThreadFactory;
@@ -632,7 +633,8 @@ public Builder setUser(final String user) {
     }
 
     public Builder setDomain(final String domain) {
-      return setEndpointSupplier(Endpoints.of(Resolver.supplier("helios", domain)));
+      return setEndpointSupplier(Endpoints.of(PeriodicResolver.create("helios", domain,
+          Executors.newSingleThreadScheduledExecutor())));
     }
 
     public Builder setEndpoints(final List<URI> endpoints) {

helios-client/src/main/java/com/spotify/helios/common/PeriodicResolver.java

@@ -0,0 +1,64 @@
+/*-
+ * -\-\-
+ * Helios Client
+ * --
+ * Copyright (C) 2016 - 2019 Spotify AB
+ * --
+ * 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 com.spotify.helios.common;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Supplier;
+import java.net.URI;
+import java.util.List;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A wrapper around {@link Resolver} that periodically refreshes SRV records.
+ */
+public class PeriodicResolver implements Supplier<List<URI>> {
+
+  private List<URI> endpoints;
+
+  private PeriodicResolver(final String srvName,
+                           final String domain,
+                           final Resolver resolver,
+                           final ScheduledExecutorService executorService) {
+    endpoints = resolver.resolve(srvName, domain);
+    executorService.scheduleWithFixedDelay(() ->
+        endpoints = resolver.resolve(srvName, domain), 0, 1, TimeUnit.MINUTES);
+  }
+
+  public static PeriodicResolver create(final String srvName,
+                                        final String domain,
+                                        final ScheduledExecutorService executorService) {
+    return new PeriodicResolver(srvName, domain, new Resolver(), executorService);
+  }
+
+  @VisibleForTesting
+  public static PeriodicResolver create(final String srvName,
+                                        final String domain,
+                                        final Resolver resolver,
+                                        final ScheduledExecutorService executorService) {
+    return new PeriodicResolver(srvName, domain, resolver, executorService);
+  }
+
+  @Override
+  public List<URI> get() {
+    return endpoints;
+  }
+}

helios-client/src/main/java/com/spotify/helios/common/Resolver.java

@@ -24,7 +24,6 @@
 import static java.lang.System.getenv;
 
 import com.google.common.base.Optional;
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.spotify.dns.DnsSrvResolver;
 import com.spotify.dns.DnsSrvResolvers;
@@ -33,7 +32,11 @@
 import java.net.URISyntaxException;
 import java.util.List;
 
-public abstract class Resolver {
+/**
+ * A utility class that resolves DNS SRV records. Used by {@link PeriodicResolver} to discover
+ * helios masters.
+ */
+class Resolver {
 
   private static final String HTTPS_SRV_FORMAT = env("HELIOS_HTTPS_SRV_FORMAT", "_%s._https.%s");
   private static final String HTTP_SRV_FORMAT = env("HELIOS_HTTP_SRV_FORMAT", "_%s._http.%s");
@@ -44,23 +47,16 @@ private static String env(final String name, final String defaultValue) {
     return Optional.fromNullable(getenv(name)).or(defaultValue);
   }
 
-  public static Supplier<List<URI>> supplier(final String srvName, final String domain) {
-    return supplier(srvName, domain, DEFAULT_RESOLVER);
+  List<URI> resolve(final String srvName, final String domain) {
+    return resolve(srvName, domain, DEFAULT_RESOLVER);
   }
 
-  static Supplier<List<URI>> supplier(final String srvName, final String domain,
-                                      final DnsSrvResolver resolver) {
-    return new Supplier<List<URI>>() {
-      @Override
-      public List<URI> get() {
-        // Try to get HTTPS SRV records first and fallback to HTTP
-        List<URI> uris = resolve(srvName, "https", domain, resolver);
-        if (uris.isEmpty()) {
-          uris = resolve(srvName, "http", domain, resolver);
-        }
-        return uris;
-      }
-    };
+  List<URI> resolve(final String srvName, final String domain, final DnsSrvResolver resolver) {
+    List<URI> uris = resolve(srvName, "https", domain, resolver);
+    if (uris.isEmpty()) {
+      return resolve(srvName, "http", domain, resolver);
+    }
+    return uris;
   }
 
   private static List<URI> resolve(final String srvName,

helios-client/src/test/java/com/spotify/helios/common/PeriodicResolverTest.java

@@ -0,0 +1,45 @@
+/*-
+ * -\-\-
+ * Helios Client
+ * --
+ * Copyright (C) 2016 - 2019 Spotify AB
+ * --
+ * 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 com.spotify.helios.common;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.ImmutableList;
+import java.net.URI;
+import java.util.concurrent.Executors;
+import org.hamcrest.Matchers;
+import org.junit.Test;
+
+public class PeriodicResolverTest {
+
+  private final Resolver resolver = mock(Resolver.class);
+
+  @Test
+  public void testGet() {
+    final URI uri = URI.create("https://foo.bar.com");
+    when(resolver.resolve("foo", "bar")).thenReturn(ImmutableList.of(uri));
+    final PeriodicResolver sut = PeriodicResolver.create("foo", "bar", resolver,
+        Executors.newSingleThreadScheduledExecutor());
+    assertThat(sut.get(), Matchers.contains(uri));
+  }
+}

helios-client/src/test/java/com/spotify/helios/common/ResolverTest.java

@@ -24,7 +24,6 @@
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.when;
 
-import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.spotify.dns.DnsSrvResolver;
 import com.spotify.dns.LookupResult;
@@ -44,7 +43,7 @@ public class ResolverTest {
   DnsSrvResolver resolver;
 
   @Test
-  public void testSupplier() throws Exception {
+  public void testResolve() throws Exception {
     final List<LookupResult> lookupResults = ImmutableList.of(
         LookupResult.create("master1.example.com", 443, 1, 1, 1),
         LookupResult.create("master2.example.com", 443, 1, 1, 1),
@@ -58,15 +57,14 @@ public void testSupplier() throws Exception {
 
     when(resolver.resolve("_helios._https.example.com")).thenReturn(lookupResults);
 
-    final Supplier<List<URI>> supplier = Resolver.supplier("helios", "example.com", resolver);
-    final List<URI> uris = supplier.get();
+    final List<URI> uris = new Resolver().resolve("helios", "example.com", resolver);
 
     assertThat(uris.size(), equalTo(3));
     assertThat(uris, Matchers.containsInAnyOrder(expectedUris));
   }
 
   @Test
-  public void testSupplierWithHttpFallback() throws Exception {
+  public void testResolveWithHttpFallback() throws Exception {
     final List<LookupResult> lookupResults = ImmutableList.of(
         LookupResult.create("master1.example.com", 80, 1, 1, 1),
         LookupResult.create("master2.example.com", 80, 1, 1, 1),
@@ -79,11 +77,10 @@ public void testSupplierWithHttpFallback() throws Exception {
     };
 
     when(resolver.resolve("_helios._https.example.com"))
-        .thenReturn(Collections.<LookupResult>emptyList());
+        .thenReturn(Collections.emptyList());
     when(resolver.resolve("_helios._http.example.com")).thenReturn(lookupResults);
 
-    final Supplier<List<URI>> supplier = Resolver.supplier("helios", "example.com", resolver);
-    final List<URI> uris = supplier.get();
+    final List<URI> uris = new Resolver().resolve("helios", "example.com", resolver);
 
     assertThat(uris.size(), equalTo(3));
     assertThat(uris, Matchers.containsInAnyOrder(expectedUris));

helios-tools/src/main/java/com/spotify/helios/cli/Target.java

@@ -23,9 +23,10 @@
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
-import com.spotify.helios.common.Resolver;
+import com.spotify.helios.common.PeriodicResolver;
 import java.net.URI;
 import java.util.List;
+import java.util.concurrent.Executors;
 
 /**
  * A target cluster identified by an endpoint string that can be used with a {@link
@@ -64,7 +65,7 @@ public String getDomain() {
 
     @Override
     public Supplier<List<URI>> getEndpointSupplier() {
-      return Resolver.supplier(srv, domain);
+      return PeriodicResolver.create(srv, domain, Executors.newSingleThreadScheduledExecutor());
     }
 
     @Override