Merge pull request #15206 from vespa-engine/jonmv/display-reindexing-actions-in-controller-deployments

ConfigChangeActions now includes ReindexActions as well

4 files changed, 52 insertions, 19 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index a97bf55e17d..e275b4e9dfd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -572,7 +572,7 @@ public class ApplicationController {
                            " as a deployment is not currently expected";
         PrepareResponse prepareResponse = new PrepareResponse();
         prepareResponse.log = List.of(logEntry);
-        prepareResponse.configChangeActions = new ConfigChangeActions(List.of(), List.of());
+        prepareResponse.configChangeActions = new ConfigChangeActions(List.of(), List.of(), List.of());
         return new ActivateResult(new RevisionId("0"), prepareResponse, 0);
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index cba78843cba..224f7734766 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -236,6 +236,23 @@ public class InternalStepRunner implements StepRunner {
                 return Optional.of(deploymentFailed);
             }
 
+            if ( ! prepareResponse.configChangeActions.reindexActions.stream().allMatch(action -> action.allowed)) {
+                List<String> messages = new ArrayList<>();
+                messages.add("Deploy failed due to non-compatible changes that require re-index.");
+                messages.add("Your options are:");
+                messages.add("1. Revert the incompatible changes.");
+                messages.add("2. If you think it is safe in your case, you can override this validation, see");
+                messages.add("   http://docs.vespa.ai/documentation/reference/validation-overrides.html");
+                messages.add("3. Deploy as a new application under a different name.");
+                messages.add("Illegal actions:");
+                prepareResponse.configChangeActions.reindexActions.stream()
+                                                                 .filter(action -> ! action.allowed)
+                                                                 .flatMap(action -> action.messages.stream())
+                                                                 .forEach(messages::add);
+                logger.log(messages);
+                return Optional.of(deploymentFailed);
+            }
+
             logger.log("Deployment successful.");
             if (prepareResponse.message != null)
                 logger.log(prepareResponse.message);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index a58a12a1861..7b248052eac 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -16,6 +16,7 @@ import com.yahoo.slime.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.RoutingController;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RefeedAction;
+import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ReindexAction;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RestartAction;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ServiceInfo;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
@@ -124,14 +125,29 @@ public class InternalStepRunnerTest {
     }
 
     @Test
+    public void reindexRequirementBlocksDeployment() {
+        tester.configServer().setConfigChangeActions(new ConfigChangeActions(List.of(),
+                                                                             List.of(),
+                                                                             List.of(new ReindexAction("Reindex",
+                                                                                                       false,
+                                                                                                       "doctype",
+                                                                                                       "cluster",
+                                                                                                       Collections.emptyList(),
+                                                                                                       List.of("Reindex it!")))));
+        tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.empty(), applicationPackage);
+        assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.devUsEast1).get().stepStatuses().get(Step.deployReal));
+    }
+
+    @Test
     public void refeedRequirementBlocksDeployment() {
-        tester.configServer().setConfigChangeActions(new ConfigChangeActions(Collections.emptyList(),
-                                                                             singletonList(new RefeedAction("Refeed",
-                                                                                                            false,
-                                                                                                            "doctype",
-                                                                                                            "cluster",
-                                                                                                            Collections.emptyList(),
-                                                                                                            singletonList("Refeed it!")))));
+        tester.configServer().setConfigChangeActions(new ConfigChangeActions(List.of(),
+                                                                             List.of(new RefeedAction("Refeed",
+                                                                                                      false,
+                                                                                                      "doctype",
+                                                                                                      "cluster",
+                                                                                                      Collections.emptyList(),
+                                                                                                      singletonList("Refeed it!"))),
+                                                                             List.of()));
         tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.empty(), applicationPackage);
         assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.devUsEast1).get().stepStatuses().get(Step.deployReal));
     }
@@ -142,15 +158,16 @@ public class InternalStepRunnerTest {
         ZoneId zone = id.type().zone(system());
         HostName host = tester.configServer().hostFor(instanceId, zone);
 
-        tester.configServer().setConfigChangeActions(new ConfigChangeActions(singletonList(new RestartAction("cluster",
-                                                                                                             "container",
-                                                                                                             "search",
-                                                                                                             singletonList(new ServiceInfo("queries",
-                                                                                                                                                                   "search",
-                                                                                                                                                                   "config",
-                                                                                                                                                                   host.value())),
-                                                                                                             singletonList("Restart it!"))),
-                                                                             Collections.emptyList()));
+        tester.configServer().setConfigChangeActions(new ConfigChangeActions(List.of(new RestartAction("cluster",
+                                                                                                       "container",
+                                                                                                       "search",
+                                                                                                       List.of(new ServiceInfo("queries",
+                                                                                                                               "search",
+                                                                                                                               "config",
+                                                                                                                               host.value())),
+                                                                                                       List.of("Restart it!"))),
+                                                                             List.of(),
+                                                                             List.of()));
         tester.runner().run();
         assertEquals(succeeded, tester.jobs().run(id).get().stepStatuses().get(Step.deployReal));
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 9337adb01de..1465e1775dd 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -405,8 +405,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
             prepareResponse.message = "foo";
             prepareResponse.configChangeActions = configChangeActions != null
                     ? configChangeActions
-                    : new ConfigChangeActions(Collections.emptyList(),
-                                              Collections.emptyList());
+                    : new ConfigChangeActions(List.of(), List.of(), List.of());
             setConfigChangeActions(null);
             prepareResponse.tenant = new TenantId("tenant");
             prepareResponse.log = warnings.getOrDefault(id, Collections.emptyList());