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

[on hold] Verify, that grpc can be used for Openshift probes #1212

Closed
wants to merge 1 commit into from

Conversation

fedinskiy
Copy link
Contributor

@fedinskiy fedinskiy commented May 4, 2023

Summary

See quarkusio/quarkus#32113 for details.

Requires quarkus-qe/quarkus-test-framework#772

Waits for quarkusio/quarkus#33219 to be fixed, since right now it is not possible for user to create a custom health service using GRPC.

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

.withProperty("quarkus.opentelemetry.tracer.exporter.otlp.endpoint", jaeger::getCollectorUrl);
.withProperty("quarkus.opentelemetry.tracer.exporter.otlp.endpoint", jaeger::getCollectorUrl)
.withProperty("quarkus.openshift.readiness-probe.grpc-action", "9000:io.quarkus.example.PongService")
.withProperty("quarkus.openshift.liveness-probe.grpc-action", "9000:io.quarkus.example.PongService");
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to specify also method? e.g. does this call sayPong or returnLastTraceId? and can we set different method for liveness and readiness probe so that we have verified it works when liveness != readiness?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good question.
According to the specification[1], a cluster should use Check method of the service automatically [2] and "unimplemented health checking protocol <...> considered a probe failure". I would say our cluster doesn't use grpc probes, but they are enabled by default since 1.24[3] and we use 1.25 and there were no complaints during deployment.

[1] https://github.com/grpc/grpc/blob/master/doc/health-checking.md
[2] https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#define-a-grpc-liveness-probe
[2] https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-gates-for-graduated-or-deprecated-features

Copy link
Contributor

Choose a reason for hiding this comment

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

So if you added SmallRye Health this gRPC Health service would be generated automatically for you right? Do we have a way to verify that gRPC probes were actually used? e.g. right now we check that template is there and we assume that pods are ready because they were not killed and we are running tests, but we don't check that template doesn't contain also HTTP probes that could be used?

Unlike you, I didn't test it so you can lead me there, but I'd really like to:

  • check that it fails when such Health service is not available (e.g. custom health check that passes for liveness but not for readiness and we check that liveness succeeded but readiness failed)
  • check it doesn't fail when it is available and pod is ready and there are no other probes that could be used

Copy link
Contributor Author

@fedinskiy fedinskiy May 5, 2023

Choose a reason for hiding this comment

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

I verified, that the deployment fails, if the service is misspelled (eg HelathService). At the same time, I was not able to force deployment to fail because of error in liveness probe (see the code for an example). I will create a stand-alone reproducer and an issue on Tuesday

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fedinskiy
Copy link
Contributor Author

run tests

@michalvavrik
Copy link
Contributor

run tests

I thought we need FW release.

@fedinskiy fedinskiy marked this pull request as draft May 15, 2023 13:32
@fedinskiy fedinskiy changed the title Verify, that grpc can be used for Openshift probes [on hold] Verify, that grpc can be used for Openshift probes May 15, 2023
@fedinskiy
Copy link
Contributor Author

fedinskiy commented Jun 1, 2023

Since adding custom gRPC Health endpoints is not supported[1] I am closing this PR

[1] quarkusio/quarkus#33219 (comment)

@fedinskiy fedinskiy closed this Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants