diff options
76 files changed, 1300 insertions, 424 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 c4eeb41a663..0a59392789a 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 @@ -82,10 +82,9 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default double feedConcurrency() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForLidSpaceCompact() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean useBucketExecutorForBucketMove() { throw new UnsupportedOperationException("TODO specify default value"); } - @ModelFeatureFlag(owners = {"musum", "mpolden"}, comment = "Revisit in February 2021", removeAfter = "7.370") default boolean reconfigurableZookeeperServer() { return true; } @ModelFeatureFlag(owners = {"geirst"}) default boolean enableFeedBlockInDistributor() { return true; } @ModelFeatureFlag(owners = {"baldersheim", "geirst", "toregge"}) default double maxDeadBytesRatio() { return 0.2; } - @ModelFeatureFlag(owners = {"hmusum"}) default int clusterControllerMaxHeapSizeInMb() { return 256; } + @ModelFeatureFlag(owners = {"hmusum"}) default int clusterControllerMaxHeapSizeInMb() { return 128; } @ModelFeatureFlag(owners = {"hmusum"}) default int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return 256; } @ModelFeatureFlag(owners = {"bjorncs", "tokle"}) default List<String> allowedAthenzProxyIdentities() { return List.of(); } @ModelFeatureFlag(owners = {"tokle"}) default boolean tenantIamRole() { return false; } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index c1d45c213fb..306f36e7674 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -8,6 +8,7 @@ import com.yahoo.config.model.api.ContainerEndpoint; import com.yahoo.config.model.api.EndpointCertificateSecrets; import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.Quota; +import com.yahoo.config.model.api.TenantSecretStore; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.ClusterSpec; @@ -58,6 +59,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private int clusterControllerMaxHeapSizeInMb = 256; private int metricsProxyMaxHeapSizeInMb = 256; private int maxActivationInhibitedOutOfSyncGroups = 0; + private List<TenantSecretStore> tenantSecretStores = Collections.emptyList(); @Override public ModelContext.FeatureFlags featureFlags() { return this; } @Override public boolean multitenant() { return multitenant; } @@ -97,6 +99,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public int clusterControllerMaxHeapSizeInMb() { return clusterControllerMaxHeapSizeInMb; } @Override public int metricsProxyMaxHeapSizeInMb(ClusterSpec.Type type) { return metricsProxyMaxHeapSizeInMb; } @Override public int maxActivationInhibitedOutOfSyncGroups() { return maxActivationInhibitedOutOfSyncGroups; } + @Override public List<TenantSecretStore> tenantSecretStores() { return tenantSecretStores; } public TestProperties setFeedConcurrency(double feedConcurrency) { this.feedConcurrency = feedConcurrency; @@ -233,6 +236,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } + public TestProperties setTenantSecretStores(List<TenantSecretStore> secretStores) { + this.tenantSecretStores = List.copyOf(secretStores); + return this; + } + public static class Spec implements ConfigServerSpec { private final String hostName; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecretStore.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecretStore.java index 39f9a627e0c..bf6a0275a94 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecretStore.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/CloudSecretStore.java @@ -33,9 +33,9 @@ public class CloudSecretStore extends SimpleComponent implements SecretStoreConf @Override public void getConfig(SecretStoreConfig.Builder builder) { - builder.groups( + builder.awsParameterStores( configList.stream() - .map(config -> new SecretStoreConfig.Groups.Builder() + .map(config -> new SecretStoreConfig.AwsParameterStores.Builder() .name(config.name) .region(config.region) .awsId(config.awsId) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 21aad7a565c..7b3bc498164 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -281,7 +281,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { store -> store )); - for (Element group : XML.getChildren(secretStoreElement, "group")) { + for (Element group : XML.getChildren(secretStoreElement, "aws-parameter-store")) { String name = group.getAttribute("name"); String region = group.getAttribute("region"); TenantSecretStore secretStore = secretStoresByName.get(name); diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc index abe7386fa00..9313d91ea55 100644 --- a/config-model/src/main/resources/schema/containercluster.rnc +++ b/config-model/src/main/resources/schema/containercluster.rnc @@ -89,9 +89,12 @@ SecretStore = element secret-store { attribute type { string "oath-ckms" | string "cloud" } & element group { attribute name { string } & - (attribute environment { string "alpha" | string "corp" | string "prod" | string "aws" | string "aws_stage" } | - attribute region { string } ) - } + + attribute environment { string "alpha" | string "corp" | string "prod" | string "aws" | string "aws_stage" } + } * & + element aws-parameter-store { + attribute name { string } & + attribute region { string } + } * } ZooKeeper = element zookeeper { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 92e0b116878..e43b5085528 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -7,6 +7,7 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.NullConfigModelRegistry; import com.yahoo.config.model.api.ContainerEndpoint; import com.yahoo.config.model.api.EndpointCertificateSecrets; +import com.yahoo.config.model.api.TenantSecretStore; import com.yahoo.config.model.builder.xml.test.DomBuilderTest; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; @@ -15,6 +16,7 @@ import com.yahoo.config.model.provision.InMemoryProvisioner; import com.yahoo.config.model.provision.SingleNodeProvisioner; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.model.test.MockRoot; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.RegionName; @@ -29,6 +31,7 @@ import com.yahoo.container.handler.VipStatusHandler; import com.yahoo.container.handler.metrics.MetricsV2Handler; import com.yahoo.container.handler.observability.ApplicationStatusHandler; import com.yahoo.container.jdisc.JdiscBindingsConfig; +import com.yahoo.container.jdisc.secretstore.SecretStoreConfig; import com.yahoo.container.servlet.ServletConfigConfig; import com.yahoo.container.usability.BindingsOverviewHandler; import com.yahoo.jdisc.http.ConnectorConfig; @@ -715,6 +718,54 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } @Test + public void cloud_secret_store_requires_configured_secret_store() { + Element clusterElem = DomBuilderTest.parse( + "<container version='1.0'>", + " <secret-store type='cloud'>", + " <aws-parameter-store name='store1' region='eu-north-1'/>", + " </secret-store>", + "</container>"); + try { + createModel(root, clusterElem); + fail("secret store not defined"); + } catch (RuntimeException e) { + assertEquals("No configured secret store named store1", e.getMessage()); + } + } + + + @Test + public void cloud_secret_store_can_be_set_up() { + Element clusterElem = DomBuilderTest.parse( + "<container version='1.0'>", + " <secret-store type='cloud'>", + " <aws-parameter-store name='store1' region='eu-north-1'/>", + " </secret-store>", + "</container>"); + + DeployState state = new DeployState.Builder() + .properties( + new TestProperties() + .setHostedVespa(true) + .setTenantSecretStores(List.of(new TenantSecretStore("store1", "1234", "role", Optional.of("externalid"))))) + .zone(new Zone(SystemName.Public, Environment.prod, RegionName.defaultName())) + .build(); + createModel(root, state, null, clusterElem); + + ApplicationContainerCluster container = getContainerCluster("container"); + assertComponentConfigured(container, "com.yahoo.jdisc.cloud.aws.AwsParameterStore"); + CloudSecretStore secretStore = (CloudSecretStore) container.getComponentsMap().get(ComponentId.fromString("com.yahoo.jdisc.cloud.aws.AwsParameterStore")); + + + SecretStoreConfig.Builder configBuilder = new SecretStoreConfig.Builder(); + secretStore.getConfig(configBuilder); + SecretStoreConfig secretStoreConfig = configBuilder.build(); + + assertEquals(1, secretStoreConfig.awsParameterStores().size()); + assertEquals("store1", secretStoreConfig.awsParameterStores().get(0).name()); + } + + @Test public void missing_security_clients_pem_fails_in_public() { Element clusterElem = DomBuilderTest.parse("<container version='1.0' />"); diff --git a/container-disc/abi-spec.json b/container-disc/abi-spec.json index 63f6b241750..735211ff47c 100644 --- a/container-disc/abi-spec.json +++ b/container-disc/abi-spec.json @@ -60,6 +60,43 @@ ], "fields": [] }, + "com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder": { + "superClass": "java.lang.Object", + "interfaces": [ + "com.yahoo.config.ConfigBuilder" + ], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>()", + "public void <init>(com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder name(java.lang.String)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder region(java.lang.String)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder awsId(java.lang.String)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder role(java.lang.String)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder externalId(java.lang.String)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores build()" + ], + "fields": [] + }, + "com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores": { + "superClass": "com.yahoo.config.InnerNode", + "interfaces": [], + "attributes": [ + "public", + "final" + ], + "methods": [ + "public void <init>(com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder)", + "public java.lang.String name()", + "public java.lang.String region()", + "public java.lang.String awsId()", + "public java.lang.String role()", + "public java.lang.String externalId()" + ], + "fields": [] + }, "com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Builder": { "superClass": "java.lang.Object", "interfaces": [ @@ -73,6 +110,8 @@ "public void <init>(com.yahoo.container.jdisc.secretstore.SecretStoreConfig)", "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Builder groups(com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Groups$Builder)", "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Builder groups(java.util.List)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Builder awsParameterStores(com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores$Builder)", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Builder awsParameterStores(java.util.List)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", "public final java.lang.String getDefMd5()", "public final java.lang.String getDefName()", @@ -82,7 +121,8 @@ "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig build()" ], "fields": [ - "public java.util.List groups" + "public java.util.List groups", + "public java.util.List awsParameterStores" ] }, "com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Groups$Builder": { @@ -151,7 +191,9 @@ "public static java.lang.String getDefVersion()", "public void <init>(com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Builder)", "public java.util.List groups()", - "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Groups groups(int)" + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$Groups groups(int)", + "public java.util.List awsParameterStores()", + "public com.yahoo.container.jdisc.secretstore.SecretStoreConfig$AwsParameterStores awsParameterStores(int)" ], "fields": [ "public static final java.lang.String CONFIG_DEF_MD5", diff --git a/container-disc/src/main/resources/configdefinitions/container.jdisc.secretstore.secret-store.def b/container-disc/src/main/resources/configdefinitions/container.jdisc.secretstore.secret-store.def index c3edc10c0bc..67ef4744da8 100644 --- a/container-disc/src/main/resources/configdefinitions/container.jdisc.secretstore.secret-store.def +++ b/container-disc/src/main/resources/configdefinitions/container.jdisc.secretstore.secret-store.def @@ -1,8 +1,15 @@ # Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. namespace=container.jdisc.secretstore +# remove groups after 7.376 is oldest model groups[].name string groups[].region string groups[].awsId string groups[].role string -groups[].externalId string
\ No newline at end of file +groups[].externalId string + +awsParameterStores[].name string +awsParameterStores[].region string +awsParameterStores[].awsId string +awsParameterStores[].role string +awsParameterStores[].externalId string diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequest.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequest.java index a56f96955de..31665c8ae0a 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequest.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/ChangeRequest.java @@ -13,15 +13,15 @@ public class ChangeRequest { private final ChangeRequestSource changeRequestSource; private final List<String> impactedSwitches; private final List<String> impactedHosts; - private final boolean isApproved; + private final Approval approval; private final Impact impact; - private ChangeRequest(String id, ChangeRequestSource changeRequestSource, List<String> impactedSwitches, List<String> impactedHosts, boolean isApproved, Impact impact) { + private ChangeRequest(String id, ChangeRequestSource changeRequestSource, List<String> impactedSwitches, List<String> impactedHosts, Approval approval, Impact impact) { this.id = Objects.requireNonNull(id); this.changeRequestSource = Objects.requireNonNull(changeRequestSource); this.impactedSwitches = Objects.requireNonNull(impactedSwitches); this.impactedHosts = Objects.requireNonNull(impactedHosts); - this.isApproved = Objects.requireNonNull(isApproved); + this.approval = Objects.requireNonNull(approval); this.impact = Objects.requireNonNull(impact); } @@ -41,8 +41,8 @@ public class ChangeRequest { return impactedHosts; } - public boolean isApproved() { - return isApproved; + public Approval getApproval() { + return approval; } public Impact getImpact() { @@ -56,7 +56,7 @@ public class ChangeRequest { ", changeRequestSource=" + changeRequestSource + ", impactedSwitches=" + impactedSwitches + ", impactedHosts=" + impactedHosts + - ", isApproved=" + isApproved + + ", approval=" + approval + ", impact=" + impact + '}'; } @@ -66,7 +66,7 @@ public class ChangeRequest { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ChangeRequest that = (ChangeRequest) o; - return isApproved == that.isApproved && + return approval == that.approval && Objects.equals(id, that.id) && Objects.equals(changeRequestSource, that.changeRequestSource) && Objects.equals(impactedSwitches, that.impactedSwitches) && @@ -76,7 +76,7 @@ public class ChangeRequest { @Override public int hashCode() { - return Objects.hash(id, changeRequestSource, impactedSwitches, impactedHosts, isApproved, impact); + return Objects.hash(id, changeRequestSource, impactedSwitches, impactedHosts, approval, impact); } public static class Builder { @@ -84,7 +84,7 @@ public class ChangeRequest { private ChangeRequestSource changeRequestSource; private List<String> impactedSwitches; private List<String> impactedHosts; - private boolean isApproved; + private Approval approval; private Impact impact; @@ -108,8 +108,8 @@ public class ChangeRequest { return this; } - public Builder isApproved(boolean isApproved) { - this.isApproved = isApproved; + public Builder approval(Approval approval) { + this.approval = approval; return this; } @@ -119,7 +119,7 @@ public class ChangeRequest { } public ChangeRequest build() { - return new ChangeRequest(id, changeRequestSource, impactedSwitches, impactedHosts, isApproved, impact); + return new ChangeRequest(id, changeRequestSource, impactedSwitches, impactedHosts, approval, impact); } public String getId() { @@ -136,4 +136,11 @@ public class ChangeRequest { UNKNOWN } + public enum Approval { + REQUESTED, + APPROVED, + REJECTED, + OTHER + } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/MockChangeRequestClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/MockChangeRequestClient.java new file mode 100644 index 00000000000..e85c0afcb0e --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/MockChangeRequestClient.java @@ -0,0 +1,33 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.vcmr; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author olaa + */ +public class MockChangeRequestClient implements ChangeRequestClient { + + private List<ChangeRequest> upcomingChangeRequests = new ArrayList<>(); + private List<ChangeRequest> approvedChangeRequests = new ArrayList<>(); + + @Override + public List<ChangeRequest> getUpcomingChangeRequests() { + return upcomingChangeRequests; + } + + @Override + public void approveChangeRequests(List<ChangeRequest> changeRequests) { + approvedChangeRequests.addAll(changeRequests); + } + + public void setUpcomingChangeRequests(List<ChangeRequest> changeRequests) { + upcomingChangeRequests = changeRequests; + } + + public List<ChangeRequest> getApprovedChangeRequests() { + return approvedChangeRequests; + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/NoopChangeRequestClient.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/NoopChangeRequestClient.java deleted file mode 100644 index d548620b062..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/NoopChangeRequestClient.java +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration.vcmr; - -import java.util.List; - -/** - * @author olaa - */ -public class NoopChangeRequestClient implements ChangeRequestClient { - - @Override - public List<ChangeRequest> getUpcomingChangeRequests() { - return List.of(); - } - - @Override - public void approveChangeRequests(List<ChangeRequest> changeRequests) {} - -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index ccb9dccdb3b..159e0bb1f0f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -185,16 +185,14 @@ public class Application { /** Returns the total quota usage for this application, excluding temporary deployments */ public QuotaUsage quotaUsage() { return instances().values().stream() - .filter(instance -> !instance.id().instance().isTester()) // Exclude temporary deployments - .map(Instance::quotaUsage).reduce(QuotaUsage::add).orElse(QuotaUsage.none); + .map(Instance::quotaUsage).reduce(QuotaUsage::add).orElse(QuotaUsage.none); } /** Returns the total quota usage for this application, excluding one specific deployment (and temporary deployments) */ public QuotaUsage quotaUsage(ApplicationId application, ZoneId zone) { return instances().values().stream() - .filter(instance -> !instance.id().instance().isTester()) // Exclude temporary deployments - .map(instance -> instance.quotaUsageExcluding(application, zone)) - .reduce(QuotaUsage::add).orElse(QuotaUsage.none); + .map(instance -> instance.quotaUsageExcluding(application, zone)) + .reduce(QuotaUsage::add).orElse(QuotaUsage.none); } /** Returns the set of deploy keys for this application. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index 22fa25fc711..f25f1c64372 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -121,22 +121,26 @@ public abstract class LockedTenant { private final BiMap<PublicKey, Principal> developerKeys; private final TenantInfo info; private final List<TenantSecretStore> tenantSecretStores; + private final Optional<String> archiveAccessRole; - private Cloud(TenantName name, Instant createdAt, LastLoginInfo lastLoginInfo, Optional<Principal> creator, BiMap<PublicKey, Principal> developerKeys, TenantInfo info, List<TenantSecretStore> tenantSecretStores) { + private Cloud(TenantName name, Instant createdAt, LastLoginInfo lastLoginInfo, Optional<Principal> creator, + BiMap<PublicKey, Principal> developerKeys, TenantInfo info, + List<TenantSecretStore> tenantSecretStores, Optional<String> archiveAccessRole) { super(name, createdAt, lastLoginInfo); this.developerKeys = ImmutableBiMap.copyOf(developerKeys); this.creator = creator; this.info = info; this.tenantSecretStores = tenantSecretStores; + this.archiveAccessRole = archiveAccessRole; } private Cloud(CloudTenant tenant) { - this(tenant.name(), tenant.createdAt(), tenant.lastLoginInfo(), Optional.empty(), tenant.developerKeys(), tenant.info(), tenant.tenantSecretStores()); + this(tenant.name(), tenant.createdAt(), tenant.lastLoginInfo(), Optional.empty(), tenant.developerKeys(), tenant.info(), tenant.tenantSecretStores(), tenant.archiveAccessRole()); } @Override public CloudTenant get() { - return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores); + return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccessRole); } public Cloud withDeveloperKey(PublicKey key, Principal principal) { @@ -144,34 +148,38 @@ public abstract class LockedTenant { if (keys.containsKey(key)) throw new IllegalArgumentException("Key " + KeyUtils.toPem(key) + " is already owned by " + keys.get(key)); keys.put(key, principal); - return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores); + return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccessRole); } public Cloud withoutDeveloperKey(PublicKey key) { BiMap<PublicKey, Principal> keys = HashBiMap.create(developerKeys); keys.remove(key); - return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores); + return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccessRole); } public Cloud withInfo(TenantInfo newInfo) { - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, newInfo, tenantSecretStores); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, newInfo, tenantSecretStores, archiveAccessRole); } @Override public LockedTenant with(LastLoginInfo lastLoginInfo) { - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccessRole); } public Cloud withSecretStore(TenantSecretStore tenantSecretStore) { ArrayList<TenantSecretStore> secretStores = new ArrayList<>(tenantSecretStores); secretStores.add(tenantSecretStore); - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores, archiveAccessRole); } public Cloud withoutSecretStore(TenantSecretStore tenantSecretStore) { ArrayList<TenantSecretStore> secretStores = new ArrayList<>(tenantSecretStores); secretStores.remove(tenantSecretStore); - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores, archiveAccessRole); + } + + public Cloud withArchiveAccessRole(Optional<String> role) { + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, role); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index e4b0ff0f6fa..3f1e8831e83 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -26,6 +26,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -39,7 +40,6 @@ import static java.util.Comparator.naturalOrder; import static java.util.Objects.requireNonNull; import static java.util.function.BinaryOperator.maxBy; import static java.util.stream.Collectors.collectingAndThen; -import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toUnmodifiableList; @@ -117,14 +117,12 @@ public class DeploymentStatus { return allJobs.asList().stream() .filter(job -> job.id().application().equals(application.id().instance(instance))) .collect(Collectors.toUnmodifiableMap(job -> job.id().type(), - job -> job)); + Function.identity())); } /** Filterable job status lists for each instance of this application. */ public Map<ApplicationId, JobList> instanceJobs() { - return allJobs.asList().stream() - .collect(groupingBy(job -> job.id().application(), - collectingAndThen(toUnmodifiableList(), JobList::from))); + return allJobs.groupingBy(job -> job.id().application()); } /** diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/NodeList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/NodeList.java index 2d336143138..080d600005f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/NodeList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/NodeList.java @@ -10,8 +10,6 @@ import java.util.Collection; import java.util.List; import java.util.stream.Collectors; -import static java.util.stream.Collectors.groupingBy; - /** * @author jonmv */ @@ -26,7 +24,7 @@ public class NodeList extends AbstractFilteringList<NodeWithServices, NodeList> public static NodeList of(List<Node> nodes, List<Node> parents, ServiceConvergence services) { var servicesByHostName = services.services().stream() - .collect(groupingBy(service -> service.host())); + .collect(Collectors.groupingBy(service -> service.host())); var parentsByHostName = parents.stream() .collect(Collectors.toMap(node -> node.hostname(), node -> node)); return new NodeList(nodes.stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessor.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessor.java new file mode 100644 index 00000000000..5226f8b39bd --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessor.java @@ -0,0 +1,92 @@ +package com.yahoo.vespa.hosted.controller.maintenance; + +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; + +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +public class ChangeManagementAssessor { + + private final NodeRepository nodeRepository; + + public static class Assessment { + public String app; + public String zone; + public String cluster; + public long clusterImpact; + public long clusterSize; + public long groupsImpact; + public long groupsTotal; + public String upgradePolicy; + public String suggestedAction; + public String impact; + } + + public ChangeManagementAssessor(Controller controller) { + this.nodeRepository = controller.serviceRegistry().configServer().nodeRepository(); + } + + public List<Assessment> assessment(List<String> impactedHostnames, ZoneId zone) { + return assessmentInner(impactedHostnames, nodeRepository.listNodes(zone).nodes(), zone); + } + + static List<Assessment> assessmentInner(List<String> impactedHostnames, List<NodeRepositoryNode> allNodes, ZoneId zone) { + // Get all active nodes running on the impacted hosts + List<NodeRepositoryNode> containerNodes = allNodes.stream() + .filter(nodeRepositoryNode -> nodeRepositoryNode.getState() == NodeState.active) //TODO look at more states? + .filter(node -> impactedHostnames.contains(node.getParentHostname() == null ? "" : node.getParentHostname())).collect(Collectors.toList()); + + // Group nodes pr cluster + Map<String, List<NodeRepositoryNode>> prCluster = containerNodes.stream().collect(Collectors.groupingBy(ChangeManagementAssessor::clusterKey)); + + // Report assessment pr cluster + return prCluster.entrySet().stream().map((entry) -> { + String key = entry.getKey(); + List<NodeRepositoryNode> nodes = entry.getValue(); + String app = Arrays.stream(key.split(":")).limit(3).collect(Collectors.joining(":")); + String cluster = Arrays.stream(key.split(":")).skip(3).collect(Collectors.joining(":")); + + long[] totalStats = clusterStats(key, allNodes); + long[] impactedStats = clusterStats(key, nodes); + + Assessment assessment = new Assessment(); + assessment.app = app; + assessment.zone = zone.value(); + assessment.cluster = cluster; + assessment.clusterSize = totalStats[0]; + assessment.clusterImpact = impactedStats[0]; + assessment.groupsTotal = totalStats[1]; + assessment.groupsImpact = impactedStats[1]; + + // TODO check upgrade policy + assessment.upgradePolicy = "na"; + // TODO do some heuristic on suggestion action + assessment.suggestedAction = "nothing"; + // TODO do some heuristic on impact + assessment.impact = "na"; + + return assessment; + }).collect(Collectors.toList()); + } + + private static String clusterKey(NodeRepositoryNode node) { + if (node.getOwner() != null && node.getMembership() != null) { + String appId = String.format("%s:%s:%s", node.getOwner().tenant, node.getOwner().application, node.getOwner().instance); + String cluster = String.format("%s:%s", node.getMembership().clustertype, node.getMembership().clusterid); + return appId + ":" + cluster; + } + return ""; + } + + private static long[] clusterStats(String key, List<NodeRepositoryNode> containerNodes) { + List<NodeRepositoryNode> clusterNodes = containerNodes.stream().filter(nodeRepositoryNode -> clusterKey(nodeRepositoryNode).equals(key)).collect(Collectors.toList()); + long groups = clusterNodes.stream().map(nodeRepositoryNode -> nodeRepositoryNode.getMembership() != null ? nodeRepositoryNode.getMembership().group : "").distinct().count(); + return new long[] { clusterNodes.size(), groups}; + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java index 71291e5d766..ca9ebe132fd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainer.java @@ -3,9 +3,11 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequest; import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequestClient; import java.time.Duration; +import java.util.List; import java.util.function.Predicate; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -17,10 +19,12 @@ public class ChangeRequestMaintainer extends ControllerMaintainer { private final Logger logger = Logger.getLogger(ChangeRequestMaintainer.class.getName()); private final ChangeRequestClient changeRequestClient; + private final SystemName system; public ChangeRequestMaintainer(Controller controller, Duration interval) { super(controller, interval, null, SystemName.allOf(Predicate.not(SystemName::isPublic))); this.changeRequestClient = controller.serviceRegistry().changeRequestClient(); + this.system = controller.system(); } @@ -29,18 +33,23 @@ public class ChangeRequestMaintainer extends ControllerMaintainer { var changeRequests = changeRequestClient.getUpcomingChangeRequests(); if (!changeRequests.isEmpty()) { - logger.fine(() -> "Found the following upcoming change requests:"); - changeRequests.forEach(changeRequest -> logger.fine(changeRequest::toString)); + logger.info(() -> "Found the following upcoming change requests:"); + changeRequests.forEach(changeRequest -> logger.info(changeRequest::toString)); } + if (system.equals(SystemName.main)) + approveChanges(changeRequests); + + // TODO: Store in curator? + return true; + } + + private void approveChanges(List<ChangeRequest> changeRequests) { var unapprovedRequests = changeRequests .stream() - .filter(changeRequest -> !changeRequest.isApproved()) + .filter(changeRequest -> changeRequest.getApproval() == ChangeRequest.Approval.REQUESTED) .collect(Collectors.toList()); changeRequestClient.approveChangeRequests(unapprovedRequests); - - // TODO: Store in curator? - return true; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index beaf546930f..8cb87b8a72d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -71,6 +71,7 @@ public class TenantSerializer { private static final String tenantInfoField = "info"; private static final String lastLoginInfoField = "lastLoginInfo"; private static final String secretStoresField = "secretStores"; + private static final String archiveAccessRoleField = "archiveAccessRole"; private static final String awsIdField = "awsId"; private static final String roleField = "role"; @@ -110,6 +111,7 @@ public class TenantSerializer { toSlime(legacyBillingInfo, root.setObject(billingInfoField)); toSlime(tenant.info(), root); toSlime(tenant.tenantSecretStores(), root); + tenant.archiveAccessRole().ifPresent(role -> root.setString(archiveAccessRoleField, role)); } private void developerKeysToSlime(BiMap<PublicKey, Principal> keys, Cursor array) { @@ -162,7 +164,8 @@ public class TenantSerializer { BiMap<PublicKey, Principal> developerKeys = developerKeysFromSlime(tenantObject.field(pemDeveloperKeysField)); TenantInfo info = tenantInfoFromSlime(tenantObject.field(tenantInfoField)); List<TenantSecretStore> tenantSecretStores = secretStoresFromSlime(tenantObject.field(secretStoresField)); - return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores); + Optional<String> archiveAccessRole = SlimeUtils.optionalString(tenantObject.field(archiveAccessRoleField)); + return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccessRole); } private BiMap<PublicKey, Principal> developerKeysFromSlime(Inspector array) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index f252e532ea8..9aa5c17270b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -36,6 +36,7 @@ import com.yahoo.slime.Inspector; import com.yahoo.slime.JsonParseException; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.LockedTenant; @@ -51,7 +52,6 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbi import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.integration.billing.Quota; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.Application; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Cluster; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; @@ -140,6 +140,7 @@ import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; import static java.util.Map.Entry.comparingByKey; import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toUnmodifiableList; /** @@ -270,6 +271,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse handlePUT(Path path, HttpRequest request) { if (path.matches("/application/v4/tenant/{tenant}")) return updateTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/info")) return updateTenantInfo(path.get("tenant"), request); + if (path.matches("/application/v4/tenant/{tenant}/archive-access")) return allowArchiveAccess(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}")) return addSecretStore(path.get("tenant"), path.get("name"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), false, request); @@ -314,6 +316,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse handleDELETE(Path path, HttpRequest request) { if (path.matches("/application/v4/tenant/{tenant}")) return deleteTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/key")) return removeDeveloperKey(path.get("tenant"), request); + if (path.matches("/application/v4/tenant/{tenant}/archive-access")) return removeArchiveAccess(path.get("tenant")); if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}")) return deleteSecretStore(path.get("tenant"), path.get("name"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return deleteApplication(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deployment")) return removeAllProdDeployments(path.get("tenant"), path.get("application")); @@ -345,8 +348,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse recursiveRoot(HttpRequest request) { Slime slime = new Slime(); Cursor tenantArray = slime.setArray(); + List<Application> applications = controller.applications().asList(); for (Tenant tenant : controller.tenants().asList()) - toSlime(tenantArray.addObject(), tenant, request); + toSlime(tenantArray.addObject(), + tenant, + applications.stream().filter(app -> app.id().tenant().equals(tenant.name())).collect(toList()), + request); return new SlimeJsonResponse(slime); } @@ -372,7 +379,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse tenant(Tenant tenant, HttpRequest request) { Slime slime = new Slime(); - toSlime(slime.setObject(), tenant, request); + toSlime(slime.setObject(), tenant, controller.applications().asList(tenant.name()), request); return new SlimeJsonResponse(slime); } @@ -482,7 +489,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { Slime slime = new Slime(); Cursor applicationArray = slime.setArray(); - for (com.yahoo.vespa.hosted.controller.Application application : controller.applications().asList(tenant)) { + for (Application application : controller.applications().asList(tenant)) { if (applicationName.map(application.id().application().value()::equals).orElse(true)) { Cursor applicationObject = applicationArray.addObject(); applicationObject.setString("tenant", application.id().tenant().value()); @@ -728,6 +735,37 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new SlimeJsonResponse(slime); } + private HttpResponse allowArchiveAccess(String tenantName, HttpRequest request) { + if (controller.tenants().require(TenantName.from(tenantName)).type() != Tenant.Type.cloud) + throw new IllegalArgumentException("Tenant '" + tenantName + "' is not a cloud tenant"); + + var data = toSlime(request.getData()).get(); + var role = mandatory("role", data).asString(); + + if (role.isBlank()) { + return ErrorResponse.badRequest("Archive access role can't be whitespace only"); + } + + controller.tenants().lockOrThrow(TenantName.from(tenantName), LockedTenant.Cloud.class, lockedTenant -> { + lockedTenant = lockedTenant.withArchiveAccessRole(Optional.of(role)); + controller.tenants().store(lockedTenant); + }); + + return new MessageResponse("Archive access role set to '" + role + "' for tenant " + tenantName + "."); + } + + private HttpResponse removeArchiveAccess(String tenantName) { + if (controller.tenants().require(TenantName.from(tenantName)).type() != Tenant.Type.cloud) + throw new IllegalArgumentException("Tenant '" + tenantName + "' is not a cloud tenant"); + + controller.tenants().lockOrThrow(TenantName.from(tenantName), LockedTenant.Cloud.class, lockedTenant -> { + lockedTenant = lockedTenant.withArchiveAccessRole(Optional.empty()); + controller.tenants().store(lockedTenant); + }); + + return new MessageResponse("Archive access role removed for tenant " + tenantName + "."); + } + private HttpResponse patchApplication(String tenantName, String applicationName, HttpRequest request) { Inspector requestObject = toSlime(request.getData()).get(); StringJoiner messageBuilder = new StringJoiner("\n").setEmptyValue("No applicable changes."); @@ -753,7 +791,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new MessageResponse(messageBuilder.toString()); } - private com.yahoo.vespa.hosted.controller.Application getApplication(String tenantName, String applicationName) { + private Application getApplication(String tenantName, String applicationName) { TenantAndApplicationId applicationId = TenantAndApplicationId.from(tenantName, applicationName); return controller.applications().getApplication(applicationId) .orElseThrow(() -> new NotExistsException(applicationId + " not found")); @@ -791,7 +829,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse clusters(String tenantName, String applicationName, String instanceName, String environment, String region) { ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName); ZoneId zone = requireZone(environment, region); - Application application = controller.serviceRegistry().configServer().nodeRepository().getApplication(zone, id); + com.yahoo.vespa.hosted.controller.api.integration.configserver.Application application = controller.serviceRegistry().configServer().nodeRepository().getApplication(zone, id); Slime slime = new Slime(); Cursor clustersObject = slime.setObject().setObject("clusters"); @@ -932,7 +970,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new MessageResponse(type.jobName() + " for " + id + " resumed"); } - private void toSlime(Cursor object, com.yahoo.vespa.hosted.controller.Application application, HttpRequest request) { + private void toSlime(Cursor object, Application application, HttpRequest request) { object.setString("tenant", application.id().tenant().value()); object.setString("application", application.id().application().value()); object.setString("deployments", withPath("/application/v4" + @@ -1070,7 +1108,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private void toSlime(Cursor object, Instance instance, DeploymentStatus status, HttpRequest request) { - com.yahoo.vespa.hosted.controller.Application application = status.application(); + Application application = status.application(); object.setString("tenant", instance.id().tenant().value()); object.setString("application", instance.id().application().value()); object.setString("instance", instance.id().instance().value()); @@ -1572,7 +1610,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { Inspector requestObject = toSlime(request.getData()).get(); TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); Credentials credentials = accessControlRequests.credentials(id.tenant(), requestObject, request.getJDiscRequest()); - com.yahoo.vespa.hosted.controller.Application application = controller.applications().createApplication(id, credentials); + Application application = controller.applications().createApplication(id, credentials); Slime slime = new Slime(); toSlime(id, slime.setObject(), request); return new SlimeJsonResponse(slime); @@ -1837,7 +1875,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { Optional<ApplicationPackage> applicationPackage = Optional.ofNullable(dataParts.get("applicationZip")) .map(ApplicationPackage::new); - Optional<com.yahoo.vespa.hosted.controller.Application> application = controller.applications().getApplication(TenantAndApplicationId.from(applicationId)); + Optional<Application> application = controller.applications().getApplication(TenantAndApplicationId.from(applicationId)); Inspector sourceRevision = deployOptions.field("sourceRevision"); Inspector buildNumber = deployOptions.field("buildNumber"); @@ -1977,10 +2015,9 @@ public class ApplicationApiHandler extends LoggingRequestHandler { .orElseThrow(() -> new NotExistsException(new TenantId(tenantName))); } - private void toSlime(Cursor object, Tenant tenant, HttpRequest request) { + private void toSlime(Cursor object, Tenant tenant, List<Application> applications, HttpRequest request) { object.setString("tenant", tenant.name().value()); object.setString("type", tenantType(tenant)); - List<com.yahoo.vespa.hosted.controller.Application> applications = controller.applications().asList(tenant.name()); switch (tenant.type()) { case athenz: AthenzTenant athenzTenant = (AthenzTenant) tenant; @@ -2013,7 +2050,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { var tenantQuota = controller.serviceRegistry().billingController().getQuota(tenant.name()); var usedQuota = applications.stream() - .map(com.yahoo.vespa.hosted.controller.Application::quotaUsage) + .map(Application::quotaUsage) .reduce(QuotaUsage.none, QuotaUsage::add); toSlime(tenantQuota, usedQuota, object.setObject("quota")); @@ -2024,16 +2061,19 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } // TODO jonmv: This should list applications, not instances. Cursor applicationArray = object.setArray("applications"); - for (com.yahoo.vespa.hosted.controller.Application application : applications) { - DeploymentStatus status = controller.jobController().deploymentStatus(application); + for (Application application : applications) { + DeploymentStatus status = null; for (Instance instance : showOnlyProductionInstances(request) ? application.productionInstances().values() : application.instances().values()) - if (recurseOverApplications(request)) + if (recurseOverApplications(request)) { + if (status == null) status = controller.jobController().deploymentStatus(application); toSlime(applicationArray.addObject(), instance, status, request); - else + } + else { toSlime(instance.id(), applicationArray.addObject(), request); + } } - tenantMetaDataToSlime(tenant, object.setObject("metaData")); + tenantMetaDataToSlime(tenant, applications, object.setObject("metaData")); } private void toSlime(Quota quota, QuotaUsage usage, Cursor object) { @@ -2101,8 +2141,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { object.setString("url", withPath("/application/v4/tenant/" + tenant.name().value(), requestURI).toString()); } - private void tenantMetaDataToSlime(Tenant tenant, Cursor object) { - List<com.yahoo.vespa.hosted.controller.Application> applications = controller.applications().asList(tenant.name()); + private void tenantMetaDataToSlime(Tenant tenant, List<Application> applications, Cursor object) { Optional<Instant> lastDev = applications.stream() .flatMap(application -> application.instances().values().stream()) .flatMap(instance -> controller.jobController().jobs(instance.id()).stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java index 1dbeaf7c235..ff45955ee76 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java @@ -1,26 +1,35 @@ package com.yahoo.vespa.hosted.controller.restapi.changemanagement; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.restapi.ErrorResponse; import com.yahoo.restapi.Path; import com.yahoo.restapi.SlimeJsonResponse; +import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.auditlog.AuditLoggingRequestHandler; +import com.yahoo.vespa.hosted.controller.maintenance.ChangeManagementAssessor; import com.yahoo.yolean.Exceptions; +import javax.ws.rs.BadRequestException; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.logging.Level; public class ChangeManagementApiHandler extends AuditLoggingRequestHandler { - private final Controller controller; + private final ChangeManagementAssessor assessor; public ChangeManagementApiHandler(LoggingRequestHandler.Context ctx, Controller controller) { super(ctx, controller.auditLogger()); - this.controller = controller; + assessor = new ChangeManagementAssessor(controller); } @Override @@ -42,48 +51,78 @@ public class ChangeManagementApiHandler extends AuditLoggingRequestHandler { private HttpResponse post(HttpRequest request) { Path path = new Path(request.getUri()); - if (path.matches("/changemanagement/v1/assessment")) return new SlimeJsonResponse(assessment()); + if (path.matches("/changemanagement/v1/assessment")) return new SlimeJsonResponse(doAssessment(request)); return ErrorResponse.notFoundError("Nothing at " + path); } - private Slime assessment() { + private Inspector inspectorOrThrow(HttpRequest request) { + try { + return SlimeUtils.jsonToSlime(request.getData().readAllBytes()).get(); + } catch (IOException e) { + throw new BadRequestException("Failed to parse request body"); + } + } + + private static Inspector getInspectorFieldOrThrow(Inspector inspector, String field) { + if (!inspector.field(field).valid()) + throw new BadRequestException("Field " + field + " cannot be null"); + return inspector.field(field); + } + + // The structure here should be + // + // { + // zone: string + // hosts: string[] + // switches: string[] + // switchInSequence: boolean + // } + // + // Only zone and host are supported right now + private Slime doAssessment(HttpRequest request) { + + Inspector inspector = inspectorOrThrow(request); + + // For now; mandatory fields + String zoneStr = getInspectorFieldOrThrow(inspector, "zone").asString(); + Inspector hostArray = getInspectorFieldOrThrow(inspector, "hosts"); + + // The impacted hostnames + List<String> hostNames = new ArrayList<>(); + if (hostArray.valid()) { + hostArray.traverse((ArrayTraverser) (i, host) -> hostNames.add(host.asString())); + } + + List<ChangeManagementAssessor.Assessment> assessments = assessor.assessment(hostNames, ZoneId.from(zoneStr)); + Slime slime = new Slime(); Cursor root = slime.setObject(); // This is the main structure that might be part of something bigger later - Cursor assessment = root.setObject("assessment"); + Cursor assessmentCursor = root.setObject("assessment"); - // Updated gives clue to if the assessement is old - assessment.setString("updated", "2021-03-12:12:12:12Z"); + // Updated gives clue to if the assessment is old + assessmentCursor.setString("updated", "2021-03-12:12:12:12Z"); // Assessment on the cluster level - Cursor apps = assessment.setArray("clusters"); - Cursor oneCluster = apps.addObject(); - oneCluster.setString("app", "mytenant:myapp:myinstance"); - oneCluster.setString("zone", "prod.us-east-3"); - oneCluster.setString("cluster", "mycontent"); - oneCluster.setLong("clusterSize", 19); - oneCluster.setLong("clusterImpact", 3); - oneCluster.setString("upgradePolicy","10%"); - oneCluster.setDouble("utilizationInWindow", 0.5); - oneCluster.setString("suggestedAction", "nothing|bcp"); - oneCluster.setString("impact", "low|degraded|outage"); - - Cursor groups = oneCluster.setArray("groups"); - Cursor oneGroup = groups.addObject(); - oneGroup.setString("groupId", "23"); - oneGroup.setLong("groupSize", 4); - oneGroup.setLong("groupImpact", 2); - - // Assessment on the host level - Cursor hosts = assessment.setArray("hosts"); - Cursor oneHost = hosts.addObject(); - oneHost.setString("hostname", "myhostname"); - oneHost.setString("resources", "vcpu:memory:disk"); - oneHost.setString("state", "active"); - oneHost.setLong("containers", 10); - oneHost.setString("suggestedAction", "nothing|retire"); - oneHost.setString("impact", "low|degraded|outage"); //For hosts this is mostly decided by request + Cursor clustersCursor = assessmentCursor.setArray("clusters"); + + assessments.forEach(assessment -> { + Cursor oneCluster = clustersCursor.addObject(); + oneCluster.setString("app", assessment.app); + oneCluster.setString("zone", assessment.zone); + oneCluster.setString("cluster", assessment.cluster); + oneCluster.setLong("clusterSize", assessment.clusterSize); + oneCluster.setLong("clusterImpact", assessment.clusterImpact); + oneCluster.setLong("groupsTotal", assessment.groupsTotal); + oneCluster.setLong("groupsImpact", assessment.groupsImpact); + oneCluster.setString("upgradePolicy", assessment.upgradePolicy); + oneCluster.setString("suggestedAction", assessment.suggestedAction); + oneCluster.setString("impact", assessment.impact); + }); + + // Assessment on the host level - TODO + assessmentCursor.setArray("hosts"); return slime; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java index 888bfd7fe42..b5126b7973b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java @@ -24,16 +24,18 @@ public class CloudTenant extends Tenant { private final BiMap<PublicKey, Principal> developerKeys; private final TenantInfo info; private final List<TenantSecretStore> tenantSecretStores; - + private final Optional<String> archiveAccessRole; /** Public for the serialization layer — do not use! */ public CloudTenant(TenantName name, Instant createdAt, LastLoginInfo lastLoginInfo, Optional<Principal> creator, - BiMap<PublicKey, Principal> developerKeys, TenantInfo info, List<TenantSecretStore> tenantSecretStores) { + BiMap<PublicKey, Principal> developerKeys, TenantInfo info, + List<TenantSecretStore> tenantSecretStores, Optional<String> archiveAccessRole) { super(name, createdAt, lastLoginInfo, Optional.empty()); this.creator = creator; this.developerKeys = developerKeys; this.info = Objects.requireNonNull(info); this.tenantSecretStores = tenantSecretStores; + this.archiveAccessRole = archiveAccessRole; } /** Creates a tenant with the given name, provided it passes validation. */ @@ -42,7 +44,7 @@ public class CloudTenant extends Tenant { createdAt, LastLoginInfo.EMPTY, Optional.ofNullable(creator), - ImmutableBiMap.of(), TenantInfo.EMPTY, List.of()); + ImmutableBiMap.of(), TenantInfo.EMPTY, List.of(), Optional.empty()); } /** The user that created the tenant */ @@ -55,6 +57,11 @@ public class CloudTenant extends Tenant { return info; } + /** An iam role which is allowed to access the S3 (log, dump) archive) */ + public Optional<String> archiveAccessRole() { + return archiveAccessRole; + } + /** Returns the set of developer keys and their corresponding developers for this tenant. */ public BiMap<PublicKey, Principal> developerKeys() { return developerKeys; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java index e47a7d305e6..737807b5dd3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java @@ -82,42 +82,4 @@ public class DeploymentQuotaCalculatorTest { assertEquals(tenantQuota, calculated); } - @Test - public void temporary_deployments_are_excluded() { - var tenantQuota = Quota.unlimited().withBudget(42); - - var instanceInTestEnv = new Instance(ApplicationId.from("default", "default", "foo")).withNewDeployment( - ZoneId.from("test", "apac1"), - ApplicationVersion.unknown, - Version.emptyVersion, - Instant.EPOCH, - Map.of(), - QuotaUsage.create(1)); - - var testerInstance = new Instance(ApplicationId.from("default", "default", "bar-t")).withNewDeployment( - ZoneId.from("prod", "apac1"), - ApplicationVersion.unknown, - Version.emptyVersion, - Instant.EPOCH, - Map.of(), - QuotaUsage.create(1)); - - var app = new Application( - TenantAndApplicationId.from(ApplicationId.defaultId()), - Instant.EPOCH, - DeploymentSpec.empty, - ValidationOverrides.empty, - Optional.empty(), - Optional.empty(), - Optional.empty(), - OptionalInt.empty(), - new ApplicationMetrics(100, 100), - Set.of(), - OptionalLong.empty(), - Optional.empty(), - List.of(instanceInTestEnv, testerInstance)); - - Quota calculated = DeploymentQuotaCalculator.calculate(tenantQuota, List.of(app), ApplicationId.defaultId(), ZoneId.from("dev", "apac1"), DeploymentSpec.empty); - assertEquals(tenantQuota, calculated); - } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java index 0c558ef3ea8..a84e7242495 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java @@ -21,6 +21,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeStat import java.net.URI; import java.time.Duration; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -29,6 +30,7 @@ import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -46,6 +48,10 @@ public class NodeRepositoryMock implements NodeRepository { private final Map<DeploymentId, Pair<Double, Double>> trafficFractions = new HashMap<>(); private final Map<ZoneId, Map<TenantName, URI>> archiveUris = new HashMap<>(); + // A separate/alternative list of NodeRepositoryNode nodes. + // Methods operating with Node and NodeRepositoryNode lives separate lives. + private final Map<ZoneId, List<NodeRepositoryNode>> nodeRepoNodes = new HashMap<>(); + private boolean allowPatching = false; /** Add or update given nodes in zone */ @@ -77,6 +83,7 @@ public class NodeRepositoryMock implements NodeRepository { /** Remove all nodes in all zones */ public void clear() { nodeRepository.clear(); + nodeRepoNodes.clear(); } /** Replace nodes in zone with given nodes */ @@ -131,7 +138,7 @@ public class NodeRepositoryMock implements NodeRepository { @Override public void addNodes(ZoneId zone, Collection<NodeRepositoryNode> nodes) { - throw new UnsupportedOperationException(); + nodeRepoNodes.put(zone, new ArrayList<>(nodes)); } @Override @@ -151,7 +158,7 @@ public class NodeRepositoryMock implements NodeRepository { @Override public NodeList listNodes(ZoneId zone) { - throw new UnsupportedOperationException(); + return new NodeList(nodeRepoNodes.get(zone)); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java index e96eed2b6d9..326928b9463 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java @@ -27,7 +27,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.resource.CostReportCons import com.yahoo.vespa.hosted.controller.api.integration.routing.GlobalRoutingService; import com.yahoo.vespa.hosted.controller.api.integration.routing.MemoryGlobalRoutingService; import com.yahoo.vespa.hosted.controller.api.integration.secrets.NoopTenantSecretService; -import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretService; import com.yahoo.vespa.hosted.controller.api.integration.stubs.DummyOwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.stubs.DummySystemMonitor; import com.yahoo.vespa.hosted.controller.api.integration.stubs.LoggingDeploymentIssues; @@ -36,7 +35,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMeteringClien import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud; import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequestClient; -import com.yahoo.vespa.hosted.controller.api.integration.vcmr.NoopChangeRequestClient; +import com.yahoo.vespa.hosted.controller.api.integration.vcmr.MockChangeRequestClient; /** * A mock implementation of a {@link ServiceRegistry} for testing purposes. @@ -72,7 +71,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg private final ContainerRegistryMock containerRegistry = new ContainerRegistryMock(); private final NoopTenantSecretService tenantSecretService = new NoopTenantSecretService(); private final ArchiveService archiveService = new MockArchiveService(); - private final ChangeRequestClient changeRequestClient = new NoopChangeRequestClient(); + private final MockChangeRequestClient changeRequestClient = new MockChangeRequestClient(); public ServiceRegistryMock(SystemName system) { this.zoneRegistryMock = new ZoneRegistryMock(system); @@ -225,7 +224,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg } @Override - public ChangeRequestClient changeRequestClient() { + public MockChangeRequestClient changeRequestClient() { return changeRequestClient; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessorTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessorTest.java new file mode 100644 index 00000000000..f3cb996c048 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessorTest.java @@ -0,0 +1,124 @@ +package com.yahoo.vespa.hosted.controller.maintenance; + + +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeMembership; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeOwner; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; +import org.junit.Assert; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +public class ChangeManagementAssessorTest { + + @Test + public void empty_input_variations() { + ZoneId zone = ZoneId.from("prod", "eu-trd"); + List<String> hostNames = new ArrayList<>(); + List<NodeRepositoryNode> allNodesInZone = new ArrayList<>(); + + // Both zone and hostnames are empty + List<ChangeManagementAssessor.Assessment> assessments + = ChangeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); + Assert.assertEquals(0, assessments.size()); + } + + @Test + public void one_host_one_cluster_no_groups() { + ZoneId zone = ZoneId.from("prod", "eu-trd"); + List<String> hostNames = Collections.singletonList("host1"); + List<NodeRepositoryNode> allNodesInZone = new ArrayList<>(); + allNodesInZone.add(createNode("node1", "host1", "myapp", "default", 0 )); + allNodesInZone.add(createNode("node2", "host1", "myapp", "default", 0 )); + allNodesInZone.add(createNode("node3", "host1", "myapp", "default", 0 )); + + // Add an not impacted hosts + allNodesInZone.add(createNode("node4", "host2", "myapp", "default", 0 )); + + // Make Assessment + List<ChangeManagementAssessor.Assessment> assessments + = ChangeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); + + // Assess the assessment :-o + Assert.assertEquals(1, assessments.size()); + Assert.assertEquals(3, assessments.get(0).clusterImpact); + Assert.assertEquals(4, assessments.get(0).clusterSize); + Assert.assertEquals(1, assessments.get(0).groupsImpact); + Assert.assertEquals(1, assessments.get(0).groupsTotal); + Assert.assertEquals("content:default", assessments.get(0).cluster); + Assert.assertEquals("mytenant:myapp:default", assessments.get(0).app); + Assert.assertEquals("prod.eu-trd", assessments.get(0).zone); + } + + @Test + public void one_of_two_groups_in_one_of_two_clusters() { + ZoneId zone = ZoneId.from("prod", "eu-trd"); + List<String> hostNames = Arrays.asList("host1", "host2"); + List<NodeRepositoryNode> allNodesInZone = new ArrayList<>(); + + // Two impacted nodes on host1 + allNodesInZone.add(createNode("node1", "host1", "myapp", "default", 0 )); + allNodesInZone.add(createNode("node2", "host1", "myapp", "default", 0 )); + + // One impacted nodes on host2 + allNodesInZone.add(createNode("node3", "host2", "myapp", "default", 0 )); + + // Another group on hosts not impacted + allNodesInZone.add(createNode("node4", "host3", "myapp", "default", 1 )); + allNodesInZone.add(createNode("node5", "host3", "myapp", "default", 1 )); + allNodesInZone.add(createNode("node6", "host3", "myapp", "default", 1 )); + + // Another cluster on hosts not impacted - this one also with three different groups (should all be ignored here) + allNodesInZone.add(createNode("node4", "host4", "myapp", "myman", 4 )); + allNodesInZone.add(createNode("node5", "host4", "myapp", "myman", 5 )); + allNodesInZone.add(createNode("node6", "host4", "myapp", "myman", 6 )); + + // Make Assessment + List<ChangeManagementAssessor.Assessment> assessments + = ChangeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); + + // Assess the assessment :-o + Assert.assertEquals(1, assessments.size()); //One cluster is impacted + Assert.assertEquals(3, assessments.get(0).clusterImpact); + Assert.assertEquals(6, assessments.get(0).clusterSize); + Assert.assertEquals(1, assessments.get(0).groupsImpact); + Assert.assertEquals(2, assessments.get(0).groupsTotal); + Assert.assertEquals("content:default", assessments.get(0).cluster); + Assert.assertEquals("mytenant:myapp:default", assessments.get(0).app); + Assert.assertEquals("prod.eu-trd", assessments.get(0).zone); + } + + private NodeOwner createOwner(String tenant, String application, String instance) { + NodeOwner owner = new NodeOwner(); + owner.tenant = tenant; + owner.application = application; + owner.instance = instance; + return owner; + } + + private NodeMembership createMembership(String clusterId, int group) { + NodeMembership membership = new NodeMembership(); + membership.group = "" + group; + membership.clusterid = clusterId; + membership.clustertype = "content"; + membership.index = 2; + membership.retired = false; + return membership; + } + + private NodeRepositoryNode createNode(String nodename, String hostname, String appName, String clusterId, int group) { + NodeRepositoryNode node = new NodeRepositoryNode(); + node.setHostname(nodename); + node.setParentHostname(hostname); + node.setState(NodeState.active); + node.setOwner(createOwner("mytenant", appName, "default")); + node.setMembership(createMembership(clusterId, group)); + + return node; + } +}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainerTest.java new file mode 100644 index 00000000000..b4df16348d2 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeRequestMaintainerTest.java @@ -0,0 +1,59 @@ +package com.yahoo.vespa.hosted.controller.maintenance; + +import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequest; +import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequestSource; +import com.yahoo.vespa.hosted.controller.api.integration.vcmr.MockChangeRequestClient; +import org.junit.Test; + +import java.time.Duration; +import java.time.ZonedDateTime; +import java.util.List; + +import static org.junit.Assert.*; + +/** + * @author olaa + */ +public class ChangeRequestMaintainerTest { + + private final ControllerTester tester = new ControllerTester(); + private final MockChangeRequestClient changeRequestClient = tester.serviceRegistry().changeRequestClient(); + private final ChangeRequestMaintainer changeRequestMaintainer = new ChangeRequestMaintainer(tester.controller(), Duration.ofMinutes(1)); + + @Test + public void only_approve_requests_pending_approval() { + + var upcomingChangeRequests = List.of( + newChangeRequest("id1", ChangeRequest.Approval.APPROVED), + newChangeRequest("id2", ChangeRequest.Approval.REQUESTED) + ); + + changeRequestClient.setUpcomingChangeRequests(upcomingChangeRequests); + changeRequestMaintainer.maintain(); + + var approvedChangeRequests = changeRequestClient.getApprovedChangeRequests(); + + assertEquals(1, approvedChangeRequests.size()); + assertEquals("id2", approvedChangeRequests.get(0).getId()); + } + + private ChangeRequest newChangeRequest(String id, ChangeRequest.Approval approval) { + return new ChangeRequest.Builder() + .id(id) + .approval(approval) + .impact(ChangeRequest.Impact.VERY_HIGH) + .impactedSwitches(List.of()) + .impactedHosts(List.of()) + .changeRequestSource(new ChangeRequestSource.Builder() + .plannedStartTime(ZonedDateTime.now()) + .plannedEndTime(ZonedDateTime.now()) + .id("some-id") + .url("some-url") + .system("some-system") + .status(ChangeRequestSource.Status.CLOSED) + .build()) + .build(); + + } +}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java index 1467c1f6392..9790c8244c8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java @@ -97,7 +97,8 @@ public class TenantSerializerTest { ImmutableBiMap.of(publicKey, new SimplePrincipal("joe"), otherPublicKey, new SimplePrincipal("jane")), TenantInfo.EMPTY, - List.of() + List.of(), + Optional.empty() ); CloudTenant serialized = (CloudTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.name(), serialized.name()); @@ -118,7 +119,8 @@ public class TenantSerializerTest { List.of( new TenantSecretStore("ss1", "123", "role1"), new TenantSecretStore("ss2", "124", "role2") - ) + ), + Optional.of("role3") ); CloudTenant serialized = (CloudTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.info(), serialized.info()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java index ff0b3b83383..28f2b836365 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java @@ -2,8 +2,13 @@ package com.yahoo.vespa.hosted.controller.restapi.changemanagement; import com.yahoo.application.container.handler.Request; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzUser; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeMembership; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeOwner; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; +import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import org.intellij.lang.annotations.Language; @@ -11,6 +16,8 @@ import org.junit.Before; import org.junit.Test; import java.io.File; +import java.util.ArrayList; +import java.util.List; public class ChangeManagementApiHandlerTest extends ControllerContainerTest { @@ -23,11 +30,12 @@ public class ChangeManagementApiHandlerTest extends ControllerContainerTest { public void before() { tester = new ContainerTester(container, responses); addUserToHostedOperatorRole(operator); + tester.serviceRegistry().configServer().nodeRepository().addNodes(ZoneId.from("prod.us-east-3"), createNodes()); } @Test public void test_api() { - assertFile(new Request("http://localhost:8080/changemanagement/v1/assessment", "{}", Request.Method.POST), "initial.json"); + assertFile(new Request("http://localhost:8080/changemanagement/v1/assessment", "{\"zone\":\"prod.us-east-3\", \"hosts\": [\"host1\"]}", Request.Method.POST), "initial.json"); } private void assertResponse(Request request, @Language("JSON") String body, int statusCode) { @@ -39,4 +47,42 @@ public class ChangeManagementApiHandlerTest extends ControllerContainerTest { addIdentityToRequest(request, operator); tester.assertResponse(request, new File(filename)); } + + private List<NodeRepositoryNode> createNodes() { + List<NodeRepositoryNode> nodes = new ArrayList<>(); + nodes.add(createNode("node1", "host1", "myapp", "default", 0 )); + nodes.add(createNode("node2", "host1", "myapp", "default", 0 )); + nodes.add(createNode("node3", "host1", "myapp", "default", 0 )); + nodes.add(createNode("node4", "host2", "myapp", "default", 0 )); + return nodes; + } + + private NodeOwner createOwner(String tenant, String application, String instance) { + NodeOwner owner = new NodeOwner(); + owner.tenant = tenant; + owner.application = application; + owner.instance = instance; + return owner; + } + + private NodeMembership createMembership(String clusterId, int group) { + NodeMembership membership = new NodeMembership(); + membership.group = "" + group; + membership.clusterid = clusterId; + membership.clustertype = "content"; + membership.index = 2; + membership.retired = false; + return membership; + } + + private NodeRepositoryNode createNode(String nodename, String hostname, String appName, String clusterId, int group) { + NodeRepositoryNode node = new NodeRepositoryNode(); + node.setHostname(nodename); + node.setParentHostname(hostname); + node.setState(NodeState.active); + node.setOwner(createOwner("mytenant", appName, "default")); + node.setMembership(createMembership(clusterId, group)); + + return node; + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json index b402ff313dc..a9b0dacb968 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json @@ -3,33 +3,18 @@ "updated": "2021-03-12:12:12:12Z", "clusters": [ { - "app": "mytenant:myapp:myinstance", + "app": "mytenant:myapp:default", "zone": "prod.us-east-3", - "cluster": "mycontent", - "clusterSize": 19, + "cluster": "content:default", + "clusterSize": 4, "clusterImpact": 3, - "upgradePolicy": "10%", - "utilizationInWindow": 0.5, - "suggestedAction": "nothing|bcp", - "impact": "low|degraded|outage", - "groups": [ - { - "groupId": "23", - "groupSize": 4, - "groupImpact": 2 - } - ] + "groupsTotal": 1, + "groupsImpact": 1, + "upgradePolicy": "na", + "suggestedAction": "nothing", + "impact": "na" } ], - "hosts": [ - { - "hostname": "myhostname", - "resources": "vcpu:memory:disk", - "state": "active", - "containers": 10, - "suggestedAction": "nothing|retire", - "impact": "low|degraded|outage" - } - ] + "hosts": [] } }
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java index 85aa01a11be..efca19a61e1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java @@ -75,7 +75,8 @@ public class SignatureFilterTest { Optional.empty(), ImmutableBiMap.of(), TenantInfo.EMPTY, - List.of())); + List.of(), + Optional.empty())); tester.curator().writeApplication(new Application(appId, tester.clock().instant())); } @@ -120,7 +121,8 @@ public class SignatureFilterTest { Optional.empty(), ImmutableBiMap.of(publicKey, () -> "user"), TenantInfo.EMPTY, - List.of())); + List.of(), + Optional.empty())); verifySecurityContext(requestOf(signer.signed(request.copy(), Method.POST, () -> new ByteArrayInputStream(hiBytes)), hiBytes), new SecurityContext(new SimplePrincipal("user"), Set.of(Role.reader(id.tenant()), diff --git a/dist/vespa-engine.repo b/dist/vespa-engine.repo index 14e522f6993..33d439a9572 100644 --- a/dist/vespa-engine.repo +++ b/dist/vespa-engine.repo @@ -1,6 +1,8 @@ [vespa-engine-stable] name=vespa-engine-stable -baseurl=https://yahoo.bintray.com/vespa-engine/centos/$releasever/stable/$basearch -gpgcheck=0 +baseurl=https://download.copr.fedorainfracloud.org/results/@vespa/vespa/epel-7-$basearch/ +type=rpm-md +gpgcheck=1 +gpgkey=https://download.copr.fedorainfracloud.org/results/@vespa/vespa/pubkey.gpg repo_gpgcheck=0 enabled=1 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 478331dd513..38c3bd47e0d 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -228,7 +228,7 @@ public class Flags { TENANT_ID, ZONE_ID); public static final UnboundIntFlag CLUSTER_CONTROLLER_MAX_HEAP_SIZE_IN_MB = defineIntFlag( - "cluster-controller-max-heap-size-in-mb", 256, + "cluster-controller-max-heap-size-in-mb", 128, List.of("hmusum"), "2021-02-10", "2021-04-10", "JVM max heap size for cluster controller in Mb", "Takes effect when restarting cluster controller"); diff --git a/jdisc-cloud-aws/src/main/java/com/yahoo/jdisc/cloud/aws/AwsParameterStore.java b/jdisc-cloud-aws/src/main/java/com/yahoo/jdisc/cloud/aws/AwsParameterStore.java index f2cac68c030..595bb4f6786 100644 --- a/jdisc-cloud-aws/src/main/java/com/yahoo/jdisc/cloud/aws/AwsParameterStore.java +++ b/jdisc-cloud-aws/src/main/java/com/yahoo/jdisc/cloud/aws/AwsParameterStore.java @@ -82,7 +82,7 @@ public class AwsParameterStore extends AbstractComponent implements SecretStore } private static List<AwsSettings> translateConfig(SecretStoreConfig secretStoreConfig) { - return secretStoreConfig.groups() + return secretStoreConfig.awsParameterStores() .stream() .map(config -> new AwsSettings(config.name(), config.role(), config.awsId(), config.externalId(), config.region())) .collect(Collectors.toList()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java index a435814c21e..3f5255c6618 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java @@ -2,16 +2,11 @@ package com.yahoo.vespa.hosted.provision.autoscale; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.applications.Cluster; import java.time.Duration; -import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.function.Predicate; -import java.util.stream.Collectors; /** * A list of metric snapshots from a cluster, sorted by increasing time (newest last). @@ -85,8 +80,8 @@ public class ClusterTimeseries { else return 0.0; // ... because load is stable } - if (queryRateNow() == 0) return 0.1; // Growth not expressible as a fraction of the current rate - return maxGrowthRate / queryRateNow(); + if (currentQueryRate() == 0) return 0.1; // Growth not expressible as a fraction of the current rate + return maxGrowthRate / currentQueryRate(); } /** The current query rate as a fraction of the peak rate in this timeseries */ @@ -97,12 +92,22 @@ public class ClusterTimeseries { return snapshots.get(snapshots.size() - 1).queryRate() / max; } + public double currentQueryRate() { + return queryRateAt(snapshots.size() - 1); + } + + public double currentWriteRate() { + return writeRateAt(snapshots.size() - 1); + } + private double queryRateAt(int index) { + if (snapshots.isEmpty()) return 0.0; return snapshots.get(index).queryRate(); } - private double queryRateNow() { - return queryRateAt(snapshots.size() - 1); + private double writeRateAt(int index) { + if (snapshots.isEmpty()) return 0.0; + return snapshots.get(index).writeRate(); } private Duration durationBetween(int startIndex, int endIndex) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java index ab6a6d548e9..35717b97cf4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java @@ -75,6 +75,8 @@ public class ResourceTarget { public static double idealCpuLoad(Duration scalingDuration, ClusterTimeseries clusterTimeseries, Application application) { + double queryCpuFraction = queryCpuFraction(clusterTimeseries); + // What's needed to have headroom for growth during scale-up as a fraction of current resources? double maxGrowthRate = clusterTimeseries.maxQueryGrowthRate(); // in fraction per minute of the current traffic double growthRateHeadroom = 1 + maxGrowthRate * scalingDuration.toMinutes(); @@ -84,18 +86,40 @@ public class ResourceTarget { growthRateHeadroom = Math.min(growthRateHeadroom, 1 / fractionOfMax + 0.1); // How much headroom is needed to handle sudden arrival of additional traffic due to another zone going down? + double maxTrafficShiftHeadroom = 10.0; // Cap to avoid extreme sizes from a current very small share double trafficShiftHeadroom; if (application.status().maxReadShare() == 0) // No traffic fraction data trafficShiftHeadroom = 2.0; // assume we currently get half of the global share of traffic + else if (application.status().currentReadShare() == 0) + trafficShiftHeadroom = maxTrafficShiftHeadroom; else trafficShiftHeadroom = application.status().maxReadShare() / application.status().currentReadShare(); + trafficShiftHeadroom = Math.min(trafficShiftHeadroom, maxTrafficShiftHeadroom); - if (trafficShiftHeadroom > 2.0) // The expectation that we have almost no load with almost no queries is incorrect due - trafficShiftHeadroom = 2.0; // to write traffic; once that is separated we can increase this threshold + // Assumptions: 1) Write load is not organic so we should not grow to handle more. + // (TODO: But allow applications to set their target write rate and size for that) + // 2) Write load does not change in BCP scenarios. + return queryCpuFraction * 1 / growthRateHeadroom * 1 / trafficShiftHeadroom * idealQueryCpuLoad() + + (1 - queryCpuFraction) * idealWriteCpuLoad(); + } + + private static double queryCpuFraction(ClusterTimeseries clusterTimeseries) { + double queryRate = clusterTimeseries.currentQueryRate(); + double writeRate = clusterTimeseries.currentWriteRate(); + if (queryRate == 0 && writeRate == 0) return queryCpuFraction(0.5); + return queryCpuFraction(queryRate / (queryRate + writeRate)); + } - return 1 / growthRateHeadroom * 1 / trafficShiftHeadroom * Resource.cpu.idealAverageLoad(); + private static double queryCpuFraction(double queryFraction) { + double relativeQueryCost = 9; // How much more expensive are queries than writes? TODO: Measure + double writeFraction = 1 - queryFraction; + return queryFraction * relativeQueryCost / (queryFraction * relativeQueryCost + writeFraction); } + public static double idealQueryCpuLoad() { return Resource.cpu.idealAverageLoad(); } + + public static double idealWriteCpuLoad() { return 0.95; } + public static double idealMemoryLoad() { return Resource.memory.idealAverageLoad(); } public static double idealDiskLoad() { return Resource.disk.idealAverageLoad(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java index 9df6af4d02a..6ff4e1cc20d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java @@ -6,7 +6,6 @@ import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Deployer; import com.yahoo.jdisc.Metric; -import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Application; @@ -14,17 +13,15 @@ import com.yahoo.vespa.hosted.provision.applications.Applications; import com.yahoo.vespa.hosted.provision.applications.Cluster; import com.yahoo.vespa.hosted.provision.autoscale.AllocatableClusterResources; import com.yahoo.vespa.hosted.provision.autoscale.Autoscaler; -import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; +import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; import com.yahoo.vespa.hosted.provision.autoscale.NodeTimeseries; import com.yahoo.vespa.hosted.provision.node.History; import java.time.Duration; import java.time.Instant; -import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; /** * Maintainer making automatic scaling decisions @@ -57,12 +54,12 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { boolean success = true; if ( ! nodeRepository().zone().environment().isProduction()) return success; - activeNodesByApplication().forEach((applicationId, nodes) -> autoscale(applicationId, nodes)); + activeNodesByApplication().forEach(this::autoscale); return success; } - private void autoscale(ApplicationId application, List<Node> applicationNodes) { - nodesByCluster(applicationNodes).forEach((clusterId, clusterNodes) -> autoscale(application, clusterId, NodeList.copyOf(clusterNodes))); + private void autoscale(ApplicationId application, NodeList applicationNodes) { + nodesByCluster(applicationNodes).forEach((clusterId, clusterNodes) -> autoscale(application, clusterId, clusterNodes)); } private void autoscale(ApplicationId applicationId, ClusterSpec.Id clusterId, NodeList clusterNodes) { @@ -143,8 +140,8 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { return r + " (total: " + r.totalResources() + ")"; } - private Map<ClusterSpec.Id, List<Node>> nodesByCluster(List<Node> applicationNodes) { - return applicationNodes.stream().collect(Collectors.groupingBy(n -> n.allocation().get().membership().cluster().id())); + private Map<ClusterSpec.Id, NodeList> nodesByCluster(NodeList applicationNodes) { + return applicationNodes.groupingBy(n -> n.allocation().get().membership().cluster().id()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java index d0c02d7baaf..55548e70ddd 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java @@ -90,13 +90,14 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { /** Resume provisioning of already provisioned hosts and their children */ private void resumeProvisioning(NodeList nodes, Mutex lock) { - Map<String, Set<Node>> nodesByProvisionedParentHostname = nodes.nodeType(NodeType.tenant, NodeType.config).asList().stream() - .filter(node -> node.parentHostname().isPresent()) - .collect(Collectors.groupingBy( - node -> node.parentHostname().get(), - Collectors.toSet())); - - nodes.state(Node.State.provisioned).nodeType(NodeType.host, NodeType.confighost).forEach(host -> { + Map<String, Set<Node>> nodesByProvisionedParentHostname = + nodes.nodeType(NodeType.tenant, NodeType.config, NodeType.controller) + .asList() + .stream() + .filter(node -> node.parentHostname().isPresent()) + .collect(Collectors.groupingBy(node -> node.parentHostname().get(), Collectors.toSet())); + + nodes.state(Node.State.provisioned).nodeType(NodeType.host, NodeType.confighost, NodeType.controllerhost).forEach(host -> { Set<Node> children = nodesByProvisionedParentHostname.getOrDefault(host.hostname(), Set.of()); try { List<Node> updatedNodes = hostProvisioner.provision(host, children); @@ -189,6 +190,7 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer { // TODO: Mark empty tenant hosts as wanttoretire & wanttodeprovision elsewhere, then handle as confighost here return node.state() != Node.State.parked || node.status().wantToDeprovision(); case confighost: + case controllerhost: return node.state() == Node.State.parked && node.status().wantToDeprovision(); default: return false; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java index e6338d73a17..025c8be449c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java @@ -7,13 +7,12 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.NodeType; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import java.time.Clock; import java.time.Duration; -import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** * A maintainer is some job which runs at a fixed rate to perform some maintenance task on the node repo. @@ -41,13 +40,12 @@ public abstract class NodeRepositoryMaintainer extends Maintainer { protected Clock clock() { return nodeRepository.clock(); } /** A utility to group active tenant nodes by application */ - protected Map<ApplicationId, List<Node>> activeNodesByApplication() { - return nodeRepository().nodes().list(Node.State.active) + protected Map<ApplicationId, NodeList> activeNodesByApplication() { + return nodeRepository().nodes() + .list(Node.State.active) .nodeType(NodeType.tenant) - .asList() - .stream() - .filter(node -> ! node.allocation().get().owner().instance().isTester()) - .collect(Collectors.groupingBy(node -> node.allocation().get().owner())); + .matching(node -> ! node.allocation().get().owner().instance().isTester()) + .groupingBy(node -> node.allocation().get().owner()); } private static JobMetrics jobMetrics(Metric metric) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java index 49a33c4d120..f620a6d113d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java @@ -5,7 +5,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.NodeType; import com.yahoo.jdisc.Metric; -import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.History; @@ -13,7 +13,6 @@ import com.yahoo.vespa.hosted.provision.node.History; import java.time.Duration; import java.time.Instant; import java.util.LinkedHashSet; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -38,15 +37,14 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer { @Override protected Set<ApplicationId> applicationsNeedingMaintenance() { - Map<ApplicationId, List<Node>> nodesByApplication = nodeRepository().nodes().list() - .nodeType(NodeType.tenant, NodeType.proxy).asList().stream() - .filter(node -> node.allocation().isPresent()) - .collect(Collectors.groupingBy(node -> node.allocation().get().owner(), Collectors.toList())); - + Map<ApplicationId, NodeList> nodesByApplication = nodeRepository().nodes().list() + .nodeType(NodeType.tenant, NodeType.proxy) + .matching(node -> node.allocation().isPresent()) + .groupingBy(node -> node.allocation().get().owner()); return nodesByApplication.entrySet().stream() - .filter(entry -> hasNodesWithChanges(entry.getKey(), entry.getValue())) - .map(Map.Entry::getKey) - .collect(Collectors.toCollection(LinkedHashSet::new)); + .filter(entry -> hasNodesWithChanges(entry.getKey(), entry.getValue())) + .map(Map.Entry::getKey) + .collect(Collectors.toCollection(LinkedHashSet::new)); } /** @@ -61,15 +59,15 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer { " as a manual change was made to its nodes"); } - private boolean hasNodesWithChanges(ApplicationId applicationId, List<Node> nodes) { + private boolean hasNodesWithChanges(ApplicationId applicationId, NodeList nodes) { Optional<Instant> lastDeployTime = deployer().lastDeployTime(applicationId); if (lastDeployTime.isEmpty()) return false; return nodes.stream() - .flatMap(node -> node.history().events().stream()) - .filter(event -> event.agent() == Agent.operator) - .map(History.Event::at) - .anyMatch(e -> lastDeployTime.get().isBefore(e)); + .flatMap(node -> node.history().events().stream()) + .filter(event -> event.agent() == Agent.operator) + .map(History.Event::at) + .anyMatch(e -> lastDeployTime.get().isBefore(e)); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java index 1274e83fb3a..f72daf1bc2b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java @@ -50,15 +50,10 @@ public class RetiredExpirer extends NodeRepositoryMaintainer { @Override protected boolean maintain() { NodeList activeNodes = nodeRepository().nodes().list(Node.State.active); - - Map<ApplicationId, List<Node>> retiredNodesByApplication = activeNodes.stream() - .filter(node -> node.allocation().isPresent()) - .filter(node -> node.allocation().get().membership().retired()) - .collect(Collectors.groupingBy(node -> node.allocation().get().owner())); - - for (Map.Entry<ApplicationId, List<Node>> entry : retiredNodesByApplication.entrySet()) { + Map<ApplicationId, NodeList> retiredNodesByApplication = activeNodes.retired().groupingBy(node -> node.allocation().get().owner()); + for (Map.Entry<ApplicationId, NodeList> entry : retiredNodesByApplication.entrySet()) { ApplicationId application = entry.getKey(); - List<Node> retiredNodes = entry.getValue(); + NodeList retiredNodes = entry.getValue(); List<Node> nodesToRemove = retiredNodes.stream().filter(n -> canRemove(n, activeNodes)).collect(Collectors.toList()); if (nodesToRemove.isEmpty()) continue; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java index 3d6130c4116..e2b89879141 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainer.java @@ -7,7 +7,6 @@ import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.jdisc.Metric; import com.yahoo.transaction.Mutex; -import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Application; @@ -17,10 +16,8 @@ import com.yahoo.vespa.hosted.provision.autoscale.Autoscaler; import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; import java.time.Duration; -import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; /** * Maintainer computing scaling suggestions for all clusters @@ -49,10 +46,10 @@ public class ScalingSuggestionsMaintainer extends NodeRepositoryMaintainer { return successes > 0; } - private int suggest(ApplicationId application, List<Node> applicationNodes) { + private int suggest(ApplicationId application, NodeList applicationNodes) { int successes = 0; for (var cluster : nodesByCluster(applicationNodes).entrySet()) - successes += suggest(application, cluster.getKey(), NodeList.copyOf(cluster.getValue())) ? 1 : 0; + successes += suggest(application, cluster.getKey(), cluster.getValue()) ? 1 : 0; return successes; } @@ -99,8 +96,8 @@ public class ScalingSuggestionsMaintainer extends NodeRepositoryMaintainer { return r1.totalResources().cost() > r2.totalResources().cost(); } - private Map<ClusterSpec.Id, List<Node>> nodesByCluster(List<Node> applicationNodes) { - return applicationNodes.stream().collect(Collectors.groupingBy(n -> n.allocation().get().membership().cluster().id())); + private Map<ClusterSpec.Id, NodeList> nodesByCluster(NodeList applicationNodes) { + return applicationNodes.groupingBy(n -> n.allocation().get().membership().cluster().id()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java index 3c936e4e6ba..49eb44a4ec0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -100,13 +101,12 @@ class Activator { Optional<Application> application = nodeRepository.applications().get(transaction.application()); if (application.isEmpty()) return; // infrastructure app, hopefully :-| - var currentNodesByCluster = newNodes.stream() - .collect(Collectors.groupingBy(node -> node.allocation().get().membership().cluster().id())); + Map<ClusterSpec.Id, NodeList> currentNodesByCluster = newNodes.groupingBy(node -> node.allocation().get().membership().cluster().id()); Application modified = application.get(); for (var clusterEntry : currentNodesByCluster.entrySet()) { var cluster = modified.cluster(clusterEntry.getKey()).get(); var previousResources = oldNodes.cluster(clusterEntry.getKey()).toResources(); - var currentResources = NodeList.copyOf(clusterEntry.getValue()).toResources(); + var currentResources = clusterEntry.getValue().toResources(); if ( ! previousResources.justNumbers().equals(currentResources.justNumbers())) { cluster = cluster.with(ScalingEvent.create(previousResources, currentResources, generation, at)); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 597c4c1bd8c..0f725e6447a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -92,7 +92,7 @@ public class GroupPreparer { allocateOsRequirement); NodeType hostType = allocation.nodeType().hostType(); boolean hostTypeSupportsDynamicProvisioning = hostType == NodeType.host || - (hostType == NodeType.confighost && + (hostType.isConfigServerHostLike() && provisionConfigServerDynamically.value()); if (nodeRepository.zone().getCloud().dynamicProvisioning() && hostTypeSupportsDynamicProvisioning) { final Version osVersion; 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 b1bba656dc8..499eb3f23c0 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 @@ -82,7 +82,7 @@ public class LoadBalancerProvisioner { if (application.instance().isTester()) return; // Do not provision for tester instances try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); - List<Node> nodes = nodesOf(clusterId, application); + NodeList nodes = nodesOf(clusterId, application); LoadBalancerId loadBalancerId = requireNonClashing(new LoadBalancerId(application, clusterId)); ApplicationTransaction transaction = new ApplicationTransaction(new ProvisionLock(application, lock), new NestedTransaction()); provision(transaction, loadBalancerId, nodes, false); @@ -167,7 +167,7 @@ public class LoadBalancerProvisioner { } /** Idempotently provision a load balancer for given application and cluster */ - private void provision(ApplicationTransaction transaction, LoadBalancerId id, List<Node> nodes, boolean activate) { + private void provision(ApplicationTransaction transaction, LoadBalancerId id, NodeList nodes, boolean activate) { Instant now = nodeRepository.clock().instant(); Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id); if (loadBalancer.isEmpty() && activate) return; // Nothing to activate as this load balancer was never prepared @@ -185,7 +185,7 @@ public class LoadBalancerProvisioner { db.writeLoadBalancers(List.of(newLoadBalancer), transaction.nested()); } - private void provision(ApplicationTransaction transaction, ClusterSpec.Id clusterId, List<Node> nodes) { + private void provision(ApplicationTransaction transaction, ClusterSpec.Id clusterId, NodeList nodes) { provision(transaction, new LoadBalancerId(transaction.application(), clusterId), nodes, true); } @@ -204,12 +204,12 @@ public class LoadBalancerProvisioner { } /** Returns the nodes allocated to the given load balanced cluster */ - private List<Node> nodesOf(ClusterSpec.Id loadBalancedCluster, ApplicationId application) { - return loadBalancedClustersOf(application).getOrDefault(loadBalancedCluster, List.of()); + private NodeList nodesOf(ClusterSpec.Id loadBalancedCluster, ApplicationId application) { + return loadBalancedClustersOf(application).getOrDefault(loadBalancedCluster, NodeList.copyOf(List.of())); } /** Returns the load balanced clusters of given application and their nodes */ - private Map<ClusterSpec.Id, List<Node>> loadBalancedClustersOf(ApplicationId application) { + private Map<ClusterSpec.Id, NodeList> loadBalancedClustersOf(ApplicationId application) { NodeList nodes = nodeRepository.nodes().list(Node.State.reserved, Node.State.active).owner(application); if (nodes.stream().anyMatch(node -> node.type() == NodeType.config)) { nodes = nodes.nodeType(NodeType.config).type(ClusterSpec.Type.admin); @@ -218,11 +218,11 @@ public class LoadBalancerProvisioner { } else { nodes = nodes.nodeType(NodeType.tenant).container(); } - return nodes.stream().collect(Collectors.groupingBy(node -> effectiveId(node.allocation().get().membership().cluster()))); + return nodes.groupingBy(node -> effectiveId(node.allocation().get().membership().cluster())); } /** Returns real servers for given nodes */ - private Set<Real> realsOf(List<Node> nodes) { + private Set<Real> realsOf(NodeList nodes) { var reals = new LinkedHashSet<Real>(); for (var node : nodes) { for (var ip : reachableIpAddresses(node)) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 19c8d68963a..cd5355befbe 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -296,8 +296,8 @@ class NodeAllocation { * flavor and host count required to cover the deficit. */ Optional<HostDeficit> hostDeficit() { - if (nodeType() != NodeType.config && nodeType() != NodeType.tenant) { - return Optional.empty(); // Requests for these node types never have a deficit + if (nodeType().isHost()) { + return Optional.empty(); // Hosts are provisioned as required by the child application } return Optional.of(new HostDeficit(requestedNodes.resources().orElseGet(NodeResources::unspecified), requestedNodes.fulfilledDeficitCount(accepted()))) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index 3ff4765dd00..482f0f2e011 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -179,7 +179,9 @@ public interface NodeSpec { /** A node spec specifying a node type. This will accept all nodes of this type. */ class TypeNodeSpec implements NodeSpec { - private static final Map<NodeType, Integer> WANTED_NODE_COUNT = Map.of(NodeType.config, 3); + private static final Map<NodeType, Integer> WANTED_NODE_COUNT = Map.of( + NodeType.config, 3, + NodeType.controller, 3); private final NodeType type; @@ -207,10 +209,8 @@ public interface NodeSpec { @Override public int idealRetiredCount(int acceptedCount, int currentRetiredCount) { - // All nodes marked with wantToRetire get marked as retired just before this function is called, - // the job of this function is to throttle the retired count. If no nodes are marked as retired - // then continue this way, otherwise allow only 1 node to be retired - return Math.min(1, currentRetiredCount); + // All nodes marked with wantToRetire get marked as retired just before this function is called + return currentRetiredCount; } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index baf7d2dbe15..650bfe761b5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -15,7 +15,6 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.Nodelike; -import com.yahoo.vespa.hosted.provision.applications.Application; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import org.junit.Test; @@ -54,6 +53,7 @@ public class AutoscalingTest { assertTrue("Too few measurements -> No change", tester.autoscale(application1, cluster1.id(), min, max).isEmpty()); tester.clock().advance(Duration.ofDays(1)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); ClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high", 14, 1, 1.4, 30.8, 30.8, @@ -93,6 +93,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 1, resources); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); ClusterResources scaledResources = tester.assertResources("Scaling up since cpu usage is too high", @@ -122,6 +123,7 @@ public class AutoscalingTest { .allMatch(n -> n.allocation().get().requestedResources().diskSpeed() == NodeResources.DiskSpeed.slow); tester.clock().advance(Duration.ofDays(2)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); // Changing min and max from slow to any ClusterResources min = new ClusterResources( 2, 1, @@ -181,6 +183,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 1, new NodeResources(1.9, 70, 70, 1)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addMeasurements(0.25f, 0.95f, 0.95f, 0, 120, application1); tester.assertResources("Scaling up to limit since resource usage is too high", 6, 1, 2.4, 78.0, 79.0, @@ -217,6 +220,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 5, new NodeResources(3.0, 10, 10, 1)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements( 0.3f, 1f, 240, application1); tester.assertResources("Scaling up since resource usage is too high", 6, 6, 3.6, 8.0, 10.0, @@ -252,6 +256,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 1, resources); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); tester.assertResources("Scaling up since resource usage is too high", 7, 1, 2.5, 80.0, 80.0, @@ -304,6 +309,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 5, 5, resources); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); tester.assertResources("Scaling up since resource usage is too high", 7, 7, 2.5, 80.0, 80.0, @@ -322,6 +328,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 6, 2, resources); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); tester.assertResources("Scaling up since resource usage is too high, changing to 1 group is cheaper", 8, 1, 2.7, 83.3, 83.3, @@ -341,6 +348,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 6, 2, new NodeResources(10, 100, 100, 1)); tester.clock().advance(Duration.ofDays(1)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addMemMeasurements(1.0f, 1f, 1000, application1); tester.assertResources("Increase group size to reduce memory load", 8, 2, 13.6, 89.3, 62.5, @@ -360,6 +368,7 @@ public class AutoscalingTest { // deploy tester.deploy(application1, cluster1, 6, 1, hostResources.withVcpu(hostResources.vcpu() / 2)); tester.clock().advance(Duration.ofDays(2)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addMemMeasurements(0.02f, 0.95f, 120, application1); tester.assertResources("Scaling down", 6, 1, 2.9, 4.0, 95.0, @@ -377,6 +386,7 @@ public class AutoscalingTest { ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.content, "cluster1"); tester.deploy(application1, cluster1, 6, 1, hostResources.withVcpu(hostResources.vcpu() / 2)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only // No autoscaling as it is too soon to scale down after initial deploy (counting as a scaling event) tester.addMemMeasurements(0.02f, 0.95f, 120, application1); @@ -391,7 +401,7 @@ public class AutoscalingTest { } @Test - public void real_resources_are_taken_into_account() { + public void test_autoscaling_considers_real_resources() { NodeResources hostResources = new NodeResources(60, 100, 1000, 10); ClusterResources min = new ClusterResources(2, 1, new NodeResources( 2, 20, 200, 1)); ClusterResources max = new ClusterResources(4, 1, new NodeResources(60, 100, 1000, 1)); @@ -403,6 +413,7 @@ public class AutoscalingTest { ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.content, "cluster1"); tester.deploy(application1, cluster1, min); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addMeasurements(1.0f, 1.0f, 0.7f, 0, 1000, application1); tester.assertResources("Scaling up", 4, 1, 7.4, 20, 200, @@ -416,6 +427,7 @@ public class AutoscalingTest { ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.content, "cluster1"); tester.deploy(application1, cluster1, min); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addMeasurements(1.0f, 1.0f, 0.7f, 0, 1000, application1); tester.assertResources("Scaling up", 4, 1, 7.4, 34, 200, @@ -455,6 +467,7 @@ public class AutoscalingTest { tester.deactivateRetired(application1, cluster1, scaledResources); tester.clock().advance(Duration.ofDays(2)); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addMemMeasurements(0.3f, 0.6f, 1000, application1); tester.assertResources("Scaling down since resource usage has gone down", 6, 1, 3, 83, 28.8, @@ -472,6 +485,7 @@ public class AutoscalingTest { ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); tester.deploy(application1, cluster1, 5, 1, resources); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); // (no read share stored) @@ -502,6 +516,7 @@ public class AutoscalingTest { ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); tester.deploy(application1, cluster1, 5, 1, resources); + tester.addQueryRateMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0); // Query traffic only tester.addCpuMeasurements(0.25f, 1f, 120, application1); // (no query rate data) @@ -529,6 +544,50 @@ public class AutoscalingTest { } @Test + public void test_autoscaling_considers_query_vs_write_rate() { + NodeResources minResources = new NodeResources( 1, 100, 100, 1); + NodeResources midResources = new NodeResources( 5, 100, 100, 1); + NodeResources maxResources = new NodeResources(10, 100, 100, 1); + ClusterResources min = new ClusterResources(5, 1, minResources); + ClusterResources max = new ClusterResources(5, 1, maxResources); + AutoscalingTester tester = new AutoscalingTester(maxResources.withVcpu(maxResources.vcpu() * 2)); + + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); + + tester.deploy(application1, cluster1, 5, 1, midResources); + tester.addCpuMeasurements(0.4f, 1f, 120, application1); + + // Why twice the query rate at time = 0? + // This makes headroom for queries doubling, which we want to observe the effect of here + + tester.addLoadMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0, t -> 10.0); + tester.assertResources("Query and write load is equal -> scale up somewhat", + 5, 1, 7.3, 100, 100, + tester.autoscale(application1, cluster1.id(), min, max).target()); + + tester.addLoadMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 100.0 : 50.0, t -> 10.0); + tester.assertResources("Query load is 5x write load -> scale up more", + 5, 1, 9.7, 100, 100, + tester.autoscale(application1, cluster1.id(), min, max).target()); + + tester.addLoadMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0, t -> 100.0); + tester.assertResources("Write load is 10x query load -> scale down", + 5, 1, 3.8, 100, 100, + tester.autoscale(application1, cluster1.id(), min, max).target()); + + tester.addLoadMeasurements(application1, cluster1.id(), 10, t -> t == 0 ? 20.0 : 10.0, t-> 0.0); + tester.assertResources("Query only -> largest possible", + 5, 1, 10.0, 100, 100, + tester.autoscale(application1, cluster1.id(), min, max).target()); + + tester.addLoadMeasurements(application1, cluster1.id(), 10, t -> 0.0, t -> 10.0); + tester.assertResources("Write only -> smallest possible", + 5, 1, 2.1, 100, 100, + tester.autoscale(application1, cluster1.id(), min, max).target()); + } + + @Test public void test_cd_autoscaling_test() { NodeResources resources = new NodeResources(1, 4, 50, 1); ClusterResources min = new ClusterResources( 2, 1, resources); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java index 1949a6116d8..e24146d4752 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java @@ -236,6 +236,19 @@ class AutoscalingTester { } /** Creates the given number of measurements, spaced 5 minutes between, using the given function */ + public void addLoadMeasurements(ApplicationId application, + ClusterSpec.Id cluster, + int measurements, + IntFunction<Double> queryRate, + IntFunction<Double> writeRate) { + Instant time = clock().instant(); + for (int i = 0; i < measurements; i++) { + db.addClusterMetrics(application, Map.of(cluster, new ClusterMetricSnapshot(time, queryRate.apply(i), writeRate.apply(i)))); + time = time.plus(Duration.ofMinutes(5)); + } + } + + /** Creates the given number of measurements, spaced 5 minutes between, using the given function */ public void addQueryRateMeasurements(ApplicationId application, ClusterSpec.Id cluster, int measurements, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTargetTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTargetTest.java new file mode 100644 index 00000000000..f616e3e8b9d --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTargetTest.java @@ -0,0 +1,75 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.autoscale; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterResources; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.test.ManualClock; +import com.yahoo.vespa.hosted.provision.applications.Application; +import com.yahoo.vespa.hosted.provision.applications.Cluster; +import com.yahoo.vespa.hosted.provision.applications.Status; +import org.junit.Test; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.function.IntFunction; + +import static org.junit.Assert.assertEquals; + +/** + * @author bratseth + */ +public class ResourceTargetTest { + + private static final double delta = 0.001; + + @Test + public void test_traffic_headroom() { + Application application = Application.empty(ApplicationId.from("t1", "a1", "i1")); + Cluster cluster = new Cluster(ClusterSpec.Id.from("test"), + false, + new ClusterResources(5, 1, new NodeResources(1, 10, 100, 1)), + new ClusterResources(5, 1, new NodeResources(1, 10, 100, 1)), + Optional.empty(), + Optional.empty(), + List.of(), + ""); + application = application.with(cluster); + + // No current traffic: Ideal load is low but capped + application = application.with(new Status(0.0, 1.0)); + assertEquals(0.131, + ResourceTarget.idealCpuLoad(Duration.ofMinutes(10), + new ClusterTimeseries(cluster.id(), + loadSnapshots(100, t -> t == 0 ? 10000.0 : 0.0, t -> 0.0)), + application), + delta); + + // Almost current traffic: Ideal load is low but capped + application = application.with(new Status(0.0001, 1.0)); + assertEquals(0.131, + ResourceTarget.idealCpuLoad(Duration.ofMinutes(10), + new ClusterTimeseries(cluster.id(), + loadSnapshots(100, t -> t == 0 ? 10000.0 : 0.0, t -> 0.0)), + application), + delta); + } + + + /** Creates the given number of measurements, spaced 5 minutes between, using the given function */ + private List<ClusterMetricSnapshot> loadSnapshots(int measurements, + IntFunction<Double> queryRate, + IntFunction<Double> writeRate) { + List<ClusterMetricSnapshot> snapshots = new ArrayList<>(measurements); + ManualClock clock = new ManualClock(); + for (int i = 0; i < measurements; i++) { + snapshots.add(new ClusterMetricSnapshot(clock.instant(), queryRate.apply(i), writeRate.apply(i))); + clock.advance(Duration.ofMinutes(5)); + } + return snapshots; + } + +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java index 0c1a59c883d..f292ab8ccf1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -152,6 +152,7 @@ public class AutoscalingMaintainerTest { // deploy tester.deploy(app1, cluster1, app1Capacity); + tester.addQueryRateMeasurements(app1, cluster1.id(), 12, t -> t == 0 ? 20.0 : 10.0); for (int i = 0; i < 20; i++) { // Record completion to keep scaling window at minimum diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java index e8cfe6a2310..755f7608cd9 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTester.java @@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Cluster; +import com.yahoo.vespa.hosted.provision.autoscale.ClusterMetricSnapshot; import com.yahoo.vespa.hosted.provision.autoscale.NodeMetricSnapshot; import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; @@ -27,6 +28,7 @@ import java.time.Instant; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.function.IntFunction; import java.util.stream.Collectors; /** @@ -85,6 +87,18 @@ public class AutoscalingMaintainerTester { } } + /** Creates the given number of measurements, spaced 5 minutes between, using the given function */ + public void addQueryRateMeasurements(ApplicationId application, + ClusterSpec.Id cluster, + int measurements, + IntFunction<Double> queryRate) { + Instant time = clock().instant(); + for (int i = 0; i < measurements; i++) { + metricsDb.addClusterMetrics(application, Map.of(cluster, new ClusterMetricSnapshot(time, queryRate.apply(i), 0.0))); + time = time.plus(Duration.ofMinutes(5)); + } + } + public Cluster cluster(ApplicationId application, ClusterSpec cluster) { return nodeRepository().applications().get(application).get().cluster(cluster.id()).get(); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index 076a0e24620..48a6e03f646 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -36,6 +36,8 @@ import com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; import com.yahoo.vespa.service.duper.ConfigServerApplication; import com.yahoo.vespa.service.duper.ConfigServerHostApplication; +import com.yahoo.vespa.service.duper.ControllerApplication; +import com.yahoo.vespa.service.duper.ControllerHostApplication; import org.junit.Test; import java.time.Duration; @@ -421,6 +423,30 @@ public class DynamicProvisioningMaintainerTest { @Test public void replace_config_server() { + replace_config_server_like(NodeType.confighost); + } + + @Test + public void replace_controller() { + replace_config_server_like(NodeType.controllerhost); + } + + public void replace_config_server_like(NodeType hostType) { + final ApplicationId hostApp; + final ApplicationId configSrvApp; + switch (hostType) { + case confighost: + hostApp = new ConfigServerHostApplication().getApplicationId(); + configSrvApp = new ConfigServerApplication().getApplicationId(); + break; + case controllerhost: + hostApp = new ControllerHostApplication().getApplicationId(); + configSrvApp = new ControllerApplication().getApplicationId(); + break; + default: + throw new IllegalArgumentException("Unexpected config server host like node type: " + hostType); + } + Cloud cloud = Cloud.builder().dynamicProvisioning(true).build(); DynamicProvisioningTester dynamicProvisioningTester = new DynamicProvisioningTester(cloud, new MockNameResolver().mockAnyLookup()); ProvisioningTester tester = dynamicProvisioningTester.provisioningTester; @@ -428,24 +454,22 @@ public class DynamicProvisioningMaintainerTest { dynamicProvisioningTester.flagSource.withBooleanFlag(Flags.DYNAMIC_CONFIG_SERVER_PROVISIONING.id(), true); // Initial config server hosts are provisioned manually - ApplicationId hostApp = ApplicationId.from("hosted-vespa", "configserver-host", "default"); - List<Node> provisionedHosts = tester.makeReadyNodes(3, "default", NodeType.confighost).stream() + List<Node> provisionedHosts = tester.makeReadyNodes(3, "default", hostType).stream() .sorted(Comparator.comparing(Node::hostname)) .collect(Collectors.toList()); - tester.prepareAndActivateInfraApplication(hostApp, NodeType.confighost); + tester.prepareAndActivateInfraApplication(hostApp, hostType); // Provision config servers - ApplicationId configSrvApp = ApplicationId.from("hosted-vespa", "zone-config-servers", "default"); for (int i = 0; i < provisionedHosts.size(); i++) { - tester.makeReadyChildren(1, i + 1, NodeResources.unspecified(), NodeType.config, - provisionedHosts.get(i).hostname(), (nodeIndex) -> "cfg" + nodeIndex); + tester.makeReadyChildren(1, i + 1, NodeResources.unspecified(), hostType.childNodeType(), + provisionedHosts.get(i).hostname(), (nodeIndex) -> "cfg" + nodeIndex); } - tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); // Expected number of hosts and children are provisioned NodeList allNodes = tester.nodeRepository().nodes().list(); - NodeList configHosts = allNodes.nodeType(NodeType.confighost); - NodeList configNodes = allNodes.nodeType(NodeType.config); + NodeList configHosts = allNodes.nodeType(hostType); + NodeList configNodes = allNodes.nodeType(hostType.childNodeType()); assertEquals(3, configHosts.size()); assertEquals(3, configNodes.size()); String hostnameToRemove = provisionedHosts.get(1).hostname(); @@ -456,20 +480,20 @@ public class DynamicProvisioningMaintainerTest { tester.nodeRepository().nodes().deprovision(hostToRemove.get(), Agent.system, tester.clock().instant()); // Redeployment of config server application retires node - tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); assertTrue("Redeployment retires node", nodeToRemove.get().allocation().get().membership().retired()); // Config server becomes removable (done by RetiredExpirer in a real system) and redeployment moves it // to inactive tester.nodeRepository().nodes().setRemovable(configSrvApp, List.of(nodeToRemove.get())); - tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); assertEquals("Node moves to inactive", Node.State.inactive, nodeToRemove.get().state()); // Node is completely removed (done by InactiveExpirer and host-admin in a real system) Node inactiveConfigServer = nodeToRemove.get(); int removedIndex = inactiveConfigServer.allocation().get().membership().index(); tester.nodeRepository().nodes().removeRecursively(inactiveConfigServer, true); - assertEquals(2, tester.nodeRepository().nodes().list().nodeType(NodeType.config).size()); + assertEquals(2, tester.nodeRepository().nodes().list().nodeType(hostType.childNodeType()).size()); // ExpiredRetirer moves host to inactive after child has moved to parked tester.nodeRepository().nodes().deallocate(hostToRemove.get(), Agent.system, getClass().getSimpleName()); @@ -477,38 +501,38 @@ public class DynamicProvisioningMaintainerTest { // Host is removed dynamicProvisioningTester.maintainer.maintain(); - assertEquals(2, tester.nodeRepository().nodes().list().nodeType(NodeType.confighost).size()); + assertEquals(2, tester.nodeRepository().nodes().list().nodeType(hostType).size()); // Deployment by the removed host has no effect HostName.setHostNameForTestingOnly("cfg2.example.com"); - tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); assertEquals(List.of(), dynamicProvisioningTester.hostProvisioner.provisionedHosts()); // Deployment on another config server starts provisioning a new host and child HostName.setHostNameForTestingOnly("cfg3.example.com"); - assertEquals(0, tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(NodeType.config).size()); - assertEquals(2, tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config).size()); - assertEquals(1, tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(NodeType.config).size()); - Node newNode = tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(NodeType.config).first().get(); + assertEquals(0, tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(hostType.childNodeType()).size()); + assertEquals(2, tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()).size()); + assertEquals(1, tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(hostType.childNodeType()).size()); + Node newNode = tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(hostType.childNodeType()).first().get(); // Resume provisioning and activate host dynamicProvisioningTester.maintainer.maintain(); List<ProvisionedHost> newHosts = dynamicProvisioningTester.hostProvisioner.provisionedHosts(); assertEquals(1, newHosts.size()); tester.nodeRepository().nodes().setReady(newHosts.get(0).hostHostname(), Agent.operator, getClass().getSimpleName()); - tester.prepareAndActivateInfraApplication(hostApp, NodeType.confighost); - assertEquals(3, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.confighost).size()); + tester.prepareAndActivateInfraApplication(hostApp, hostType); + assertEquals(3, tester.nodeRepository().nodes().list(Node.State.active).nodeType(hostType).size()); // Redeployment of config server app actives new node - tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); + tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); newNode = tester.nodeRepository().nodes().node(newNode.hostname()).get(); assertSame(Node.State.active, newNode.state()); assertEquals("Removed index is reused", removedIndex, newNode.allocation().get().membership().index()); // Next redeployment does nothing - NodeList nodesBefore = tester.nodeRepository().nodes().list().nodeType(NodeType.config); - tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config); - NodeList nodesAfter = tester.nodeRepository().nodes().list().nodeType(NodeType.config); + NodeList nodesBefore = tester.nodeRepository().nodes().list().nodeType(hostType.childNodeType()); + tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); + NodeList nodesAfter = tester.nodeRepository().nodes().list().nodeType(hostType.childNodeType()); assertEquals(nodesBefore, nodesAfter); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java index 718facd477c..924d38cc6c2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java @@ -42,7 +42,6 @@ import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -191,25 +190,20 @@ public class RetiredExpirerTest { // Redeploy to retire all 3 config servers infraDeployer.activateAllSupportedInfraApplications(true); + List<Node> retiredNodes = tester.nodeRepository().nodes().list().retired().asList(); + assertEquals(3, retiredNodes.size()); - // Only 1 config server is allowed to retire at any given point in time - List<Node> retiredNodes = tester.nodeRepository().nodes().list(() -> {}).stream() - .filter(node -> node.allocation().map(allocation -> allocation.membership().retired()).orElse(false)) - .collect(Collectors.toList()); - assertEquals(1, retiredNodes.size()); - Node retiredNode = retiredNodes.get(0); - String retiredNodeHostname = retiredNode.hostname(); - - // Allow retiredNodeHostname to be removed + // The Orchestrator will allow only 1 to be removed, say cfg1 + Node retiredNode = tester.nodeRepository().nodes().node(cfg1.s()).orElseThrow(); doThrow(new OrchestrationException("denied")).when(orchestrator).acquirePermissionToRemove(any()); - doNothing().when(orchestrator).acquirePermissionToRemove(eq(new HostName(retiredNodeHostname))); + doNothing().when(orchestrator).acquirePermissionToRemove(eq(new HostName(retiredNode.hostname()))); // RetiredExpirer should remove cfg1 from application RetiredExpirer retiredExpirer = createRetiredExpirer(deployer); retiredExpirer.run(); var activeConfigServerHostnames = new HashSet<>(Set.of("cfg1", "cfg2", "cfg3")); - assertTrue(activeConfigServerHostnames.contains(retiredNodeHostname)); - activeConfigServerHostnames.remove(retiredNodeHostname); + assertTrue(activeConfigServerHostnames.contains(retiredNode.hostname())); + activeConfigServerHostnames.remove(retiredNode.hostname()); assertEquals(activeConfigServerHostnames, configServerHostnames(duperModel)); assertEquals(1, tester.nodeRepository().nodes().list(Node.State.inactive).nodeType(NodeType.config).size()); assertEquals(2, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.config).size()); @@ -234,8 +228,8 @@ public class RetiredExpirerTest { // Provision and ready new config server MockNameResolver nameResolver = (MockNameResolver)tester.nodeRepository().nameResolver(); String ipv4 = "127.0.1.4"; - nameResolver.addRecord(retiredNodeHostname, ipv4); - Node node = Node.create(retiredNodeHostname, new IP.Config(Set.of(ipv4), Set.of()), retiredNodeHostname, + nameResolver.addRecord(retiredNode.hostname(), ipv4); + Node node = Node.create(retiredNode.hostname(), new IP.Config(Set.of(ipv4), Set.of()), retiredNode.hostname(), tester.asFlavor("default", NodeType.config), NodeType.config).build(); var nodes = List.of(node); nodes = nodeRepository.nodes().addNodes(nodes, Agent.system); @@ -252,14 +246,16 @@ public class RetiredExpirerTest { infraDeployer.activateAllSupportedInfraApplications(true); assertEquals(3, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.config).size()); - // Another config server should now have retired + // There are now 2 retired config servers left retiredExpirer.run(); assertEquals(3, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.config).size()); - var retiredNodes2 = tester.nodeRepository().nodes().list(() -> {}).stream() - .filter(n -> n.allocation().map(allocation -> allocation.membership().retired()).orElse(false)) - .collect(Collectors.toList()); - assertEquals(1, retiredNodes2.size()); - assertNotEquals(retiredNodeHostname, retiredNodes2.get(0)); + var retiredHostnames = tester.nodeRepository() + .nodes().list(() -> {}) + .stream() + .filter(n -> n.allocation().map(allocation -> allocation.membership().retired()).orElse(false)) + .map(Node::hostname) + .collect(Collectors.toSet()); + assertEquals(Set.of("cfg2", "cfg3"), retiredHostnames); } private Set<String> configServerHostnames(MockDuperModel duperModel) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java index 88d39e887d3..9ae67cef235 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java @@ -72,9 +72,9 @@ public class ScalingSuggestionsMaintainerTest { new TestMetric()); maintainer.maintain(); - assertEquals("14 nodes with [vcpu: 6.9, memory: 5.1 Gb, disk 15.0 Gb, bandwidth: 0.1 Gbps]", + assertEquals("12 nodes with [vcpu: 6.0, memory: 5.1 Gb, disk 15.0 Gb, bandwidth: 0.1 Gbps]", suggestionOf(app1, cluster1, tester).get().resources().toString()); - assertEquals("9 nodes with [vcpu: 13.8, memory: 4.0 Gb, disk 10.3 Gb, bandwidth: 0.1 Gbps]", + assertEquals("8 nodes with [vcpu: 11.0, memory: 4.0 Gb, disk 11.8 Gb, bandwidth: 0.1 Gbps]", suggestionOf(app2, cluster2, tester).get().resources().toString()); // Utilization goes way down @@ -82,14 +82,14 @@ public class ScalingSuggestionsMaintainerTest { addMeasurements(0.10f, 0.10f, 0.10f, 0, 500, app1, tester.nodeRepository(), metricsDb); maintainer.maintain(); assertEquals("Suggestion stays at the peak value observed", - "14 nodes with [vcpu: 6.9, memory: 5.1 Gb, disk 15.0 Gb, bandwidth: 0.1 Gbps]", + "12 nodes with [vcpu: 6.0, memory: 5.1 Gb, disk 15.0 Gb, bandwidth: 0.1 Gbps]", suggestionOf(app1, cluster1, tester).get().resources().toString()); // Utilization is still way down and a week has passed tester.clock().advance(Duration.ofDays(7)); addMeasurements(0.10f, 0.10f, 0.10f, 0, 500, app1, tester.nodeRepository(), metricsDb); maintainer.maintain(); assertEquals("Peak suggestion has been outdated", - "6 nodes with [vcpu: 2.0, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps]", + "5 nodes with [vcpu: 1.8, memory: 4.0 Gb, disk 10.0 Gb, bandwidth: 0.1 Gbps]", suggestionOf(app1, cluster1, tester).get().resources().toString()); assertTrue(shouldSuggest(app1, cluster1, tester)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java index 6e50c934047..e94d1c1230e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeTypeProvisioningTest.java @@ -19,10 +19,10 @@ import java.time.Duration; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; /** @@ -183,7 +183,6 @@ public class NodeTypeProvisioningTest { List<Node> nodesToRetire = tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.proxy).asList() .subList(3, 3 + numNodesToRetire); - String currentyRetiringHostname; { nodesToRetire.forEach(nodeToRetire -> tester.nodeRepository().nodes().write(nodeToRetire.withWantToRetire(true, Agent.system, tester.clock().instant()), () -> {})); @@ -198,14 +197,13 @@ public class NodeTypeProvisioningTest { List<Node> nodesCurrentlyRetiring = nodes.stream() .filter(node -> node.allocation().get().membership().retired()) .collect(Collectors.toList()); - assertEquals(1, nodesCurrentlyRetiring.size()); + assertEquals(5, nodesCurrentlyRetiring.size()); - // The retiring node should be one of the nodes we marked for retirement - currentyRetiringHostname = nodesCurrentlyRetiring.get(0).hostname(); - assertEquals(1, nodesToRetire.stream().map(Node::hostname).filter(hostname -> hostname.equals(currentyRetiringHostname)).count()); + // The retiring nodes should be the nodes we marked for retirement + assertTrue(Set.copyOf(nodesToRetire).containsAll(nodesCurrentlyRetiring)); } - { // Redeploying while the node is still retiring has no effect + { // Redeploying while the nodes are still retiring has no effect List<HostSpec> hosts = deployProxies(application, tester); assertEquals(11, hosts.size()); tester.activate(application, new HashSet<>(hosts)); @@ -216,57 +214,29 @@ public class NodeTypeProvisioningTest { List<Node> nodesCurrentlyRetiring = nodes.stream() .filter(node -> node.allocation().get().membership().retired()) .collect(Collectors.toList()); - assertEquals(1, nodesCurrentlyRetiring.size()); - - // The node that started retiring is still the only one retiring - assertEquals(currentyRetiringHostname, nodesCurrentlyRetiring.get(0).hostname()); + assertEquals(5, nodesCurrentlyRetiring.size()); } { + // Let all retired nodes expire tester.advanceTime(Duration.ofMinutes(11)); retiredExpirer.run(); List<HostSpec> hosts = deployProxies(application, tester); - assertEquals(10, hosts.size()); + assertEquals(6, hosts.size()); tester.activate(application, new HashSet<>(hosts)); - NodeList nodes = tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.proxy); - assertEquals(10, nodes.size()); - // Verify the node we previously set to retire has finished retiring - assertEquals(Node.State.dirty, tester.nodeRepository().nodes().node(currentyRetiringHostname) - .orElseThrow(RuntimeException::new).state()); - - // Verify that a node is currently retiring - List<Node> nodesCurrentlyRetiring = nodes.stream() - .filter(node -> node.allocation().get().membership().retired()) - .collect(Collectors.toList()); - assertEquals(1, nodesCurrentlyRetiring.size()); + // All currently active proxy nodes are not marked with wantToRetire or as retired + long numRetiredActiveProxyNodes = tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.proxy).stream() + .filter(node -> !node.status().wantToRetire()) + .filter(node -> !node.allocation().get().membership().retired()) + .count(); + assertEquals(6, numRetiredActiveProxyNodes); - // This node is different from the one that was retiring previously - String newRetiringHostname = nodesCurrentlyRetiring.get(0).hostname(); - assertNotEquals(currentyRetiringHostname, newRetiringHostname); - // ... but is one of the nodes that were put to wantToRetire earlier - assertTrue(nodesToRetire.stream().map(Node::hostname).filter(hostname -> hostname.equals(newRetiringHostname)).count() == 1); + // All the nodes that were marked with wantToRetire earlier are now dirty + assertEquals(nodesToRetire.stream().map(Node::hostname).collect(Collectors.toSet()), + tester.nodeRepository().nodes().list(Node.State.dirty).stream().map(Node::hostname).collect(Collectors.toSet())); } - - - for (int i = 0; i < 10; i++){ - tester.advanceTime(Duration.ofMinutes(11)); - retiredExpirer.run(); - List<HostSpec> hosts = deployProxies(application, tester); - tester.activate(application, new HashSet<>(hosts)); - } - - // After a long time, all currently active proxy nodes are not marked with wantToRetire or as retired - long numRetiredActiveProxyNodes = tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.proxy).stream() - .filter(node -> !node.status().wantToRetire()) - .filter(node -> !node.allocation().get().membership().retired()) - .count(); - assertEquals(11 - numNodesToRetire, numRetiredActiveProxyNodes); - - // All the nodes that were marked with wantToRetire earlier are now dirty - assertEquals(nodesToRetire.stream().map(Node::hostname).collect(Collectors.toSet()), - tester.nodeRepository().nodes().list(Node.State.dirty).stream().map(Node::hostname).collect(Collectors.toSet())); } private List<HostSpec> deployProxies(ApplicationId application, ProvisioningTester tester) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json index 1e9a2d60837..65e07c46242 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application1.json @@ -66,7 +66,7 @@ }, "utilization" : { "cpu" : 0.0, - "idealCpu": 0.2, + "idealCpu": 0.275, "memory" : 0.0, "idealMemory": 0.7, "disk" : 0.0, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json index 376b748ff8e..ecab55d19d4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/application2.json @@ -42,7 +42,7 @@ }, "utilization" : { "cpu" : 0.0, - "idealCpu": 0.19047619047619047, + "idealCpu": 0.2664285714285714, "memory" : 0.0, "idealMemory": 0.7, "disk" : 0.0, diff --git a/searchlib/abi-spec.json b/searchlib/abi-spec.json index d412f408350..9e958dd4d4c 100644 --- a/searchlib/abi-spec.json +++ b/searchlib/abi-spec.json @@ -897,6 +897,7 @@ "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorXwPlusB()", "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorArgmax()", "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorArgmin()", + "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorCellCast()", "public final com.yahoo.searchlib.rankingexpression.rule.LambdaFunctionNode lambdaFunction()", "public final com.yahoo.tensor.functions.Reduce$Aggregator tensorReduceAggregator()", "public final com.yahoo.tensor.TensorType tensorType(java.util.List)", @@ -1046,6 +1047,7 @@ "public static final int XW_PLUS_B", "public static final int ARGMAX", "public static final int ARGMIN", + "public static final int CELL_CAST", "public static final int AVG", "public static final int COUNT", "public static final int MAX", diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj index 36b1f9627bb..7506fe250fc 100755 --- a/searchlib/src/main/javacc/RankingExpressionParser.jj +++ b/searchlib/src/main/javacc/RankingExpressionParser.jj @@ -141,6 +141,7 @@ TOKEN : <XW_PLUS_B: "xw_plus_b"> | <ARGMAX: "argmax"> | <ARGMIN: "argmin"> | + <CELL_CAST: "cell_cast"> | <AVG: "avg" > | <COUNT: "count"> | @@ -380,7 +381,8 @@ TensorFunctionNode tensorFunction() : tensorExpression = tensorSoftmax() | tensorExpression = tensorXwPlusB() | tensorExpression = tensorArgmax() | - tensorExpression = tensorArgmin() + tensorExpression = tensorArgmin() | + tensorExpression = tensorCellCast() ) { return tensorExpression; } } @@ -597,6 +599,16 @@ TensorFunctionNode tensorArgmin() : { return new TensorFunctionNode(new Argmin(TensorFunctionNode.wrap(tensor), dimensions)); } } +TensorFunctionNode tensorCellCast() : +{ + ExpressionNode tensor; + String valueType; +} +{ + <CELL_CAST> <LBRACE> tensor = expression() <COMMA> valueType = identifier() <RBRACE> + { return new TensorFunctionNode(new CellCast(TensorFunctionNode.wrap(tensor), TensorType.Value.fromId(valueType)));} +} + LambdaFunctionNode lambdaFunction() : { List<String> variables; @@ -667,7 +679,7 @@ String tensorFunctionName() : ( <MAP> { return token.image; } ) | ( <REDUCE> { return token.image; } ) | ( <JOIN> { return token.image; } ) | - ( <MERGE> { return token.image; } ) | + ( <MERGE> { return token.image; } ) | ( <RENAME> { return token.image; } ) | ( <CONCAT> { return token.image; } ) | ( <TENSOR> { return token.image; } ) | @@ -681,6 +693,7 @@ String tensorFunctionName() : ( <XW_PLUS_B> { return token.image; } ) | ( <ARGMAX> { return token.image; } ) | ( <ARGMIN> { return token.image; } ) | + ( <CELL_CAST> { return token.image; } ) | ( aggregator = tensorReduceAggregator() { return aggregator.toString(); } ) } diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java index 123fa5ac43b..5c4840a555d 100644 --- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java @@ -394,6 +394,22 @@ public class EvaluationTestCase { } @Test + public void testCellTypeCasting() { + EvaluationTester tester = new EvaluationTester(); + + tester.assertEvaluates("tensor<float>(x[3]):[1.0, 2.0, 3.0]", + "cell_cast(tensor0, float)", + "tensor<double>(x[3]):[1, 2, 3]"); + tester.assertEvaluates("tensor<float>():{1}", + "cell_cast(tensor0{x:1}, float)", + "tensor<double>(x{}):{1:1, 2:2, 3:3}"); + tester.assertEvaluates("tensor<float>(x[2]):[3,8]", + "cell_cast(tensor0 * tensor1, float)", + "tensor<float>(x[2]):[1,2]", + "tensor<double>(x[2]):[3,4]"); + } + + @Test public void testMixedTensorType() throws ParseException { String expected = "tensor(x[1],y{},z[2]):{{x:0,y:a,z:0}:4.0,{x:0,y:a,z:1}:5.0,{x:0,y:b,z:0}:7.0,{x:0,y:b,z:1}:8.0}"; String a = "tensor(x[1],y{}):{ {x:0,y:a}:1, {x:0,y:b}:2 }"; diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp index f7a2a507aec..0f0b9bfc78d 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.cpp @@ -129,6 +129,13 @@ EnumStoreDictionary<DictionaryT>::find_matching_enums(const vespalib::datastore: return result; } +template <typename DictionaryT> +IEnumStore::Index +EnumStoreDictionary<DictionaryT>::remap_index(Index idx) +{ + return idx; +} + template <> void EnumStoreDictionary<EnumTree>::clear_all_posting_lists(std::function<void(EntryRef)>) @@ -254,6 +261,14 @@ EnumStoreFoldedDictionary::remove(const EntryComparator& comp, EntryRef ref) } } +IEnumStore::Index +EnumStoreFoldedDictionary::remap_index(Index idx) +{ + auto itr = _dict.find(idx, *_folded_compare); + assert(itr.valid()); + return itr.getKey(); +} + template class EnumStoreDictionary<EnumTree>; template class EnumStoreDictionary<EnumPostingTree>; diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h index 67bbb1cd3e9..085806e3446 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_dictionary.h @@ -16,9 +16,9 @@ template <typename DictionaryT> class EnumStoreDictionary : public vespalib::datastore::UniqueStoreDictionary<DictionaryT, IEnumStoreDictionary> { protected: using EntryRef = IEnumStoreDictionary::EntryRef; + using Index = IEnumStoreDictionary::Index; private: using EnumVector = IEnumStoreDictionary::EnumVector; - using Index = IEnumStoreDictionary::Index; using IndexSet = IEnumStoreDictionary::IndexSet; using IndexVector = IEnumStoreDictionary::IndexVector; using ParentUniqueStoreDictionary = vespalib::datastore::UniqueStoreDictionary<DictionaryT, IEnumStoreDictionary>; @@ -49,6 +49,7 @@ public: std::vector<attribute::IAttributeVector::EnumHandle> find_matching_enums(const vespalib::datastore::EntryComparator& cmp) const override; + Index remap_index(Index idx) override; void clear_all_posting_lists(std::function<void(EntryRef)> clearer) override; void update_posting_list(Index idx, const vespalib::datastore::EntryComparator& cmp, std::function<EntryRef(EntryRef)> updater) override; EnumPostingTree& get_posting_dictionary() override; @@ -74,6 +75,7 @@ public: ~EnumStoreFoldedDictionary() override; vespalib::datastore::UniqueStoreAddResult add(const vespalib::datastore::EntryComparator& comp, std::function<vespalib::datastore::EntryRef(void)> insertEntry) override; void remove(const vespalib::datastore::EntryComparator& comp, vespalib::datastore::EntryRef ref) override; + Index remap_index(Index idx) override; }; } diff --git a/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h b/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h index 8bbd7677595..2abb91f94a6 100644 --- a/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h +++ b/searchlib/src/vespa/searchlib/attribute/i_enum_store_dictionary.h @@ -47,6 +47,7 @@ public: virtual std::vector<attribute::IAttributeVector::EnumHandle> find_matching_enums(const vespalib::datastore::EntryComparator& cmp) const = 0; + virtual Index remap_index(Index idx) = 0; virtual void clear_all_posting_lists(std::function<void(EntryRef)> clearer) = 0; virtual void update_posting_list(Index idx, const vespalib::datastore::EntryComparator& cmp, std::function<EntryRef(EntryRef)> updater) = 0; virtual EnumPostingTree& get_posting_dictionary() = 0; diff --git a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.cpp b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.cpp index 3c41265620f..0ce8711ccfc 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.cpp @@ -9,9 +9,9 @@ LOG_SETUP(".searchlib.attribute.multi_string_post_attribute"); namespace search { IEnumStore::Index -StringEnumIndexMapper::map(IEnumStore::Index original, const vespalib::datastore::EntryComparator& compare) const +StringEnumIndexMapper::map(IEnumStore::Index original) const { - return _dictionary.find(original, compare).getKey(); + return _dictionary.remap_index(original); } } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp index f97a4e281a8..afd7da6911d 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp @@ -29,11 +29,11 @@ MultiValueStringPostingAttributeT<B, T>::~MultiValueStringPostingAttributeT() class StringEnumIndexMapper : public EnumIndexMapper { public: - StringEnumIndexMapper(const EnumPostingTree & dictionary) : _dictionary(dictionary) { } - IEnumStore::Index map(IEnumStore::Index original, const vespalib::datastore::EntryComparator& compare) const override; + StringEnumIndexMapper(IEnumStoreDictionary & dictionary) : _dictionary(dictionary) { } + IEnumStore::Index map(IEnumStore::Index original) const override; virtual bool hasFold() const override { return true; } private: - const EnumPostingTree & _dictionary; + IEnumStoreDictionary& _dictionary; }; template <typename B, typename T> @@ -43,10 +43,10 @@ applyValueChanges(const DocIndices& docIndices, EnumStoreBatchUpdater &updater) { using PostingChangeComputer = PostingChangeComputerT<WeightedIndex, PostingMap>; EnumStore &enumStore(this->getEnumStore()); - Dictionary &dict(enumStore.get_posting_dictionary()); + IEnumStoreDictionary& dictionary(enumStore.get_dictionary()); auto compare = enumStore.make_folded_comparator(); - StringEnumIndexMapper mapper(dict); + StringEnumIndexMapper mapper(dictionary); PostingMap changePost(PostingChangeComputer::compute(this->getMultiValueMapping(), docIndices, compare, mapper)); this->updatePostings(changePost); MultiValueStringAttributeT<B, T>::applyValueChanges(docIndices, updater); diff --git a/searchlib/src/vespa/searchlib/attribute/postingchange.cpp b/searchlib/src/vespa/searchlib/attribute/postingchange.cpp index fc8a3c7deed..4d4fd3aa9bf 100644 --- a/searchlib/src/vespa/searchlib/attribute/postingchange.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postingchange.cpp @@ -102,9 +102,8 @@ removeDupRemovals(std::vector<uint32_t> &removals) } IEnumStore::Index -EnumIndexMapper::map(IEnumStore::Index original, const vespalib::datastore::EntryComparator& compare) const +EnumIndexMapper::map(IEnumStore::Index original) const { - (void) compare; return original; } @@ -159,7 +158,7 @@ private: EnumIndex mapEnumIndex(EnumIndex unmapped) { auto itr = _cachedMapping.insert(std::make_pair(unmapped.ref(), 0)); if (itr.second) { - itr.first->second = _mapper.map(unmapped, _compare).ref(); + itr.first->second = _mapper.map(unmapped).ref(); } return EnumIndex(vespalib::datastore::EntryRef(itr.first->second)); } diff --git a/searchlib/src/vespa/searchlib/attribute/postingchange.h b/searchlib/src/vespa/searchlib/attribute/postingchange.h index 459fd2b8211..4a358b63046 100644 --- a/searchlib/src/vespa/searchlib/attribute/postingchange.h +++ b/searchlib/src/vespa/searchlib/attribute/postingchange.h @@ -49,7 +49,7 @@ class EnumIndexMapper { public: virtual ~EnumIndexMapper() { } - virtual IEnumStore::Index map(IEnumStore::Index original, const vespalib::datastore::EntryComparator& compare) const; + virtual IEnumStore::Index map(IEnumStore::Index original) const; virtual bool hasFold() const { return false; } }; diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp index 29e9ad39073..325874091ef 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp @@ -78,7 +78,7 @@ void SingleValueNumericPostingAttribute<B>::applyValueChanges(EnumStoreBatchUpdater& updater) { EnumStore & enumStore = this->getEnumStore(); - Dictionary & dict = enumStore.get_posting_dictionary(); + IEnumStoreDictionary& dictionary = enumStore.get_dictionary(); auto cmp = enumStore.make_comparator(); PostingMap changePost; @@ -101,9 +101,8 @@ SingleValueNumericPostingAttribute<B>::applyValueChanges(EnumStoreBatchUpdater& if (oldIdx.valid()) { T oldValue = enumStore.get_value(oldIdx); T newValue = this->applyArithmetic(oldValue, change); - - auto addItr = dict.find(EnumIndex(), enumStore.make_comparator(newValue)); - EnumIndex newIdx = addItr.getKey(); + EnumIndex newIdx; + (void) dictionary.find_index(enumStore.make_comparator(newValue), newIdx); currEnumIndices[change._doc] = newIdx; } } else if(change._type == ChangeBase::CLEARDOC) { diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h index 60a4bc022be..397fad60be3 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.h @@ -67,7 +67,7 @@ private: void makePostingChange(const vespalib::datastore::EntryComparator *cmp, - Dictionary &dict, + IEnumStoreDictionary& dictionary, const std::map<DocId, EnumIndex> &currEnumIndices, PostingMap &changePost); diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp index e91deb61f36..50ed621b13e 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp @@ -53,7 +53,7 @@ template <typename B> void SingleValueStringPostingAttributeT<B>:: makePostingChange(const vespalib::datastore::EntryComparator *cmpa, - Dictionary &dict, + IEnumStoreDictionary& dictionary, const std::map<DocId, EnumIndex> &currEnumIndices, PostingMap &changePost) { @@ -63,13 +63,13 @@ makePostingChange(const vespalib::datastore::EntryComparator *cmpa, EnumIndex newIdx = elem.second; // add new posting - auto addItr = dict.find(newIdx, *cmpa); - changePost[EnumPostingPair(addItr.getKey(), cmpa)].add(docId, 1); + auto remapped_new_idx = dictionary.remap_index(newIdx); + changePost[EnumPostingPair(remapped_new_idx, cmpa)].add(docId, 1); // remove old posting if ( oldIdx.valid()) { - auto rmItr = dict.find(oldIdx, *cmpa); - changePost[EnumPostingPair(rmItr.getKey(), cmpa)].remove(docId); + auto remapped_old_idx = dictionary.remap_index(oldIdx); + changePost[EnumPostingPair(remapped_old_idx, cmpa)].remove(docId); } } } @@ -79,7 +79,7 @@ void SingleValueStringPostingAttributeT<B>::applyValueChanges(EnumStoreBatchUpdater& updater) { EnumStore & enumStore = this->getEnumStore(); - Dictionary & dict = enumStore.get_posting_dictionary(); + IEnumStoreDictionary& dictionary = enumStore.get_dictionary(); auto cmp = enumStore.make_folded_comparator(); PostingMap changePost; @@ -104,7 +104,7 @@ SingleValueStringPostingAttributeT<B>::applyValueChanges(EnumStoreBatchUpdater& } } - makePostingChange(&cmp, dict, currEnumIndices, changePost); + makePostingChange(&cmp, dictionary, currEnumIndices, changePost); this->updatePostings(changePost); diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index c6727aa372e..e51569da988 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -1167,6 +1167,7 @@ "public com.yahoo.tensor.Tensor concat(com.yahoo.tensor.Tensor, java.lang.String)", "public com.yahoo.tensor.Tensor rename(java.util.List, java.util.List)", "public static com.yahoo.tensor.Tensor generate(com.yahoo.tensor.TensorType, java.util.function.Function)", + "public com.yahoo.tensor.Tensor cellCast(com.yahoo.tensor.TensorType$Value)", "public com.yahoo.tensor.Tensor l1Normalize(java.lang.String)", "public com.yahoo.tensor.Tensor l2Normalize(java.lang.String)", "public com.yahoo.tensor.Tensor matmul(com.yahoo.tensor.Tensor, java.lang.String)", @@ -1569,6 +1570,23 @@ ], "fields": [] }, + "com.yahoo.tensor.functions.CellCast": { + "superClass": "com.yahoo.tensor.functions.PrimitiveTensorFunction", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(com.yahoo.tensor.functions.TensorFunction, com.yahoo.tensor.TensorType$Value)", + "public java.util.List arguments()", + "public com.yahoo.tensor.functions.TensorFunction withArguments(java.util.List)", + "public com.yahoo.tensor.functions.PrimitiveTensorFunction toPrimitive()", + "public com.yahoo.tensor.TensorType type(com.yahoo.tensor.evaluation.TypeContext)", + "public com.yahoo.tensor.Tensor evaluate(com.yahoo.tensor.evaluation.EvaluationContext)", + "public java.lang.String toString(com.yahoo.tensor.functions.ToStringContext)" + ], + "fields": [] + }, "com.yahoo.tensor.functions.CompositeTensorFunction": { "superClass": "com.yahoo.tensor.functions.TensorFunction", "interfaces": [], @@ -3445,4 +3463,4 @@ ], "fields": [] } -}
\ No newline at end of file +} diff --git a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java index 4a24cdcc7bf..ec85cb6de56 100644 --- a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java +++ b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java @@ -5,15 +5,18 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Random; import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; import java.util.stream.Stream; import static java.util.stream.Collectors.toUnmodifiableList; @@ -86,6 +89,14 @@ public abstract class AbstractFilteringList<Type, ListType extends AbstractFilte return constructor.apply(items.stream().sorted(comparator).collect(toUnmodifiableList()), false); } + /** Returns the items grouped by the given classifier. */ + public final <OtherType> Map<OtherType, ListType> groupingBy(Function<Type, OtherType> classifier) { + return items.stream().collect(Collectors.groupingBy(classifier, + HashMap::new, + Collectors.collectingAndThen(toUnmodifiableList(), + (list) -> constructor.apply(list, false)))); + } + public final boolean isEmpty() { return items.isEmpty(); } public final int size() { return items.size(); } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java index fbf5bc35129..3378520dc91 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java @@ -4,6 +4,7 @@ package com.yahoo.tensor; import com.yahoo.tensor.evaluation.TypeContext; import com.yahoo.tensor.functions.Argmax; import com.yahoo.tensor.functions.Argmin; +import com.yahoo.tensor.functions.CellCast; import com.yahoo.tensor.functions.Concat; import com.yahoo.tensor.functions.ConstantTensor; import com.yahoo.tensor.functions.Diag; @@ -179,6 +180,10 @@ public interface Tensor { return new Generate<>(type, valueSupplier).evaluate(); } + default Tensor cellCast(TensorType.Value valueType) { + return new CellCast<>(new ConstantTensor<>(this), valueType).evaluate(); + } + // ----------------- Composite tensor functions which have a defined primitive mapping default Tensor l1Normalize(String dimension) { diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/CellCast.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/CellCast.java new file mode 100644 index 00000000000..d052e383c85 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/CellCast.java @@ -0,0 +1,83 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.tensor.functions; + +import com.yahoo.tensor.Tensor; +import com.yahoo.tensor.TensorType; +import com.yahoo.tensor.evaluation.EvaluationContext; +import com.yahoo.tensor.evaluation.Name; +import com.yahoo.tensor.evaluation.TypeContext; + +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Objects; + +/** + * The <i>cell_cast</i> tensor function creates a new tensor with the specified cell value type. + * + * @author lesters + */ +public class CellCast<NAMETYPE extends Name> extends PrimitiveTensorFunction<NAMETYPE> { + + private final TensorFunction<NAMETYPE> argument; + private final TensorType.Value valueType; + + public CellCast(TensorFunction<NAMETYPE> argument, TensorType.Value valueType) { + Objects.requireNonNull(argument, "The argument tensor cannot be null"); + Objects.requireNonNull(valueType, "The value type cannot be null"); + this.argument = argument; + this.valueType = valueType; + } + + @Override + public List<TensorFunction<NAMETYPE>> arguments() { return Collections.singletonList(argument); } + + @Override + public TensorFunction<NAMETYPE> withArguments(List<TensorFunction<NAMETYPE>> arguments) { + if ( arguments.size() != 1) + throw new IllegalArgumentException("CellCast must have 1 argument, got " + arguments.size()); + return new CellCast<>(arguments.get(0), valueType); + } + + @Override + public PrimitiveTensorFunction<NAMETYPE> toPrimitive() { + return new CellCast<>(argument.toPrimitive(), valueType); + } + + @Override + public TensorType type(TypeContext<NAMETYPE> context) { + return new TensorType(valueType, argument.type(context).dimensions()); + } + + @Override + public Tensor evaluate(EvaluationContext<NAMETYPE> context) { + Tensor tensor = argument.evaluate(context); + if (tensor.type().valueType() == valueType) { + return tensor; + } + TensorType type = new TensorType(valueType, tensor.type().dimensions()); + return cast(tensor, type); + } + + private Tensor cast(Tensor tensor, TensorType type) { + Tensor.Builder builder = Tensor.Builder.of(type); + TensorType.Value fromValueType = tensor.type().valueType(); + for (Iterator<Tensor.Cell> i = tensor.cellIterator(); i.hasNext(); ) { + Tensor.Cell cell = i.next(); + if (fromValueType == TensorType.Value.FLOAT) { + builder.cell(cell.getKey(), cell.getFloatValue()); + } else if (fromValueType == TensorType.Value.DOUBLE) { + builder.cell(cell.getKey(), cell.getDoubleValue()); + } else { + builder.cell(cell.getKey(), cell.getValue()); + } + } + return builder.build(); + } + + @Override + public String toString(ToStringContext context) { + return "cell_cast(" + argument.toString(context) + ", " + valueType + ")"; + } + +} diff --git a/vespajlib/src/test/java/com/yahoo/tensor/functions/CellCastTestCase.java b/vespajlib/src/test/java/com/yahoo/tensor/functions/CellCastTestCase.java new file mode 100644 index 00000000000..bc10ecc3abd --- /dev/null +++ b/vespajlib/src/test/java/com/yahoo/tensor/functions/CellCastTestCase.java @@ -0,0 +1,38 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.tensor.functions; + +import com.yahoo.tensor.Tensor; +import com.yahoo.tensor.TensorType; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author lesters + */ +public class CellCastTestCase { + + @Test + public void testCellCasting() { + Tensor tensor; + + tensor = Tensor.from("tensor(x[3]):[1.0, 2.0, 3.0]"); + assertEquals(TensorType.Value.DOUBLE, tensor.type().valueType()); + assertEquals(TensorType.Value.DOUBLE, tensor.cellCast(TensorType.Value.DOUBLE).type().valueType()); + assertEquals(TensorType.Value.FLOAT, tensor.cellCast(TensorType.Value.FLOAT).type().valueType()); + assertEquals(tensor, tensor.cellCast(TensorType.Value.FLOAT)); + + tensor = Tensor.from("tensor<double>(x{}):{{x:0}:1.0,{x:1}:2.0,{x:2}:3.0}"); + assertEquals(TensorType.Value.DOUBLE, tensor.type().valueType()); + assertEquals(TensorType.Value.DOUBLE, tensor.cellCast(TensorType.Value.DOUBLE).type().valueType()); + assertEquals(TensorType.Value.FLOAT, tensor.cellCast(TensorType.Value.FLOAT).type().valueType()); + assertEquals(tensor, tensor.cellCast(TensorType.Value.FLOAT)); + + tensor = Tensor.from("tensor<float>(x[3],y{}):{a:[1.0, 2.0, 3.0],b:[4.0,5.0,6.0]}"); + assertEquals(TensorType.Value.FLOAT, tensor.type().valueType()); + assertEquals(TensorType.Value.DOUBLE, tensor.cellCast(TensorType.Value.DOUBLE).type().valueType()); + assertEquals(TensorType.Value.FLOAT, tensor.cellCast(TensorType.Value.FLOAT).type().valueType()); + assertEquals(tensor, tensor.cellCast(TensorType.Value.DOUBLE)); + } + +} diff --git a/zookeeper-command-line-client/src/main/sh/vespa-zkcli b/zookeeper-command-line-client/src/main/sh/vespa-zkcli index 1a5858b3222..0406e3d89c2 100755 --- a/zookeeper-command-line-client/src/main/sh/vespa-zkcli +++ b/zookeeper-command-line-client/src/main/sh/vespa-zkcli @@ -80,19 +80,15 @@ usage() { echo "" echo "-h|-help) print this help text" - echo "-nosudo do not use sudo when running command" } -sudo="/usr/bin/sudo -u ${VESPA_USER}" while [ $# -gt 0 ]; do case $1 in -h|-help) usage; exit 0;; - -nosudo) shift; sudo="" ;; *) echo "Unrecognized option '$1'" >&2; exit 1;; esac done -$sudo java \ - -cp $VESPA_HOME/lib/jars/zookeeper-command-line-client-jar-with-dependencies.jar \ - -Dlog4j.configuration="log4j-vespa.properties" -Xms32m -Xmx512m \ - com.yahoo.vespa.zookeeper.cli.Main "$@" +java -cp $VESPA_HOME/lib/jars/zookeeper-command-line-client-jar-with-dependencies.jar \ + -Dlog4j.configuration="log4j-vespa.properties" -Xms32m -Xmx512m \ + com.yahoo.vespa.zookeeper.cli.Main "$@" |