Netflix / karyon

The nucleus or the base container for Applications and Services built using the NetflixOSS ecosystem
Apache License 2.0
497 stars 153 forks source link

Karyon 2.0 seems to force Amazon Datacenter for Eureka #80

Open aspyker opened 10 years ago

aspyker commented 10 years ago

On adding in KaryonEurekaModule to my Modules, I get:

1) Error in custom provider, java.lang.RuntimeException: Your datacenter is defined as cloud but we are not able to get the amazon metadata to register. Set the property eureka.validateInstanceId to false to ignore themetadata call while locating com.netflix.appinfo.providers.CloudInstanceConfigProvider while locating com.netflix.appinfo.CloudInstanceConfig while locating com.netflix.appinfo.EurekaInstanceConfig for parameter 0 at com.netflix.appinfo.providers.EurekaConfigBasedInstanceInfoProvider.(EurekaConfigBasedInstanceInfoProvider.java:35) at com.netflix.appinfo.providers.EurekaConfigBasedInstanceInfoProvider.class(EurekaConfigBasedInstanceInfoProvider.java:26) while locating com.netflix.appinfo.providers.EurekaConfigBasedInstanceInfoProvider while locating com.netflix.appinfo.InstanceInfo for parameter 0 at com.netflix.discovery.DiscoveryClient.(DiscoveryClient.java:198) at com.netflix.discovery.DiscoveryClient.class(DiscoveryClient.java:108) while locating com.netflix.discovery.DiscoveryClient at com.netflix.karyon.eureka.KaryonEurekaModule.configure(KaryonEurekaModule.java:21) while locating com.netflix.discovery.shared.LookupService Caused by: java.lang.RuntimeException: Your datacenter is defined as cloud but we are not able to get the amazon metadata to register. Set the property eureka.validateInstanceId to false to ignore themetadata call at com.netflix.appinfo.CloudInstanceConfig.initDataCenterInfo(CloudInstanceConfig.java:83) at com.netflix.appinfo.CloudInstanceConfig.initCloudInstanceConfig(CloudInstanceConfig.java:67) at com.netflix.appinfo.CloudInstanceConfig.(CloudInstanceConfig.java:57) at com.netflix.appinfo.providers.CloudInstanceConfigProvider.get(CloudInstanceConfigProvider.java:27) at com.netflix.appinfo.providers.CloudInstanceConfigProvider.get(CloudInstanceConfigProvider.java:15) at com.google.inject.internal.InjectorImpl$2.get(InjectorImpl.java:687) at com.google.internal.InjectorImpl$3.get(InjectorImpl.java:737) at com.google.inject.internal.SingleParameterInjector.inject(SingleParameterInjector.java:38) at com.google.inject.internal.SingleParameterInjector.getAll(SingleParameterInjector.java:62) at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:84) at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:254) at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1031) at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40) at com.google.inject.Scopes$1$1.get(Scopes.java:65) at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:40) at com.google.inject.internal.InjectorImpl$2.get(InjectorImpl.java:684) at com.google.inject.internal.SingleParameterInjector.inject(SingleParameterInjector.java:38) at com.google.inject.internal.SingleParameterInjector.getAll(SingleParameterInjector.java:62) at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:84) at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:254) at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1031) at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40) at com.netflix.governator.guice.lazy.FineGrainedLazySingletonScopeImpl$1.get(FineGrainedLazySingletonScopeImpl.java:62) at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:40) at com.google.inject.internal.FactoryProxy.get(FactoryProxy.java:54) at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInteroryAdapter.java:46) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1031) at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40) at com.google.inject.Scopes$1$1.get(Scopes.java:65) at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:40) at com.google.inject.internal.InternalInjectorCreator$1.call(InternalInjectorCreator.java:204) at com.google.inject.internal.InternalInjectorCreator$1.call(InternalInjectorCreator.java:198) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1024) at com.google.inject.internal.InternalInjectorCreator.loadEagerSingletons(InternalInjectorCreator.java:198) at com.google.inject.internal.InternalInjectorCreator.injectDynamically(InternalInjectorCreator.java:179) at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:109) at com.google.inject.Guice.createInjector(Guice.java:95) at com.netflix.governator.guice.LifecycleInjector.createSimulatedChildInjector(LifecycleInjector.java:371) at com.netflix.governator.guice.LifecycleInjector.createChildInjector(LifecycleInjector.java:240) at com.netflix.governator.guice.LifecycleInjector.createInjector(LifecycleInjector.java:303) at com.netflix.governator.guice.LifecycleInjector.createInjector(LifecycleInjector.java:257) at com.netflix.governator.guice.LifecycleInjector.bootstrap(LifecycleInjector.java:160) at com.netflix.karyon.KaryonServer.startAndAwait(KaryonServer.java:26) at com.netflix.karyon.KaryonServer.main(KaryonServer.java:44)

It looks like Amazon is assumed by

ApplicationInfoManager initialization to EurekaInstanceConfig -> implemented by CloudInstanceConfig. This seems like this is the Amazon one. I tried to do the older Karyon 1.0 approach of com.netflix.karyon.eureka.datacenter.type=MyOwn in my config properties and as a -D parameter, but neither avoided the issue.

aspyker commented 10 years ago

I see this in 1.x:

https://github.com/Netflix/karyon/blob/1.x/karyon-eureka/src/main/java/com/netflix/karyon/server/eureka/EurekaHandler.java#L80

