Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Container image registry is ignored when using quarkus-openshift #28284

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

Sgitario
Copy link
Contributor

This commit fixes several issues when using the quarkus-openshift extension:

  • When providing the registry using the property quarkus.container-image.image (for example, quay.io/user/app:version, the registry is quay.io), the extension quarkus-openshift ignores the registry and uses the fallback property which is docker.io.

  • The quarkus-openshift extension always creates the ImageStream resource regardless of the container image builder used (s2i, docker, jib, ...). However, when the user provides a docker image, it was using it for the container image of the Deployment resource, and this ImageStream was never in use. After these changes, the Deployment resource always uses the ImageStream resource, so no changes in the documentation are necessary.

  • When the user provides a docker image and the deployment kind is "DeploymentConfig" (the default option), the quarkus-openshift extension removes the trigger image change params, so when deploying the application, the application is not automatically started.

@Sgitario
Copy link
Contributor Author

Relates to quarkiverse/quarkus-helm#100
cc @pjgg

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me

@Sgitario
Copy link
Contributor Author

Apart from the issues described in the description, I found another one related to the user permissions when using the default Dockerfile (the one provided by Quarkus when creating a new application) with the following content:

FROM registry.access.redhat.com/ubi8/ubi-minimal:8.3

ARG JAVA_PACKAGE=java-11-openjdk-headless
ARG RUN_JAVA_VERSION=1.3.8
ENV LANG='en_US.UTF-8' LANGUAGE='en_US:en'
# Install java and the run-java script
# Also set up permissions for user `1001`
RUN microdnf install curl ca-certificates ${JAVA_PACKAGE} \
    && microdnf update \
    && microdnf clean all \
    && mkdir /deployments \
    && chown 1001 /deployments \
    && chmod "g+rwX" /deployments \
    && chown 1001:root /deployments \
    && curl https://repo1.maven.org/maven2/io/fabric8/run-java-sh/${RUN_JAVA_VERSION}/run-java-sh-${RUN_JAVA_VERSION}-sh.sh -o /deployments/run-java.sh \
    && chown 1001 /deployments/run-java.sh \
    && chmod 540 /deployments/run-java.sh \
    && echo "securerandom.source=file:/dev/urandom" >> /etc/alternatives/jre/lib/security/java.security

# Configure the JAVA_OPTIONS, you can add -XshowSettings:vm to also display the heap size.
ENV JAVA_OPTIONS="-Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager"
# We make four distinct layers so if there are application changes the library layers can be re-used
COPY --chown=1001 target/quarkus-app/lib/ /deployments/lib/
COPY --chown=1001 target/quarkus-app/*.jar /deployments/
COPY --chown=1001 target/quarkus-app/app/ /deployments/app/
COPY --chown=1001 target/quarkus-app/quarkus/ /deployments/quarkus/

EXPOSE 8080
USER 1001

ENTRYPOINT [ "/deployments/run-java.sh" ]

When deploying the application to OpenShift, it will fail because it does not have permission to run the script run-java.sh. Note that OpenShift always uses a different user when running a pod (not the 1001). This issue is related to #25499.

The workaround I found for my testing is to change this line: && chmod 540 /deployments/run-java.sh \ to && chmod 775 /deployments/run-java.sh \. Basically, it relaxes the permissions for running the script. I'm not saying this is the right fix as I'm not an expert on the matters, and also because it already works on Kubernetes, but what do you think we should address this? @iocanel @geoand

@quarkus-bot

This comment has been minimized.

This commit fixes several issues when using the `quarkus-openshift` extension:
- When providing the registry using the property `quarkus.container-image.image` (for example, quay.io/user/app:version, the registry is `quay.io`), the extension `quarkus-openshift` ignores the registry and uses the fallback property which is `docker.io`.

- The `quarkus-openshift` extension always creates the ImageStream resource regardless of the container image builder used (s2i, docker, jib, ...). However, when the user provides a docker image, it was using it for the container image of the Deployment resource, and this ImageStream was never in use. After these changes, the Deployment resource always uses the ImageStream resource, so no changes in the documentation are necessary. 

- When the user provides a docker image and the deployment kind is "DeploymentConfig" (the default option), the `quarkus-openshift` extension removes the trigger image change params, so when deploying the application, the application is not automatically started.
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 29, 2022

Failing Jobs - Building 827b1ab

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@geoand
Copy link
Contributor

geoand commented Oct 2, 2022

The workaround I found for my testing is to change this line: && chmod 540 /deployments/run-java.sh \ to && chmod 775 /deployments/run-java.sh . Basically, it relaxes the permissions for running the script. I'm not saying this is the right fix as I'm not an expert on the matters, and also because it already works on Kubernetes, but what do you think we should address this? @iocanel @geoand

Yeah, it's not great but I don't really know what else we can do...

@Sgitario
Copy link
Contributor Author

Sgitario commented Oct 3, 2022

The workaround I found for my testing is to change this line: && chmod 540 /deployments/run-java.sh \ to && chmod 775 /deployments/run-java.sh . Basically, it relaxes the permissions for running the script. I'm not saying this is the right fix as I'm not an expert on the matters, and also because it already works on Kubernetes, but what do you think we should address this? @iocanel @geoand

Yeah, it's not great but I don't really know what else we can do...

It seems that the Dockerfile that is provided by Quarkus 2.13 has changed. I'm trying the new version to check whether this issue with OpenShift is gone.

@Sgitario
Copy link
Contributor Author

Sgitario commented Oct 3, 2022

The workaround I found for my testing is to change this line: && chmod 540 /deployments/run-java.sh \ to && chmod 775 /deployments/run-java.sh . Basically, it relaxes the permissions for running the script. I'm not saying this is the right fix as I'm not an expert on the matters, and also because it already works on Kubernetes, but what do you think we should address this? @iocanel @geoand

Yeah, it's not great but I don't really know what else we can do...

It seems that the Dockerfile that is provided by Quarkus 2.13 has changed. I'm trying the new version to check whether this issue with OpenShift is gone.

I confirmed that the new Dockerfile is now working in both K8s and OpenShift environments. It was fixed by c24179a

Copy link
Contributor

@iocanel iocanel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@geoand geoand merged commit a0be8a8 into quarkusio:main Oct 4, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Oct 4, 2022
@gsmet gsmet modified the milestones: 2.14 - main, 2.13.1.Final Oct 4, 2022
@Sgitario Sgitario deleted the fix_openshift_builder_image branch October 5, 2022 05:17
@gsmet gsmet modified the milestones: 2.13.1.Final, 2.13.2.Final Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants