Skip to content

Commit

Permalink
Test failure fix #14235 (#14260)
Browse files Browse the repository at this point in the history
  • Loading branch information
deepthi912 authored Oct 20, 2024
1 parent 7129392 commit 1c41a1e
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.pinot.controller;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.apache.helix.ConfigAccessor;
import org.apache.helix.model.ClusterConfig;
import org.apache.helix.model.HelixConfigScope;
import org.apache.helix.model.InstanceConfig;
import org.apache.helix.model.builder.HelixConfigScopeBuilder;
import org.apache.pinot.common.utils.helix.HelixHelper;
import org.apache.pinot.controller.helix.ControllerTest;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.NetUtils;
import org.apache.pinot.spi.utils.builder.ControllerRequestURLBuilder;
import org.testng.annotations.Test;

import static org.apache.pinot.controller.ControllerConf.CONTROLLER_HOST;
import static org.apache.pinot.controller.ControllerConf.CONTROLLER_PORT;
import static org.apache.pinot.spi.utils.CommonConstants.Controller.CONFIG_OF_INSTANCE_ID;
import static org.apache.pinot.spi.utils.CommonConstants.Helix.CONTROLLER_INSTANCE;
import static org.testng.Assert.*;


/**
* This class tests env variables when starting controller from configs
*/
public class ControllerStarterDynamicEnvTest extends ControllerTest {
private final Map<String, Object> _configOverride = new HashMap<>();

@Override
protected void overrideControllerConf(Map<String, Object> properties) {
//To not test dynamic config env variables through this class. Host is not dynamic for this class config
properties.putAll(_configOverride);
}

@Test
public void testNoVariable()
throws Exception {
_configOverride.clear();
_configOverride.put(CONTROLLER_HOST, "myHost");
_configOverride.put(CONFIG_OF_INSTANCE_ID, "Controller_myInstance");
_configOverride.put(CONTROLLER_PORT, 1234);

startZk();
this.startController();

String instanceId = _controllerStarter.getInstanceId();
assertEquals(instanceId, "Controller_myInstance");
InstanceConfig instanceConfig = HelixHelper.getInstanceConfig(_helixManager, instanceId);
assertEquals(instanceConfig.getInstanceName(), instanceId);
assertEquals(instanceConfig.getHostName(), "myHost");
assertEquals(instanceConfig.getPort(), "1234");
assertEquals(instanceConfig.getTags(), Collections.singleton(CONTROLLER_INSTANCE));

stopController();
stopZk();
}

@Test
public void testOneVariable()
throws Exception {
_configOverride.clear();
_configOverride.put("dynamic.env.config", "controller.host");
_configOverride.put(CONTROLLER_HOST, "HOST");
_configOverride.put(CONFIG_OF_INSTANCE_ID, "Controller_myInstance");
_configOverride.put(CONTROLLER_PORT, 1234);

startZk();
this.startController();

String instanceId = _controllerStarter.getInstanceId();
assertEquals(instanceId, "Controller_myInstance");
InstanceConfig instanceConfig = HelixHelper.getInstanceConfig(_helixManager, instanceId);
assertEquals(instanceConfig.getInstanceName(), instanceId);
assertEquals(instanceConfig.getHostName(), "myHost");
assertEquals(instanceConfig.getPort(), "1234");
assertEquals(instanceConfig.getTags(), Collections.singleton(CONTROLLER_INSTANCE));

stopController();
stopZk();
}

@Test
public void testMultipleVariables()
throws Exception {
_configOverride.clear();
_configOverride.put("dynamic.env.config", "controller.host,controller.port");
_configOverride.put(CONTROLLER_HOST, "HOST");
_configOverride.put(CONTROLLER_PORT, "PORT");
_configOverride.put(CONFIG_OF_INSTANCE_ID, "Controller_myInstance");

startZk();
this.startController();

String instanceId = _controllerStarter.getInstanceId();
assertEquals(instanceId, "Controller_myInstance");
InstanceConfig instanceConfig = HelixHelper.getInstanceConfig(_helixManager, instanceId);
assertEquals(instanceConfig.getInstanceName(), instanceId);
assertEquals(instanceConfig.getHostName(), "myHost");
assertEquals(instanceConfig.getPort(), "1234");
assertEquals(instanceConfig.getTags(), Collections.singleton(CONTROLLER_INSTANCE));

stopController();
stopZk();
}

@Override
public Map<String, Object> getDefaultControllerConfiguration() {
Map<String, Object> properties = new HashMap<>();
properties.put(ControllerConf.ZK_STR, getZkUrl());
properties.put(ControllerConf.HELIX_CLUSTER_NAME, getHelixClusterName());
properties.put("dynamic.env.config", "controller.host,controller.port");
properties.put(CONTROLLER_HOST, "HOST");
properties.put(CONTROLLER_PORT, "PORT");

int controllerPort = NetUtils.findOpenPort(_nextControllerPort);
properties.put(ControllerConf.CONTROLLER_PORT, controllerPort);
if (_controllerPort == 0) {
_controllerPort = controllerPort;
}
_nextControllerPort = controllerPort + 1;
properties.put(ControllerConf.DATA_DIR, DEFAULT_DATA_DIR);
properties.put(ControllerConf.LOCAL_TEMP_DIR, DEFAULT_LOCAL_TEMP_DIR);
// Enable groovy on the controller
properties.put(ControllerConf.DISABLE_GROOVY, false);
properties.put(CommonConstants.CONFIG_OF_TIMEZONE, "UTC");
overrideControllerConf(properties);
return properties;
}

@Override
public void startController(Map<String, Object> properties)
throws Exception {
Map<String, String> envVariables = new HashMap<>();
envVariables.put("HOST", "myHost");
envVariables.put("PORT", "1234");
_controllerStarter = createControllerStarter();
_controllerStarter.init(new PinotConfiguration(properties, envVariables));
_controllerStarter.start();
_controllerConfig = _controllerStarter.getConfig();
_controllerBaseApiUrl = _controllerConfig.generateVipUrl();
_controllerRequestURLBuilder = ControllerRequestURLBuilder.baseUrl(_controllerBaseApiUrl);
_controllerDataDir = _controllerConfig.getDataDir();
_helixResourceManager = _controllerStarter.getHelixResourceManager();
_helixManager = _controllerStarter.getHelixControllerManager();
_helixDataAccessor = _helixManager.getHelixDataAccessor();
ConfigAccessor configAccessor = _helixManager.getConfigAccessor();
// HelixResourceManager is null in Helix only mode, while HelixManager is null in Pinot only mode.
HelixConfigScope scope =
new HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.CLUSTER).forCluster(getHelixClusterName())
.build();
switch (_controllerStarter.getControllerMode()) {
case DUAL:
case PINOT_ONLY:
_helixAdmin = _helixResourceManager.getHelixAdmin();
_propertyStore = _helixResourceManager.getPropertyStore();
// TODO: Enable periodic rebalance per 10 seconds as a temporary work-around for the Helix issue:
// https://github.com/apache/helix/issues/331 and https://github.com/apache/helix/issues/2309.
// Remove this after Helix fixing the issue.
configAccessor.set(scope, ClusterConfig.ClusterConfigProperty.REBALANCE_TIMER_PERIOD.name(), "10000");
break;
case HELIX_ONLY:
_helixAdmin = _helixManager.getClusterManagmentTool();
_propertyStore = _helixManager.getHelixPropertyStore();
break;
default:
break;
}
assertEquals(System.getProperty("user.timezone"), "UTC");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@

public class ControllerTest {
public static final String LOCAL_HOST = "localhost";
public static final String LOCAL_HOST_CONTROLLER = "LC";
public static final String DEFAULT_DATA_DIR = new File(FileUtils.getTempDirectoryPath(),
"test-controller-data-dir" + System.currentTimeMillis()).getAbsolutePath();
public static final String DEFAULT_LOCAL_TEMP_DIR = new File(FileUtils.getTempDirectoryPath(),
Expand Down Expand Up @@ -137,7 +136,6 @@ public class ControllerTest {
protected HelixDataAccessor _helixDataAccessor;
protected HelixAdmin _helixAdmin;
protected ZkHelixPropertyStore<ZNRecord> _propertyStore;
protected Map<String, String> _envVariables = new HashMap<>();

/**
* Acquire the {@link ControllerTest} default instance that can be shared across different test cases.
Expand Down Expand Up @@ -166,11 +164,6 @@ public static HttpClient getHttpClient() {
return _httpClient;
}

public Map<String, String> getEnvVariables() {
_envVariables.putIfAbsent("LC", "localhost");
return _envVariables;
}

/**
* ControllerRequestClient is lazy evaluated, static object, only instantiate when first use.
*
Expand Down Expand Up @@ -216,8 +209,7 @@ public Map<String, Object> getDefaultControllerConfiguration() {
Map<String, Object> properties = new HashMap<>();
properties.put(ControllerConf.ZK_STR, getZkUrl());
properties.put(ControllerConf.HELIX_CLUSTER_NAME, getHelixClusterName());
properties.put(ControllerConf.CONTROLLER_HOST, LOCAL_HOST_CONTROLLER);
properties.put("dynamic.env.config", ControllerConf.CONTROLLER_HOST);
properties.put(ControllerConf.CONTROLLER_HOST, LOCAL_HOST);
int controllerPort = NetUtils.findOpenPort(_nextControllerPort);
properties.put(ControllerConf.CONTROLLER_PORT, controllerPort);
if (_controllerPort == 0) {
Expand Down Expand Up @@ -255,10 +247,8 @@ public void startController(Map<String, Object> properties)
throws Exception {
assertNull(_controllerStarter, "Controller is already started");
assertTrue(_controllerPort > 0, "Controller port is not assigned");
Map<String, String> envVariables = new HashMap<>();
envVariables.put("LC", "localhost");
_controllerStarter = createControllerStarter();
_controllerStarter.init(new PinotConfiguration(properties, envVariables));
_controllerStarter.init(new PinotConfiguration(properties));
_controllerStarter.start();
_controllerConfig = _controllerStarter.getConfig();
_controllerBaseApiUrl = _controllerConfig.generateVipUrl();
Expand Down Expand Up @@ -978,8 +968,7 @@ public ControllerConf getControllerConfig() {
/**
* Do not override this method as the configuration is shared across all default TestNG group.
*/
public final Map<String, Object> getSharedControllerConfiguration()
throws IOException {
public final Map<String, Object> getSharedControllerConfiguration() {
Map<String, Object> properties = getDefaultControllerConfiguration();

// TODO: move these test specific configs into respective test classes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void testDualModeController()
properties = getDefaultControllerConfiguration();
properties.put(ControllerConf.CONTROLLER_MODE, ControllerConf.ControllerMode.DUAL);
ControllerStarter secondDualModeController = new ControllerStarter();
secondDualModeController.init(new PinotConfiguration(properties, getEnvVariables()));
secondDualModeController.init(new PinotConfiguration(properties));
secondDualModeController.start();
TestUtils
.waitForCondition(aVoid -> secondDualModeController.getHelixResourceManager().getHelixZkManager().isConnected(),
Expand Down Expand Up @@ -169,7 +169,7 @@ public void testDualModeController()
properties.put(ControllerConf.CONTROLLER_MODE, ControllerConf.ControllerMode.DUAL);

ControllerStarter thirdDualModeController = new ControllerStarter();
thirdDualModeController.init(new PinotConfiguration(properties, getEnvVariables()));
thirdDualModeController.init(new PinotConfiguration(properties));
thirdDualModeController.start();
PinotHelixResourceManager pinotHelixResourceManager = thirdDualModeController.getHelixResourceManager();
_helixManager = pinotHelixResourceManager.getHelixZkManager();
Expand Down Expand Up @@ -210,7 +210,7 @@ public void testPinotOnlyController()
properties.put(ControllerConf.CONTROLLER_MODE, ControllerConf.ControllerMode.PINOT_ONLY);

ControllerStarter firstPinotOnlyController = new ControllerStarter();
firstPinotOnlyController.init(new PinotConfiguration(properties, getEnvVariables()));
firstPinotOnlyController.init(new PinotConfiguration(properties));

// Starting Pinot-only controller without Helix controller should fail
try {
Expand All @@ -225,7 +225,7 @@ public void testPinotOnlyController()
properties.put(ControllerConf.CONTROLLER_MODE, ControllerConf.ControllerMode.HELIX_ONLY);

ControllerStarter helixOnlyController = new ControllerStarter();
helixOnlyController.init(new PinotConfiguration(properties, getEnvVariables()));
helixOnlyController.init(new PinotConfiguration(properties));
helixOnlyController.start();
HelixManager helixControllerManager = helixOnlyController.getHelixControllerManager();
HelixAdmin helixAdmin = helixControllerManager.getClusterManagmentTool();
Expand All @@ -252,7 +252,7 @@ public void testPinotOnlyController()
properties.put(ControllerConf.CONTROLLER_MODE, ControllerConf.ControllerMode.PINOT_ONLY);

ControllerStarter secondPinotOnlyController = new ControllerStarter();
secondPinotOnlyController.init(new PinotConfiguration(properties, getEnvVariables()));
secondPinotOnlyController.init(new PinotConfiguration(properties));
secondPinotOnlyController.start();
TestUtils.waitForCondition(
aVoid -> secondPinotOnlyController.getHelixResourceManager().getHelixZkManager().isConnected(), TIMEOUT_IN_MS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void testControllerLeaderLocator()
// Use custom instance id
properties.put(Controller.CONFIG_OF_INSTANCE_ID, "Controller_myInstance");
ControllerStarter secondControllerStarter = new ControllerStarter();
secondControllerStarter.init(new PinotConfiguration(properties, getEnvVariables()));
secondControllerStarter.init(new PinotConfiguration(properties));
secondControllerStarter.start();

TestUtils
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,15 @@ public static List<Configuration> applyDynamicEnvConfig(List<Configuration> conf
return configurations.stream().peek(configuration -> {
if (!configuration.getBoolean(TEMPLATED_KEY, false)) {
for (String dynamicEnvConfigVarName : configuration.getStringArray(ENV_DYNAMIC_CONFIG_KEY)) {
configuration.setProperty(dynamicEnvConfigVarName,
environmentVariables.get(configuration.getString(dynamicEnvConfigVarName)));
LOGGER.info("The environment variable is set to the property {} through dynamic configs",
dynamicEnvConfigVarName);
String envVariable = configuration.getString(dynamicEnvConfigVarName);
Object envVarValue = environmentVariables.get(envVariable);
if (envVarValue != null) {
configuration.setProperty(dynamicEnvConfigVarName, envVarValue);
LOGGER.info("The environment variable is set to the property {} through dynamic configs",
dynamicEnvConfigVarName);
} else {
LOGGER.error("The environment variable {} is not found", envVariable);
}
}
configuration.setProperty(TEMPLATED_KEY, true);
}
Expand Down

0 comments on commit 1c41a1e

Please sign in to comment.