You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by sm...@apache.org on 2023/07/24 09:15:26 UTC
[knox] branch master updated: KNOX-2943 - Handling CORE-SETTINGS service properly (#780)
This is an automated email from the ASF dual-hosted git repository.
smolnar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new 1111d04ad KNOX-2943 - Handling CORE-SETTINGS service properly (#780)
1111d04ad is described below
commit 1111d04ad7f15627b9d034d3460cd7698e88e72f
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Mon Jul 24 11:15:21 2023 +0200
KNOX-2943 - Handling CORE-SETTINGS service properly (#780)
---
.../cm/ClouderaManagerServiceDiscovery.java | 14 ++++++++-----
.../ClouderaManagerServiceDiscoveryMessages.java | 3 +++
.../ClouderaManagerServiceDiscoveryRepository.java | 6 ++++--
...uderaManagerServiceDiscoveryRepositoryTest.java | 24 ++++++++++++++++++++++
4 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
index f503771a5..c03b36938 100644
--- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
+++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
@@ -279,14 +279,14 @@ public class ClouderaManagerServiceDiscovery implements ServiceDiscovery, Cluste
serviceConfig = getServiceConfig(client.getConfig(), servicesResourceApi, service);
}
ApiRoleList roleList = getRoles(client.getConfig(), rolesResourceApi, clusterName, service);
- if (roleList != null) {
+ if (roleList != null && roleList.getItems() != null) {
for (ApiRole role : roleList.getItems()) {
String roleName = role.getName();
log.discoveringServiceRole(roleName, role.getType());
ApiConfigList roleConfig = null;
- /* no reason to check role config for CM service */
- if (!CM_SERVICE_TYPE.equals(service.getType())) {
+ /* no reason to check role config for CM or CORE_SETTINGS services */
+ if (!CM_SERVICE_TYPE.equals(service.getType()) && !CORE_SETTINGS_TYPE.equals(service.getType())) {
roleConfig = getRoleConfig(client.getConfig(), rolesResourceApi, service, role);
}
@@ -381,7 +381,6 @@ public class ClouderaManagerServiceDiscovery implements ServiceDiscovery, Cluste
// no roles in the repository -> query CM
final String serviceName = service.getName();
try {
- log.lookupRolesFromCM();
/* Populate roles for CM Service since they are not discoverable */
if (CM_SERVICE_TYPE.equalsIgnoreCase(serviceName)) {
roles = new ApiRoleList();
@@ -390,11 +389,16 @@ public class ClouderaManagerServiceDiscovery implements ServiceDiscovery, Cluste
cmRole.setType(CM_ROLE_TYPE);
roles.addItemsItem(cmRole);
} else if (!CORE_SETTINGS_TYPE.equalsIgnoreCase(serviceName)) { //CORE_SETTINGS has no roles, it does not make sense to discover them
+ log.lookupRolesFromCM();
roles = rolesResourceApi.readRoles(clusterName, serviceName, "", VIEW_SUMMARY);
+ } else {
+ log.noRoles();
}
// make sure that role is populated in the service discovery repository to avoid subsequent CM calls
- repository.addRoles(serviceDiscoveryConfig, service, roles);
+ if (roles != null && roles.getItems() != null) {
+ repository.addRoles(serviceDiscoveryConfig, service, roles);
+ }
} catch (ApiException e) {
log.failedToAccessServiceRoleConfigs(serviceName, "N/A", clusterName, e);
throw e;
diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java
index 06f6f7ff2..4c1842fa2 100644
--- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java
+++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java
@@ -253,6 +253,9 @@ public interface ClouderaManagerServiceDiscoveryMessages {
@Message(level = MessageLevel.DEBUG, text = "Looking up roles from the configured Cloudera Manager discovery endpoint...")
void lookupRolesFromCM();
+ @Message(level = MessageLevel.DEBUG, text = "No roles to look up for this service.")
+ void noRoles();
+
@Message(level = MessageLevel.DEBUG, text = "Looking up role configuration from the configured Cloudera Manager discovery endpoint...")
void lookupRoleConfigsFromCM();
diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java
index bff8a7b5d..5329cd8ce 100644
--- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java
+++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java
@@ -176,8 +176,10 @@ class ClouderaManagerServiceDiscoveryRepository {
}
public void addRoles(ApiRoleList roles) {
- for (ApiRole role : roles.getItems()) {
- roleConfigsMap.put(role, new ApiConfigList());
+ if (roles != null && roles.getItems() != null) {
+ for (ApiRole role : roles.getItems()) {
+ roleConfigsMap.put(role, new ApiConfigList());
+ }
}
}
diff --git a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java
index 2de186242..b20f93d6f 100644
--- a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java
+++ b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java
@@ -119,6 +119,30 @@ public class ClouderaManagerServiceDiscoveryRepositoryTest {
assertTrue(containsRole(service, roleName));
}
+ @Test
+ public void testAddNullRoles() throws Exception {
+ repository.registerCluster(serviceDiscoveryConfig);
+ final ApiService service = EasyMock.createNiceMock(ApiService.class);
+ EasyMock.expect(service.getName()).andReturn(ClouderaManagerServiceDiscovery.CORE_SETTINGS_TYPE).anyTimes();
+ EasyMock.replay(service);
+ repository.addService(serviceDiscoveryConfig, service);
+ repository.addRoles(serviceDiscoveryConfig, service, null);
+ assertNull(repository.getRoles(serviceDiscoveryConfig, service).getItems());
+ }
+
+ @Test
+ public void testAddNullRoleItems() throws Exception {
+ repository.registerCluster(serviceDiscoveryConfig);
+ final ApiService service = EasyMock.createNiceMock(ApiService.class);
+ EasyMock.expect(service.getName()).andReturn(ClouderaManagerServiceDiscovery.CORE_SETTINGS_TYPE).anyTimes();
+ final ApiRoleList roles = EasyMock.createNiceMock(ApiRoleList.class);
+ EasyMock.expect(roles.getItems()).andReturn(null).anyTimes();
+ EasyMock.replay(service, roles);
+ repository.addService(serviceDiscoveryConfig, service);
+ repository.addRoles(serviceDiscoveryConfig, service, roles);
+ assertNull(repository.getRoles(serviceDiscoveryConfig, service).getItems());
+ }
+
@Test
public void testAddRoleConfig() throws Exception {
final String serviceName = "HDFS-1";