diff --git a/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImages.java b/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImages.java index cb5e113de5f..7b470b034a8 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImages.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImages.java @@ -49,6 +49,12 @@ public class UploadDockerImages { private static final Logger LOGGER = LoggerFactory.getLogger(UploadDockerImages.class); + // A throwaway local registry used to serve base images to the buildx builder during a build. It is not a + // deployment registry: nothing is ever deployed or run from it, and it is torn down at the end of the build. + private static final String LOCAL_REGISTRY_PORT = "5000"; + private static final String LOCAL_REGISTRY_HOST = "localhost:" + LOCAL_REGISTRY_PORT; + private static final String LOCAL_REGISTRY_CONTAINER = "sleeper-base-registry"; + private final Path baseDockerDirectory; private final Path jarsDirectory; private final DeployConfiguration deployConfig; @@ -89,7 +95,17 @@ public boolean isDockerCli() { } public static void useBuildXBuilder(CommandPipelineRunner commandRunner) throws IOException, InterruptedException { - commandRunner.run("docker", "buildx", "create", "--name", "sleeper"); + createBuildXBuilder(commandRunner, false); + } + + private static void createBuildXBuilder(CommandPipelineRunner commandRunner, boolean useHostNetwork) throws IOException, InterruptedException { + if (useHostNetwork) { + // Host networking lets the containerised buildx builder reach the local registry serving base images on + // localhost. Without it the builder cannot resolve FROM a locally-built base image. + commandRunner.run("docker", "buildx", "create", "--name", "sleeper", "--driver-opt", "network=host"); + } else { + commandRunner.run("docker", "buildx", "create", "--name", "sleeper"); + } commandRunner.runOrThrow("docker", "buildx", "use", "sleeper"); } @@ -101,19 +117,34 @@ public void upload(String repositoryPrefix, List imagesToUploa LOGGER.info("Building and uploading images: {}", imagesToUpload); boolean anyUseBaseImage = imagesToUpload.stream().anyMatch(StackDockerImage::isUseDefaultBaseImage); - if (deployConfig.dockerImageLocation() == DockerImageLocation.LOCAL_BUILD - && createMultiplatformBuilder && anyUseBaseImage) { - useBuildXBuilder(commandRunner); - } - if (deployConfig.dockerImageLocation() == DockerImageLocation.LOCAL_BUILD) { - String baseTag = buildTag(repositoryPrefix, baseImage); - if (anyUseBaseImage) { - buildAndPushImage(baseTag, baseImage, Map.of()); + // A multiplatform image is built in the buildx "sleeper" builder, which resolves a FROM image from a + // registry and cannot see the local Docker image store. When such an image builds on a base image, we + // serve base images from a throwaway local registry that both the plain Docker builder and the buildx + // builder can pull from. When no multiplatform image needs a base, base images are built straight into + // the local Docker image store, and no registry is needed. + boolean useLocalRegistry = imagesToUpload.stream() + .anyMatch(image -> image.isMultiplatform() && usesBaseImage(image)); + + if (createMultiplatformBuilder && (anyUseBaseImage || useLocalRegistry)) { + createBuildXBuilder(commandRunner, useLocalRegistry); } - for (StackDockerImage image : imagesToUpload) { - Map buildArgs = createBuildArgs(repositoryPrefix, image, baseTag); - buildAndPushImage(buildTag(repositoryPrefix, image), image, buildArgs); + if (useLocalRegistry) { + startLocalRegistry(); + } + try { + String baseTag = baseImageTag(repositoryPrefix, baseImage, useLocalRegistry); + if (anyUseBaseImage) { + buildBaseImage(baseTag, baseImage, useLocalRegistry); + } + for (StackDockerImage image : imagesToUpload) { + Map buildArgs = createBuildArgs(repositoryPrefix, image, baseTag, useLocalRegistry); + buildAndPushImage(buildTag(repositoryPrefix, image), image, buildArgs); + } + } finally { + if (useLocalRegistry) { + stopLocalRegistry(); + } } } else if (deployConfig.dockerImageLocation() == DockerImageLocation.REPOSITORY) { for (StackDockerImage image : imagesToUpload) { @@ -122,6 +153,21 @@ public void upload(String repositoryPrefix, List imagesToUploa } } + private boolean usesBaseImage(StackDockerImage image) { + return image.isUseDefaultBaseImage() || image.createOverrideBaseImage(deployConfig).isPresent(); + } + + private void startLocalRegistry() throws IOException, InterruptedException { + // Best-effort start; tolerate a registry left running by a previous build, as we do for the buildx builder. + commandRunner.run("docker", "run", "-d", "-p", LOCAL_REGISTRY_PORT + ":" + LOCAL_REGISTRY_PORT, + "--name", LOCAL_REGISTRY_CONTAINER, "registry:2"); + } + + private void stopLocalRegistry() throws IOException, InterruptedException { + // Best-effort teardown so a failed build does not mask the original error, and no registry is left running. + commandRunner.run("docker", "rm", "-f", LOCAL_REGISTRY_CONTAINER); + } + private void buildAndPushImage(String tag, StackDockerImage image, Map buildArgs) throws IOException, InterruptedException { Path dockerfileDirectory = image.resolveBuildContext(baseDockerDirectory, deployConfig); image.getLambdaJar().ifPresent(jar -> { @@ -154,11 +200,43 @@ private void buildAndPushImage(String tag, StackDockerImage image, Map createBuildArgs(String repositoryPrefix, StackDockerImage image, String baseTag) throws IOException, InterruptedException { + private void buildBaseImage(String tag, StackDockerImage image, boolean useLocalRegistry) throws IOException, InterruptedException { + // A base image is only a build input for other images, which resolve it via the BASE_IMAGE build argument. It + // is never deployed or run directly, so it is never pushed to a deployment registry (e.g. ECR). It is made + // available either in the local Docker image store, or in a throwaway local registry when a multiplatform + // build needs it (the buildx builder cannot see the local image store). + Path dockerfileDirectory = image.resolveBuildContext(baseDockerDirectory, deployConfig); + if (image.isMultiplatform()) { + String platformList = ContainerPlatform.buildPlatformListArgument(image.getPlatforms()); + String loadOrPush = useLocalRegistry ? "--push" : "--load"; + commandRunner.runOrThrow(dockerBuild( + List.of("docker", "buildx", "build"), + List.of("--platform", platformList, loadOrPush, "-t", tag), + dockerfileDirectory)); + } else { + commandRunner.runOrThrow(dockerBuild( + List.of("docker", "build"), + List.of("-t", tag), + dockerfileDirectory)); + if (useLocalRegistry) { + commandRunner.runOrThrow("docker", "push", tag); + } + } + } + + private String baseImageTag(String repositoryPrefix, StackDockerImage image, boolean useLocalRegistry) { + if (useLocalRegistry) { + return LOCAL_REGISTRY_HOST + "/" + image.getImageName() + ":" + version; + } else { + return buildTag(repositoryPrefix, image); + } + } + + private Map createBuildArgs(String repositoryPrefix, StackDockerImage image, String baseTag, boolean useLocalRegistry) throws IOException, InterruptedException { StackDockerImage overrideBaseImage = image.createOverrideBaseImage(deployConfig).orElse(null); if (overrideBaseImage != null) { - String overrideBaseTag = buildTag(repositoryPrefix, overrideBaseImage); - buildAndPushImage(overrideBaseTag, overrideBaseImage, Map.of()); + String overrideBaseTag = baseImageTag(repositoryPrefix, overrideBaseImage, useLocalRegistry); + buildBaseImage(overrideBaseTag, overrideBaseImage, useLocalRegistry); return Map.of("BASE_IMAGE", overrideBaseTag); } else if (image.isUseDefaultBaseImage()) { return Map.of("BASE_IMAGE", baseTag); diff --git a/java/clients/src/test/java/sleeper/clients/deploy/container/DockerImageCommandTestData.java b/java/clients/src/test/java/sleeper/clients/deploy/container/DockerImageCommandTestData.java index 939cdac37bd..4735a66b25e 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/container/DockerImageCommandTestData.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/container/DockerImageCommandTestData.java @@ -42,7 +42,7 @@ public static List commandsToLoginDockerAndPushImages(InstanceP commands.add(createBuildxBuilderInstanceCommand()); commands.add(useBuildxBuilderInstanceCommand()); String baseTag = tag(instanceProperties, "base"); - commands.add(buildAndPushMultiplatformImageCommand(baseTag, "./docker/base")); + commands.add(buildAndLoadMultiplatformImageCommand(baseTag, "./docker/base")); for (String image : images) { String tag = tag(instanceProperties, image); commands.add(buildImageCommand(tag, "./docker/" + image, baseTag)); @@ -100,23 +100,35 @@ public static CommandPipeline createBuildxBuilderInstanceCommand() { return pipeline(command("docker", "buildx", "create", "--name", "sleeper")); } + public static CommandPipeline createBuildxBuilderWithHostNetworkCommand() { + return pipeline(command("docker", "buildx", "create", "--name", "sleeper", "--driver-opt", "network=host")); + } + public static CommandPipeline useBuildxBuilderInstanceCommand() { return pipeline(command("docker", "buildx", "use", "sleeper")); } + public static CommandPipeline startLocalRegistryCommand() { + return pipeline(command("docker", "run", "-d", "-p", "5000:5000", "--name", "sleeper-base-registry", "registry:2")); + } + + public static CommandPipeline stopLocalRegistryCommand() { + return pipeline(command("docker", "rm", "-f", "sleeper-base-registry")); + } + public static CommandPipeline buildAndPushMultiplatformImageCommand(String tag, String dockerDirectory, String baseTag) { return pipeline(command("docker", "buildx", "build", "--build-arg", "BASE_IMAGE=" + baseTag, "--platform", "linux/amd64,linux/arm64", "--push", "-t", tag, dockerDirectory)); } - public static CommandPipeline buildAndPushMultiplatformImageCommand(String tag, String dockerDirectory) { + public static CommandPipeline buildAndLoadMultiplatformImageCommand(String tag, String dockerDirectory) { return pipeline(command("docker", "buildx", "build", "--platform", "linux/amd64,linux/arm64", - "--push", "-t", tag, dockerDirectory)); + "--load", "-t", tag, dockerDirectory)); } - public static CommandPipeline buildAndLoadMultiplatformImageCommand(String tag, String dockerDirectory) { + public static CommandPipeline pushMultiplatformBaseToLocalRegistryCommand(String tag, String dockerDirectory) { return pipeline(command("docker", "buildx", "build", "--platform", "linux/amd64,linux/arm64", - "--load", "-t", tag, dockerDirectory)); + "--push", "-t", tag, dockerDirectory)); } } diff --git a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java index b64c3c04dd7..3c77c4f5502 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java @@ -34,7 +34,7 @@ import static java.util.stream.Collectors.toUnmodifiableList; import static java.util.stream.Collectors.toUnmodifiableMap; import static org.assertj.core.api.Assertions.assertThat; -import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildAndPushMultiplatformImageCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildAndLoadMultiplatformImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildLambdaImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.createBuildxBuilderInstanceCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.dockerLoginToEcrCommand; @@ -77,7 +77,7 @@ void shouldUploadTwoLambdaImagesOverwritingJarEachTime() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, dockerDir.toString() + "/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, dockerDir.toString() + "/base"), buildLambdaImageCommand(expectedTag1, lambdaImageDir.toString(), expectedBaseTag), pushImageCommand(expectedTag1), buildLambdaImageCommand(expectedTag2, lambdaImageDir.toString(), expectedBaseTag), diff --git a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java index 743abd76cb4..3fc7a917c59 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java @@ -38,12 +38,17 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildAndLoadMultiplatformImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildAndPushMultiplatformImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildLambdaImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.createBuildxBuilderInstanceCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.createBuildxBuilderWithHostNetworkCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.dockerLoginToEcrCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.pushImageCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.pushMultiplatformBaseToLocalRegistryCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.startLocalRegistryCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.stopLocalRegistryCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.useBuildxBuilderInstanceCommand; import static sleeper.core.properties.instance.CommonProperty.ECR_REPOSITORY_PREFIX; import static sleeper.core.properties.instance.CommonProperty.LAMBDA_DEPLOY_TYPE; @@ -76,7 +81,7 @@ void shouldPushImageForIngestStack() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag, "./docker/ingest", expectedBaseTag), pushImageCommand(expectedTag)); } @@ -97,7 +102,7 @@ void shouldPushImagesForTwoStacks() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag1, "./docker/ingest", expectedBaseTag), pushImageCommand(expectedTag1), buildImageCommand(expectedTag2, "./docker/bulk-import-runner"), @@ -120,7 +125,7 @@ void shouldPushImageWhenEcrRepositoryPrefixIsSet() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag, "./docker/ingest", expectedBaseTag), pushImageCommand(expectedTag)); } @@ -147,7 +152,7 @@ void shouldPushCoreImage() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildLambdaImageCommand(expectedTag, "./docker/lambda", expectedBaseTag), pushImageCommand(expectedTag)); assertThat(files).isEqualTo(Map.of( @@ -174,7 +179,7 @@ void shouldPushImageForCoreAndOptionalLambdaInNewInstance() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildLambdaImageCommand(expectedTag1, "./docker/lambda", expectedBaseTag), pushImageCommand(expectedTag1), buildLambdaImageCommand(expectedTag2, "./docker/lambda", expectedBaseTag), @@ -202,7 +207,7 @@ void shouldPushImageForOptionalLambdaWhenSeveralOfItsStacksAreEnabled() throws E dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildLambdaImageCommand(expectedTag, "./docker/lambda", expectedBaseTag), pushImageCommand(expectedTag)); assertThat(files).isEqualTo(Map.of( @@ -240,7 +245,7 @@ void shouldDeployLambdaByDockerWhenConfiguredToAlwaysDeployByDocker() throws Exc dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildLambdaImageCommand(expectedTag, "./docker/lambda", expectedBaseTag), pushImageCommand(expectedTag)); assertThat(files).isEqualTo(Map.of( @@ -279,7 +284,7 @@ void shouldPushImageWhenPreviousStackHasNoDockerImage() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag, "./docker/ingest", expectedBaseTag), pushImageCommand(expectedTag)); } @@ -297,15 +302,18 @@ void shouldPushImageWhenCompactionImageNeedsToBeBuiltByBuildx() throws Exception // When uploadForDeployment(dockerDeploymentImageConfig()); - // Then - String expectedBaseTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/base:1.0.0"; + // Then the multiplatform compaction image is built in the buildx builder, which cannot see the local image + // store, so its base is served from a throwaway local registry that is torn down afterwards. + String expectedBaseTag = "localhost:5000/base:1.0.0"; String expectedTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/compaction:1.0.0"; assertThat(commandsThatRan).containsExactly( dockerLoginToEcrCommand(), - createBuildxBuilderInstanceCommand(), + createBuildxBuilderWithHostNetworkCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), - buildAndPushMultiplatformImageCommand(expectedTag, "./docker/compaction", expectedBaseTag)); + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedBaseTag, "./docker/base"), + buildAndPushMultiplatformImageCommand(expectedTag, "./docker/compaction", expectedBaseTag), + stopLocalRegistryCommand()); } @Test @@ -316,18 +324,21 @@ void shouldPushImagesWhenOnlyOneImageNeedsToBeBuiltByBuildx() throws Exception { // When uploadForDeployment(dockerDeploymentImageConfig()); - // Then - String expectedBaseTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/base:1.0.0"; + // Then the single-platform ingest image and the multiplatform compaction image both resolve their base from + // the local registry, which both the plain Docker builder and the buildx builder can pull from. + String expectedBaseTag = "localhost:5000/base:1.0.0"; String expectedTag1 = "123.dkr.ecr.test-region.amazonaws.com/test-instance/ingest:1.0.0"; String expectedTag2 = "123.dkr.ecr.test-region.amazonaws.com/test-instance/compaction:1.0.0"; assertThat(commandsThatRan).containsExactly( dockerLoginToEcrCommand(), - createBuildxBuilderInstanceCommand(), + createBuildxBuilderWithHostNetworkCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag1, "./docker/ingest", expectedBaseTag), pushImageCommand(expectedTag1), - buildAndPushMultiplatformImageCommand(expectedTag2, "./docker/compaction", expectedBaseTag)); + buildAndPushMultiplatformImageCommand(expectedTag2, "./docker/compaction", expectedBaseTag), + stopLocalRegistryCommand()); } } @@ -361,14 +372,16 @@ void shouldNotFailWhenCreateBuildxBuilderFailsForCompactionImage() throws Except uploadForDeployment(dockerDeploymentImageConfig()); // Then - String expectedBaseTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/base:1.0.0"; + String expectedBaseTag = "localhost:5000/base:1.0.0"; String expectedTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/compaction:1.0.0"; assertThat(commandsThatRan).containsExactly( dockerLoginToEcrCommand(), - createBuildxBuilderInstanceCommand(), + createBuildxBuilderWithHostNetworkCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), - buildAndPushMultiplatformImageCommand(expectedTag, "./docker/compaction", expectedBaseTag)); + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedBaseTag, "./docker/base"), + buildAndPushMultiplatformImageCommand(expectedTag, "./docker/compaction", expectedBaseTag), + stopLocalRegistryCommand()); } @Test @@ -385,7 +398,7 @@ void shouldFailWhenUseBuildxBuilderFails() { }); assertThat(commandsThatRan).containsExactly( dockerLoginToEcrCommand(), - createBuildxBuilderInstanceCommand(), + createBuildxBuilderWithHostNetworkCommand(), useBuildxBuilderInstanceCommand()); } @@ -411,7 +424,7 @@ void shouldFailWhenDockerBuildFails() { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand); } @@ -435,7 +448,7 @@ void shouldFailWhenDockerPushFailsAfterBuild() { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(ecrTag, "./docker/ingest", expectedBaseTag), pushCommand); } @@ -538,7 +551,7 @@ void shouldPushStateStoreCommitterImageForEc2Platform() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag, "./docker/statestore-committer", expectedBaseTag), pushImageCommand(expectedTag)); } @@ -584,7 +597,7 @@ void shouldPushImageWhenCdkAppMatches() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag, "./docker/data-generation", expectedBaseTag), pushImageCommand(expectedTag)); } @@ -619,7 +632,7 @@ void shouldBuildBaseImageFromOverrideDirectoryWhenSet() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./custom/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./custom/base"), buildImageCommand(expectedTag, "./docker/ingest", expectedBaseTag), pushImageCommand(expectedTag)); } @@ -659,14 +672,60 @@ void shouldOverrideBaseForSingleImage() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag1, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag1, "./docker/base"), buildImageCommand(expectedTag1, "./docker/ingest", expectedBaseTag1), pushImageCommand(expectedTag1), buildImageCommand(expectedBaseTag2, "./custom/base"), - pushImageCommand(expectedBaseTag2), buildImageCommand(expectedTag2, "./docker/bulk-import-runner", expectedBaseTag2), pushImageCommand(expectedTag2)); } + + @Test + void shouldNotPushOverrideBaseImageWhenOverridingBaseForSingleImageOnly() throws Exception { + // Given the exact scenario that reported the bug: an override base is set for one image, but no + // ECR repository exists for the synthesised -base, so it must be built locally and never pushed. + deployConfig = DeployConfiguration.fromLocalBuild().withImageToOverrideBaseDir(Map.of("bulk-import-runner", "./custom/base")); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.EksBulkImportStack); + + // When + uploadForDeployment(dockerDeploymentImageConfig()); + + // Then + String expectedBaseTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/bulk-import-runner-base:1.0.0"; + String expectedTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/bulk-import-runner:1.0.0"; + assertThat(commandsThatRan).containsExactly( + dockerLoginToEcrCommand(), + buildImageCommand(expectedBaseTag, "./custom/base"), + buildImageCommand(expectedTag, "./docker/bulk-import-runner", expectedBaseTag), + pushImageCommand(expectedTag)); + } + + @Test + void shouldServeMultiplatformOverrideBaseViaLocalRegistry() throws Exception { + // Given a multiplatform image with an overridden base. This is the shape of the reported bug (an override + // base for a multiplatform image, e.g. bulk-import-runner-base). The override base must be reachable by the + // buildx builder, which cannot see the local Docker image store, so it is served from a local registry and + // never pushed to ECR. + deployConfig = DeployConfiguration.fromLocalBuild().withImageToOverrideBaseDir(Map.of("compaction", "./custom/base")); + properties.setEnum(OPTIONAL_STACKS, OptionalStack.CompactionStack); + + // When + uploadForDeployment(dockerDeploymentImageConfig()); + + // Then + String expectedDefaultBaseTag = "localhost:5000/base:1.0.0"; + String expectedOverrideBaseTag = "localhost:5000/compaction-base:1.0.0"; + String expectedTag = "123.dkr.ecr.test-region.amazonaws.com/test-instance/compaction:1.0.0"; + assertThat(commandsThatRan).containsExactly( + dockerLoginToEcrCommand(), + createBuildxBuilderWithHostNetworkCommand(), + useBuildxBuilderInstanceCommand(), + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedDefaultBaseTag, "./docker/base"), + pushMultiplatformBaseToLocalRegistryCommand(expectedOverrideBaseTag, "./custom/base"), + buildAndPushMultiplatformImageCommand(expectedTag, "./docker/compaction", expectedOverrideBaseTag), + stopLocalRegistryCommand()); + } } @Nested @@ -688,7 +747,7 @@ void shouldUploadOneImageWithBase() throws Exception { dockerLoginToEcrCommand(), createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedSystemTestTag, "./docker/system-test", expectedBaseTag), pushImageCommand(expectedSystemTestTag)); } diff --git a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToRepositoryTest.java b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToRepositoryTest.java index 92fd346532a..61a8f927dd7 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToRepositoryTest.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToRepositoryTest.java @@ -31,11 +31,16 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildAndLoadMultiplatformImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildAndPushMultiplatformImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.buildLambdaImageCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.createBuildxBuilderInstanceCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.createBuildxBuilderWithHostNetworkCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.pushImageCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.pushMultiplatformBaseToLocalRegistryCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.startLocalRegistryCommand; +import static sleeper.clients.deploy.container.DockerImageCommandTestData.stopLocalRegistryCommand; import static sleeper.clients.deploy.container.DockerImageCommandTestData.useBuildxBuilderInstanceCommand; @DisplayName("Upload Docker images") @@ -52,17 +57,19 @@ void shouldBuildAndPushDockerDeploymentImages() throws Exception { // When uploadAllImages(dockerImageConfiguration); - // Then - String expectedBaseTag = "www.somedocker.com/prefix/base:1.0.0"; + // Then the multiplatform compaction image is built in the buildx builder, so base images are served from a + // throwaway local registry that both the plain Docker builder and the buildx builder can pull from. + String expectedBaseTag = "localhost:5000/base:1.0.0"; String expectedCommitterTag = "www.somedocker.com/prefix/statestore-committer:1.0.0"; String expectedIngestTag = "www.somedocker.com/prefix/ingest:1.0.0"; String expectedBulkImportTag = "www.somedocker.com/prefix/bulk-import-runner:1.0.0"; String expectedCompactionTag = "www.somedocker.com/prefix/compaction:1.0.0"; String expectedEmrTag = "www.somedocker.com/prefix/bulk-import-runner-emr-serverless:1.0.0"; assertThat(commandsThatRan).containsExactly( - createBuildxBuilderInstanceCommand(), + createBuildxBuilderWithHostNetworkCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedCommitterTag, "./docker/statestore-committer", expectedBaseTag), pushImageCommand(expectedCommitterTag), buildImageCommand(expectedIngestTag, "./docker/ingest", expectedBaseTag), @@ -71,7 +78,8 @@ void shouldBuildAndPushDockerDeploymentImages() throws Exception { pushImageCommand(expectedBulkImportTag), buildAndPushMultiplatformImageCommand(expectedCompactionTag, "./docker/compaction", expectedBaseTag), buildImageCommand(expectedEmrTag, "./docker/bulk-import-runner-emr-serverless", expectedBaseTag), - pushImageCommand(expectedEmrTag)); + pushImageCommand(expectedEmrTag), + stopLocalRegistryCommand()); } @Test @@ -83,15 +91,17 @@ void shouldDisableCreatingBuildxBuilder() throws Exception { // When uploadAllImagesNoBuildxBuilder(dockerImageConfiguration); - // Then - String expectedBaseTag = "www.somedocker.com/prefix/base:1.0.0"; + // Then the buildx builder is not created (the caller set one up), but base images are still served from a + // local registry because the multiplatform compaction image cannot resolve its base from the local image store. + String expectedBaseTag = "localhost:5000/base:1.0.0"; String expectedCommitterTag = "www.somedocker.com/prefix/statestore-committer:1.0.0"; String expectedIngestTag = "www.somedocker.com/prefix/ingest:1.0.0"; String expectedBulkImportTag = "www.somedocker.com/prefix/bulk-import-runner:1.0.0"; String expectedCompactionTag = "www.somedocker.com/prefix/compaction:1.0.0"; String expectedEmrTag = "www.somedocker.com/prefix/bulk-import-runner-emr-serverless:1.0.0"; assertThat(commandsThatRan).containsExactly( - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedCommitterTag, "./docker/statestore-committer", expectedBaseTag), pushImageCommand(expectedCommitterTag), buildImageCommand(expectedIngestTag, "./docker/ingest", expectedBaseTag), @@ -100,7 +110,8 @@ void shouldDisableCreatingBuildxBuilder() throws Exception { pushImageCommand(expectedBulkImportTag), buildAndPushMultiplatformImageCommand(expectedCompactionTag, "./docker/compaction", expectedBaseTag), buildImageCommand(expectedEmrTag, "./docker/bulk-import-runner-emr-serverless", expectedBaseTag), - pushImageCommand(expectedEmrTag)); + pushImageCommand(expectedEmrTag), + stopLocalRegistryCommand()); } @Test @@ -126,7 +137,7 @@ void shouldBuildAndPushLambdaImages() throws Exception { assertThat(commandsThatRan).containsExactly( createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildLambdaImageCommand(expectedStatestoreTag, "./docker/lambda", expectedBaseTag), pushImageCommand(expectedStatestoreTag), buildLambdaImageCommand(expectedIngestTaskTag, "./docker/lambda", expectedBaseTag), @@ -163,7 +174,7 @@ void shouldBuildAndPushImageForDemonstrationCdkApp() throws Exception { assertThat(commandsThatRan).containsExactly( createBuildxBuilderInstanceCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), + buildAndLoadMultiplatformImageCommand(expectedBaseTag, "./docker/base"), buildImageCommand(expectedTag, "./docker/data-generation", expectedBaseTag), pushImageCommand(expectedTag)); } @@ -181,18 +192,20 @@ void shouldBuildBaseImageFromOverrideDirectoryWhenSet() throws Exception { // When uploadAllImages(dockerImageConfiguration, uploader); - // Then - String expectedBaseTag = "www.somedocker.com/prefix/base:1.0.0"; - String expectedSparkBaseTag = "www.somedocker.com/prefix/bulk-import-runner-base:1.0.0"; + // Then base images (the overridden default base and the per-image spark base) are served from the local + // registry, since the multiplatform compaction image builds in the buildx builder. + String expectedBaseTag = "localhost:5000/base:1.0.0"; + String expectedSparkBaseTag = "localhost:5000/bulk-import-runner-base:1.0.0"; String expectedCommitterTag = "www.somedocker.com/prefix/statestore-committer:1.0.0"; String expectedIngestTag = "www.somedocker.com/prefix/ingest:1.0.0"; String expectedBulkImportTag = "www.somedocker.com/prefix/bulk-import-runner:1.0.0"; String expectedCompactionTag = "www.somedocker.com/prefix/compaction:1.0.0"; String expectedEmrTag = "www.somedocker.com/prefix/bulk-import-runner-emr-serverless:1.0.0"; assertThat(commandsThatRan).containsExactly( - createBuildxBuilderInstanceCommand(), + createBuildxBuilderWithHostNetworkCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./custom/base"), + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedBaseTag, "./custom/base"), buildImageCommand(expectedCommitterTag, "./docker/statestore-committer", expectedBaseTag), pushImageCommand(expectedCommitterTag), buildImageCommand(expectedIngestTag, "./docker/ingest", expectedBaseTag), @@ -203,7 +216,8 @@ void shouldBuildBaseImageFromOverrideDirectoryWhenSet() throws Exception { pushImageCommand(expectedBulkImportTag), buildAndPushMultiplatformImageCommand(expectedCompactionTag, "./docker/compaction", expectedBaseTag), buildImageCommand(expectedEmrTag, "./docker/bulk-import-runner-emr-serverless", expectedBaseTag), - pushImageCommand(expectedEmrTag)); + pushImageCommand(expectedEmrTag), + stopLocalRegistryCommand()); } @Test @@ -213,21 +227,23 @@ void shouldFailWhenDockerBuildFails() { CommandPipeline buildImageCommand = buildImageCommand( "www.somedocker.com/prefix/statestore-committer:1.0.0", "./docker/statestore-committer", - "www.somedocker.com/prefix/base:1.0.0"); + "localhost:5000/base:1.0.0"); setReturnExitCodeForCommand(42, buildImageCommand); - // When / Then - String expectedBaseTag = "www.somedocker.com/prefix/base:1.0.0"; + // When / Then the local registry is torn down even though the build fails. + String expectedBaseTag = "localhost:5000/base:1.0.0"; assertThatThrownBy(() -> uploadAllImages(dockerImageConfiguration)) .isInstanceOfSatisfying(CommandFailedException.class, e -> { assertThat(e.getCommand()).isEqualTo(buildImageCommand); assertThat(e.getExitCode()).isEqualTo(42); }); assertThat(commandsThatRan).containsExactly( - createBuildxBuilderInstanceCommand(), + createBuildxBuilderWithHostNetworkCommand(), useBuildxBuilderInstanceCommand(), - buildAndPushMultiplatformImageCommand(expectedBaseTag, "./docker/base"), - buildImageCommand); + startLocalRegistryCommand(), + pushMultiplatformBaseToLocalRegistryCommand(expectedBaseTag, "./docker/base"), + buildImageCommand, + stopLocalRegistryCommand()); } protected void uploadAllImages(DockerImageConfiguration imageConfig) throws Exception { diff --git a/java/deployment/cdk/src/main/java/sleeper/cdk/artefacts/SleeperArtefactRepositories.java b/java/deployment/cdk/src/main/java/sleeper/cdk/artefacts/SleeperArtefactRepositories.java index 3b1c354d4cc..e0ad594f015 100644 --- a/java/deployment/cdk/src/main/java/sleeper/cdk/artefacts/SleeperArtefactRepositories.java +++ b/java/deployment/cdk/src/main/java/sleeper/cdk/artefacts/SleeperArtefactRepositories.java @@ -107,6 +107,10 @@ private void deployImages() { } for (DockerDeployment deployment : DockerDeployment.all()) { + // A base image is only ever a build input, built locally and never uploaded, so it needs no repository. + if (deployment.isDefaultBaseImage()) { + continue; + } Repository repository = createRepository(deployment.getDeploymentName()); if (deployment.isCreateEmrServerlessPolicy()) { diff --git a/java/deployment/cdk/src/test/java/sleeper/cdk/artefacts/repositories.approved.json b/java/deployment/cdk/src/test/java/sleeper/cdk/artefacts/repositories.approved.json index 0d8bff9e5c7..56e49953d5f 100644 --- a/java/deployment/cdk/src/test/java/sleeper/cdk/artefacts/repositories.approved.json +++ b/java/deployment/cdk/src/test/java/sleeper/cdk/artefacts/repositories.approved.json @@ -366,18 +366,6 @@ "UpdateReplacePolicy": "Delete", "DeletionPolicy": "Delete" }, - "Repositorybase41FBA974": { - "Type": "AWS::ECR::Repository", - "Properties": { - "EmptyOnDelete": true, - "LifecyclePolicy": { - "LifecyclePolicyText": "{\"rules\":[{\"rulePriority\":1,\"description\":\"Delete untagged images\",\"selection\":{\"tagStatus\":\"untagged\",\"countType\":\"sinceImagePushed\",\"countNumber\":1,\"countUnit\":\"days\"},\"action\":{\"type\":\"expire\"}},{\"rulePriority\":2,\"description\":\"Keep images for 365 days\",\"selection\":{\"tagStatus\":\"any\",\"countType\":\"sinceImagePushed\",\"countNumber\":365,\"countUnit\":\"days\"},\"action\":{\"type\":\"expire\"}}]}" - }, - "RepositoryName": "test-deployment/base" - }, - "UpdateReplacePolicy": "Delete", - "DeletionPolicy": "Delete" - }, "Repositoryingest2C547FF9": { "Type": "AWS::ECR::Repository", "Properties": {