Dynamic environment for apollo-portal #2867

Anilople · 2019-12-24T12:35:30Z

What's the purpose of this PR

Add a feature, make portal support self-define environment without change 1 line code.
Only config will be changed.

Brief changelog

Dynamic environment setting when portal service start.
A new way to forbidden changing hard code to add new environment.
Just change 2 configuration properties.
Suppose that you want to add a new environment beta, and you have been add dev, just do like follow step.

change apollo.portal.envs.Just change its value to dev,beta in db or add VM option -Dapollo.portal.envs=dev,beta
add property beta.meta=your-meta-address or add VM option -Dbeta_meta=your-meta-address

There are 4 fake classes added under package com.ctrip.framework.apollo.portal.environment,
the purpose is using them to replace the classes in apollo-core.

But only 1 fake class exists testing, the rest 3 classes lack the unit testing.

Could I cancel the function 3rd party MetaServerProvider could be injected by typical Java Service Loader pattern. in com.ctrip.framework.apollo.portal.environment.PortalMetaDomainConsts, and make its loading to be not lazy ?

Follow this checklist to help us incorporate your contribution quickly and easily:

Read the Contributing Guide before making this pull request.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit tests to verify the code.
Run mvn clean test to make sure this pull request doesn't break anything.

…o-portal)

coveralls · 2019-12-24T12:48:23Z

Coverage remained the same at ?% when pulling 27dc65b on Anilople:dynamic-portal-env-nochange-with-apollo-core into b718007 on ctripcorp:master.

… string

…ter.

codecov-io · 2019-12-24T14:36:17Z

Codecov Report

Merging #2867 into master will not change coverage.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master    #2867   +/-   ##
=========================================
  Coverage     51.05%   51.05%           
  Complexity     2197     2197           
=========================================
  Files           429      429           
  Lines         13235    13235           
  Branches       1360     1360           
=========================================
  Hits           6757     6757           
  Misses         6005     6005           
  Partials        473      473

