Merge pull request #3368 from vespa-engine/mpolden/fix-deployment-spec-cast

Fix deployment spec cast in container cluster model

7 files changed, 73 insertions, 25 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
index 8530679e82f..098f5620723 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
@@ -140,10 +140,11 @@ public class DeploymentSpec {
     /** Returns the deployment steps of this in the order they will be performed */
     public List<Step> steps() { return steps; }
 
-    /** Returns only the DeclaredZone deployment steps of this in the order they will be performed */
+    /** Returns all the DeclaredZone deployment steps in the order they are declared */
     public List<DeclaredZone> zones() {
-        return steps.stream().filter(step -> step instanceof DeclaredZone).map(DeclaredZone.class::cast)
-                             .collect(Collectors.toList());
+        return steps.stream()
+                .flatMap(step -> step.zones().stream())
+                .collect(Collectors.toList());
     }
 
     /** Returns the XML form of this spec, or null if it was not created by fromXml, nor is empty */
diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
index 8b97eb2503e..ddee0be6e9c 100644
--- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
+++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
@@ -22,7 +22,7 @@ import java.util.*;
 /**
  * For testing purposes only
  *
- * @author tonytv
+ * @author Tony Vaagenes
  */
 public class MockApplicationPackage implements ApplicationPackage {
 
@@ -33,17 +33,17 @@ public class MockApplicationPackage implements ApplicationPackage {
     private final String servicesS;
     private final List<String> searchDefinitions;
     private final String searchDefinitionDir;
-    private final Optional<String> deploymentInfo;
+    private final Optional<String> deploymentSpec;
     private final Optional<String> validationOverrides;
     private final boolean failOnValidateXml;
 
     private MockApplicationPackage(String hosts, String services, List<String> searchDefinitions, String searchDefinitionDir,
-                                   String deploymentInfo, String validationOverrides, boolean failOnValidateXml) {
+                                   String deploymentSpec, String validationOverrides, boolean failOnValidateXml) {
         this.hostsS = hosts;
         this.servicesS = services;
         this.searchDefinitions = searchDefinitions;
         this.searchDefinitionDir = searchDefinitionDir;
-        this.deploymentInfo = Optional.ofNullable(deploymentInfo);
+        this.deploymentSpec = Optional.ofNullable(deploymentSpec);
         this.validationOverrides = Optional.ofNullable(validationOverrides);
         this.failOnValidateXml = failOnValidateXml;
     }
@@ -111,7 +111,7 @@ public class MockApplicationPackage implements ApplicationPackage {
 
     @Override
     public Optional<Reader> getDeployment() {
-        return deploymentInfo.map(StringReader::new);
+        return deploymentSpec.map(StringReader::new);
     }
 
     @Override
@@ -151,7 +151,7 @@ public class MockApplicationPackage implements ApplicationPackage {
         private String services = null;
         private List<String> searchDefinitions = Collections.emptyList();
         private String searchDefinitionDir = null;
-        private String deploymentInfo = null;
+        private String deploymentSpec = null;
         private String validationOverrides = null;
         private boolean failOnValidateXml = false;
 
@@ -191,8 +191,8 @@ public class MockApplicationPackage implements ApplicationPackage {
             return this;
         }
 
-        public Builder withDeploymentInfo(String deploymentInfo) {
-            this.deploymentInfo = deploymentInfo;
+        public Builder withDeploymentSpec(String deploymentSpec) {
+            this.deploymentSpec = deploymentSpec;
             return this;
         }
 
@@ -208,7 +208,7 @@ public class MockApplicationPackage implements ApplicationPackage {
 
         public ApplicationPackage build() {
                 return new MockApplicationPackage(hosts, services, searchDefinitions, searchDefinitionDir,
-                                                  deploymentInfo, validationOverrides, failOnValidateXml);
+                                                  deploymentSpec, validationOverrides, failOnValidateXml);
         }
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
index 8af6f94d903..32a9158dbcf 100755
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
@@ -107,7 +107,7 @@ import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX
 /**
  * @author gjoranv
  * @author Einar M R Rosenvinge
- * @author tonytv
+ * @author Tony Vaagenes
  */
 public final class ContainerCluster
         extends AbstractConfigProducer<AbstractConfigProducer<?>>
@@ -411,12 +411,13 @@ public final class ContainerCluster
     }
 
     private boolean zoneHasActiveRotation(Zone zone) {
-        return getDeploymentSpec()
-                .flatMap(spec -> spec.steps().stream()
-                        .filter(dz -> dz.deploysTo(zone.environment(), Optional.of(zone.region())))
-                        .findFirst())
-                .map(step -> ((DeploymentSpec.DeclaredZone)step).active())
-                .orElse(false);
+        Optional<DeploymentSpec> spec = getDeploymentSpec();
+        if (!spec.isPresent()) {
+            return false;
+        }
+        return spec.get().zones().stream()
+                .anyMatch(declaredZone -> declaredZone.deploysTo(zone.environment(), Optional.of(zone.region())) &&
+                                          declaredZone.active());
     }
 
     private Optional<DeploymentSpec> getDeploymentSpec() {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidatorTest.java
index 152cae6aff4..58b2c09fc74 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidatorTest.java
@@ -16,7 +16,7 @@ import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
 /**
- * @author <a href="mailto:simon@yahoo-inc.com">Harald Musum</a>
+ * @author hmusum
  */
 public class DeploymentFileValidatorTest {
 
@@ -41,7 +41,7 @@ public class DeploymentFileValidatorTest {
                 "   </jdisc>" +
                 "</services>";
 
-        final String deploymentInfo = "<?xml version='1.0' encoding='UTF-8'?>" +
+        final String deploymentSpec = "<?xml version='1.0' encoding='UTF-8'?>" +
                 "<deployment version='1.0'>" +
                 "  <test />" +
                 "  <prod global-service-id='non-existing'>" +
@@ -52,7 +52,7 @@ public class DeploymentFileValidatorTest {
         ApplicationPackage app = new MockApplicationPackage.Builder()
                 .withHosts(simpleHosts)
                 .withServices(services)
-                .withDeploymentInfo(deploymentInfo)
+                .withDeploymentSpec(deploymentSpec)
                 .build();
         DeployState.Builder builder = new DeployState.Builder().applicationPackage(app);
         try {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
index 1a655510052..af846f10ffe 100755
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
@@ -4,8 +4,10 @@ package com.yahoo.vespa.model.container;
 import com.yahoo.cloud.config.ClusterInfoConfig;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.cloud.config.RoutingProviderConfig;
+import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.deploy.DeployProperties;
 import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.config.model.test.MockApplicationPackage;
 import com.yahoo.config.model.test.MockRoot;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
@@ -24,6 +26,7 @@ import com.yahoo.vespa.model.container.search.ContainerSearch;
 import com.yahoo.vespa.model.container.search.searchchain.SearchChains;
 import org.junit.Test;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.Optional;
@@ -33,7 +36,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 /**
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a>
+ * @author Simon Thoresen Hult
  */
 public class ContainerClusterTest {
 
@@ -263,6 +266,49 @@ public class ContainerClusterTest {
         assertEquals(0, cluster.getAllComponents().stream().map(c -> c.getClassId().getName()).filter(c -> c.equals("com.yahoo.jdisc.http.filter.security.RoutingConfigProvider")).count());
     }
 
+    @Test
+    public void setsRotationActiveAccordingToDeploymentSpec() {
+        String deploymentSpec = "<deployment>\n" +
+                                "  <prod>    \n" +
+                                "    <region active='true'>us-north-1</region>\n" +
+                                "    <parallel>\n" +
+                                "      <region active='false'>us-north-2</region>\n" +
+                                "      <region active='true'>us-north-3</region>\n" +
+                                "    </parallel>\n" +
+                                "    <region active='false'>us-north-4</region>\n" +
+                                "  </prod>\n" +
+                                "</deployment>";
+        for (String region : Arrays.asList("us-north-1", "us-north-3")) {
+            Container container = containerIn(region, deploymentSpec);
+            assertEquals("Region " + region + " is active", "true",
+                         container.getServicePropertyString("activeRotation"));
+        }
+        for (String region : Arrays.asList("us-north-2", "us-north-4")) {
+            Container container = containerIn(region, deploymentSpec);
+            assertEquals("Region " + region + " is inactive", "false",
+                         container.getServicePropertyString("activeRotation"));
+        }
+        Container container = containerIn("unknown", deploymentSpec);
+        assertEquals("Unknown region is inactive", "false",
+                     container.getServicePropertyString("activeRotation"));
+    }
+
+    private static Container containerIn(String regionName, String deploymentSpec) {
+        ApplicationPackage applicationPackage = new MockApplicationPackage.Builder()
+                .withDeploymentSpec(deploymentSpec)
+                .build();
+        DeployState state = new DeployState.Builder()
+                .applicationPackage(applicationPackage)
+                .properties(new DeployProperties.Builder().hostedVespa(true).zone(
+                        new Zone(Environment.prod, RegionName.from(regionName))).build()
+                )
+                .build();
+        MockRoot root = new MockRoot("foo", state);
+        ContainerCluster cluster = new ContainerCluster(root, "container0", "container1");
+        addContainer(cluster, "c1", "c1.domain");
+        return cluster.getContainers().get(0);
+    }
+
     private static void addContainer(ContainerCluster cluster, String name, String hostName) {
         Container container = new Container(cluster, name, 0);
         container.setHostResource(new HostResource(new Host(null, hostName)));
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java
index 108d54ae546..4d7f33e6ef9 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java
@@ -190,7 +190,7 @@ public class LbServicesProducerTest {
                 "  </prod>" +
                 "</deployment>";
 
-        return new MockApplicationPackage.Builder().withHosts(hosts).withServices(services).withDeploymentInfo(deploymentInfo).build();
+        return new MockApplicationPackage.Builder().withHosts(hosts).withServices(services).withDeploymentSpec(deploymentInfo).build();
     }
 
     private Model createVespaModel(ApplicationPackage applicationPackage, DeployState.Builder deployStateBuilder) throws IOException, SAXException {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java
index f4fee50170f..f50dc7828b0 100755
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/model/RoutingProducerTest.java
@@ -100,7 +100,7 @@ public class RoutingProducerTest {
         return new MockApplicationPackage.Builder()
                 .withHosts(hosts)
                 .withServices(services)
-                .withDeploymentInfo(deploymentInfo)
+                .withDeploymentSpec(deploymentInfo)
                 .build();
     }