Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SCB-1128] Omega discovers alpha service from eureka #394

Merged
merged 22 commits into from
Jan 27, 2019

Conversation

coolbeevip
Copy link
Member

@coolbeevip coolbeevip commented Jan 23, 2019

Get the access address of Alpah Server from Eureka Server
Turn this feautre on by set alpha.cluster.register.type=spring-cloud
First omega gets the Alpha address from Eureka with ${alpha.cluster.serviceId}
If omega can't get it in Eureka then use ${alpha.cluster.address}

Add dependencie

 <dependency>
      <groupId>org.apache.servicecomb.pack</groupId>
      <artifactId>omega-spring-starter</artifactId>
      <version>0.4.0-SNAPSHOT</version>
 </dependency>
 <dependency>
      <groupId>org.apache.servicecomb.pack</groupId>
      <artifactId>omega-spring-cloud-starter</artifactId>
      <version>0.4.0-SNAPSHOT</version>
 </dependency>

About AutoConfiguration

Deprecated @EnableOmega

add omega-spring-starter dependency, then it will automatically enabled omega,You can set omega.enabled=true/false to turn this feature ON or OFF

Add Alpha configuration to the application.yml

  • Default Alpha use ${spring.application.name} as the Eureka serviceId
alpha.cluster.serviceId=servicecomb-alpha-server
  • Define the registry type

When the value is spring-cloud, it is get from Eureka. otherwise use ${alpha.cluster.address}

alpha.cluster.register.type=spring-cloud
  • e.g.
alpha:
  cluster:
    address: localhost:8080
    serviceId: servicecomb-alpha-server
    register:
      type: spring-cloud
omega:
  instance:
    instanceId: ${spring.application.name}-${spring.cloud.client.hostname}-${server.port}

Add Eureka Client configuration to the application.yml

eureka:
  client:
    service-url:
      defaultZone: http://127.0.0.1:8761/eureka

@coveralls
Copy link

coveralls commented Jan 23, 2019

Coverage Status

Coverage increased (+0.03%) to 90.375% when pulling cea2b8a on coolbeevip:SCB-1128 into 43894d0 on apache:master.

@@ -0,0 +1,9 @@
info:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need the license header, please take the application.yaml as an example.

* to pass the transactions id across the application.
* @see OmegaContext
*/
public @interface EnableOmegaSpringCloud {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already has the EnableOmega, so I don't think we need to add this annotation here.
We may need to find a way to enable the service discovery by looking up the configuration atomatically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can use spring.factories

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can let SpringFactoriesLoader automatically configure OmegaSpringConfig through spring.factories, so @EnableOmega is not needed


@Bean(name = {"alphaClusterEurekaConfig"})
@ConditionalOnProperty(name = "alpha.cluster.register.type", havingValue = "spring-cloud")
AlphaClusterConfig alphaClusterEurekaConfig(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to find a way to reuse the old configuration, it could be in our todo list.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe define an object AlphaClusterAddress, Attribute contains Alpha's address, registry center information...

Add AlphaClusterAddress to the parameters of AlphaClusterConfig

  AlphaClusterConfig alphaClusterConfig(
      @Value("${alpha.cluster.address:localhost:8080}") String[] addresses,
      @Value("${alpha.cluster.ssl.enable:false}") boolean enableSSL,
      @Value("${alpha.cluster.ssl.mutualAuth:false}") boolean mutualAuth,
      @Value("${alpha.cluster.ssl.cert:client.crt}") String cert,
      @Value("${alpha.cluster.ssl.key:client.pem}") String key,
      @Value("${alpha.cluster.ssl.certChain:ca.crt}") String certChain,
      @Lazy AlphaClusterAddress alphaClusterAddress,
      @Lazy MessageHandler handler,
      @Lazy TccMessageHandler tccMessageHandler) {
   ...
}

Define an default AlphaClusterAddress Bean in OmegaSpringConfig,it's Null
Define an default AlphaClusterAddress Bean in OmegaSpringEurekaConfig,it's from Eureka

use this AlphaClusterAddress if it is not empty, otherwise use the default ${alpha.cluster.address address}

@coolbeevip
Copy link
Member Author

@WillemJiang I reconstructed the code according to your idea

@coolbeevip coolbeevip closed this Jan 25, 2019
@coolbeevip coolbeevip reopened this Jan 25, 2019
@coolbeevip coolbeevip closed this Jan 25, 2019
@coolbeevip coolbeevip reopened this Jan 25, 2019
@WillemJiang WillemJiang changed the title [SCB 1128] Omega discovers alpha service from eureka [SCB-1128] Omega discovers alpha service from eureka Jan 25, 2019
MessageFormat messageFormat = new KryoMessageFormat();
AlphaClusterConfig clusterConfig = AlphaClusterConfig.builder()
.addresses(ImmutableList.copyOf(addresses))
.addresses(ImmutableList.copyOf(alphaClusterDiscovery.getAddresses()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we implement the dynamic address lookup? I think we need to add the alphaClusterDiscovery address here.

@WillemJiang WillemJiang merged commit 928e9d6 into apache:master Jan 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants