diff options
4 files changed, 24 insertions, 13 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index 850e14b25e8..367b258e7b6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -125,9 +126,9 @@ public class SystemFlagsDataArchive { uncheck(zipOut::flush); } - public Set<FlagData> flagData(FlagsTarget target) { + public List<FlagData> flagData(FlagsTarget target) { List<String> filenames = target.flagDataFilesPrioritized(); - Set<FlagData> targetData = new HashSet<>(); + List<FlagData> targetData = new ArrayList<>(); files.forEach((flagId, fileMap) -> { for (String filename : filenames) { FlagData data = fileMap.get(filename); 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 d169cd97df7..f140648c6dd 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 @@ -5,12 +5,14 @@ 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.PermanentFlags; 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; import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult.WireFlagDataChange; import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult.WireOperationFailure; import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult.WireWarning; +import org.apache.zookeeper.Op; import java.util.ArrayList; import java.util.HashMap; @@ -251,6 +253,13 @@ class SystemFlagsDeployResult { return new OperationError(message, Set.of(), OperationType.VALIDATE_ARCHIVE, null, null); } + static OperationError dataForUndefinedFlag(FlagsTarget target, FlagId id) { + return new OperationError("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(), OperationType.DATA_FOR_UNDEFINED_FLAG, id, null); + } + String message() { return message; } Set<FlagsTarget> targets() { return targets; } OperationType operation() { return operation; } @@ -284,7 +293,8 @@ class SystemFlagsDeployResult { } enum OperationType { - CREATE("create"), DELETE("delete"), UPDATE("update"), LIST("list"), VALIDATE_ARCHIVE("validate-archive"); + CREATE("create"), DELETE("delete"), UPDATE("update"), LIST("list"), VALIDATE_ARCHIVE("validate-archive"), + DATA_FOR_UNDEFINED_FLAG("data-for-undefined-flag"); private final String stringValue; 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 21a429b59ad..e0b65b0834d 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 @@ -78,7 +78,7 @@ class SystemFlagsDeployer { return SystemFlagsDeployResult.merge(results); } - private SystemFlagsDeployResult deployFlags(FlagsTarget target, Set<FlagData> flagData, boolean dryRun) { + private SystemFlagsDeployResult deployFlags(FlagsTarget target, List<FlagData> flagData, boolean dryRun) { Map<FlagId, FlagData> wantedFlagData = lookupTable(flagData); Map<FlagId, FlagData> currentFlagData; List<FlagId> definedFlags; @@ -98,7 +98,7 @@ class SystemFlagsDeployer { updateExistingFlagData(target, dryRun, wantedFlagData, currentFlagData, results, errors); removeOldFlagData(target, dryRun, wantedFlagData, currentFlagData, results, errors); failOnNewFlagDataForUndefinedFlags(target, wantedFlagData, currentFlagData, definedFlags, errors); - warnOnExistingFlagDataForUndefinedFlags(target, wantedFlagData, currentFlagData, definedFlags, warnings); + failOnFlagDataForUndefinedFlags(target, wantedFlagData, currentFlagData, definedFlags, errors); return new SystemFlagsDeployResult(results, errors, warnings); } @@ -191,14 +191,14 @@ class SystemFlagsDeployer { } } - private static void warnOnExistingFlagDataForUndefinedFlags(FlagsTarget target, - Map<FlagId, FlagData> wantedFlagData, - Map<FlagId, FlagData> currentFlagData, - List<FlagId> definedFlags, - List<Warning> warnings) { + private static void failOnFlagDataForUndefinedFlags(FlagsTarget target, + Map<FlagId, FlagData> wantedFlagData, + Map<FlagId, FlagData> currentFlagData, + List<FlagId> definedFlags, + List<OperationError> errors) { for (FlagId flagId : currentFlagData.keySet()) { if (wantedFlagData.containsKey(flagId) && !definedFlags.contains(flagId)) { - warnings.add(Warning.dataForUndefinedFlag(target, flagId)); + errors.add(OperationError.dataForUndefinedFlag(target, 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 35a13cdeeec..549dd1ed253 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 @@ -166,8 +166,8 @@ public class SystemFlagsDeployerTest { .build(); SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(prodUsEast3Target)); SystemFlagsDeployResult result = deployer.deployFlags(archive, true); - assertThat(result.warnings()) - .containsOnly(SystemFlagsDeployResult.Warning.dataForUndefinedFlag(prodUsEast3Target, new FlagId("my-flag"))); + assertThat(result.errors()) + .containsOnly(OperationError.dataForUndefinedFlag(prodUsEast3Target, new FlagId("my-flag"))); } private static FlagData flagData(String filename) throws IOException { |