Skip to content

Commit 0cd9b9e

Browse files
authored
[controller] Fix bug where setting the cloud config was only done on the controller cluster, not on storage clusters (#1231)
Fix bug where setting the cloud config was only done on the controller cluster, not on storage clusters. This was done by adding clusterName to the method signature of setCloudConfig.
1 parent 5cc32a7 commit 0cd9b9e

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

services/venice-controller/src/main/java/com/linkedin/venice/controller/ZkHelixAdminClient.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public void createVeniceControllerCluster() {
103103
helixAdmin.addStateModelDef(controllerClusterName, LeaderStandbySMD.name, LeaderStandbySMD.build());
104104

105105
if (commonConfig.isControllerClusterHelixCloudEnabled()) {
106-
setCloudConfig(commonConfig);
106+
setCloudConfig(controllerClusterName, commonConfig);
107107
}
108108
}
109109
return true;
@@ -130,7 +130,7 @@ public void createVeniceStorageCluster(String clusterName, Map<String, String> h
130130

131131
VeniceControllerClusterConfig config = multiClusterConfigs.getControllerConfig(clusterName);
132132
if (config.isStorageClusterHelixCloudEnabled()) {
133-
setCloudConfig(config);
133+
setCloudConfig(clusterName, config);
134134
}
135135
}
136136
return true;
@@ -327,7 +327,7 @@ public void addInstanceTag(String clusterName, String instanceName, String tag)
327327
helixAdmin.addInstanceTag(clusterName, instanceName, tag);
328328
}
329329

330-
public void setCloudConfig(VeniceControllerClusterConfig config) {
330+
public void setCloudConfig(String clusterName, VeniceControllerClusterConfig config) {
331331
String cloudId = config.getHelixCloudId();
332332
List<String> cloudInfoSources = config.getHelixCloudInfoSources();
333333
String cloudInfoProcessorName = config.getHelixCloudInfoProcessorName();
@@ -345,6 +345,6 @@ public void setCloudConfig(VeniceControllerClusterConfig config) {
345345
if (!cloudInfoProcessorName.isEmpty()) {
346346
cloudConfigBuilder.setCloudInfoProcessorName(cloudInfoProcessorName);
347347
}
348-
helixAdmin.addCloudConfig(controllerClusterName, cloudConfigBuilder.build());
348+
helixAdmin.addCloudConfig(clusterName, cloudConfigBuilder.build());
349349
}
350350
}

services/venice-controller/src/test/java/com/linkedin/venice/controller/TestZkHelixAdminClient.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ public class TestZkHelixAdminClient {
2828
private VeniceControllerMultiClusterConfig mockMultiClusterConfigs;
2929
private VeniceControllerClusterConfig mockClusterConfig;
3030

31+
private static final String controllerClusterName = "venice-controllers";
32+
3133
@BeforeMethod
3234
public void setUp() throws NoSuchFieldException, IllegalAccessException {
3335
zkHelixAdminClient = mock(ZkHelixAdminClient.class);
@@ -78,8 +80,8 @@ public void testSetCloudConfig() {
7880
when(mockClusterConfig.getHelixCloudInfoSources()).thenReturn(cloudInfoSources);
7981
when(mockClusterConfig.getHelixCloudInfoProcessorName()).thenReturn("TestProcessor");
8082

81-
doCallRealMethod().when(zkHelixAdminClient).setCloudConfig(mockClusterConfig);
82-
zkHelixAdminClient.setCloudConfig(mockClusterConfig);
83+
doCallRealMethod().when(zkHelixAdminClient).setCloudConfig(controllerClusterName, mockClusterConfig);
84+
zkHelixAdminClient.setCloudConfig(controllerClusterName, mockClusterConfig);
8385

8486
verify(mockHelixAdmin).addCloudConfig(any(), any());
8587
}
@@ -92,8 +94,10 @@ public void testControllerCloudInfoSourcesNotSet() {
9294
when(mockClusterConfig.getHelixCloudInfoSources()).thenReturn(Collections.emptyList());
9395
when(mockClusterConfig.getHelixCloudInfoProcessorName()).thenReturn("TestProcessor");
9496

95-
doCallRealMethod().when(zkHelixAdminClient).setCloudConfig(mockClusterConfig);
96-
assertThrows(HelixException.class, () -> zkHelixAdminClient.setCloudConfig(mockClusterConfig));
97+
doCallRealMethod().when(zkHelixAdminClient).setCloudConfig(controllerClusterName, mockClusterConfig);
98+
assertThrows(
99+
HelixException.class,
100+
() -> zkHelixAdminClient.setCloudConfig(controllerClusterName, mockClusterConfig));
97101
}
98102

99103
@Test
@@ -107,7 +111,9 @@ public void testControllerCloudInfoProcessorNameNotSet() {
107111
when(mockClusterConfig.getHelixCloudInfoSources()).thenReturn(cloudInfoSources);
108112
when(mockClusterConfig.getHelixCloudInfoProcessorName()).thenReturn("");
109113

110-
doCallRealMethod().when(zkHelixAdminClient).setCloudConfig(mockClusterConfig);
111-
assertThrows(HelixException.class, () -> zkHelixAdminClient.setCloudConfig(mockClusterConfig));
114+
doCallRealMethod().when(zkHelixAdminClient).setCloudConfig(controllerClusterName, mockClusterConfig);
115+
assertThrows(
116+
HelixException.class,
117+
() -> zkHelixAdminClient.setCloudConfig(controllerClusterName, mockClusterConfig));
112118
}
113119
}

0 commit comments

Comments
 (0)