From 8bc5a9376a1c2192b20650d1f20e0a76877cb3d8 Mon Sep 17 00:00:00 2001 From: Ioannis Canellos Date: Tue, 28 Nov 2023 11:59:39 +0200 Subject: [PATCH 1/2] refactor: remove OpenshiftBaseXXXImage classes --- .../deployment/OpenshiftBaseJavaImage.java | 86 ------------------- .../deployment/OpenshiftBaseNativeImage.java | 57 ------------ .../deployment/OpenshiftProcessor.java | 32 +------ 3 files changed, 3 insertions(+), 172 deletions(-) delete mode 100644 extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseJavaImage.java delete mode 100644 extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseNativeImage.java diff --git a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseJavaImage.java b/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseJavaImage.java deleted file mode 100644 index 4cee5990eb667..0000000000000 --- a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseJavaImage.java +++ /dev/null @@ -1,86 +0,0 @@ - -package io.quarkus.container.image.openshift.deployment; - -import java.util.Optional; - -import io.quarkus.container.image.deployment.util.ImageUtil; - -public enum OpenshiftBaseJavaImage { - - //We only compare `repositories` so registries and tags are stripped - FABRIC8("fabric8/s2i-java:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", "JAVA_CLASSPATH", - "JAVA_OPTIONS"), - OPENJDK_8_RHEL7("redhat-openjdk-18/openjdk18-openshift:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", - "JAVA_LIB_DIR", "JAVA_CLASSPATH", "JAVA_OPTIONS"), - OPENJDK_8_RHEL8("openjdk/openjdk-8-rhel8:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", - "JAVA_CLASSPATH", "JAVA_OPTIONS"), - OPENJDK_11_RHEL7("openjdk/openjdk-11-rhel7:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", - "JAVA_CLASSPATH", "JAVA_OPTIONS"), - OPENJDK_11_RHEL8("openjdk/openjdk-11-rhel8:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", - "JAVA_CLASSPATH", "JAVA_OPTIONS"), - OPENJ9_8_RHEL7("openj9/openj9-8-rhel7:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", - "JAVA_CLASSPATH", "JAVA_OPTIONS"), - OPENJ9_8_RHEL8("openj9/openj9-8-rhel8:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", - "JAVA_CLASSPATH", "JAVA_OPTIONS"), - OPENJ9_11_RHEL7("openj9/openj9-11-rhel7:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", - "JAVA_CLASSPATH", "JAVA_OPTIONS"), - OPENJ9_11_RHEL8("openj9/openj9-11-rhel8:latest", "/deployments", "JAVA_MAIN_CLASS", "JAVA_APP_JAR", "JAVA_LIB_DIR", - "JAVA_CLASSPATH", "JAVA_OPTIONS"); - - private final String image; - private final String jarDirectory; - private final String javaMainClassEnvVar; - private final String jarEnvVar; - private final String jarLibEnvVar; - private final String classpathEnvVar; - private final String jvmOptionsEnvVar; - - public static Optional findMatching(String image) { - for (OpenshiftBaseJavaImage candidate : OpenshiftBaseJavaImage.values()) { - if (ImageUtil.getRepository(candidate.getImage()).equals(ImageUtil.getRepository(image))) { - return Optional.of(candidate); - } - } - return Optional.empty(); - } - - private OpenshiftBaseJavaImage(String image, String jarDirectory, String javaMainClassEnvVar, String jarEnvVar, - String jarLibEnvVar, String classpathEnvVar, String jvmOptionsEnvVar) { - this.image = image; - this.jarDirectory = jarDirectory; - this.javaMainClassEnvVar = javaMainClassEnvVar; - this.jarEnvVar = jarEnvVar; - this.jarLibEnvVar = jarLibEnvVar; - this.classpathEnvVar = classpathEnvVar; - this.jvmOptionsEnvVar = jvmOptionsEnvVar; - } - - public String getImage() { - return image; - } - - public String getJarDirectory() { - return this.jarDirectory; - } - - public String getJavaMainClassEnvVar() { - return javaMainClassEnvVar; - } - - public String getJvmOptionsEnvVar() { - return jvmOptionsEnvVar; - } - - public String getClasspathEnvVar() { - return classpathEnvVar; - } - - public String getJarLibEnvVar() { - return jarLibEnvVar; - } - - public String getJarEnvVar() { - return jarEnvVar; - } - -} diff --git a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseNativeImage.java b/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseNativeImage.java deleted file mode 100644 index b1f143efac5c6..0000000000000 --- a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftBaseNativeImage.java +++ /dev/null @@ -1,57 +0,0 @@ - -package io.quarkus.container.image.openshift.deployment; - -import java.util.Optional; - -import io.quarkus.container.image.deployment.util.ImageUtil; - -public enum OpenshiftBaseNativeImage { - - //We only compare `repositories` so registries and tags are stripped - QUARKUS("quarkus/ubi-quarkus-native-binary-s2i:2.0", "/home/quarkus/", "application", "QUARKUS_HOME", "QUARKUS_OPTS"); - - private final String image; - private final String nativeBinaryDirectory; - private final String fixedNativeBinaryName; - private final String homeDirEnvVar; - private final String optsEnvVar; - - public static Optional findMatching(String image) { - for (OpenshiftBaseNativeImage candidate : OpenshiftBaseNativeImage.values()) { - if (ImageUtil.getRepository(candidate.getImage()).equals(ImageUtil.getRepository(image))) { - return Optional.of(candidate); - } - } - return Optional.empty(); - } - - private OpenshiftBaseNativeImage(String image, String nativeBinaryDirectory, String fixedNativeBinaryName, - String homeDirEnvVar, String optsEnvVar) { - this.image = image; - this.nativeBinaryDirectory = nativeBinaryDirectory; - this.fixedNativeBinaryName = fixedNativeBinaryName; - this.homeDirEnvVar = homeDirEnvVar; - this.optsEnvVar = optsEnvVar; - } - - public String getImage() { - return image; - } - - public String getNativeBinaryDirectory() { - return nativeBinaryDirectory; - } - - public String getFixedNativeBinaryName() { - return this.fixedNativeBinaryName; - } - - public String getHomeDirEnvVar() { - return homeDirEnvVar; - } - - public String getOptsEnvVar() { - return optsEnvVar; - } - -} diff --git a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java b/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java index 7354e21cab4e2..f445a45abb515 100644 --- a/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java +++ b/extensions/container-image/container-image-openshift/deployment/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java @@ -146,24 +146,14 @@ public void openshiftRequirementsJvm(ContainerImageOpenshiftConfig openshiftConf boolean hasCustomJvmArguments = config.jvmArguments.isPresent(); builderImageProducer.produce(new BaseImageInfoBuildItem(baseJvmImage)); - Optional baseImage = OpenshiftBaseJavaImage.findMatching(baseJvmImage); if (config.buildStrategy == BuildStrategy.BINARY) { // Jar directory priorities: // 1. explicitly specified by the user. - // 2. detected via OpenshiftBaseJavaImage // 3. fallback value - String jarDirectory = config.jarDirectory - .orElse(baseImage.map(i -> i.getJarDirectory()).orElse(config.FALLBACK_JAR_DIRECTORY)); + String jarDirectory = config.jarDirectory.orElse(config.FALLBACK_JAR_DIRECTORY); String pathToJar = concatUnixPaths(jarDirectory, jarFileName); - // If the image is known, we can define env vars for classpath, jar, lib etc. - baseImage.ifPresent(b -> { - envProducer.produce(KubernetesEnvBuildItem.createSimpleVar(b.getJarEnvVar(), pathToJar, null)); - envProducer.produce(KubernetesEnvBuildItem.createSimpleVar(b.getJvmOptionsEnvVar(), - String.join(" ", config.getEffectiveJvmArguments()), null)); - }); - //In all other cases its the responsibility of the image to set those up correctly. if (hasCustomJarPath || hasCustomJvmArguments) { List cmd = new ArrayList<>(); @@ -172,8 +162,6 @@ public void openshiftRequirementsJvm(ContainerImageOpenshiftConfig openshiftConf cmd.addAll(Arrays.asList("-jar", pathToJar)); envProducer.produce(KubernetesEnvBuildItem.createSimpleVar(JAVA_APP_JAR, pathToJar, null)); commandProducer.produce(KubernetesCommandBuildItem.command(cmd)); - } else if (baseImage.isEmpty()) { - envProducer.produce(KubernetesEnvBuildItem.createSimpleVar(JAVA_APP_JAR, pathToJar, null)); } } } @@ -209,26 +197,12 @@ public void openshiftRequirementsNative(ContainerImageOpenshiftConfig openshiftC if (config.buildStrategy == BuildStrategy.BINARY) { builderImageProducer.produce(new BaseImageInfoBuildItem(config.baseNativeImage)); - Optional baseImage = OpenshiftBaseNativeImage.findMatching(config.baseNativeImage); // Native binary directory priorities: // 1. explicitly specified by the user. - // 2. detected via OpenshiftBaseNativeImage - // 3. fallback value + // 2. fallback vale - String nativeBinaryDirectory = config.nativeBinaryDirectory - .orElse(baseImage.map(i -> i.getNativeBinaryDirectory()).orElse(config.FALLBACK_NATIVE_BINARY_DIRECTORY)); + String nativeBinaryDirectory = config.nativeBinaryDirectory.orElse(config.FALLBACK_NATIVE_BINARY_DIRECTORY); String pathToNativeBinary = concatUnixPaths(nativeBinaryDirectory, nativeBinaryFileName); - - baseImage.ifPresent(b -> { - envProducer.produce( - KubernetesEnvBuildItem.createSimpleVar(b.getHomeDirEnvVar(), nativeBinaryDirectory, OPENSHIFT)); - config.nativeArguments.ifPresent(nativeArguments -> { - envProducer.produce(KubernetesEnvBuildItem.createSimpleVar(b.getOptsEnvVar(), - String.join(" ", nativeArguments), OPENSHIFT)); - }); - - }); - if (hasCustomNativePath || hasCustomNativeArguments) { commandProducer .produce(KubernetesCommandBuildItem.commandWithArgs(pathToNativeBinary, config.nativeArguments.get())); From f6f8626f4d661cbcaab0d960c891e9a63f2e4187 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Mon, 4 Dec 2023 14:07:38 +0100 Subject: [PATCH 2/2] Fix OpenshiftWithS2iTest JAVA_OPTS are not automatically added anymore. But TBH, I wonder if it's the right move to not have these JAVA_OPTS as we need to set the logger anyway. --- .../java/io/quarkus/it/kubernetes/OpenshiftWithS2iTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithS2iTest.java b/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithS2iTest.java index 136f04936feb5..d3049dc5063de 100644 --- a/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithS2iTest.java +++ b/integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithS2iTest.java @@ -86,11 +86,6 @@ public void assertGeneratedResources() throws IOException { assertThat(envVar.getName()).isEqualTo("JAVA_APP_JAR"); //assertThat(envVar.getValue()).isEqualTo("/deployments/quarkus-run.jar"); // this is flaky }); - assertThat(envVars).anySatisfy(envVar -> { - assertThat(envVar.getName()).isEqualTo("JAVA_OPTIONS"); - assertThat(envVar.getValue()) - .contains("-Djava.util.logging.manager=org.jboss.logmanager.LogManager"); - }); }); });