From 0ee0e88852701d0748cbed83525b974e5501afdb Mon Sep 17 00:00:00 2001 From: "dongsheng.qi" Date: Wed, 14 Aug 2024 11:11:35 +0800 Subject: [PATCH 1/2] fix: The configfile interface cannot correctly obtain configuration through grayscale labels --- .../GrayReleaseRulesHolder.java | 33 +++++++++++++++---- .../GrayReleaseRulesHolderTest.java | 22 ++++++------- .../controller/ConfigFileController.java | 8 +++-- .../controller/ConfigFileControllerTest.java | 4 +-- 4 files changed, 45 insertions(+), 22 deletions(-) diff --git a/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolder.java b/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolder.java index e7f3b904301..9a1fde4a6f7 100644 --- a/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolder.java +++ b/apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolder.java @@ -152,15 +152,36 @@ public Long findReleaseIdFromGrayReleaseRule(String clientAppId, String clientIp } /** - * Check whether there are gray release rules for the clientAppId, clientIp, namespace + * Check whether there are gray release rules for the clientAppId, clusterName, clientIp, clientLabel, namespace * combination. Please note that even there are gray release rules, it doesn't mean it will always * load gray releases. Because gray release rules actually apply to one more dimension - cluster. */ - public boolean hasGrayReleaseRule(String clientAppId, String clientIp, String namespaceName) { - return reversedGrayReleaseRuleCache.containsKey(assembleReversedGrayReleaseRuleKey(clientAppId, - namespaceName, clientIp)) || reversedGrayReleaseRuleCache.containsKey - (assembleReversedGrayReleaseRuleKey(clientAppId, namespaceName, GrayReleaseRuleItemDTO - .ALL_IP)); + public boolean hasGrayReleaseRule(String clientAppId, String clusterName, String clientIp, String clientLabel, String namespaceName) { + // check ip gray rule + if (reversedGrayReleaseRuleCache.containsKey(assembleReversedGrayReleaseRuleKey(clientAppId, + namespaceName, clientIp)) || reversedGrayReleaseRuleCache.containsKey + (assembleReversedGrayReleaseRuleKey(clientAppId, namespaceName, GrayReleaseRuleItemDTO + .ALL_IP))) { + return true; + } + + // check label gray rule + String key = assembleGrayReleaseRuleKey(clientAppId, clusterName, namespaceName); + if (!grayReleaseRuleCache.containsKey(key)) { + return false; + } + List rules = Lists.newArrayList(grayReleaseRuleCache.get(key)); + for (GrayReleaseRuleCache rule : rules) { + //check branch status + if (rule.getBranchStatus() != NamespaceBranchStatus.ACTIVE) { + continue; + } + if (rule.matches(clientAppId, clientIp, clientLabel)) { + return true; + } + }; + + return false; } private void scanGrayReleaseRules() { diff --git a/apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolderTest.java b/apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolderTest.java index 503f36ae3d6..9f61d35cd3c 100644 --- a/apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolderTest.java +++ b/apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolderTest.java @@ -109,18 +109,18 @@ public void testScanGrayReleaseRules() throws Exception { assertNull(grayReleaseRulesHolder.findReleaseIdFromGrayReleaseRule(anotherClientAppId, anotherClientIp, anotherClientLabel, someAppId, someClusterName, someNamespaceName)); - assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp, + assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel, someNamespaceName)); - assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId.toUpperCase(), someClientIp, + assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId.toUpperCase(), someClusterName, someClientIp, someClientLabel, someNamespaceName.toUpperCase())); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, anotherClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, anotherClientIp, someClientLabel, someNamespaceName)); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel, anotherNamespaceName)); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel, someNamespaceName)); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel, anotherNamespaceName)); GrayReleaseRule anotherRule = assembleGrayReleaseRule(someAppId, someClusterName, @@ -144,16 +144,16 @@ public void testScanGrayReleaseRules() throws Exception { (anotherClientAppId, anotherClientIp, anotherClientLabel, someAppId, someClusterName, someNamespaceName)); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel, someNamespaceName)); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel, anotherNamespaceName)); - assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp, + assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel, someNamespaceName)); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, someClientIp, someClientLabel, someNamespaceName)); - assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp, + assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel, anotherNamespaceName)); } diff --git a/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java b/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java index 297de2ee0aa..f286fcf04f0 100644 --- a/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java +++ b/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java @@ -149,6 +149,7 @@ public ResponseEntity queryConfigAsJson(@PathVariable String appId, HttpServletRequest request, HttpServletResponse response) throws IOException { + System.out.println("requeset1: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel); String result = queryConfig(ConfigFileOutputFormat.JSON, appId, clusterName, namespace, dataCenter, clientIp, clientLabel, request, response); @@ -174,14 +175,14 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu } //1. check whether this client has gray release rules - boolean hasGrayReleaseRule = grayReleaseRulesHolder.hasGrayReleaseRule(appId, clientIp, - namespace); + boolean hasGrayReleaseRule = grayReleaseRulesHolder.hasGrayReleaseRule(appId, clusterName, clientIp, clientLabel, namespace); String cacheKey = assembleCacheKey(outputFormat, appId, clusterName, namespace, dataCenter); //2. try to load gray release and return if (hasGrayReleaseRule) { Tracer.logEvent("ConfigFile.Cache.GrayRelease", cacheKey); + System.out.println("requeset2: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel); return loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel, request, response); } @@ -192,6 +193,7 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu //4. if not exists, load from ConfigController if (Strings.isNullOrEmpty(result)) { Tracer.logEvent("ConfigFile.Cache.Miss", cacheKey); + System.out.println("requeset3: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel); result = loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel, request, response); @@ -200,7 +202,7 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu } //5. Double check if this client needs to load gray release, if yes, load from db again //This step is mainly to avoid cache pollution - if (grayReleaseRulesHolder.hasGrayReleaseRule(appId, clientIp, namespace)) { + if (grayReleaseRulesHolder.hasGrayReleaseRule(appId, clusterName, clientIp, clientLabel, namespace)) { Tracer.logEvent("ConfigFile.Cache.GrayReleaseConflict", cacheKey); return loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel, request, response); diff --git a/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileControllerTest.java b/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileControllerTest.java index 62e66b3933c..2560787f764 100644 --- a/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileControllerTest.java +++ b/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileControllerTest.java @@ -95,7 +95,7 @@ public void setUp() throws Exception { when(namespaceUtil.filterNamespaceName(someNamespace)).thenReturn(someNamespace); when(namespaceUtil.normalizeNamespace(someAppId, someNamespace)).thenReturn(someNamespace); - when(grayReleaseRulesHolder.hasGrayReleaseRule(anyString(), anyString(), anyString())) + when(grayReleaseRulesHolder.hasGrayReleaseRule(anyString(), anyString(), anyString(), anyString(), anyString())) .thenReturn(false); watchedKeys2CacheKey = @@ -199,7 +199,7 @@ public void testQueryConfigWithGrayRelease() throws Exception { Map configurations = ImmutableMap.of(someKey, someValue); - when(grayReleaseRulesHolder.hasGrayReleaseRule(someAppId, someClientIp, someNamespace)) + when(grayReleaseRulesHolder.hasGrayReleaseRule(someAppId, someClusterName, someClientIp, someClientLabel, someNamespace)) .thenReturn(true); ApolloConfig someApolloConfig = mock(ApolloConfig.class); From 18a98e2efc813c244e06687f140829f23694fb0e Mon Sep 17 00:00:00 2001 From: "dongsheng.qi" Date: Wed, 14 Aug 2024 11:22:36 +0800 Subject: [PATCH 2/2] fix: remove debug log --- .../apollo/configservice/controller/ConfigFileController.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java b/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java index f286fcf04f0..3ab90ec9cca 100644 --- a/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java +++ b/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java @@ -149,7 +149,6 @@ public ResponseEntity queryConfigAsJson(@PathVariable String appId, HttpServletRequest request, HttpServletResponse response) throws IOException { - System.out.println("requeset1: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel); String result = queryConfig(ConfigFileOutputFormat.JSON, appId, clusterName, namespace, dataCenter, clientIp, clientLabel, request, response); @@ -182,7 +181,6 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu //2. try to load gray release and return if (hasGrayReleaseRule) { Tracer.logEvent("ConfigFile.Cache.GrayRelease", cacheKey); - System.out.println("requeset2: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel); return loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel, request, response); } @@ -193,7 +191,6 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu //4. if not exists, load from ConfigController if (Strings.isNullOrEmpty(result)) { Tracer.logEvent("ConfigFile.Cache.Miss", cacheKey); - System.out.println("requeset3: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel); result = loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel, request, response);