Skip to content

Commit

Permalink
Merge pull request #1307 from yobennett/feature/add-should-enforce-fe…
Browse files Browse the repository at this point in the history
…tch-registry-at-init

Add shouldEnforceFetchRegistryAtInit config
  • Loading branch information
yobennett committed Jun 18, 2020
2 parents 6d7a1e1 + 2b8c018 commit 6438fba
Show file tree
Hide file tree
Showing 7 changed files with 251 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ public boolean shouldFetchRegistry() {
return prefixedConfig.getBoolean(FETCH_REGISTRY_ENABLED_KEY, true);
}

public boolean shouldEnforceFetchRegistryAtInit() {
return prefixedConfig.getBoolean(SHOULD_ENFORCE_FETCH_REGISTRY_AT_INIT_KEY, false);
}

public String getRegistryRefreshSingleVipAddress() {
return prefixedConfig.getString(FETCH_SINGLE_VIP_ONLY_KEY, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,12 @@ public boolean shouldFetchRegistry() {
namespace + FETCH_REGISTRY_ENABLED_KEY, true).get();
}

@Override
public boolean shouldEnforceFetchRegistryAtInit() {
return configInstance.getBooleanProperty(
namespace + SHOULD_ENFORCE_FETCH_REGISTRY_AT_INIT_KEY, false).get();
}

/*
* (non-Javadoc)
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,24 @@ public synchronized BackupRegistry get() {
throw new RuntimeException("Failed to initialize DiscoveryClient!", e);
}

if (clientConfig.shouldFetchRegistry() && !fetchRegistry(false)) {
fetchRegistryFromBackup();
if (clientConfig.shouldFetchRegistry()) {
try {
boolean primaryFetchRegistryResult = fetchRegistry(false);
if (!primaryFetchRegistryResult) {
logger.info("Initial registry fetch from primary servers failed");
}
boolean backupFetchRegistryResult = true;
if (!primaryFetchRegistryResult && !fetchRegistryFromBackup()) {
backupFetchRegistryResult = false;
logger.info("Initial registry fetch from backup servers failed");
}
if (!primaryFetchRegistryResult && !backupFetchRegistryResult && clientConfig.shouldEnforceFetchRegistryAtInit()) {
throw new IllegalStateException("Fetch registry error at startup. Initial fetch failed.");
}
} catch (Throwable th) {
logger.error("Fetch registry error at startup: {}", th.getMessage());
throw new IllegalStateException(th);
}
}

// call and execute the pre registration handler before all background tasks (inc registration) is started
Expand Down Expand Up @@ -1543,8 +1559,10 @@ void refreshRegistry() {
/**
* Fetch the registry information from back up registry if all eureka server
* urls are unreachable.
*
* @return true if the registry was fetched
*/
private void fetchRegistryFromBackup() {
private boolean fetchRegistryFromBackup() {
try {
@SuppressWarnings("deprecation")
BackupRegistry backupRegistryInstance = newBackupRegistryInstance();
Expand All @@ -1568,13 +1586,15 @@ private void fetchRegistryFromBackup() {
localRegionApps.set(applications);
logTotalInstances();
logger.info("Fetched registry successfully from the backup");
return true;
}
} else {
logger.warn("No backup registry instance defined & unable to find any discovery servers.");
}
} catch (Throwable e) {
logger.warn("Cannot fetch applications from apps although backup registry was specified", e);
}
return false;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,16 @@ default boolean shouldUnregisterOnShutdown() {
*/
boolean shouldFetchRegistry();

/**
* If set to true, the {@link EurekaClient} initialization should throw an exception at constructor time
* if an initial fetch of eureka registry information from the remote servers is unsuccessful.
*
* Note that if {@link #shouldFetchRegistry()} is set to false, then this config is a no-op
*
* @return true or false for whether the client initialization should enforce an initial fetch
*/
boolean shouldEnforceFetchRegistryAtInit();

/**
* Indicates whether the client is only interested in the registry information for a single VIP.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ final class PropertyBasedClientConfigConstants {

static final String REGISTRATION_ENABLED_KEY = "registration.enabled";
static final String FETCH_REGISTRY_ENABLED_KEY = "shouldFetchRegistry";
static final String SHOULD_ENFORCE_FETCH_REGISTRY_AT_INIT_KEY = "shouldEnforceFetchRegistryAtInit";

static final String REGISTRY_REFRESH_INTERVAL_KEY = "client.refresh.interval";
static final String REGISTRATION_REPLICATION_INTERVAL_KEY = "appinfo.replicate.interval";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
package com.netflix.discovery;

import com.google.inject.AbstractModule;
import com.google.inject.Injector;
import com.google.inject.ProvisionException;
import com.google.inject.Scopes;
import com.netflix.appinfo.EurekaInstanceConfig;
import com.netflix.appinfo.PropertiesInstanceConfig;
import com.netflix.discovery.shared.Applications;
import com.netflix.discovery.shared.resolver.EndpointRandomizer;
import com.netflix.discovery.shared.resolver.ResolverUtils;
import com.netflix.discovery.shared.transport.EurekaHttpClient;
import com.netflix.discovery.shared.transport.SimpleEurekaHttpServer;
import com.netflix.discovery.shared.transport.jersey.Jersey1DiscoveryClientOptionalArgs;
import com.netflix.discovery.util.InstanceInfoGenerator;
import com.netflix.governator.guice.LifecycleInjector;
import com.netflix.governator.lifecycle.LifecycleManager;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;

import javax.ws.rs.core.MediaType;
import java.io.IOException;
import java.util.Collections;
import java.util.List;

import static com.netflix.discovery.shared.transport.EurekaHttpResponse.anEurekaHttpResponse;
import static java.util.Collections.singletonList;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

/**
* Tests the `shouldEnforceFetchRegistryAtInit` configuration property, which throws an exception during `DiscoveryClient`
* construction if set to `true` and both the primary and backup registry fail to return a successful response.
*/
public class EurekaClientLifecycleServerFailureTest {

private static final String MY_APPLICATION_NAME = "MYAPPLICATION";
private static final String MY_INSTANCE_ID = "myInstanceId";

private static final Applications APPLICATIONS = InstanceInfoGenerator.newBuilder(1, 1).build().toApplications();
private static final Applications APPLICATIONS_DELTA = new Applications(APPLICATIONS.getAppsHashCode(), 1L, Collections.emptyList());

public static final EurekaHttpClient requestHandler = mock(EurekaHttpClient.class);

public static SimpleEurekaHttpServer eurekaHttpServer;

@BeforeClass
public static void setupClass() throws IOException {
eurekaHttpServer = new SimpleEurekaHttpServer(requestHandler);
when(requestHandler.getApplications()).thenReturn(
anEurekaHttpResponse(500, APPLICATIONS).type(MediaType.APPLICATION_JSON_TYPE).build()
);
when(requestHandler.getDelta()).thenReturn(
anEurekaHttpResponse(500, APPLICATIONS_DELTA).type(MediaType.APPLICATION_JSON_TYPE).build()
);
}

@AfterClass
public static void tearDownClass() {
if (eurekaHttpServer != null) {
eurekaHttpServer.shutdown();
}
}

private static class LocalEurekaInstanceConfig extends PropertiesInstanceConfig {

@Override
public String getInstanceId() {
return MY_INSTANCE_ID;
}

@Override
public String getAppname() {
return MY_APPLICATION_NAME;
}

@Override
public int getLeaseRenewalIntervalInSeconds() {
return 1;
}
}

/**
* EurekaClientConfig configured to enforce fetch registry at init
*/
private static class LocalEurekaClientConfig1 extends DefaultEurekaClientConfig {
@Override
public List<String> getEurekaServerServiceUrls(String myZone) {
return singletonList(eurekaHttpServer.getServiceURI().toString());
}

@Override
public boolean shouldEnforceFetchRegistryAtInit() {
return true;
}
}

/**
* EurekaClientConfig configured to enforce fetch registry at init but not to fetch registry
*/
private static class LocalEurekaClientConfig2 extends DefaultEurekaClientConfig {
@Override
public List<String> getEurekaServerServiceUrls(String myZone) {
return singletonList(eurekaHttpServer.getServiceURI().toString());
}

@Override
public boolean shouldEnforceFetchRegistryAtInit() {
return true;
}

@Override
public boolean shouldFetchRegistry() {
return false;
}
}

/**
* n.b. without a configured backup registry, the default backup registry is set to `NotImplementedRegistryImpl`,
* which returns `null` for its list of applications and thus results in a failure to return a successful response
* for registry data when used.
*/
@Test(expected = ProvisionException.class)
public void testEnforceFetchRegistryAtInitPrimaryAndBackupFailure() {
Injector injector = LifecycleInjector.builder()
.withModules(
new AbstractModule() {
@Override
protected void configure() {
bind(EurekaInstanceConfig.class).to(LocalEurekaInstanceConfig.class);
bind(EurekaClientConfig.class).to(LocalEurekaClientConfig1.class);
bind(AbstractDiscoveryClientOptionalArgs.class).to(Jersey1DiscoveryClientOptionalArgs.class).in(Scopes.SINGLETON);
bind(EndpointRandomizer.class).toInstance(ResolverUtils::randomize);
}
}
)
.build().createInjector();
LifecycleManager lifecycleManager = injector.getInstance(LifecycleManager.class);
try {
lifecycleManager.start();
} catch (Exception e) {
throw new RuntimeException(e);
}

// this will throw a Guice ProvisionException for the constructor failure
injector.getInstance(EurekaClient.class);
}

@Test
public void testEnforceFetchRegistryAtInitPrimaryFailureAndBackupSuccess() {
Injector injector = LifecycleInjector.builder()
.withModules(
new AbstractModule() {
@Override
protected void configure() {
bind(EurekaInstanceConfig.class).to(LocalEurekaInstanceConfig.class);
bind(EurekaClientConfig.class).to(LocalEurekaClientConfig1.class);
bind(AbstractDiscoveryClientOptionalArgs.class).to(Jersey1DiscoveryClientOptionalArgs.class).in(Scopes.SINGLETON);
bind(EndpointRandomizer.class).toInstance(ResolverUtils::randomize);
bind(BackupRegistry.class).toInstance(new MockBackupRegistry()); // returns empty list on registry fetch
}
}
)
.build().createInjector();
LifecycleManager lifecycleManager = injector.getInstance(LifecycleManager.class);
try {
lifecycleManager.start();
} catch (Exception e) {
throw new RuntimeException(e);
}

// this will not throw a Guice ProvisionException for the constructor failure
EurekaClient client = injector.getInstance(EurekaClient.class);
Assert.assertNotNull(client);
}

@Test
public void testEnforceFetchRegistryAtInitPrimaryFailureNoop() {
Injector injector = LifecycleInjector.builder()
.withModules(
new AbstractModule() {
@Override
protected void configure() {
bind(EurekaInstanceConfig.class).to(LocalEurekaInstanceConfig.class);
bind(EurekaClientConfig.class).to(LocalEurekaClientConfig2.class);
bind(AbstractDiscoveryClientOptionalArgs.class).to(Jersey1DiscoveryClientOptionalArgs.class).in(Scopes.SINGLETON);
bind(EndpointRandomizer.class).toInstance(ResolverUtils::randomize);
}
}
)
.build().createInjector();
LifecycleManager lifecycleManager = injector.getInstance(LifecycleManager.class);
try {
lifecycleManager.start();
} catch (Exception e) {
throw new RuntimeException(e);
}

// this will not throw a Guice ProvisionException for the constructor failure
EurekaClient client = injector.getInstance(EurekaClient.class);
Assert.assertNotNull(client);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ public static void clearDiscoveryClientConfig() {
ConfigurationManager.getConfigInstance().clearProperty("eureka.fetchRemoteRegionsRegistry");
ConfigurationManager.getConfigInstance().clearProperty("eureka.myregion.availabilityZones");
ConfigurationManager.getConfigInstance().clearProperty("eureka.serviceUrl.default");
ConfigurationManager.getConfigInstance().clearProperty("eureka.shouldEnforceFetchRegistryAtInit");
}

public static EurekaClient setupDiscoveryClient(InstanceInfo clientInstanceInfo) {
Expand Down

0 comments on commit 6438fba

Please sign in to comment.