Improve feedback from /system-flags/v1 responses

Add flag owners to result/warning/error whenever available.
Make warning/error message from undefined flags more informative.

3 files changed, 19 insertions, 4 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java
index 57d47757c5e..d169cd97df7 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java
@@ -2,7 +2,9 @@
 package com.yahoo.vespa.hosted.controller.restapi.systemflags;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import com.yahoo.vespa.flags.FlagDefinition;
 import com.yahoo.vespa.flags.FlagId;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.json.FlagData;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult;
@@ -100,6 +102,7 @@ class SystemFlagsDeployResult {
         for (FlagDataChange change : flagChanges) {
             var wireChange = new WireFlagDataChange();
             wireChange.flagId = change.flagId().toString();
+            wireChange.owners = owners(change.flagId());
             wireChange.operation = change.operation().asString();
             wireChange.targets = change.targets().stream().map(FlagsTarget::asString).collect(toList());
             wireChange.data = change.data().map(FlagData::toWire).orElse(null);
@@ -113,6 +116,7 @@ class SystemFlagsDeployResult {
             wireError.operation = error.operation().asString();
             wireError.targets = error.targets().stream().map(FlagsTarget::asString).collect(toList());
             wireError.flagId = error.flagId().map(FlagId::toString).orElse(null);
+            wireError.owners = error.flagId().map(id -> owners(id)).orElse(List.of());
             wireError.data = error.flagData().map(FlagData::toWire).orElse(null);
             wireResult.errors.add(wireError);
         }
@@ -121,12 +125,17 @@ class SystemFlagsDeployResult {
             var wireWarning = new WireWarning();
             wireWarning.message = warning.message();
             wireWarning.flagId = warning.flagId().toString();
+            wireWarning.owners = owners(warning.flagId());
             wireWarning.targets = warning.targets().stream().map(FlagsTarget::asString).collect(toList());
             wireResult.warnings.add(wireWarning);
         }
         return wireResult;
     }
 
+    private static List<String> owners(FlagId id) {
+        return Flags.getFlag(id).map(FlagDefinition::getOwners).orElse(List.of());
+    }
+
     static class FlagDataChange {
 
         private final FlagId flagId;
@@ -296,7 +305,9 @@ class SystemFlagsDeployResult {
         }
 
         static Warning dataForUndefinedFlag(FlagsTarget target, FlagId flagId) {
-            return new Warning("Flag data present for undefined flag", Set.of(target), flagId);
+            return new Warning(
+                    "Flag data present for undefined flag. Remove flag data files if flag's definition is already removed from Flags/PermanentFlags. " +
+                            "Consult ModelContext.FeatureFlags for safe removal of flag used by config-model.", Set.of(target), flagId);
         }
 
         String message() { return message; }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java
index 5ea277f5101..0331bf292a0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.restapi.systemflags;
 
 import com.yahoo.concurrent.DaemonThreadFactory;
 import com.yahoo.config.provision.SystemName;
-import java.util.logging.Level;
 import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider;
 import com.yahoo.vespa.flags.FlagId;
 import com.yahoo.vespa.flags.Flags;
@@ -25,6 +24,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.function.Function;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -181,9 +181,11 @@ class SystemFlagsDeployer  {
                                                            Map<FlagId, FlagData> currentFlagData,
                                                            List<FlagId> definedFlags,
                                                            List<OperationError> errors) {
+        String errorMessage = "Flag not defined in target zone. If zone/configserver cluster is new, add an empty flag " +
+                "data file for this zone as a temporary measure until the stale flag data files are removed.";
         for (FlagId flagId : wantedFlagData.keySet()) {
             if (!currentFlagData.containsKey(flagId) && !definedFlags.contains(flagId)) {
-                errors.add(OperationError.createFailed("Flag not defined in target zone", target, wantedFlagData.get(flagId)));
+                errors.add(OperationError.createFailed(errorMessage, target, wantedFlagData.get(flagId)));
             }
         }
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java
index 789be26db1f..35a13cdeeec 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java
@@ -147,8 +147,10 @@ public class SystemFlagsDeployerTest {
                 .build();
         SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(prodUsEast3Target));
         SystemFlagsDeployResult result = deployer.deployFlags(archive, true);
+        String expectedErrorMessage = "Flag not defined in target zone. If zone/configserver cluster is new, " +
+                "add an empty flag data file for this zone as a temporary measure until the stale flag data files are removed.";
         assertThat(result.errors())
-                .containsOnly(SystemFlagsDeployResult.OperationError.createFailed("Flag not defined in target zone", prodUsEast3Target, prodUsEast3Data));
+                .containsOnly(SystemFlagsDeployResult.OperationError.createFailed(expectedErrorMessage, prodUsEast3Target, prodUsEast3Data));
     }
 
     @Test