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

Quarkus-openshift extension fails to deploy application which use management interface+https #32225

Closed
fedinskiy opened this issue Mar 29, 2023 · 7 comments · Fixed by #32902
Closed
Assignees
Labels
area/kubernetes kind/bug Something isn't working
Milestone

Comments

@fedinskiy
Copy link
Contributor

fedinskiy commented Mar 29, 2023

Describe the bug

I have an application, which uses separate management interface[1] with HTTPS and deployed on Openshift via quarkus-openshift extension[2]. The application fails to start after the deployment.

[1] #30506
[2] https://quarkus.io/guides/deploying-to-openshift

Expected behavior

Quarkus openshift extension should deploy an application in a completely working state.

Actual behavior

error: update acceptor rejected openshift-quickstart-1: pods for rc 'fvd-management-https/openshift-quickstart-1' took longer than 600 seconds to become available in the logs of deployment container and Warning Unhealthy 3s (x5 over 43s) kubelet Startup probe failed: Get "http://10.128.2.196:9000/q/health/started": EOF in the events (oc describe pod openshift-quickstart-1-$whatever) for the app itself

How to Reproduce?

  1. Clone: git clone [email protected]:fedinskiy/reproducer.git -b openshift-extension-management-https
  2. Create new openshift project oc new-project fvd-test-management-https
  3. Deploy the app according to manual: mvn clean install -Dquarkus.kubernetes.deploy=true -Dquarkus.openshift.route.expose=true -Dquarkus.kubernetes-client.trust-certs=true -Dquarkus.management.ssl.certificate.key-store-file=META-INF/server.keystore -Dquarkus.management.ssl.certificate.key-store-password=password
    Log contains something like
    [INFO] [io.quarkus.kubernetes.deployment.KubernetesDeployer] The deployed application can be accessed at: http://openshift-quickstart-fvd-management-https.apps.ocp4-12.rest.of.url
  4. Check the app
[fedinskiy@localhost reproducer]$ curl -v http://openshift-quickstart-fvd-management-https.apps.ocp4-12
<omitted for brevity>
> User-Agent: curl/7.85.0
< HTTP/1.0 503 Service Unavailable

For comparison:

  1. oc new-project fvd-management
  2. mvn clean install -Dquarkus.kubernetes.deploy=true -Dquarkus.openshift.route.expose=true -Dquarkus.kubernetes-client.trust-certs=true
  3. The application can be accessed: curl http://openshift-quickstart-fvd-management.apps.ocp4-12<omitted>

Output of uname -a or ver

6.0.18-300.fc37.x86_64

Output of java -version

17.0.5, vendor: GraalVM Community

GraalVM version (if different from Java)

No response

Quarkus version or git rev

34da513

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)

Additional information

This is a follow-up to #32135

@fedinskiy fedinskiy added the kind/bug Something isn't working label Mar 29, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 29, 2023

/cc @Sgitario (kubernetes), @geoand (kubernetes,openshift), @iocanel (kubernetes,openshift)

@Sgitario
Copy link
Contributor

According to this comment #32135 (comment) from @cescoffier , we could move the SSL config properties from runtime to build time, to know which schema to use (either HTTP or HTTPS).

Assuming that we move these properties, I have a couple of questions:

  • Moving these properties from runtime to build time means that users won't be able to modify them at runtime. Isn't this a regression or this was never the intention / or does not make sense for users to update this configuration at runtime?

  • I see that there are a lot of properties related to the SSL (keystore, truststore, ...), so maybe users use one property, but it does not activate the HTTPS protocol. Is there an easy logic to ensure that HTTPS will be used according to a combination of properties set?

An alternative solution is to allow users to explicitly configure the port and schema to use via properties (so the probes would not be configured automatically).

@cescoffier
Copy link
Member

The management interface has not been shipped yet (in a GA release), so we can still modify the behavior.

Yes we reduce the flexibility, but at the same time, you probably know at build time whether or not you use ssl, as you need the certs and so on (even if the certs are provisioned separately).

@Sgitario
Copy link
Contributor

Something else I saw today is that the port configuration (for Management and HTTP configuration) are runtime properties.

This means that if users change the port number at runtime, the generated manifests for OpenShift / Kubernetes will stop working.

I would say this is a very major issue, though the fix (changing these properties to be build-time-only properties) implies adding a strong limitation for users (since users won't be able to update the port to use any longer).

Not sure if we can / or make sense to allow changing these properties if and only if the Kubernetes extension is not present.

@rsvoboda
Copy link
Member

@Sgitario @cescoffier is this gonna be fixed for 3.0.0.Final? CC @gsmet

@Sgitario
Copy link
Contributor

Unfortunately, not. Still, we need to think of a solution for #32225 (comment).

@Sgitario
Copy link
Contributor

Something else I saw today is that the port configuration (for Management and HTTP configuration) are runtime properties.

This means that if users change the port number at runtime, the generated manifests for OpenShift / Kubernetes will stop working.

I would say this is a very major issue, though the fix (changing these properties to be build-time-only properties) implies adding a strong limitation for users (since users won't be able to update the port to use any longer).

Not sure if we can / or make sense to allow changing these properties if and only if the Kubernetes extension is not present.

I've reported #32882 to fix it.

I will be working on a fix for the issue management interface+https.

@Sgitario Sgitario self-assigned this Apr 25, 2023
Sgitario added a commit to Sgitario/quarkus that referenced this issue Apr 26, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue Apr 26, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue May 15, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue May 15, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue May 17, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue May 26, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue May 26, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue May 29, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sgitario added a commit to Sgitario/quarkus that referenced this issue May 30, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone May 30, 2023
sberyozkin pushed a commit to sberyozkin/quarkus that referenced this issue Jun 21, 2023
Note that these changes will move the management ssl configuration to built-time configuration. 

Moreover, the logic to select the HTTPS schema mimics to the logic in VertxHttpRecorder.initializeMainHttpServer method.

Fix quarkusio#32225
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubernetes kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants