diff options
author | Håkon Hallingstad <hakon@yahooinc.com> | 2023-07-28 14:33:32 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@yahooinc.com> | 2023-07-28 14:33:32 +0200 |
commit | 2ee24c4628c75abb8f8495eac978b3eb75c66162 (patch) | |
tree | f7d2d7a182ecee2f2387f97a930d666f81e78bcb /controller-server/src/test/java/com | |
parent | e9613c844167b4e8a55096043f42953182cd3482 (diff) |
Add cloud flag dimension
Diffstat (limited to 'controller-server/src/test/java/com')
4 files changed, 102 insertions, 20 deletions
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java index 6fd44e09d8d..21fe1f66bc5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneApiMock.java @@ -36,8 +36,8 @@ public class ZoneApiMock implements ZoneApi { } } - public static ZoneApiMock fromId(String id) { - return from(ZoneId.from(id)); + public static ZoneApiMock fromId(String zoneId) { + return from(ZoneId.from(zoneId)); } public static ZoneApiMock from(Environment environment, RegionName region) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java index e6a9014df94..63d479d4c6c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java @@ -161,7 +161,7 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry @Override public ZoneApi systemZone() { - return ZoneApiMock.fromId("prod.controller"); + return ZoneApiMock.newBuilder().withSystem(system).withVirtualId(ZoneId.ofVirtualControllerZone()).build(); } @Override @@ -180,7 +180,7 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry private ZoneApiMock systemAsZone() { return ZoneApiMock.newBuilder() .with(ZoneId.from("prod.us-east-1")) - .withVirtualId(ZoneId.from("prod.controller")) + .withVirtualId(ZoneId.ofVirtualControllerZone()) .build(); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResultTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResultTest.java index 36679e0dd91..d0d362abcfc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResultTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResultTest.java @@ -20,9 +20,12 @@ import static org.assertj.core.api.Assertions.assertThat; * @author bjorncs */ public class SystemFlagsDeployResultTest { + private final ZoneApiMock prodUsWest1Zone = ZoneApiMock.fromId("prod.us-west-1"); + private final ZoneRegistryMock registry = new ZoneRegistryMock(SystemName.cd).setZones(prodUsWest1Zone); + @Test void changes_and_errors_are_present_in_wire_format() { - FlagsTarget controllerTarget = FlagsTarget.forController(SystemName.cd); + FlagsTarget controllerTarget = FlagsTarget.forController(registry.systemZone()); FlagId flagOne = new FlagId("flagone"); FlagId flagTwo = new FlagId("flagtwo"); SystemFlagsDeployResult result = new SystemFlagsDeployResult( @@ -41,10 +44,8 @@ public class SystemFlagsDeployResultTest { @Test void identical_errors_and_changes_from_multiple_targets_are_merged() { - ZoneApiMock prodUsWest1Zone = ZoneApiMock.fromId("prod.us-west-1"); - ZoneRegistryMock registry = new ZoneRegistryMock(SystemName.cd).setZones(prodUsWest1Zone); - FlagsTarget prodUsWest1Target = FlagsTarget.forConfigServer(registry, prodUsWest1Zone.getId()); - FlagsTarget controllerTarget = FlagsTarget.forController(SystemName.cd); + FlagsTarget prodUsWest1Target = FlagsTarget.forConfigServer(registry, prodUsWest1Zone); + FlagsTarget controllerTarget = FlagsTarget.forController(registry.systemZone()); FlagId flagOne = new FlagId("flagone"); FlagId flagTwo = new FlagId("flagtwo"); 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 50354639f6f..14e37853191 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 @@ -2,7 +2,9 @@ package com.yahoo.vespa.hosted.controller.restapi.systemflags; import com.yahoo.config.provision.SystemName; +import com.yahoo.vespa.flags.FetchVector; 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.SystemFlagsDataArchive; @@ -15,11 +17,16 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Paths; import java.util.List; +import java.util.Optional; import java.util.Set; import static com.yahoo.vespa.hosted.controller.restapi.systemflags.SystemFlagsDeployResult.FlagDataChange; import static com.yahoo.vespa.hosted.controller.restapi.systemflags.SystemFlagsDeployResult.OperationError; +import static com.yahoo.yolean.Exceptions.uncheck; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -38,12 +45,12 @@ public class SystemFlagsDeployerTest { private final ZoneApiMock prodUsEast3Zone = ZoneApiMock.fromId("prod.us-east-3"); private final ZoneRegistryMock registry = new ZoneRegistryMock(SYSTEM).setZones(prodUsWest1Zone, prodUsEast3Zone); - private final FlagsTarget controllerTarget = FlagsTarget.forController(SYSTEM); - private final FlagsTarget prodUsWest1Target = FlagsTarget.forConfigServer(registry, prodUsWest1Zone.getId()); - private final FlagsTarget prodUsEast3Target = FlagsTarget.forConfigServer(registry, prodUsEast3Zone.getId()); + private final FlagsTarget controllerTarget = FlagsTarget.forController(registry.systemZone()); + private final FlagsTarget prodUsWest1Target = FlagsTarget.forConfigServer(registry, prodUsWest1Zone); + private final FlagsTarget prodUsEast3Target = FlagsTarget.forConfigServer(registry, prodUsEast3Zone); @Test - void deploys_flag_data_to_targets() throws IOException { + void deploys_flag_data_to_targets() { FlagsClient flagsClient = mock(FlagsClient.class); when(flagsClient.listFlagData(controllerTarget)).thenReturn(List.of()); when(flagsClient.listFlagData(prodUsWest1Target)).thenReturn(List.of(flagData("existing-prod.us-west-1.json"))); @@ -74,7 +81,81 @@ public class SystemFlagsDeployerTest { } @Test - void dryrun_should_not_change_flags() throws IOException { + void deploys_partial_flag_data_to_targets() { + // default.json contains one rule with 2 conditions, one of which has a condition on the aws cloud. + // This condition IS resolved for a config server target, but NOT for a controller target, because FLAG_ID + // has the CLOUD dimension set. + deployFlags(Optional.empty(), "partial/default.json", Optional.of("partial/put-controller.json"), true, PutType.CREATE, FetchVector.Dimension.CLOUD); + deployFlags(Optional.empty(), "partial/default.json", Optional.empty(), false, PutType.NONE, FetchVector.Dimension.CLOUD); + deployFlags(Optional.of("partial/initial.json"), "partial/default.json", Optional.of("partial/put-controller-2.json"), true, PutType.UPDATE, FetchVector.Dimension.CLOUD); + deployFlags(Optional.of("partial/initial.json"), "partial/default.json", Optional.empty(), false, PutType.DELETE, FetchVector.Dimension.CLOUD); + + // When the CLOUD dimension is NOT set on the dimension, the controller target will also resolve that dimension, and + // the result should be identical to the config server target. Let's also verify the config server target is unchanged. + deployFlags(Optional.empty(), "partial/default.json", Optional.empty(), true, PutType.NONE); + deployFlags(Optional.empty(), "partial/default.json", Optional.empty(), false, PutType.NONE); + deployFlags(Optional.of("partial/initial.json"), "partial/default.json", Optional.empty(), true, PutType.DELETE); + deployFlags(Optional.of("partial/initial.json"), "partial/default.json", Optional.empty(), false, PutType.DELETE); + } + + private enum PutType { + CREATE, + UPDATE, + DELETE, + NONE + } + + /** + * @param existingFlagDataPath path to flag data the target already has + * @param defaultFlagDataPath path to default json file + * @param putFlagDataPath path to flag data pushed to target, or empty if nothing should be pushed + * @param controller whether to target the controller, or config server + */ + private void deployFlags(Optional<String> existingFlagDataPath, + String defaultFlagDataPath, + Optional<String> putFlagDataPath, + boolean controller, + PutType putType, + FetchVector.Dimension... flagDimensions) { + List<FlagData> existingFlagData = existingFlagDataPath.map(SystemFlagsDeployerTest::flagData).map(List::of).orElse(List.of()); + FlagData defaultFlagData = flagData(defaultFlagDataPath); + FlagsTarget target = controller ? controllerTarget : prodUsWest1Target; + Optional<FlagData> putFlagData = putFlagDataPath.map(SystemFlagsDeployerTest::flagData); + + try (var replacer = Flags.clearFlagsForTesting()) { + Flags.defineStringFlag(FLAG_ID.toString(), "default", List.of("hakonhall"), "2023-07-27", "2123-07-27", "", "", flagDimensions); + + FlagsClient flagsClient = mock(FlagsClient.class); + when(flagsClient.listFlagData(target)).thenReturn(existingFlagData); + + SystemFlagsDataArchive archive = new SystemFlagsDataArchive.Builder() + .addFile("default.json", defaultFlagData) + .build(); + + SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(target)); + + List<FlagDataChange> changes = deployer.deployFlags(archive, false).flagChanges(); + + putFlagData.ifPresentOrElse(flagData -> { + verify(flagsClient).putFlagData(target, flagData); + switch (putType) { + case CREATE -> assertThat(changes).containsOnly(FlagDataChange.created(FLAG_ID, target, flagData)); + case UPDATE -> assertThat(changes).containsOnly(FlagDataChange.updated(FLAG_ID, target, flagData, existingFlagData.get(0))); + case DELETE, NONE -> throw new IllegalStateException("Flag data put to the target, but change type is " + putType); + } + }, () -> { + verify(flagsClient, never()).putFlagData(eq(target), any()); + switch (putType) { + case DELETE -> assertThat(changes).containsOnly(FlagDataChange.deleted(FLAG_ID, target)); + case NONE -> assertEquals(changes, List.of()); + default -> throw new IllegalStateException("No flag data is expected to be put to the target but change type is " + putType); + } + }); + } + } + + @Test + void dryrun_should_not_change_flags() { FlagsClient flagsClient = mock(FlagsClient.class); when(flagsClient.listFlagData(controllerTarget)).thenReturn(List.of()); when(flagsClient.listDefinedFlags(controllerTarget)).thenReturn(List.of(new FlagId("my-flag"))); @@ -97,7 +178,7 @@ public class SystemFlagsDeployerTest { } @Test - void creates_error_entries_in_result_if_flag_data_operations_fail() throws IOException { + void creates_error_entries_in_result_if_flag_data_operations_fail() { FlagsClient flagsClient = mock(FlagsClient.class); UncheckedIOException exception = new UncheckedIOException(new IOException("I/O error message")); when(flagsClient.listFlagData(prodUsWest1Target)).thenThrow(exception); @@ -120,7 +201,7 @@ public class SystemFlagsDeployerTest { } @Test - void creates_error_entry_for_invalid_flag_archive() throws IOException { + void creates_error_entry_for_invalid_flag_archive() { FlagsClient flagsClient = mock(FlagsClient.class); FlagData defaultData = flagData("flags/my-flag/main.json"); SystemFlagsDataArchive archive = new SystemFlagsDataArchive.Builder() @@ -135,7 +216,7 @@ public class SystemFlagsDeployerTest { } @Test - void creates_error_entry_for_flag_data_of_undefined_flag() throws IOException { + void creates_error_entry_for_flag_data_of_undefined_flag() { FlagData prodUsEast3Data = flagData("flags/my-flag/main.prod.us-east-3.json"); FlagsClient flagsClient = mock(FlagsClient.class); when(flagsClient.listFlagData(prodUsEast3Target)) @@ -154,7 +235,7 @@ public class SystemFlagsDeployerTest { } @Test - void creates_warning_entry_for_existing_flag_data_for_undefined_flag() throws IOException { + void creates_warning_entry_for_existing_flag_data_for_undefined_flag() { FlagData prodUsEast3Data = flagData("flags/my-flag/main.prod.us-east-3.json"); FlagsClient flagsClient = mock(FlagsClient.class); when(flagsClient.listFlagData(prodUsEast3Target)) @@ -170,8 +251,8 @@ public class SystemFlagsDeployerTest { .containsOnly(OperationError.dataForUndefinedFlag(prodUsEast3Target, new FlagId("my-flag"))); } - private static FlagData flagData(String filename) throws IOException { - return FlagData.deserializeUtf8Json(Files.readAllBytes(Paths.get("src/test/resources/system-flags/" + filename))); + private static FlagData flagData(String filename) { + return FlagData.deserializeUtf8Json(uncheck(() -> Files.readAllBytes(Paths.get("src/test/resources/system-flags/" + filename)))); } } |