This used to default to MyOwn and setting archaius used to change how the switch case here worked:

https://github.com/Netflix/karyon/blob/1.x/karyon-eureka/src/main/java/com/netflix/karyon/server/eureka/EurekaHandler.java#L138

Where is the equiv in Karyon 2.0?

NiteshKant commented 10 years ago

Thanks to @elandau eureka supports injection and karyon-governator module provides eureka integration through a guice module.

Simplistically you can configure eureka to use your datacenter config, using the following guice binding:

bind(EurekaInstanceConfig.class).to(MyDataCenterInstanceConfig.class);

where MyDataCenterInstanceConfig is the config for our datacenter.

aspyker commented 10 years ago

Confirmed that this works. Two more questions:

  1. Is there a way to configure this via archaius like worked before?
  2. Do you want to change the default behavior when it's not configured as compared to Karyon 1.0 which will mean people trying this locally (not on Amazon) will get an error?
NiteshKant commented 10 years ago

Is there a way to configure this via archaius like worked before?

No. One of the key principals we follow in karyon 2.0 is to reduce change in implementations based on properties like this one. The behavior can be easily altered by a guice module and is very evident from the code as opposed to a property in a property file.

Do you want to change the default behavior when it's not configured as compared to Karyon 1.0 which will mean people trying this locally (not on Amazon) will get an error?

The sample application sets the property "eureka.validateInstanceId" to false here which mutes the error you have mentioned in this bug.

aspyker commented 10 years ago

By forcing the configuration into the code, this means I can't have a Acme Air that works across multiple environments without recompiling. We had previously used this property to, outside of the application, switch between SoftLayer, Docker (and likely would have worked for Amazon). I'm not familiar enough yet with guice. Is there a way with the new approach that I could control this from outside the code, so the same WAR binary could work across various Eureka datacenter providers?

Thanks for the pointer to the validateInstanceId. However, this is just masking a change in default behavior as compared to 1.0 (1.0 assumes MyOwn, 2.0 assumes Amazon and asks you to disable this property). What is the difference between MyOwn and Amazon with validateInstanceId = false?

NiteshKant commented 10 years ago

@aspyker I do not believe choosing a EurekaInstanceConfig type (which is based on your datacenter) is truly a configuration and it belongs to a configuration file. Apart from that, it has issues around a property value mapping to an enum which maps to an instance of an interface. This made it impossible for anyone to specify a completely different configuration. Since, guice(DI) is built for this usecase i.e. the ability to change implementations in different environments, it fits for this usecase.

Getting to how to achieve this with guice, you can have your module drive the bindings based on configuration. Modifying the KaryonJerseyModuleImpl in the hello-world example:

public static class KaryonJerseyModuleImpl extends KaryonJerseyModule {

        @Override
        protected void configure() {
            super.configure();
            String dcType = ConfigurationManager.getConfigInstance().getString("datacenter.type", null);
            if (null == dcType) {
                bind(EurekaInstanceConfig.class).to(MyDataCenterInstanceConfig.class);
            } else if ("XXX".equals(dcType)) {
                bind(EurekaInstanceConfig.class).to(... );  // Bind to your instance class.
            }
            bind(AuthenticationService.class).to(AuthenticationServiceImpl.class);
        }

        @Override
        public int serverPort() {
            return 8888;
        }

        @Override
        public int shutdownPort() {
            return 8899;
        }

        @Override
        public void configureInterceptors(GovernatorHttpInterceptorSupport<ByteBuf, ByteBuf> interceptorSupport) {
            interceptorSupport.forUri("/*").intercept(LoggingInterceptor.class);
            interceptorSupport.forUri("/hello").interceptIn(AuthInterceptor.class);
        }
    }

You can specify the property datacenter.type in any of your config files.

What is the difference between MyOwn and Amazon with validateInstanceId = false?

Even if you specify validateInstanceId = false, the datacenter is still Amazon, it just does not barf if it can not get the amazon metadata. It is really for dev environments posing as running in the cloud. The sample app should have this property defined in the hello-netflix-oss-dev.properties property though, I will fix that.

NiteshKant commented 10 years ago

@aspyker should I close this issue? I do not see any change needed from karyon side.

aspyker commented 10 years ago

Nitesh,

Sorry this project is a side project for me, so I'm slow to respond.

As you said it just means we'll have to move the multiple datacenter (cloud) support into our own apps. I understand how the enumeration support was painful, but it was nice for this to be configurable across clouds in a single app without the app code in each app having to change to support this behavior. Given this I will likely introduce a property that is IBM usage specific and code into all of our apps as you suggested.

FWIW, I was hoping to get a SoftLayerDataCenterInfo class added to Eureka over time. At that point we'd have SoftLayerDatacenterInfo, MyDataCenterInfo (better named GenericDataCenterInfo), DataCenterInfo (better named AmazonDataCenterInfo). If the NetflixOSS cloud platform in general maintained these various "supported" options, it would seem like the NetflixOSS cloud platform could switch between then for any specific application.

I would have rather seen the code you noted above in KaryonEurekaModule and maybe to get around the enumeration issue, just have the property specified as a className (com.netflix.karyon.eureka.datacenter.class=com.netflix.appinfo.SoftLayerDataCenterInfo).

But if the choice is that Karyon doesn't want to support these sort of properties long term, I think it's reasonable to close this issue.