diff options
53 files changed, 542 insertions, 240 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index bb42eb6da26..a53b0931cc6 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -7,7 +7,6 @@ import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.FileRegistry; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.Rotation; import com.yahoo.config.provision.Zone; import java.io.File; @@ -49,7 +48,6 @@ public interface ModelContext { String athenzDnsSuffix(); boolean hostedVespa(); Zone zone(); - default Set<Rotation> rotations() { return Set.of(); } // TODO(mpolden): Remove once 7.122 is the oldest config model Set<ContainerEndpoint> endpoints(); boolean isBootstrap(); boolean isFirstTimeDeployment(); diff --git a/config-model/CMakeLists.txt b/config-model/CMakeLists.txt index 309363d5be1..e5f8c388613 100644 --- a/config-model/CMakeLists.txt +++ b/config-model/CMakeLists.txt @@ -6,6 +6,6 @@ vespa_install_script(src/main/perl/vespa-expand-config.pl bin) vespa_install_script(src/main/sh/vespa-validate-application bin) install(DIRECTORY src/main/resources/schema DESTINATION share/vespa PATTERN ".gitignore" EXCLUDE) -install(DIRECTORY src/main/resources/schema DESTINATION share/vespa/schema/version/6.x PATTERN ".gitignore" EXCLUDE) +install(DIRECTORY src/main/resources/schema DESTINATION share/vespa/schema/version/7.x PATTERN ".gitignore" EXCLUDE) install_java_artifact(jing) diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 0f43be96e73..b286b94c699 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -22,7 +22,6 @@ import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.model.provision.HostsXmlProvisioner; import com.yahoo.config.model.provision.SingleNodeProvisioner; import com.yahoo.config.model.test.MockApplicationPackage; -import com.yahoo.config.provision.Rotation; import com.yahoo.config.provision.Zone; import com.yahoo.io.IOUtils; import com.yahoo.io.reader.NamedReader; @@ -47,7 +46,6 @@ import java.io.UncheckedIOException; import java.time.Instant; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index 61cbc7da52d..91907af75c4 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -592,6 +592,7 @@ "public static com.yahoo.config.provision.NodeResources$DiskSpeed[] values()", "public static com.yahoo.config.provision.NodeResources$DiskSpeed valueOf(java.lang.String)", "public static int compare(com.yahoo.config.provision.NodeResources$DiskSpeed, com.yahoo.config.provision.NodeResources$DiskSpeed)", + "public boolean compatibleWith(com.yahoo.config.provision.NodeResources$DiskSpeed)", "public boolean isDefault()", "public static com.yahoo.config.provision.NodeResources$DiskSpeed getDefault()" ], @@ -613,6 +614,7 @@ "public static com.yahoo.config.provision.NodeResources$StorageType[] values()", "public static com.yahoo.config.provision.NodeResources$StorageType valueOf(java.lang.String)", "public static int compare(com.yahoo.config.provision.NodeResources$StorageType, com.yahoo.config.provision.NodeResources$StorageType)", + "public boolean compatibleWith(com.yahoo.config.provision.NodeResources$StorageType)", "public boolean isDefault()", "public static com.yahoo.config.provision.NodeResources$StorageType getDefault()" ], @@ -757,21 +759,6 @@ ], "fields": [] }, - "com.yahoo.config.provision.Rotation": { - "superClass": "java.lang.Object", - "interfaces": [], - "attributes": [ - "public" - ], - "methods": [ - "public void <init>(java.lang.String)", - "public java.lang.String getId()", - "public boolean equals(java.lang.Object)", - "public int hashCode()", - "public java.lang.String toString()" - ], - "fields": [] - }, "com.yahoo.config.provision.SystemName": { "superClass": "java.lang.Enum", "interfaces": [], diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index 7769060dc60..ce3dd579d2f 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -29,6 +29,10 @@ public class NodeResources { return 0; } + public boolean compatibleWith(DiskSpeed other) { + return this == any || other == any || other == this; + } + private DiskSpeed combineWith(DiskSpeed other) { if (this == any) return other; if (other == any) return this; @@ -60,6 +64,10 @@ public class NodeResources { return 0; } + public boolean compatibleWith(StorageType other) { + return this == any || other == any || other == this; + } + private StorageType combineWith(StorageType other) { if (this == any) return other; if (other == any) return this; @@ -218,8 +226,8 @@ public class NodeResources { if (this.memoryGb != other.memoryGb) return false; if (this.diskGb != other.diskGb) return false; if (this.bandwidthGbps != other.bandwidthGbps) return false; - if (other.diskSpeed != DiskSpeed.any && other.diskSpeed != this.diskSpeed) return false; - if (other.storageType != StorageType.any && other.storageType != this.storageType) return false; + if ( ! this.diskSpeed.compatibleWith(other.diskSpeed)) return false; + if ( ! this.storageType.compatibleWith(other.storageType)) return false; return true; } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Rotation.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Rotation.java deleted file mode 100644 index d1cf86c6d16..00000000000 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Rotation.java +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.config.provision; - -import java.util.Objects; - -/** - * A rotation (virtual endpoint). - */ -// TODO(mpolden): Remove once 7.122 is the oldest config model -public class Rotation { - - private final String id; - - public Rotation(String id) { - this.id = Objects.requireNonNull(id, "Rotation id cannot be null"); - } - - public String getId() { - return id; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof Rotation)) { - return false; - } - final Rotation that = (Rotation) o; - return (this.id.equals(that.id)); - } - - @Override - public int hashCode() { - return id.hashCode(); - } - - @Override - public String toString() { - return "rotation '" + id + "'"; - } - -} diff --git a/configd/src/apps/sentinel/config-handler.cpp b/configd/src/apps/sentinel/config-handler.cpp index a73a5f01f33..7b68e6c8150 100644 --- a/configd/src/apps/sentinel/config-handler.cpp +++ b/configd/src/apps/sentinel/config-handler.cpp @@ -5,6 +5,8 @@ #include <vespa/vespalib/net/simple_metric_snapshot.h> #include <vespa/vespalib/net/socket_address.h> +#include <vespa/vespalib/util/exceptions.h> +#include <string> #include <fcntl.h> #include <sys/wait.h> @@ -24,9 +26,7 @@ ConfigHandler::configure_port(int port) } } if (port <= 0 || port > 65535) { - LOG(error, "Fatal: bad port %d, expected range [1,65535]", port); - EV_STOPPING("config-sentinel", "bad port"); - exit(EXIT_FAILURE); + throw vespalib::FatalException("Bad port " + std::to_string(port) + ", expected range [1, 65535]", VESPA_STRLOC); } if (port != _boundPort) { LOG(debug, "Config-sentinel accepts connections on port %d", port); diff --git a/configd/src/apps/sentinel/sentinel.cpp b/configd/src/apps/sentinel/sentinel.cpp index 023b96ce96c..126c59d42f3 100644 --- a/configd/src/apps/sentinel/sentinel.cpp +++ b/configd/src/apps/sentinel/sentinel.cpp @@ -5,6 +5,7 @@ #include <unistd.h> #include <sys/time.h> #include <vespa/vespalib/util/signalhandler.h> +#include <vespa/vespalib/util/exceptions.h> #include <vespa/defaults.h> #include "config-handler.h" @@ -85,8 +86,15 @@ main(int argc, char **argv) vespalib::SignalHandler::CHLD.clear(); handler.doWork(); // Check for child procs & commands } catch (InvalidConfigException& ex) { - LOG(warning, "Configuration problem: (ignoring): %s", - ex.what()); + LOG(warning, "Configuration problem: (ignoring): %s", ex.what()); + } catch (vespalib::PortListenException& ex) { + LOG(error, "Fatal: %s", ex.getMessage().c_str()); + EV_STOPPING("config-sentinel", ex.what()); + exit(EXIT_FAILURE); + } catch (vespalib::FatalException& ex) { + LOG(error, "Fatal: %s", ex.getMessage().c_str()); + EV_STOPPING("config-sentinel", ex.what()); + exit(EXIT_FAILURE); } if (vespalib::SignalHandler::CHLD.check()) { continue; diff --git a/configdefinitions/src/vespa/zookeeper-server.def b/configdefinitions/src/vespa/zookeeper-server.def index 55be9d2ae28..90dec3a4f4e 100644 --- a/configdefinitions/src/vespa/zookeeper-server.def +++ b/configdefinitions/src/vespa/zookeeper-server.def @@ -17,10 +17,6 @@ dataDir string default="var/zookeeper" clientPort int default=2181 -# In the hosted Vespa prod.us-east-3 zone, a snapshotCount of 50000 corresponds -# to about 5 transaction log snapshots per hour. Assuming this is a fairly -# normal zone, a snapRetainCount of 15 gives 3-4 hours of logs before they're -# purged. snapshotCount int default=50000 # Purge interval in hours autopurge.purgeInterval int default=1 @@ -41,3 +37,6 @@ server[].electionPort int default=2183 # and in general where there is a zookeeper ensemble running that has had few transactions. # TODO: Consider setting this to false by default (and override where appropriate) trustEmptySnapshot bool default=true + +# TLS options +tlsForQuorumCommunication enum { OFF, PORT_UNIFICATION, TLS_WITH_PORT_UNIFICATION, TLS_ONLY } default=OFF diff --git a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java index 911cd5ffd02..6662ba85e5d 100644 --- a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java +++ b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java @@ -154,7 +154,7 @@ public class ComponentNode extends Node { } catch (InvocationTargetException | InstantiationException | IllegalAccessException e) { StackTraceElement dependencyInjectorMarker = new StackTraceElement("============= Dependency Injection =============", "newInstance", null, -1); - throw removeStackTrace(new ComponentConstructorException("Error constructing " + idAndType(), cutStackTraceAtConstructor(e.getCause(), dependencyInjectorMarker))); + throw removeStackTrace(new ComponentConstructorException("Error constructing " + idAndType() + ": " + e.getMessage(), cutStackTraceAtConstructor(e.getCause(), dependencyInjectorMarker))); } return initId(instance); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index 9db896bbb88..bf89d072b75 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -181,7 +181,11 @@ enum PathGroup { "/zone/v1/{*}"), /** Paths used for deploying system-wide feature flags. */ - systemFlags("/system-flags/v1/{*}"); + systemFlagsDeploy("/system-flags/v1/deploy"), + + + /** Paths used for "dry-running" system-wide feature flags. */ + systemFlagsDryrun("/system-flags/v1/dryrun"); final List<String> pathSpecs; final String prefix; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java index 51f29626acf..074d3ef7e95 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java @@ -123,9 +123,14 @@ enum Policy { .on(PathGroup.publicInfo) .in(SystemName.all())), - /** Access to /system-flags/v1. */ - systemFlagsDeployment(Privilege.grant(Action.all()) - .on(PathGroup.systemFlags) + /** Access to /system-flags/v1/deploy. */ + systemFlagsDeploy(Privilege.grant(Action.update) + .on(PathGroup.systemFlagsDeploy) + .in(SystemName.all())), + + /** Access to /system-flags/v1/dryrun. */ + systemFlagsDryrun(Privilege.grant(Action.update) + .on(PathGroup.systemFlagsDryrun) .in(SystemName.all())); private final Set<Privilege> privileges; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java index e1497bd686e..b53cf9162e7 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java @@ -111,6 +111,9 @@ public abstract class Role { /** Returns the role for system flag deployer */ public static UnboundRole systemFlagsDeployer() { return new UnboundRole(RoleDefinition.systemFlagsDeployer); } + /** Returns the role for system flag dryrun */ + public static UnboundRole systemFlagsDryrunner() { return new UnboundRole(RoleDefinition.systemFlagsDryrunner); } + /** Returns the role definition of this bound role. */ public RoleDefinition definition() { return roleDefinition; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java index a261f5c7e8f..67efdc3017d 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java @@ -115,7 +115,9 @@ public enum RoleDefinition { Policy.keyManagement, Policy.developmentDeployment), - systemFlagsDeployer(hostedOperator, Policy.systemFlagsDeployment); + systemFlagsDeployer(Policy.systemFlagsDeploy, Policy.systemFlagsDryrun), + + systemFlagsDryrunner(Policy.systemFlagsDryrun); private final Set<RoleDefinition> parents; private final Set<Policy> policies; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index ec14dcb7123..83e3c03ffaa 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -62,6 +62,10 @@ public class SystemFlagsDataArchive { public static SystemFlagsDataArchive fromDirectory(Path directory) { Path root = directory.toAbsolutePath(); + Path flagsDirectory = directory.resolve("flags"); + if (!Files.isDirectory(flagsDirectory)) { + throw new IllegalArgumentException("Sub-directory 'flags' does not exist: " + flagsDirectory); + } try (Stream<Path> directoryStream = Files.walk(root)) { Builder builder = new Builder(); directoryStream.forEach(absolutePath -> { diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java index 6dd815f4f51..d153e218640 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java @@ -129,10 +129,17 @@ public class RoleTest { @Test public void system_flags() { - URI uri = URI.create("/system-flags/v1/deploy"); + URI deployUri = URI.create("/system-flags/v1/deploy"); Action action = Action.update; - assertTrue(mainEnforcer.allows(Role.systemFlagsDeployer(), action, uri)); - assertTrue(mainEnforcer.allows(Role.hostedOperator(), action, uri)); - assertFalse(mainEnforcer.allows(Role.everyone(), action, uri)); + assertTrue(mainEnforcer.allows(Role.systemFlagsDeployer(), action, deployUri)); + assertTrue(mainEnforcer.allows(Role.hostedOperator(), action, deployUri)); + assertFalse(mainEnforcer.allows(Role.systemFlagsDryrunner(), action, deployUri)); + assertFalse(mainEnforcer.allows(Role.everyone(), action, deployUri)); + + URI dryrunUri = URI.create("/system-flags/v1/dryrun"); + assertTrue(mainEnforcer.allows(Role.systemFlagsDeployer(), action, dryrunUri)); + assertTrue(mainEnforcer.allows(Role.hostedOperator(), action, dryrunUri)); + assertTrue(mainEnforcer.allows(Role.systemFlagsDryrunner(), action, dryrunUri)); + assertFalse(mainEnforcer.allows(Role.everyone(), action, dryrunUri)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 21a1b3c1eda..3f985e95a4f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -128,7 +128,6 @@ public class ApplicationController { private final RoutingPolicies routingPolicies; private final Clock clock; private final DeploymentTrigger deploymentTrigger; - private final BooleanFlag provisionApplicationCertificate; private final ApplicationPackageValidator applicationPackageValidator; ApplicationController(Controller controller, CuratorDb curator, @@ -146,7 +145,6 @@ public class ApplicationController { routingPolicies = new RoutingPolicies(controller); rotationRepository = new RotationRepository(rotationsConfig, this, curator); deploymentTrigger = new DeploymentTrigger(controller, clock); - provisionApplicationCertificate = Flags.PROVISION_APPLICATION_CERTIFICATE.bindTo(controller.flagSource()); applicationPackageValidator = new ApplicationPackageValidator(controller); // Update serialization format of all applications @@ -565,12 +563,6 @@ public class ApplicationController { } private Optional<ApplicationCertificate> getApplicationCertificate(Instance instance) { - boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, - instance.id().serializedForm()).value(); - if (!provisionCertificate) { - return Optional.empty(); - } - // Re-use certificate if already provisioned Optional<ApplicationCertificate> applicationCertificate = curator.readApplicationCertificate(instance.id()); if(applicationCertificate.isPresent()) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java index 8f84845a94b..bb6777b9e27 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzFacade.java @@ -208,8 +208,8 @@ public class AthenzFacade implements AccessControl { return hasAccess("launch", service.getDomain().getName() + ":service."+service.getName(), principal); } - public boolean hasSystemFlagsDeployAccess(AthenzIdentity identity) { - return hasAccess("deploy", new AthenzResourceName(service.getDomain(), "system-flags").toResourceNameString(), identity); + public boolean hasSystemFlagsAccess(AthenzIdentity identity, boolean dryRun) { + return hasAccess(dryRun ? "dryrun" : "deploy", new AthenzResourceName(service.getDomain(), "system-flags").toResourceNameString(), identity); } /** diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java index 2a75c7953ca..56b2de33478 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java @@ -101,10 +101,14 @@ public class AthenzRoleFilter extends JsonSecurityRequestFilterBase { && instance.get().value().equals(principal.getIdentity().getName())) roleMemberships.add(Role.athenzUser(tenant.get().name(), application.get(), instance.get())); - if (athenz.hasSystemFlagsDeployAccess(identity)) { + if (athenz.hasSystemFlagsAccess(identity, /*dryrun*/false)) { roleMemberships.add(Role.systemFlagsDeployer()); } + if (athenz.hasSystemFlagsAccess(identity, /*dryrun*/true)) { + roleMemberships.add(Role.systemFlagsDryrunner()); + } + return roleMemberships.isEmpty() ? Set.of(Role.everyone()) : Set.copyOf(roleMemberships); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java index 08bb7628080..32cb79963c1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java @@ -22,7 +22,7 @@ import java.util.concurrent.Executor; * @author bjorncs */ @SuppressWarnings("unused") // Request handler listed in controller's services.xml -class SystemFlagsHandler extends LoggingRequestHandler { +public class SystemFlagsHandler extends LoggingRequestHandler { private static final String API_PREFIX = "/system-flags/v1"; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index dbe451fd433..52ac9c8088a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -741,7 +741,6 @@ public class ControllerTest { @Test public void testDeploySelectivelyProvisionsCertificate() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.PROVISION_APPLICATION_CERTIFICATE.id(), true); Function<Instance, Optional<ApplicationCertificate>> certificate = (application) -> tester.controller().curator().readApplicationCertificate(application.id()); // Create app1 diff --git a/eval/src/vespa/eval/eval/basic_nodes.h b/eval/src/vespa/eval/eval/basic_nodes.h index 01ac86e4472..52f39510a9b 100644 --- a/eval/src/vespa/eval/eval/basic_nodes.h +++ b/eval/src/vespa/eval/eval/basic_nodes.h @@ -157,7 +157,7 @@ public: str += " in ["; CommaTracker node_list; for (const auto &node: _entries) { - node_list.maybe_comma(str); + node_list.maybe_add_comma(str); str += node->dump(ctx); } str += "])"; diff --git a/eval/src/vespa/eval/eval/function.cpp b/eval/src/vespa/eval/eval/function.cpp index 8b49278a8f0..44bb61abe17 100644 --- a/eval/src/vespa/eval/eval/function.cpp +++ b/eval/src/vespa/eval/eval/function.cpp @@ -463,10 +463,9 @@ void parse_if(ParseContext &ctx) { } void parse_call(ParseContext &ctx, Call_UP call) { + CommaTracker list; for (size_t i = 0; i < call->num_params(); ++i) { - if (i > 0) { - ctx.eat(','); - } + list.maybe_eat_comma(ctx); call->bind_next(get_expression(ctx)); } ctx.push_expression(std::move(call)); @@ -480,10 +479,9 @@ std::vector<vespalib::string> get_ident_list(ParseContext &ctx, bool wrapped) { ctx.skip_spaces(); ctx.eat('('); } + CommaTracker ident_list(wrapped); while (!ctx.find_list_end()) { - if (!list.empty() || !wrapped) { - ctx.eat(','); - } + ident_list.maybe_eat_comma(ctx); list.push_back(get_ident(ctx, false)); } if (wrapped) { @@ -580,10 +578,9 @@ TensorSpec::Address get_tensor_address(ParseContext &ctx, const ValueType &type) TensorSpec::Address addr; ctx.skip_spaces(); ctx.eat('{'); + CommaTracker list; while (!ctx.find_list_end()) { - if (!addr.empty()) { - ctx.eat(','); - } + list.maybe_eat_comma(ctx); auto dim_name = get_ident(ctx, false); size_t dim_idx = type.dimension_index(dim_name); if (dim_idx != ValueType::Dimension::npos) { @@ -617,10 +614,9 @@ void parse_tensor_create_verbose(ParseContext &ctx, const ValueType &type) { ctx.skip_spaces(); ctx.eat('{'); nodes::TensorCreate::Spec create_spec; + CommaTracker list; while (!ctx.find_list_end()) { - if (!create_spec.empty()) { - ctx.eat(','); - } + list.maybe_eat_comma(ctx); auto address = get_tensor_address(ctx, type); ctx.skip_spaces(); ctx.eat(':'); @@ -638,20 +634,18 @@ void parse_tensor_create_convenient(ParseContext &ctx, const ValueType &type, nodes::TensorCreate::Spec create_spec; using Label = TensorSpec::Label; std::vector<Label> addr; + std::vector<CommaTracker> list; for (;;) { if (addr.size() == dim_list.size()) { TensorSpec::Address address; for (size_t i = 0; i < addr.size(); ++i) { - if (addr[i].is_mapped()) { - address.emplace(dim_list[i].name, addr[i]); - } else { - address.emplace(dim_list[i].name, Label(addr[i].index-1)); - } + address.emplace(dim_list[i].name, addr[i]); } create_spec.emplace(std::move(address), get_expression(ctx)); } else { bool mapped = dim_list[addr.size()].is_mapped(); addr.push_back(mapped ? Label("") : Label(size_t(0))); + list.emplace_back(); ctx.skip_spaces(); ctx.eat(mapped ? '{' : '['); } @@ -659,25 +653,23 @@ void parse_tensor_create_convenient(ParseContext &ctx, const ValueType &type, bool mapped = addr.back().is_mapped(); ctx.eat(mapped ? '}' : ']'); addr.pop_back(); + list.pop_back(); if (addr.empty()) { return ctx.push_expression(std::make_unique<nodes::TensorCreate>(type, std::move(create_spec))); } } - if (addr.back().is_mapped()) { - if (addr.back().name != "") { - ctx.eat(','); + if (list.back().maybe_eat_comma(ctx)) { + if (addr.back().is_indexed()) { + if (++addr.back().index >= dim_list[addr.size()-1].size) { + return ctx.fail(make_string("dimension too large: '%s'", + dim_list[addr.size()-1].name.c_str())); + } } + } + if (addr.back().is_mapped()) { addr.back().name = get_ident(ctx, false); ctx.skip_spaces(); ctx.eat(':'); - } else { - if (addr.back().index != 0) { - ctx.eat(','); - } - if (++addr.back().index > dim_list[addr.size()-1].size) { - return ctx.fail(make_string("dimension too large: '%s'", - dim_list[addr.size()-1].name.c_str())); - } } } } @@ -811,11 +803,9 @@ void parse_in(ParseContext &ctx) ctx.skip_spaces(); ctx.eat('['); ctx.skip_spaces(); - size_t size = 0; - while (!ctx.eos() && ctx.get() != ']') { - if (++size > 1) { - ctx.eat(','); - } + CommaTracker list; + while (!ctx.find_list_end()) { + list.maybe_eat_comma(ctx); parse_value(ctx); ctx.skip_spaces(); auto entry = ctx.pop_expression(); diff --git a/eval/src/vespa/eval/eval/string_stuff.cpp b/eval/src/vespa/eval/eval/string_stuff.cpp index 70331a38cca..782aacb5cea 100644 --- a/eval/src/vespa/eval/eval/string_stuff.cpp +++ b/eval/src/vespa/eval/eval/string_stuff.cpp @@ -9,7 +9,7 @@ vespalib::string as_string(const TensorSpec::Address &address) { CommaTracker label_list; vespalib::string str = "{"; for (const auto &label: address) { - label_list.maybe_comma(str); + label_list.maybe_add_comma(str); if (label.second.is_mapped()) { str += make_string("%s:%s", label.first.c_str(), label.second.name.c_str()); } else { diff --git a/eval/src/vespa/eval/eval/string_stuff.h b/eval/src/vespa/eval/eval/string_stuff.h index 2e60691fa28..820196fe959 100644 --- a/eval/src/vespa/eval/eval/string_stuff.h +++ b/eval/src/vespa/eval/eval/string_stuff.h @@ -9,16 +9,30 @@ namespace vespalib::eval { /** * Helper class used to insert commas on the appropriate places in - * comma-separated textual lists. + * comma-separated textual lists. Can also be used to figure out when + * to expect commas when parsing text. **/ struct CommaTracker { bool first; CommaTracker() : first(true) {} - void maybe_comma(vespalib::string &dst) { + CommaTracker(bool first_in) : first(first_in) {} + bool maybe_add_comma(vespalib::string &dst) { if (first) { first = false; + return false; } else { dst.push_back(','); + return true; + } + } + template <typename T> + bool maybe_eat_comma(T &ctx) { + if (first) { + first = false; + return false; + } else { + ctx.eat(','); + return true; } } }; diff --git a/eval/src/vespa/eval/eval/tensor_nodes.h b/eval/src/vespa/eval/eval/tensor_nodes.h index 776275f5884..93c1da4f2a8 100644 --- a/eval/src/vespa/eval/eval/tensor_nodes.h +++ b/eval/src/vespa/eval/eval/tensor_nodes.h @@ -234,7 +234,7 @@ public: str += ":{"; CommaTracker child_list; for (const Child &child: _cells) { - child_list.maybe_comma(str); + child_list.maybe_add_comma(str); str += as_string(child.first); str += ":"; str += child.second->dump(ctx); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 4038fd49b02..64e7b338f91 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -136,19 +136,6 @@ public class Flags { "Takes effect on next iteration of HostProvisionMaintainer.", APPLICATION_ID); - public static final UnboundBooleanFlag PROVISION_APPLICATION_CERTIFICATE = defineFeatureFlag( - "provision-application-certificate", false, - "Provision certificate from CA and include reference in deployment", - "Takes effect on deployment through controller", - APPLICATION_ID); - - public static final UnboundBooleanFlag DIRECT_ROUTING_USE_HTTPS_4443 = defineFeatureFlag( - "direct-routing-use-https-4443", false, - "Decides whether NLB is pointed at container on port 4443 (https) or 4080 (http)", - "Takes effect at redeployment", - APPLICATION_ID - ); - public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( "default-term-wise-limit", 1.0, "Node resource memory in Gb for admin cluster nodes", @@ -162,7 +149,7 @@ public class Flags { HOSTNAME); public static final UnboundStringFlag ZOOKEEPER_SERVER_MAJOR_MINOR_VERSION = defineStringFlag( - "zookeeper-server-version", "3.4", + "zookeeper-server-version", "3.5", "The version of ZooKeeper server to use (major.minor, not full version)", "Takes effect on restart of Docker container", NODE_TYPE, APPLICATION_ID, HOSTNAME); diff --git a/logd/src/logd/state_reporter.cpp b/logd/src/logd/state_reporter.cpp index 713946f43db..02bd7253026 100644 --- a/logd/src/logd/state_reporter.cpp +++ b/logd/src/logd/state_reporter.cpp @@ -13,11 +13,11 @@ using vespalib::metrics::SimpleManagerConfig; StateReporter::StateReporter() : _port(-1), - _server(), _health(), _components(), _metrics(SimpleMetricsManager::create(SimpleManagerConfig())), - _producer(_metrics) + _producer(_metrics), + _server() { } diff --git a/logd/src/logd/state_reporter.h b/logd/src/logd/state_reporter.h index ecf806b7899..5c1428acc38 100644 --- a/logd/src/logd/state_reporter.h +++ b/logd/src/logd/state_reporter.h @@ -15,11 +15,11 @@ namespace logdemon { */ class StateReporter { int _port; - std::unique_ptr<vespalib::StateServer> _server; vespalib::SimpleHealthProducer _health; vespalib::SimpleComponentConfigProducer _components; std::shared_ptr<vespalib::metrics::MetricsManager> _metrics; vespalib::metrics::Producer _producer; + std::unique_ptr<vespalib::StateServer> _server; public: StateReporter(); ~StateReporter(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 8fb8823a6f8..f2aa186fe4d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -249,6 +249,11 @@ public class NodeAgentImpl implements NodeAgent { } } + private void stopServicesIfNeeded(NodeAgentContext context) { + if (hasStartedServices && context.node().owner().isEmpty()) + stopServices(context); + } + private void stopServices(NodeAgentContext context) { context.log(logger, "Stopping services"); if (containerState == ABSENT) return; @@ -402,11 +407,12 @@ public class NodeAgentImpl implements NodeAgent { switch (node.state()) { case ready: case reserved: - case parked: case failed: case inactive: + case parked: removeContainerIfNeededUpdateContainerState(context, container); updateNodeRepoWithCurrentAttributes(context); + stopServicesIfNeeded(context); break; case active: storageMaintainer.handleCoreDumpsForContainer(context, container); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index d3bf93da71b..63f980efd91 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.flags.Flags; @@ -655,6 +656,47 @@ public class NodeAgentImplTest { inOrder.verify(orchestrator).resume(hostName); } + + // Tests that only containers without owners are stopped + @Test + public void testThatStopContainerDependsOnOwnerPresent() { + verifyThatContainerIsStopped(NodeState.parked, Optional.empty()); + verifyThatContainerIsStopped(NodeState.parked, Optional.of(ApplicationId.defaultId())); + verifyThatContainerIsStopped(NodeState.failed, Optional.empty()); + verifyThatContainerIsStopped(NodeState.failed, Optional.of(ApplicationId.defaultId())); + verifyThatContainerIsStopped(NodeState.inactive, Optional.of(ApplicationId.defaultId())); + } + + private void verifyThatContainerIsStopped(NodeState nodeState, Optional<ApplicationId> owner) { + NodeSpec.Builder nodeBuilder = new NodeSpec.Builder() + .hostname(hostName) + .type(NodeType.tenant) + .flavor("docker") + .currentDockerImage(dockerImage) + .wantedDockerImage(dockerImage) + .state(nodeState); + + owner.ifPresent(nodeBuilder::owner); + NodeSpec node = nodeBuilder.build(); + + NodeAgentContext context = createContext(node); + NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); + + when(nodeRepository.getOptionalNode(eq(hostName))).thenReturn(Optional.of(node)); + + nodeAgent.doConverge(context); + + verify(dockerOperations, never()).removeContainer(eq(context), any()); + if (owner.isPresent()) { + verify(dockerOperations, never()).stopServices(eq(context)); + } else { + verify(dockerOperations, times(1)).stopServices(eq(context)); + nodeAgent.doConverge(context); + // Should not be called more than once, have already been stopped + verify(dockerOperations, times(1)).stopServices(eq(context)); + } + } + private NodeAgentImpl makeNodeAgent(DockerImage dockerImage, boolean isRunning) { mockGetContainer(dockerImage, isRunning); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerService.java new file mode 100644 index 00000000000..891faceca7a --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerService.java @@ -0,0 +1,38 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.lb; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; + +import java.util.Comparator; +import java.util.Optional; +import java.util.Set; + +/** + * Implementation of a load balancer service that returns a real as the load balancer instance. This is intended for + * development purposes. + * + * @author mpolden + */ +public class PassthroughLoadBalancerService implements LoadBalancerService { + + @Override + public LoadBalancerInstance create(ApplicationId application, ClusterSpec.Id cluster, Set<Real> reals, boolean force) { + var real = reals.stream() + .min(Comparator.naturalOrder()) + .orElseThrow(() -> new IllegalArgumentException("No reals given")); + return new LoadBalancerInstance(real.hostname(), Optional.empty(), Set.of(real.port()), + Set.of(real.ipAddress() + "/32"), Set.of()); + } + + @Override + public void remove(ApplicationId application, ClusterSpec.Id cluster) { + // Nothing to remove + } + + @Override + public Protocol protocol() { + return Protocol.ipv4; + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/Real.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/Real.java index 196a7ff2d05..3c237a9a8a0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/Real.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/Real.java @@ -13,8 +13,7 @@ import java.util.Objects; */ public class Real implements Comparable<Real> { - // TODO: Change to 4443 when moving to HTTPS - private static final int defaultPort = 4080; + private static final int defaultPort = 4443; private final HostName hostname; private final String ipAddress; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java index 331ffe7e202..f670ff93073 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java @@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.IP; +import java.util.ArrayList; import java.util.Comparator; import java.util.Objects; import java.util.Optional; @@ -23,28 +24,30 @@ import java.util.stream.Collectors; * @author ogronnesby */ public class SharedLoadBalancerService implements LoadBalancerService { + private static final Comparator<Node> hostnameComparator = Comparator.comparing(Node::hostname); + private final NodeRepository nodeRepository; @Inject public SharedLoadBalancerService(NodeRepository nodeRepository) { - this.nodeRepository = Objects.requireNonNull(nodeRepository, "Missing nodeRepository value"); + this.nodeRepository = Objects.requireNonNull(nodeRepository, "nodeRepository must be non-null"); } @Override public LoadBalancerInstance create(ApplicationId application, ClusterSpec.Id cluster, Set<Real> reals, boolean force) { - final var proxyNodes = nodeRepository.getNodes(NodeType.proxy); + var proxyNodes = new ArrayList<>(nodeRepository.getNodes(NodeType.proxy)); proxyNodes.sort(hostnameComparator); if (proxyNodes.size() == 0) { - throw new IllegalStateException("Missing proxy nodes in nodeRepository"); + throw new IllegalStateException("Missing proxy nodes in node repository"); } - final var firstProxyNode = proxyNodes.get(0); - final var networkNames = proxyNodes.stream() - .flatMap(node -> node.ipAddresses().stream()) - .map(SharedLoadBalancerService::addNetworKPrefixLength) - .collect(Collectors.toSet()); + var firstProxyNode = proxyNodes.get(0); + var networkNames = proxyNodes.stream() + .flatMap(node -> node.ipAddresses().stream()) + .map(SharedLoadBalancerService::withPrefixLength) + .collect(Collectors.toSet()); return new LoadBalancerInstance( HostName.from(firstProxyNode.hostname()), @@ -65,12 +68,11 @@ public class SharedLoadBalancerService implements LoadBalancerService { return Protocol.dualstack; } - private static String addNetworKPrefixLength(String address) { + private static String withPrefixLength(String address) { if (IP.isV6(address)) { return address + "/128"; } - else { - return address + "/32"; - } + return address + "/32"; } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java index d81d01bc941..a54287ed082 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooter.java @@ -73,7 +73,7 @@ public class NodeRebooter extends Maintainer { // as long as 0 <= overdue <= rebootInterval, with the last maintain() in that interval // naturally scheduling the remaining with probability 1. - int configServers = 3; + int configServers = nodeRepository().database().cluster().size(); long secondsRemaining = Math.max(0, rebootInterval.getSeconds() - overdue.get().getSeconds()); double runsRemaining = configServers * secondsRemaining / (double) interval().getSeconds(); double probability = 1 / (1 + runsRemaining); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 93efcbbf046..a7b37628289 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -50,13 +50,11 @@ public class LoadBalancerProvisioner { private final NodeRepository nodeRepository; private final CuratorDatabaseClient db; private final LoadBalancerService service; - private final BooleanFlag usePort4443Flag; public LoadBalancerProvisioner(NodeRepository nodeRepository, LoadBalancerService service, FlagSource flagSource) { this.nodeRepository = nodeRepository; this.db = nodeRepository.database(); this.service = service; - this.usePort4443Flag = Flags.DIRECT_ROUTING_USE_HTTPS_4443.bindTo(flagSource); // Read and write all load balancers to make sure they are stored in the latest version of the serialization format try (var lock = db.lockLoadBalancers()) { for (var id : db.readLoadBalancerIds()) { @@ -170,10 +168,9 @@ public class LoadBalancerProvisioner { Map<HostName, Set<String>> hostnameToIpAdresses = nodes.stream() .collect(Collectors.toMap(node -> HostName.from(node.hostname()), this::reachableIpAddresses)); - boolean usePort4443 = usePort4443Flag.with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm()).value(); Set<Real> reals = new LinkedHashSet<>(); hostnameToIpAdresses.forEach((hostname, ipAddresses) -> { - ipAddresses.forEach(ipAddress -> reals.add(new Real(hostname, ipAddress, usePort4443 ? 4443 : 4080))); + ipAddresses.forEach(ipAddress -> reals.add(new Real(hostname, ipAddress))); }); log.log(LogLevel.INFO, "Creating load balancer for " + cluster + " in " + application.toShortString() + ", targeting: " + reals); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerServiceTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerServiceTest.java new file mode 100644 index 00000000000..402eaf37529 --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerServiceTest.java @@ -0,0 +1,30 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.lb; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.HostName; +import org.junit.Test; + +import java.util.Set; + +import static org.junit.Assert.assertEquals; + +/** + * @author mpolden + */ +public class PassthroughLoadBalancerServiceTest { + + @Test + public void create() { + var lbService = new PassthroughLoadBalancerService(); + var real = new Real(HostName.from("host1.example.com"), "192.0.2.10"); + var reals = Set.of(real, new Real(HostName.from("host2.example.com"), "192.0.2.11")); + var instance = lbService.create(ApplicationId.from("tenant1", "app1", "default"), + ClusterSpec.Id.from("c1"), reals, false); + assertEquals(real.hostname(), instance.hostname()); + assertEquals(Set.of(real.port()), instance.ports()); + assertEquals(Set.of(real.ipAddress() + "/32"), instance.networks()); + } + +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java index 5344fbc3c5f..1829a93c34f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java @@ -17,6 +17,7 @@ import static org.junit.Assert.assertEquals; * @author ogronnesby */ public class SharedLoadBalancerServiceTest { + private final ProvisioningTester tester = new ProvisioningTester.Builder().build(); private final SharedLoadBalancerService loadBalancerService = new SharedLoadBalancerService(tester.nodeRepository()); private final ApplicationId applicationId = ApplicationId.from("tenant1", "application1", "default"); @@ -29,7 +30,7 @@ public class SharedLoadBalancerServiceTest { @Test public void test_create_lb() { tester.makeReadyNodes(2, "default", NodeType.proxy); - final var lb = loadBalancerService.create(applicationId, clusterId, reals, false); + var lb = loadBalancerService.create(applicationId, clusterId, reals, false); assertEquals(HostName.from("host-1.yahoo.com"), lb.hostname()); assertEquals(Optional.empty(), lb.dnsZone()); @@ -46,4 +47,5 @@ public class SharedLoadBalancerServiceTest { public void test_protocol() { assertEquals(LoadBalancerService.Protocol.dualstack, loadBalancerService.protocol()); } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java index afada3c6f0f..a4b66d3cf9e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java @@ -8,7 +8,6 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -29,7 +28,7 @@ import java.util.stream.Collectors; */ public class MaintenanceTester { - private final Curator curator = new MockCurator(); + private final MockCurator curator = new MockCurator(); public final ManualClock clock = new ManualClock(Instant.ofEpochMilli(0L)); // determinism private final Zone zone = new Zone(Environment.prod, RegionName.from("us-east")); private final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default"); @@ -38,6 +37,10 @@ public class MaintenanceTester { DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true); + public MaintenanceTester() { + curator.setZooKeeperEnsembleConnectionSpec("zk1.host:1,zk2.host:2,zk3.host:3"); + } + public NodeRepository nodeRepository() { return nodeRepository; } public void createReadyTenantNodes(int count) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java index 3ca159e10eb..b731ab309a6 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java @@ -11,6 +11,7 @@ import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.config.provision.ParentHostUnavailableException; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; @@ -36,7 +37,8 @@ import static org.junit.Assert.fail; */ public class DockerProvisioningTest { - private static final NodeResources dockerFlavor = new NodeResources(1, 4, 10, 1); + private static final NodeResources dockerFlavor = new NodeResources(1, 4, 10, 1, + NodeResources.DiskSpeed.fast, NodeResources.StorageType.local); @Test public void docker_application_deployment() { @@ -204,7 +206,7 @@ public class DockerProvisioningTest { } catch (Exception e) { assertEquals("No room for 3 nodes as 2 of 4 hosts are exclusive", - "Could not satisfy request for 3 nodes with [vcpu: 1.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps] for container cluster 'myContainer' group 0 6.39 in tenant1.app1: Not enough nodes available due to host exclusivity constraints.", + "Could not satisfy request for 3 nodes with [vcpu: 1.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps, storage type: local] for container cluster 'myContainer' group 0 6.39 in tenant1.app1: Not enough nodes available due to host exclusivity constraints.", e.getMessage()); } @@ -225,7 +227,25 @@ public class DockerProvisioningTest { NodeList nodes = tester.getNodes(application1, Node.State.active); assertEquals(1, nodes.size()); - assertEquals("[vcpu: 1.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps]", nodes.asList().get(0).flavor().name()); + assertEquals("[vcpu: 1.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps, storage type: local]", nodes.asList().get(0).flavor().name()); + } + + @Test + public void storage_type_must_match() { + try { + ProvisioningTester tester = new ProvisioningTester.Builder() + .zone(new Zone(Environment.prod, RegionName.from("us-east-1"))).build(); + ApplicationId application1 = tester.makeApplicationId(); + tester.makeReadyVirtualDockerNodes(1, dockerFlavor, "dockerHost1"); + tester.makeReadyVirtualDockerNodes(1, dockerFlavor, "dockerHost2"); + + List<HostSpec> hosts = tester.prepare(application1, ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent"), + Version.fromString("6.42"), false), 2, 1, + dockerFlavor.with(NodeResources.StorageType.remote)); + } + catch (OutOfCapacityException e) { + assertTrue(e.getMessage().startsWith("Could not satisfy request for 2 nodes with [vcpu: 1.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 1.0 Gbps, storage type: remote]")); + } } private Set<String> hostsOf(NodeList nodes) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index 91846d14aa1..9c42d0d95a1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -67,9 +67,9 @@ public class LoadBalancerProvisionerTest { assertEquals(containerCluster1, lbApp1.get().get(0).id().cluster()); assertEquals(Collections.singleton(4443), lbApp1.get().get(0).instance().ports()); assertEquals("127.0.0.1", get(lbApp1.get().get(0).instance().reals(), 0).ipAddress()); - assertEquals(4080, get(lbApp1.get().get(0).instance().reals(), 0).port()); + assertEquals(4443, get(lbApp1.get().get(0).instance().reals(), 0).port()); assertEquals("127.0.0.2", get(lbApp1.get().get(0).instance().reals(), 1).ipAddress()); - assertEquals(4080, get(lbApp1.get().get(0).instance().reals(), 1).port()); + assertEquals(4443, get(lbApp1.get().get(0).instance().reals(), 1).port()); // A container is failed Supplier<List<Node>> containers = () -> tester.getNodes(app1).type(ClusterSpec.Type.container).asList(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json index 19e65c2fc25..a9a728bab15 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers-single.json @@ -21,12 +21,12 @@ { "hostname": "host13.yahoo.com", "ipAddress": "127.0.13.1", - "port": 4080 + "port": 4443 }, { "hostname": "host14.yahoo.com", "ipAddress": "127.0.14.1", - "port": 4080 + "port": 4443 } ] } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json index 0b05a41af0a..515081bcb8e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json @@ -21,12 +21,12 @@ { "hostname": "host1.yahoo.com", "ipAddress": "127.0.1.1", - "port": 4080 + "port": 4443 }, { "hostname": "host10.yahoo.com", "ipAddress": "127.0.10.1", - "port": 4080 + "port": 4443 } ] }, @@ -51,12 +51,12 @@ { "hostname": "host13.yahoo.com", "ipAddress": "127.0.13.1", - "port": 4080 + "port": 4443 }, { "hostname": "host14.yahoo.com", "ipAddress": "127.0.14.1", - "port": 4080 + "port": 4443 } ] } diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index ce1344f5c26..f402f536ae2 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -2241,7 +2241,22 @@ void testNamePrefix() { EXPECT_EQUAL("sfsint32_pc", vS2->getNamePrefix()); EXPECT_EQUAL("sfsint32_pc.xyz.abc", vSS1->getName()); EXPECT_EQUAL("sfsint32_pc", vSS1->getNamePrefix()); +} + +class MyMultiValueAttribute : public ArrayStringAttribute { +public: + MyMultiValueAttribute(const vespalib::string& name) + : ArrayStringAttribute(name, Config(BasicType::STRING, CollectionType::ARRAY)) + { + } + bool has_free_lists_enabled() const { return this->_mvMapping.has_free_lists_enabled(); } +}; +void +test_multi_value_mapping_has_free_lists_enabled() +{ + MyMultiValueAttribute attr("mvtest"); + EXPECT_TRUE(attr.has_free_lists_enabled()); } void @@ -2292,6 +2307,7 @@ int AttributeTest::Main() testReaderDuringLastUpdate(); TEST_DO(testPendingCompaction()); TEST_DO(testNamePrefix()); + test_multi_value_mapping_has_free_lists_enabled(); deleteDataDirs(); TEST_DONE(); diff --git a/searchlib/src/tests/attribute/compaction/attribute_compaction_test.cpp b/searchlib/src/tests/attribute/compaction/attribute_compaction_test.cpp index c3453783e38..2766d310baf 100644 --- a/searchlib/src/tests/attribute/compaction/attribute_compaction_test.cpp +++ b/searchlib/src/tests/attribute/compaction/attribute_compaction_test.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/searchlib/attribute/attribute.h> #include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/attributeguard.h> #include <vespa/searchlib/attribute/attributevector.hpp> #include <vespa/searchlib/attribute/integerbase.h> #include <vespa/searchlib/attribute/address_space_usage.h> @@ -167,27 +168,63 @@ TEST_F("Test that compaction of integer array attribute reduces memory usage", F EXPECT_LESS(afterStatus.getUsed(), beforeStatus.getUsed()); } -TEST_F("Test that no compaction of int8 array attribute increases address space usage", Fixture(compactAddressSpaceAttributeConfig(false))) +void +populate_and_hammer(Fixture& f, bool take_attribute_guard) { DocIdRange range1 = f.addDocs(1000); DocIdRange range2 = f.addDocs(1000); - f.populate(range1, 1000); - f.hammer(range2, 101); + if (take_attribute_guard) { + { + // When attribute guard is held free lists will not be used in the hammer step. + search::AttributeGuard guard(f._v); + f.populate(range1, 1000); + f.hammer(range2, 101); + } + f._v->commit(true); + f._v->commit(); + } else { + f.populate(range1, 1000); + f.hammer(range2, 101); + } +} + +TEST_F("Address space usage (dead) increases significantly when free lists are NOT used (compaction configured off)", + Fixture(compactAddressSpaceAttributeConfig(false))) +{ + populate_and_hammer(f, true); AddressSpace afterSpace = f.getMultiValueAddressSpaceUsage("after"); // 100 * 1000 dead arrays due to new values for docids // 1 reserved array accounted as dead EXPECT_EQUAL(100001u, afterSpace.dead()); } -TEST_F("Test that compaction of int8 array attribute limits address space usage", Fixture(compactAddressSpaceAttributeConfig(true))) +TEST_F("Address space usage (dead) increases only slightly when free lists are used (compaction configured off)", + Fixture(compactAddressSpaceAttributeConfig(false))) { - DocIdRange range1 = f.addDocs(1000); - DocIdRange range2 = f.addDocs(1000); - f.populate(range1, 1000); - f.hammer(range2, 101); + populate_and_hammer(f, false); + AddressSpace afterSpace = f.getMultiValueAddressSpaceUsage("after"); + // Only 1000 dead arrays (due to new values for docids) as free lists are used. + // 1 reserved array accounted as dead + EXPECT_EQUAL(1001u, afterSpace.dead()); +} + +TEST_F("Compaction limits address space usage (dead) when free lists are NOT used", + Fixture(compactAddressSpaceAttributeConfig(true))) +{ + populate_and_hammer(f, true); AddressSpace afterSpace = f.getMultiValueAddressSpaceUsage("after"); // DEAD_ARRAYS_SLACK in multi value mapping is is 64k EXPECT_GREATER(65536u, afterSpace.dead()); } +TEST_F("Compaction is not executed when free lists are used", + Fixture(compactAddressSpaceAttributeConfig(true))) +{ + populate_and_hammer(f, false); + AddressSpace afterSpace = f.getMultiValueAddressSpaceUsage("after"); + // Only 1000 dead arrays (due to new values for docids) as free lists are used. + // 1 reserved array accounted as dead + EXPECT_EQUAL(1001u, afterSpace.dead()); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp b/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp index 9bbcd71f59d..2f74e9d31f6 100644 --- a/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp +++ b/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp @@ -80,17 +80,19 @@ public: _maxSmallArraySize() { } - void setup(uint32_t maxSmallArraySize) { - _mvMapping = std::make_unique<MvMapping>(ArrayStoreConfig(maxSmallArraySize, - ArrayStoreConfig::AllocSpec(0, RefType::offsetSize(), 8 * 1024, - ALLOC_GROW_FACTOR))); + void setup(uint32_t maxSmallArraySize, bool enable_free_lists = true) { + ArrayStoreConfig config(maxSmallArraySize, + ArrayStoreConfig::AllocSpec(0, RefType::offsetSize(), 8 * 1024, ALLOC_GROW_FACTOR)); + config.enable_free_lists(enable_free_lists); + _mvMapping = std::make_unique<MvMapping>(config); _attr = std::make_unique<AttributeType>(*_mvMapping); _maxSmallArraySize = maxSmallArraySize; } - void setup(uint32_t maxSmallArraySize, size_t minArrays, size_t maxArrays, size_t numArraysForNewBuffer) { - _mvMapping = std::make_unique<MvMapping>(ArrayStoreConfig(maxSmallArraySize, - ArrayStoreConfig::AllocSpec(minArrays, maxArrays, numArraysForNewBuffer, - ALLOC_GROW_FACTOR))); + void setup(uint32_t maxSmallArraySize, size_t minArrays, size_t maxArrays, size_t numArraysForNewBuffer, bool enable_free_lists = true) { + ArrayStoreConfig config(maxSmallArraySize, + ArrayStoreConfig::AllocSpec(minArrays, maxArrays, numArraysForNewBuffer, ALLOC_GROW_FACTOR)); + config.enable_free_lists(enable_free_lists); + _mvMapping = std::make_unique<MvMapping>(config); _attr = std::make_unique<AttributeType>(*_mvMapping); _maxSmallArraySize = maxSmallArraySize; } @@ -307,6 +309,18 @@ TEST_F(IntMappingTest, test_that_replace_works) EXPECT_EQ(4u, getTotalValueCnt()); } +TEST_F(IntMappingTest, test_that_free_lists_can_be_enabled) +{ + setup(3, true); + EXPECT_TRUE(_mvMapping->has_free_lists_enabled()); +} + +TEST_F(IntMappingTest, test_that_free_lists_can_be_disabled) +{ + setup(3, false); + EXPECT_FALSE(_mvMapping->has_free_lists_enabled()); +} + TEST_F(CompactionIntMappingTest, test_that_compaction_works) { setup(3, 64, 512, 129); diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h index 6246a656bf6..ca4303e772f 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h @@ -48,12 +48,14 @@ public: vespalib::AddressSpace getAddressSpaceUsage() const override; vespalib::MemoryUsage getArrayStoreMemoryUsage() const override; + bool has_free_lists_enabled() const { return _store.has_free_lists_enabled(); } static datastore::ArrayStoreConfig optimizedConfigForHugePage(size_t maxSmallArraySize, size_t hugePageSize, size_t smallPageSize, size_t minNumArraysForNewBuffer, - float allocGrowFactor); + float allocGrowFactor, + bool enable_free_lists); }; } diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp index 4460d1757a2..3dcc0df477d 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp @@ -80,9 +80,12 @@ MultiValueMapping<EntryT, RefT>::optimizedConfigForHugePage(size_t maxSmallArray size_t hugePageSize, size_t smallPageSize, size_t minNumArraysForNewBuffer, - float allocGrowFactor) + float allocGrowFactor, + bool enable_free_lists) { - return ArrayStore::optimizedConfigForHugePage(maxSmallArraySize, hugePageSize, smallPageSize, minNumArraysForNewBuffer, allocGrowFactor); + auto result = ArrayStore::optimizedConfigForHugePage(maxSmallArraySize, hugePageSize, smallPageSize, minNumArraysForNewBuffer, allocGrowFactor); + result.enable_free_lists(enable_free_lists); + return result; } } diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp index f07efcafa9a..b23860ebd31 100644 --- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp @@ -11,6 +11,7 @@ namespace search { namespace multivalueattribute { constexpr size_t SMALL_MEMORY_PAGE_SIZE = 4 * 1024; +constexpr bool enable_free_lists = true; } @@ -23,7 +24,8 @@ MultiValueAttribute(const vespalib::string &baseFileName, vespalib::alloc::MemoryAllocator::HUGEPAGE_SIZE, multivalueattribute::SMALL_MEMORY_PAGE_SIZE, 8 * 1024, - cfg.getGrowStrategy().getMultiValueAllocGrowFactor()), + cfg.getGrowStrategy().getMultiValueAllocGrowFactor(), + multivalueattribute::enable_free_lists), cfg.getGrowStrategy().to_generic_strategy()) { } diff --git a/vespalib/src/vespa/vespalib/datastore/array_store.h b/vespalib/src/vespa/vespalib/datastore/array_store.h index d9d5afcbd43..d5fef404a43 100644 --- a/vespalib/src/vespa/vespalib/datastore/array_store.h +++ b/vespalib/src/vespa/vespalib/datastore/array_store.h @@ -101,6 +101,8 @@ public: // Should only be used for unit testing const BufferState &bufferState(EntryRef ref) const; + bool has_free_lists_enabled() const { return _store.has_free_lists_enabled(); } + static ArrayStoreConfig optimizedConfigForHugePage(size_t maxSmallArraySize, size_t hugePageSize, size_t smallPageSize, diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.h b/vespalib/src/vespa/vespalib/datastore/datastorebase.h index 59e0d76b638..22b1a53f0ab 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.h +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.h @@ -316,6 +316,8 @@ public: void disableFreeList(uint32_t bufferId); void disableElemHoldList(); + bool has_free_lists_enabled() const { return _freeListsEnabled; } + /** * Returns the free list for the given type id. */ diff --git a/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java b/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java index 9cb87efa3c0..c4e1f8130a1 100644 --- a/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java +++ b/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java @@ -5,13 +5,17 @@ import com.google.inject.Inject; import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.component.AbstractComponent; import com.yahoo.log.LogLevel; +import com.yahoo.security.tls.TlsContext; + import static com.yahoo.vespa.defaults.Defaults.getDefaults; import java.io.File; import java.io.FileWriter; import java.io.IOException; import java.util.List; +import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -76,6 +80,45 @@ public class VespaZooKeeperServerImpl extends AbstractComponent implements Runna sb.append("serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory").append("\n"); ensureThisServerIsRepresented(config.myid(), config.server()); config.server().forEach(server -> addServerToCfg(sb, server)); + sb.append(createTlsQuorumConfig(getEnvironmentVariable("VESPA_TLS_FOR_ZOOKEEPER_QUORUM_COMMUNICATION") + .orElse(config.tlsForQuorumCommunication().name()))); + return sb.toString(); + } + + private String createTlsQuorumConfig(String tlsSetting) { + StringBuilder sb = new StringBuilder(); + + // Common config + sb.append("ssl.quorum.hostnameVerification=false\n"); + sb.append("ssl.quorum.clientAuth=NEED\n"); + sb.append("ssl.quorum.ciphersuites=").append(String.join(",", new TreeSet<>(TlsContext.ALLOWED_CIPHER_SUITES))).append("\n"); + sb.append("ssl.quorum.enabledProtocols=").append(String.join(",", new TreeSet<>(TlsContext.ALLOWED_PROTOCOLS))).append("\n"); + sb.append("ssl.quorum.protocol=TLS\n"); + + boolean sslQuorum; + boolean portUnification; + switch (tlsSetting) { + case "OFF": + sslQuorum = false; + portUnification = false; + break; + case "PORT_UNIFICATION": + sslQuorum = false; + portUnification = true; + break; + case "TLS_WITH_PORT_UNIFICATION": + sslQuorum = true; + portUnification = true; + break; + case "TLS_ONLY": + sslQuorum = true; + portUnification = false; + break; + default: throw new IllegalArgumentException("Unknown value of config setting tlsForQuorumCommunication: " + tlsSetting); + } + sb.append("sslQuorum=").append(sslQuorum).append("\n"); + sb.append("portUnification=").append(portUnification).append("\n"); + return sb.toString(); } @@ -132,4 +175,9 @@ public class VespaZooKeeperServerImpl extends AbstractComponent implements Runna return zookeeperServerConfig.server().stream().map(ZookeeperServerConfig.Server::hostname).collect(Collectors.toSet()); } + private static Optional<String> getEnvironmentVariable(String variableName) { + return Optional.ofNullable(System.getenv().get(variableName)) + .filter(var -> !var.isEmpty()); + } + } diff --git a/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java b/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java index 19690bff963..fcca602a2ed 100644 --- a/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java +++ b/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java @@ -26,11 +26,7 @@ public class VespaZooKeeperServerImplTest { public void config_is_written_correctly_when_one_server() throws IOException { File cfgFile = folder.newFile(); File idFile = folder.newFile(); - ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); - builder.zooKeeperConfigFile(cfgFile.getAbsolutePath()); - builder.myidFile(idFile.getAbsolutePath()); - builder.server(newServer(0, "foo", 123, 321)); - builder.myid(0); + ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile); createServer(builder); validateConfigFileSingleHost(cfgFile); validateIdFile(idFile, ""); @@ -52,6 +48,45 @@ public class VespaZooKeeperServerImplTest { validateIdFile(idFile, "1\n"); } + @Test + public void config_is_written_correctly_with_tls_for_quorum_communication_port_unification() throws IOException { + File cfgFile = folder.newFile(); + File idFile = folder.newFile(); + ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile); + builder.tlsForQuorumCommunication(ZookeeperServerConfig.TlsForQuorumCommunication.Enum.PORT_UNIFICATION); + createServer(builder); + validateConfigFilePortUnification(cfgFile); + } + + @Test + public void config_is_written_correctly_with_tls_for_quorum_communication_tls_with_port_unification() throws IOException { + File cfgFile = folder.newFile(); + File idFile = folder.newFile(); + ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile); + builder.tlsForQuorumCommunication(ZookeeperServerConfig.TlsForQuorumCommunication.Enum.TLS_WITH_PORT_UNIFICATION); + createServer(builder); + validateConfigFileTlsWithPortUnification(cfgFile); + } + + @Test + public void config_is_written_correctly_with_tls_for_quorum_communication_tls_only() throws IOException { + File cfgFile = folder.newFile(); + File idFile = folder.newFile(); + ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile); + builder.tlsForQuorumCommunication(ZookeeperServerConfig.TlsForQuorumCommunication.Enum.TLS_ONLY); + createServer(builder); + validateConfigFileTlsOnly(cfgFile); + } + + private ZookeeperServerConfig.Builder createConfigBuilderForSingleHost(File cfgFile, File idFile) { + ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); + builder.zooKeeperConfigFile(cfgFile.getAbsolutePath()); + builder.myidFile(idFile.getAbsolutePath()); + builder.server(newServer(0, "foo", 123, 321)); + builder.myid(0); + return builder; + } + private void createServer(ZookeeperServerConfig.Builder builder) { new VespaZooKeeperServerImpl(new ZookeeperServerConfig(builder), false); } @@ -99,41 +134,78 @@ public class VespaZooKeeperServerImplTest { assertThat(actual, is(expected)); } + private String commonConfig() { + return "tickTime=2000\n" + + "initLimit=20\n" + + "syncLimit=15\n" + + "maxClientCnxns=0\n" + + "snapCount=50000\n" + + "dataDir=" + getDefaults().underVespaHome("var/zookeeper") + "\n" + + "clientPort=2181\n" + + "autopurge.purgeInterval=1\n" + + "autopurge.snapRetainCount=15\n" + + "4lw.commands.whitelist=conf,cons,crst,dirs,dump,envi,mntr,ruok,srst,srvr,stat,wchs\n" + + "admin.enableServer=false\n" + + "serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory\n"; + } + private void validateConfigFileSingleHost(File cfgFile) throws IOException { String expected = - "tickTime=2000\n" + - "initLimit=20\n" + - "syncLimit=15\n" + - "maxClientCnxns=0\n" + - "snapCount=50000\n" + - "dataDir=" + getDefaults().underVespaHome("var/zookeeper") + "\n" + - "clientPort=2181\n" + - "autopurge.purgeInterval=1\n" + - "autopurge.snapRetainCount=15\n" + - "4lw.commands.whitelist=conf,cons,crst,dirs,dump,envi,mntr,ruok,srst,srvr,stat,wchs\n" + - "admin.enableServer=false\n" + - "serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory\n" + - "server.0=foo:321:123\n"; + commonConfig() + + "server.0=foo:321:123\n" + + commonTlsConfig() + + "sslQuorum=false\n" + + "portUnification=false\n"; validateConfigFile(cfgFile, expected); } + private String commonTlsConfig() { + return "ssl.quorum.hostnameVerification=false\n" + + "ssl.quorum.clientAuth=NEED\n" + + "ssl.quorum.ciphersuites=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384,TLS_CHACHA20_POLY1305_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256\n" + + "ssl.quorum.enabledProtocols=TLSv1.2\n" + + "ssl.quorum.protocol=TLS\n"; + } + private void validateConfigFileMultipleHosts(File cfgFile) throws IOException { String expected = - "tickTime=2000\n" + - "initLimit=20\n" + - "syncLimit=15\n" + - "maxClientCnxns=0\n" + - "snapCount=50000\n" + - "dataDir=" + getDefaults().underVespaHome("var/zookeeper") + "\n" + - "clientPort=2181\n" + - "autopurge.purgeInterval=1\n" + - "autopurge.snapRetainCount=15\n" + - "4lw.commands.whitelist=conf,cons,crst,dirs,dump,envi,mntr,ruok,srst,srvr,stat,wchs\n" + - "admin.enableServer=false\n" + - "serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory\n" + - "server.0=foo:321:123\n" + - "server.1=bar:432:234\n" + - "server.2=baz:543:345\n"; + commonConfig() + + "server.0=foo:321:123\n" + + "server.1=bar:432:234\n" + + "server.2=baz:543:345\n" + + commonTlsConfig() + + "sslQuorum=false\n" + + "portUnification=false\n"; + validateConfigFile(cfgFile, expected); + } + + private void validateConfigFilePortUnification(File cfgFile) throws IOException { + String expected = + commonConfig() + + "server.0=foo:321:123\n" + + commonTlsConfig() + + "sslQuorum=false\n" + + "portUnification=true\n"; + validateConfigFile(cfgFile, expected); + } + + private void validateConfigFileTlsWithPortUnification(File cfgFile) throws IOException { + String expected = + commonConfig() + + "server.0=foo:321:123\n" + + commonTlsConfig() + + "sslQuorum=true\n" + + "portUnification=true\n"; + validateConfigFile(cfgFile, expected); + } + + private void validateConfigFileTlsOnly(File cfgFile) throws IOException { + String expected = + commonConfig() + + "server.0=foo:321:123\n" + + commonTlsConfig() + + "sslQuorum=true\n" + + "portUnification=false\n"; validateConfigFile(cfgFile, expected); } |