Impacted Files	Coverage Δ	Complexity Δ
...framework/apollo/core/signature/HmacSha1Utils.java	`62.5% <100%> (ø)`	`1 <0> (ø)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e81618...27dc65b. Read the comment docs.

nobodyiam

This feature looks great! I think this will greatly reduce the complexitiy to add a customized environment.

.../main/java/com/ctrip/framework/apollo/portal/environment/PortalLegacyMetaServerProvider.java

...rtal/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaDomainConsts.java

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ClusterService.java

…ateCluster

.../main/java/com/ctrip/framework/apollo/portal/environment/PortalLegacyMetaServerProvider.java

...rtal/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaDomainConsts.java

…llo-portal. Initialize it without lazy.

nobodyiam · 2020-01-01T12:11:48Z

...lo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ConsumerController.java

-      for (String env : envArray) {
-        if (Strings.isNullOrEmpty(env)) {
-          continue;
+    } else {


I think this else { statement is unnecessary?

From the git history. The code is merged from other branch.

the else { statement was removed in pr #2864 , so I guess it was brought back to this branch when you resolved the merging conflicts.

Change back already.

Is the whole file rolled back? I think we still need to change the import statement import com.ctrip.framework.apollo.portal.environment.Env;?

I've changed it by accident. A new commit has pushed.

nobodyiam

There is also some conflict with master branch, please help to solve it, thx.

.../main/java/com/ctrip/framework/apollo/portal/environment/PortalLegacyMetaServerProvider.java

nobodyiam · 2020-01-01T12:17:24Z

...rtal/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaDomainConsts.java

+ * @see com.ctrip.framework.apollo.core.MetaDomainConsts
+ * @author wxq
+ */
+public class PortalMetaDomainConsts {


Please add unit test for PortalMetaDomainConsts

In apollo-core, there is a test for MetaDomain, i.e com.ctrip.framework.apollo.core.MetaDomainTest. It seems that implements com.ctrip.framework.apollo.BaseIntegrationTest, and start a internal server to test.

If i want to write the unit test of com.ctrip.framework.apollo.portal.environment.PortalMetaDomainConsts, should i use the same way?

A little Challenge to do this. (hard)

I think we need to do it and it looks like you have done that part

The unit test is done.

Most code is copied from apollo-core

…Provider

…-apollo-core' into dynamic-portal-env-nochange-with-apollo-core

nobodyiam · 2020-01-04T14:26:27Z

...al/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaServerProvider.java

+        metaServerAddressesFromSystemProperty = KeyValueUtils.removeKeySuffix(metaServerAddressesFromSystemProperty, "_meta".length());
+
+        // find key-value from OS environment variable which key ends with "_meta"
+        Map<String, String> metaServerAddressesFromOSEnvironment = KeyValueUtils.filterWithKeyEndswith(System.getenv(), "_meta");


OS Environment variables are normally in UPPER CASE, so it might be better to check with _META

I've change the rule of match suffix to case insensitive.

nobodyiam · 2020-01-04T14:34:32Z

...al/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaServerProvider.java

+        metaServerAddressesFromOSEnvironment = KeyValueUtils.removeKeySuffix(metaServerAddressesFromOSEnvironment, "_meta".length());
+
+        // find key-value from properties file which key ends with ".meta"
+        Properties properties = new Properties();


I think it might be better to also support setting meta server address from PortalDB.ServerConfig, which is the same place to store apollo.portal.envs and is therefore more intuitive, e.g. user can set env and env meta server address in System Configuration page. How do you think?

You mean the meta server addresses are save in apollo.portal.envs which type is a map?
the config like follow:

apollo: portal: envs: dev: http://ip:port other: http://ip:port self-define: http://ip:port

And in PortalDB.ServerConfig's apollo.portal.envs, there is a json object

{ "dev"" : "http://ip:port" "other" : "http://ip:port" "self-define" : "http://ip:port" }

So then portal conbine those config to get the meta server addresses?

I think this feature (add environment in portal when running) will consume more code to implement.
How about when this feature (add environment in portal when start up) finished, next plan to do it?

I think we can keep it as add environment in portal when start up, but we support configuring it via System Configuration page, e.g. we add a new configuration item like apollo.portal.meta.servers to store the mapping between env and meta server address.

We will still keep the apollo.portal.envs as it indicates the active environments so that there may be 3 environment mappings in apollo.portal.meta.servers but only 2 active envs in apollo.portal.envs.

If this involves much more effort, then it's okay to leave it when we support add environment in portal when running.

Get it.
It involves the Database operations. Add new key apollo.portal.meta.servers, and refresh it in runtime.
I feel that the database operation is a bit difficult.
I don't know how to add this feature now.

…ork.apollo.portal.environment.Env

nobodyiam

LGTM for now

Dynamic environment without apollo-core Env (make a fake Env in apoll…

75c671d

…o-portal)

Anilople added 2 commits December 24, 2019 22:24

The data return to front end cannot use wrapper class. Change them to…

953d533

… string

Fix Concurrent problem when create cluster. Add lock when create clus…

ce2d1fd

…ter.

nobodyiam requested changes Dec 29, 2019

View reviewed changes

Delete method getMetaServerAddress. Delete synchronized in method cre…

1ae3642

…ateCluster

nobodyiam reviewed Dec 29, 2019

View reviewed changes

.../main/java/com/ctrip/framework/apollo/portal/environment/PortalLegacyMetaServerProvider.java Outdated Show resolved Hide resolved

nobodyiam reviewed Dec 29, 2019

View reviewed changes

...rtal/src/main/java/com/ctrip/framework/apollo/portal/environment/PortalMetaDomainConsts.java Outdated Show resolved Hide resolved

Anilople added 2 commits December 29, 2019 20:21

Delete the PortalMetaServerProvider. Only 1 MetaServerProvider in apo…

8ec717a

…llo-portal. Initialize it without lazy.

Merge branch 'master' into dynamic-portal-env-nochange-with-apollo-core

c651b53

nobodyiam reviewed Jan 1, 2020

View reviewed changes

nobodyiam requested changes Jan 1, 2020

View reviewed changes

Anilople added 7 commits January 2, 2020 19:03

change class name: PortalLegacyMetaServerProvider -> PortalMetaServer…

579a957

…Provider

unit test of PortalMetaServerProvider

7e45725

Merge branch 'master' into dynamic-portal-env-nochange-with-apollo-core

77a124b

Just remain static method in PortalMetaServerProvider.

e1ef18e

Unit test of PortalMetaDomainConsts

3b314b1

Merge remote-tracking branch 'origin/dynamic-portal-env-nochange-with…

dc11f80

…-apollo-core' into dynamic-portal-env-nochange-with-apollo-core

change Env to new Env

0690715

nobodyiam reviewed Jan 4, 2020

View reviewed changes

Anilople added 3 commits January 4, 2020 23:34

Solve conflict, Rollback to master branch's content

cdf1695

Change: the match of key's suffix is case insensitive now

808d9be

Change: com.ctrip.framework.apollo.core.enums.Env -> com.ctrip.framew…

9e81618

…ork.apollo.portal.environment.Env

nobodyiam changed the title ~~Dynamic environment without apollo-core Env (make a fake Env in apollo-portal)~~ Dynamic environment for apollo-portal Jan 5, 2020

Merge branch 'master' into dynamic-portal-env-nochange-with-apollo-core

27dc65b

nobodyiam approved these changes Jan 5, 2020

View reviewed changes

nobodyiam merged commit 42edb9d into apolloconfig:master Jan 5, 2020

nobodyiam mentioned this pull request Jan 5, 2020

Dynamic env backward compatibility #2827

Closed

4 tasks

nobodyiam added this to the 1.6.0 milestone Feb 16, 2020

nobodyiam added a commit to nobodyiam/apollo that referenced this pull request Feb 22, 2020

a little refactoring for pr apolloconfig#2867

dea51d1

nobodyiam mentioned this pull request Feb 22, 2020

a little refactoring for pr #2867 #2930

Merged

4 tasks

nobodyiam added a commit that referenced this pull request Feb 22, 2020

a little refactoring for pr #2867

fba3930

Anilople mentioned this pull request Mar 10, 2020

Env from database #2953

Merged

4 tasks

Anilople mentioned this pull request Apr 18, 2020

completing the remaining predefined environment #3022

Closed

4 tasks

Anilople added area/portal apollo-portal feature Categorizes issue as related to a new feature. labels Dec 23, 2020

nobodyiam mentioned this pull request Dec 19, 2023

Apollo多环境下的代码优化 #5044

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic environment for apollo-portal #2867

Dynamic environment for apollo-portal #2867

Anilople commented Dec 24, 2019

coveralls commented Dec 24, 2019 •

edited

Loading

codecov-io commented Dec 24, 2019 •

edited

Loading

nobodyiam left a comment

nobodyiam Jan 1, 2020 •

edited

Loading

Anilople Jan 3, 2020

nobodyiam Jan 4, 2020

Anilople Jan 4, 2020

nobodyiam Jan 4, 2020

Anilople Jan 4, 2020

nobodyiam left a comment

nobodyiam Jan 1, 2020

Anilople Jan 2, 2020

nobodyiam Jan 3, 2020

Anilople Jan 3, 2020

Anilople Jan 3, 2020

nobodyiam Jan 4, 2020

Anilople Jan 4, 2020

nobodyiam Jan 4, 2020

Anilople Jan 4, 2020

Anilople Jan 4, 2020

nobodyiam Jan 4, 2020

Anilople Jan 5, 2020

nobodyiam left a comment

Dynamic environment for apollo-portal #2867

Dynamic environment for apollo-portal #2867

Conversation

Anilople commented Dec 24, 2019

Brief changelog

coveralls commented Dec 24, 2019 • edited Loading

codecov-io commented Dec 24, 2019 • edited Loading

Codecov Report

nobodyiam left a comment

Choose a reason for hiding this comment

nobodyiam Jan 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nobodyiam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nobodyiam left a comment

Choose a reason for hiding this comment

coveralls commented Dec 24, 2019 •

edited

Loading

codecov-io commented Dec 24, 2019 •

edited

Loading

nobodyiam Jan 1, 2020 •

edited

Loading