From 617ff8b657744baf4b540df4c82fe2fd73a54bc0 Mon Sep 17 00:00:00 2001 From: Ioannis Canellos Date: Thu, 3 Aug 2023 13:56:40 +0300 Subject: [PATCH] fix: use istag from other ns when internal reg is used --- .../main/asciidoc/deploying-to-openshift.adoc | 6 ++ .../ContainerImageOpenshiftConfig.java | 9 +- .../ApplyBuilderImageDecorator.java | 24 ++++- .../kubernetes/deployment/Constants.java | 1 + .../deployment/OpenshiftProcessor.java | 18 +++- ...WithBaseImageFromInternalRegistryTest.java | 88 +++++++++++++++++++ 6 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithBaseImageFromInternalRegistryTest.java diff --git a/docs/src/main/asciidoc/deploying-to-openshift.adoc b/docs/src/main/asciidoc/deploying-to-openshift.adoc index 1ff4619a3b958..33de117872abb 100644 --- a/docs/src/main/asciidoc/deploying-to-openshift.adoc +++ b/docs/src/main/asciidoc/deploying-to-openshift.adoc @@ -137,6 +137,12 @@ To trigger a container image build: The build that will be performed is a _s2i binary_ build. The input of the build is the jar that has been built locally and the output of the build is an `ImageStream` that is configured to automatically trigger a deployment. +The base/builder image is specified using `base-jvm-image` and `base-native-image` for jvm and native mode respectively. An ImageStream for the image is automatically generated, unless these properties are used to reference an existing ImageStreamTag in the internal openshift registry. For example: + +[source,properties] +---- +quarkus.openshift.base-jvm-image=image-registry.openshift-image-registry.svc:5000/some-project/openjdk-11:17.1.16. +---- [NOTE] ==== diff --git a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/ContainerImageOpenshiftConfig.java b/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/ContainerImageOpenshiftConfig.java index ba12fab0899d6..d2adf4fe379ff 100644 --- a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/ContainerImageOpenshiftConfig.java +++ b/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/ContainerImageOpenshiftConfig.java @@ -45,7 +45,9 @@ public static String getDefaultJvmImage(CompiledJavaVersionBuildItem.JavaVersion /** * The base image to be used when a container image is being produced for the jar build. - * + * The value of this property is used to create an ImageStream for the builder image used in the Openshift build. + * When it references images already available in the internal Openshift registry, the corresponding streams are used + * instead. * When the application is built against Java 17 or higher, {@code registry.access.redhat.com/ubi8/openjdk-17:1.16} * is used as the default. * Otherwise {@code registry.access.redhat.com/ubi8/openjdk-11:1.16} is used as the default. @@ -54,7 +56,10 @@ public static String getDefaultJvmImage(CompiledJavaVersionBuildItem.JavaVersion public Optional baseJvmImage; /** - * The base image to be used when a container image is being produced for the native binary build + * The base image to be used when a container image is being produced for the native binary build. + * The value of this property is used to create an ImageStream for the builder image used in the Openshift build. + * When it references images already available in the internal Openshift registry, the corresponding streams are used + * instead. */ @ConfigItem(defaultValue = DEFAULT_BASE_NATIVE_IMAGE) public String baseNativeImage; diff --git a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ApplyBuilderImageDecorator.java b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ApplyBuilderImageDecorator.java index 5afcc842c524a..dbc23467e9cc6 100644 --- a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ApplyBuilderImageDecorator.java +++ b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/ApplyBuilderImageDecorator.java @@ -1,10 +1,14 @@ package io.quarkus.kubernetes.deployment; +import static io.quarkus.kubernetes.deployment.Constants.OPENSHIFT_INTERNAL_REGISTRY_PROJECT; + +import java.util.Optional; + import io.dekorate.kubernetes.decorator.*; import io.dekorate.s2i.decorator.AddBuildConfigResourceDecorator; -import io.dekorate.utils.Images; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.openshift.api.model.SourceBuildStrategyFluent; +import io.quarkus.container.spi.ImageReference; public class ApplyBuilderImageDecorator extends NamedResourceDecorator> { @@ -21,11 +25,23 @@ public ApplyBuilderImageDecorator(String name, String image) { @Override public void andThenVisit(SourceBuildStrategyFluent strategy, ObjectMeta meta) { - String builderRepository = Images.getRepository(image); - String builderTag = Images.getTag(image); + ImageReference imageRef = ImageReference.parse(image); + + String builderRepository = imageRef.getRepository(); + String builderTag = imageRef.getTag(); String builderName = !builderRepository.contains("/") ? builderRepository : builderRepository.substring(builderRepository.lastIndexOf("/") + 1); - strategy.withNewFrom().withKind("ImageStreamTag").withName(builderName + ":" + builderTag).endFrom(); + Optional builderGroup = Optional.of(builderRepository) + .filter(s -> s.contains("/")) + .map(s -> s.substring(0, s.indexOf("/"))); + + boolean usesInternalRegistry = imageRef.getRegistry() + .filter(registry -> registry.contains(OPENSHIFT_INTERNAL_REGISTRY_PROJECT)).isPresent(); + strategy.withNewFrom() + .withKind("ImageStreamTag") + .withName(builderName + ":" + builderTag) + .withNamespace(builderGroup.filter(g -> usesInternalRegistry).orElse(null)) + .endFrom(); } @Override diff --git a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/Constants.java b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/Constants.java index aee8889971a89..dd11245f8abf1 100644 --- a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/Constants.java +++ b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/Constants.java @@ -38,6 +38,7 @@ public final class Constants { static final String DEFAULT_S2I_IMAGE_NAME = "s2i-java"; //refers to the Dekorate default image. static final String OPENSHIFT_INTERNAL_REGISTRY = "image-registry.openshift-image-registry.svc:5000"; + static final String OPENSHIFT_INTERNAL_REGISTRY_PROJECT = "openshift-image-registry"; //a more relaxed str to match static final String KNATIVE = "knative"; static final String KNATIVE_SERVICE = "Service"; diff --git a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/OpenshiftProcessor.java b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/OpenshiftProcessor.java index 94df9d8d36a26..ffcbf904b52f7 100644 --- a/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/OpenshiftProcessor.java +++ b/extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/OpenshiftProcessor.java @@ -5,6 +5,7 @@ import static io.quarkus.kubernetes.deployment.Constants.LIVENESS_PROBE; import static io.quarkus.kubernetes.deployment.Constants.OPENSHIFT; import static io.quarkus.kubernetes.deployment.Constants.OPENSHIFT_APP_RUNTIME; +import static io.quarkus.kubernetes.deployment.Constants.OPENSHIFT_INTERNAL_REGISTRY_PROJECT; import static io.quarkus.kubernetes.deployment.Constants.QUARKUS; import static io.quarkus.kubernetes.deployment.Constants.READINESS_PROBE; import static io.quarkus.kubernetes.deployment.Constants.ROUTE; @@ -43,6 +44,7 @@ import io.quarkus.container.spi.ContainerImageInfoBuildItem; import io.quarkus.container.spi.ContainerImageLabelBuildItem; import io.quarkus.container.spi.FallbackContainerImageRegistryBuildItem; +import io.quarkus.container.spi.ImageReference; import io.quarkus.container.spi.SingleSegmentContainerImageRequestBuildItem; import io.quarkus.deployment.Capabilities; import io.quarkus.deployment.Capability; @@ -300,14 +302,26 @@ public List createDecorators(ApplicationInfoBuildItem applic // Handle custom s2i builder images baseImage.map(BaseImageInfoBuildItem::getImage).ifPresent(builderImage -> { String builderImageName = ImageUtil.getName(builderImage); - S2iBuildConfig s2iBuildConfig = new S2iBuildConfigBuilder().withBuilderImage(builderImage).build(); if (!DEFAULT_S2I_IMAGE_NAME.equals(builderImageName)) { result.add(new DecoratorBuildItem(OPENSHIFT, new RemoveBuilderImageResourceDecorator(DEFAULT_S2I_IMAGE_NAME))); } if (containerImageConfig.builder.isEmpty() || config.isOpenshiftBuildEnabled(containerImageConfig, capabilities)) { - result.add(new DecoratorBuildItem(OPENSHIFT, new AddBuilderImageStreamResourceDecorator(s2iBuildConfig))); result.add(new DecoratorBuildItem(OPENSHIFT, new ApplyBuilderImageDecorator(name, builderImage))); + ImageReference imageRef = ImageReference.parse(builderImage); + boolean usesInternalRegistry = imageRef.getRegistry() + .filter(registry -> registry.contains(OPENSHIFT_INTERNAL_REGISTRY_PROJECT)).isPresent(); + if (usesInternalRegistry) { + // When the internal registry is specified for the image, we assume the stream already exists + // It's better if we refer to it directly as (as an ImageStreamTag). + // In this case we need to remove the ImageStream (created by dekorate). + String repository = imageRef.getRepository(); + String imageStreamName = repository.substring(repository.lastIndexOf("/")); + result.add(new DecoratorBuildItem(OPENSHIFT, new RemoveBuilderImageResourceDecorator(imageStreamName))); + } else { + S2iBuildConfig s2iBuildConfig = new S2iBuildConfigBuilder().withBuilderImage(builderImage).build(); + result.add(new DecoratorBuildItem(OPENSHIFT, new AddBuilderImageStreamResourceDecorator(s2iBuildConfig))); + } } }); diff --git a/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithBaseImageFromInternalRegistryTest.java b/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithBaseImageFromInternalRegistryTest.java new file mode 100644 index 0000000000000..003e1404cbdf7 --- /dev/null +++ b/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithBaseImageFromInternalRegistryTest.java @@ -0,0 +1,88 @@ +package io.quarkus.it.kubernetes; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.Container; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.openshift.api.model.BuildConfig; +import io.fabric8.openshift.api.model.DeploymentConfig; +import io.fabric8.openshift.api.model.DeploymentTriggerImageChangeParams; +import io.fabric8.openshift.api.model.ImageStream; +import io.quarkus.builder.Version; +import io.quarkus.maven.dependency.Dependency; +import io.quarkus.test.ProdBuildResults; +import io.quarkus.test.ProdModeTestResults; +import io.quarkus.test.QuarkusProdModeTest; + +public class OpenshiftWithBaseImageFromInternalRegistryTest { + + private static final String APP_NAME = "openshift-with-base-image-stream"; + + @RegisterExtension + static final QuarkusProdModeTest config = new QuarkusProdModeTest() + .withApplicationRoot((jar) -> jar.addClasses(GreetingResource.class)) + .setApplicationName(APP_NAME) + .setApplicationVersion("0.1-SNAPSHOT") + .overrideConfigKey("quarkus.openshift.base-jvm-image", + "image-registry.openshift-image-registry.svc:5000/myns/myimage:1.0") + .setForcedDependencies(List.of(Dependency.of("io.quarkus", "quarkus-openshift", Version.getVersion()))); + + @ProdBuildResults + private ProdModeTestResults prodModeTestResults; + + @Test + public void assertGeneratedResources() throws IOException { + Path kubernetesDir = prodModeTestResults.getBuildDir().resolve("kubernetes"); + + assertThat(kubernetesDir).isDirectoryContaining(p -> p.getFileName().endsWith("openshift.json")) + .isDirectoryContaining(p -> p.getFileName().endsWith("openshift.yml")); + List openshiftList = DeserializationUtil.deserializeAsList(kubernetesDir.resolve("openshift.yml")); + + assertThat(openshiftList).filteredOn(h -> "DeploymentConfig".equals(h.getKind())).singleElement().satisfies(h -> { + assertThat(h.getMetadata()).satisfies(m -> { + assertThat(m.getName()).isEqualTo(APP_NAME); + }); + assertThat(h).isInstanceOfSatisfying(DeploymentConfig.class, d -> { + Container container = d.getSpec().getTemplate().getSpec().getContainers().get(0); + assertThat(container.getImage()).endsWith(APP_NAME + ":0.1-SNAPSHOT"); + + DeploymentTriggerImageChangeParams imageTriggerParams = d.getSpec().getTriggers().get(0).getImageChangeParams(); + assertThat(imageTriggerParams.getFrom().getKind()).isEqualTo("ImageStreamTag"); + assertThat(imageTriggerParams.getFrom().getName()).isEqualTo(APP_NAME + ":0.1-SNAPSHOT"); + }); + }); + + assertThat(openshiftList).filteredOn(h -> "BuildConfig".equals(h.getKind())).singleElement().satisfies(h -> { + assertThat(h.getMetadata()).satisfies(m -> { + assertThat(m.getName()).isEqualTo(APP_NAME); + }); + assertThat(h).isInstanceOfSatisfying(BuildConfig.class, b -> { + assertThat(b.getSpec().getStrategy().getSourceStrategy().getFrom()).satisfies(f -> { + assertThat(f.getKind()).isEqualTo("ImageStreamTag"); + assertThat(f.getNamespace()).isEqualTo("myns"); + assertThat(f.getName()).isEqualTo("myimage:1.0"); + }); + }); + }); + + //Verify that we only got one Image + assertThat(openshiftList).filteredOn(h -> "ImageStream".equals(h.getKind())).singleElement() + .satisfies(h -> { + assertThat(h.getMetadata()).satisfies(m -> { + assertThat(m.getName()).isEqualTo(APP_NAME); + }); + + assertThat(h).isInstanceOfSatisfying(ImageStream.class, i -> { + assertThat(i.getSpec().getDockerImageRepository()).isNull(); + }); + }); + + } +}