Skip to content

Commit

Permalink
Merge pull request #32307 from vespa-engine/hmusum/move-away-from-clo…
Browse files Browse the repository at this point in the history
…udconfig-1

Move away from CloudConfig
  • Loading branch information
Harald Musum authored Aug 31, 2024
2 parents e8a5e1a + ebb4ceb commit 4b81664
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import com.yahoo.net.HostName;
import com.yahoo.vespa.defaults.Defaults;
import com.yahoo.vespa.model.container.ContainerCluster;
import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions.ConfigServer;
import com.yahoo.vespa.model.container.configserver.option.ConfigOptions;
import com.yahoo.vespa.model.container.configserver.option.ConfigOptions.ConfigServer;

import java.util.Optional;
import java.util.stream.IntStream;
Expand All @@ -33,10 +33,10 @@ public class ConfigserverCluster extends TreeConfigProducer
VipStatusConfig.Producer,
ZookeeperServerConfig.Producer {

private final CloudConfigOptions options;
private final ConfigOptions options;
private ContainerCluster<?> containerCluster;

public ConfigserverCluster(TreeConfigProducer<?> parent, String subId, CloudConfigOptions options) {
public ConfigserverCluster(TreeConfigProducer<?> parent, String subId, ConfigOptions options) {
super(parent, subId);
this.options = options;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/**
* @author Tony Vaagenes
*/
public interface CloudConfigOptions {
public interface ConfigOptions {

class ConfigServer {
public final String hostName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import com.yahoo.vespa.model.container.ContainerModel;
import com.yahoo.vespa.model.container.component.ConnectionLogComponent;
import com.yahoo.vespa.model.container.configserver.ConfigserverCluster;
import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
import com.yahoo.vespa.model.container.configserver.option.ConfigOptions;
import org.w3c.dom.Element;

/**
Expand All @@ -18,9 +18,9 @@
*/
public class ConfigServerContainerModelBuilder extends ContainerModelBuilder {

private final CloudConfigOptions options;
private final ConfigOptions options;

public ConfigServerContainerModelBuilder(CloudConfigOptions options) {
public ConfigServerContainerModelBuilder(ConfigOptions options) {
super(true, Networking.enable);
this.options = options;
}
Expand All @@ -33,7 +33,7 @@ public void doBuild(ContainerModel model, Element spec, ConfigModelContext model
cluster.setContainerCluster(model.getCluster());
}

// Need to override this method since we need to use the values in CloudConfigOptions (the ones
// Need to override this method since we need to use the values in ConfigOptions (the ones
// in ConfigModelContext.DeployState.properties are not set)
@Override
protected void addStatusHandlers(ApplicationContainerCluster cluster, boolean isHostedVespa) {
Expand All @@ -56,6 +56,6 @@ protected void addModelEvaluationRuntime(ApplicationContainerCluster cluster) {
// Model evaluation bundles are pre-installed in the standalone container.
}

/** Note: using {@link CloudConfigOptions} as {@link DeployState#isHosted()} returns <em>false</em> for hosted configserver/controller */
/** Note: using {@link ConfigOptions} as {@link DeployState#isHosted()} returns <em>false</em> for hosted configserver/controller */
private boolean isHosted() { return options.hostedVespa().orElse(Boolean.FALSE); }
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,6 @@ void testBasePorts() {
assertTrue(vespaModel.getConfig(StandardConfig.class, "simple/simpleservice.1").baseport() != 10000);
}

/**
* This test is the same as the system test cloudconfig/plugins.
* Be sure to update it as well if you change this.
*/
@Test
void testPlugins() {
VespaModel vespaModel = getVespaModelDoNotValidateXml(TESTDIR + "plugins");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import com.yahoo.vespa.model.container.Container;
import com.yahoo.vespa.model.container.ContainerModel;
import com.yahoo.vespa.model.container.ContainerModelEvaluation;
import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
import com.yahoo.vespa.model.container.configserver.option.ConfigOptions;
import com.yahoo.vespa.model.container.xml.ConfigServerContainerModelBuilder;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -153,8 +153,8 @@ private static TestOptions createTestOptions(List<String> configServerHostnames,
Optional.of(configServerHostnames)
.filter(hostnames -> !hostnames.isEmpty())
.map(hostnames -> hostnames.stream()
.map(hostname -> new CloudConfigOptions.ConfigServer(hostname, Optional.empty()))
.toArray(CloudConfigOptions.ConfigServer[]::new))
.map(hostname -> new ConfigOptions.ConfigServer(hostname, Optional.empty()))
.toArray(ConfigOptions.ConfigServer[]::new))
.ifPresent(testOptions::configServers);

Optional.of(configServerZkIds)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.model.container.configserver;

import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
import com.yahoo.vespa.model.container.configserver.option.ConfigOptions;

import java.util.Optional;

/**
* @author Ulf Lilleengen
*/
public class TestOptions implements CloudConfigOptions {
public class TestOptions implements ConfigOptions {

private ConfigServer[] configServers = new ConfigServer[0];
private int[] configServerZookeeperIds = new int[0];
Expand Down
2 changes: 0 additions & 2 deletions config/src/vespa/config/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
/.depend
/Makefile
/libconfig.so.5.1
/libcloudconfig.so.5.1
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.container.standalone;

import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
import com.yahoo.vespa.model.container.configserver.option.ConfigOptions;

import java.util.Arrays;
import java.util.Optional;
Expand All @@ -11,7 +11,7 @@
/**
* @author bjorncs
*/
public class CloudConfigInstallVariables implements CloudConfigOptions {
public class ConfigEnvironmentVariables implements ConfigOptions {

@Override
public Optional<Integer> rpcPort() {
Expand All @@ -31,14 +31,14 @@ public Optional<Boolean> multiTenant() {
public ConfigServer[] allConfigServers() {
return Optional.ofNullable(System.getenv("VESPA_CONFIGSERVERS"))
.or(() -> getRawInstallVariable("services.addr_configserver"))
.map(CloudConfigInstallVariables::toConfigServers)
.map(ConfigEnvironmentVariables::toConfigServers)
.orElseGet(() -> new ConfigServer[0]);
}

@Override
public int[] configServerZookeeperIds() {
return Optional.ofNullable(System.getenv("VESPA_CONFIGSERVER_ZOOKEEPER_IDS"))
.map(CloudConfigInstallVariables::multiValueParameterStream)
.map(ConfigEnvironmentVariables::multiValueParameterStream)
.orElseGet(Stream::empty)
.mapToInt(Integer::valueOf)
.toArray();
Expand Down Expand Up @@ -66,7 +66,7 @@ public Optional<Long> sessionLifeTimeSecs() {
@Override
public String[] configModelPluginDirs() {
return getRawInstallVariable("cloudconfig_server.config_model_plugin_dirs")
.map(CloudConfigInstallVariables::toConfigModelsPluginDir)
.map(ConfigEnvironmentVariables::toConfigModelsPluginDir)
.orElseGet(() -> new String[0]);
}

Expand Down Expand Up @@ -143,7 +143,7 @@ public String zooKeeperSnapshotMethod() {

static ConfigServer[] toConfigServers(String configserversString) {
return multiValueParameterStream(configserversString)
.map(CloudConfigInstallVariables::toConfigServer)
.map(ConfigEnvironmentVariables::toConfigServer)
.toArray(ConfigServer[]::new);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ private static void validateApplication(ApplicationPackage applicationPackage) {

private static ContainerModelBuilder newContainerModelBuilder(Networking networkingOption) {
return isConfigServer() ?
new ConfigServerContainerModelBuilder(new CloudConfigInstallVariables()) :
new ConfigServerContainerModelBuilder(new ConfigEnvironmentVariables()) :
new ContainerModelBuilder(true, networkingOption);
}

Expand Down Expand Up @@ -266,13 +266,13 @@ private static Zone getZone() {
if (!isConfigServer()) {
return Zone.defaultZone();
}
CloudConfigInstallVariables cloudConfigVariables = new CloudConfigInstallVariables();
if (!cloudConfigVariables.hostedVespa().orElse(false)) {
ConfigEnvironmentVariables variables = new ConfigEnvironmentVariables();
if (!variables.hostedVespa().orElse(false)) {
return Zone.defaultZone();
}
RegionName region = cloudConfigVariables.region().map(RegionName::from).orElseGet(RegionName::defaultName);
Environment environment = cloudConfigVariables.environment().map(Environment::from).orElseGet(Environment::defaultEnvironment);
SystemName system = cloudConfigVariables.system().map(SystemName::from).orElseGet(SystemName::defaultSystem);
RegionName region = variables.region().map(RegionName::from).orElseGet(RegionName::defaultName);
Environment environment = variables.environment().map(Environment::from).orElseGet(Environment::defaultEnvironment);
SystemName system = variables.system().map(SystemName::from).orElseGet(SystemName::defaultSystem);
return new Zone(system, environment, region);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,40 +1,40 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.container.standalone;

import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
import com.yahoo.vespa.model.container.configserver.option.ConfigOptions;
import org.junit.Test;

import java.util.Arrays;
import java.util.List;

import static com.yahoo.container.standalone.CloudConfigInstallVariables.toConfigModelsPluginDir;
import static com.yahoo.container.standalone.CloudConfigInstallVariables.toConfigServer;
import static com.yahoo.container.standalone.CloudConfigInstallVariables.toConfigServers;
import static com.yahoo.container.standalone.ConfigEnvironmentVariables.toConfigModelsPluginDir;
import static com.yahoo.container.standalone.ConfigEnvironmentVariables.toConfigServer;
import static com.yahoo.container.standalone.ConfigEnvironmentVariables.toConfigServers;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

/**
* @author Ulf Lilleengen
* @author Tony Vaagenes
*/
public class CloudConfigInstallVariablesTest {
public class ConfigEnvironmentVariablesTest {

@Test
public void test_configserver_parsing() {
CloudConfigOptions.ConfigServer[] parsed = toConfigServers("myhost.mydomain.com");
ConfigOptions.ConfigServer[] parsed = toConfigServers("myhost.mydomain.com");
assertEquals(1, parsed.length);
}

@Test
public void port_can_be_configured() {
CloudConfigOptions.ConfigServer[] parsed = toConfigServers("myhost:123");
ConfigOptions.ConfigServer[] parsed = toConfigServers("myhost:123");
int port = parsed[0].port.get();
assertEquals(123, port);
}

@Test
public void multiple_spaces_are_supported() {
CloudConfigOptions.ConfigServer[] parsed = toConfigServers("test1 test2");
ConfigOptions.ConfigServer[] parsed = toConfigServers("test1 test2");
assertEquals(2, parsed.length);

List<String> hostNames = Arrays.stream(parsed).map(cs -> cs.hostName).toList();
Expand Down

0 comments on commit 4b81664

Please sign in to comment.