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.

Sign up for GitHub

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

Do not check logs in serverless tests #2267

Merged
merged 1 commit into from
Jan 10, 2025
Merged

Do not check logs in serverless tests #2267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ...mum-reactive/src/test/java/io/quarkus/ts/http/minimum/reactive/HttpMinimumReactiveIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.List;
Expand Down Expand Up @@ -90,7 +91,7 @@ public void shortRecordReturnedDirectly() {

@Test
@Tag("https://github.com/quarkusio/quarkus/issues/44564")
void interceptedMethodFound() throws InterruptedException {
void interceptedMethodFound() {
Response operator = givenSpec()
.when()
.contentType(ContentType.JSON)
Expand All @@ -99,7 +100,7 @@ void interceptedMethodFound() throws InterruptedException {

assertEquals(200, operator.statusCode(), "Intercepted request was not processed");
assertEquals("Hello operator", operator.body().asString(), "Intercepted request was not processed properly");

assertNotEquals(0, app.getLogs().size(), "App logs are empty!");
await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> {
List<String> logs = app.getLogs();
boolean startFlag = false;
Expand Down
6 changes: 6 additions & 0 deletions ...inimum/reactive/ServerlessExtensionDockerBuildStrategyOpenShiftHttpMinimumReactiveIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static io.restassured.RestAssured.given;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;

import io.quarkus.test.scenarios.OpenShiftDeploymentStrategy;
Expand All @@ -18,4 +19,9 @@ public class ServerlessExtensionDockerBuildStrategyOpenShiftHttpMinimumReactiveI
protected RequestSpecification givenSpec() {
return HTTPS_CLIENT_SPEC;
}

@Override
@Disabled("Serverless deployments do not have logs which we can check")
void interceptedMethodFound() {
}
}
6 changes: 6 additions & 0 deletions ...o/quarkus/ts/http/minimum/reactive/ServerlessExtensionOpenShiftHttpMinimumReactiveIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static io.restassured.RestAssured.given;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;

import io.quarkus.test.scenarios.OpenShiftDeploymentStrategy;
Expand All @@ -19,4 +20,9 @@ public class ServerlessExtensionOpenShiftHttpMinimumReactiveIT extends HttpMinim
protected RequestSpecification givenSpec() {
return HTTPS_CLIENT_SPEC;
}

@Override
@Disabled("Serverless deployments do not have logs which we can check")
Copy link
Member

Choose a reason for hiding this comment

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

wow, this is surprising

Can you share why is that?

app.getLogs() can't retrieve them or even with oc tool there are no logs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you run oc status, there would be no pods to read logs from (which is, I guess, expected). Calling app.getLogs returns empty list.

Copy link
Member

Choose a reason for hiding this comment

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

ok, I guess we would need to actively collect logs in FW for any serverless deployment before it gets scaled to zero.

Copy link
Contributor Author

@fedinskiy fedinskiy Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

BTW, here how such deployment looks like:

Warning: apps.openshift.io/v1 DeploymentConfig is deprecated in v4.14+, unavailable in v4.10000+
In project fd-demonstration on server https://api.ocp4-17<...>

svc/app-00001 - <...> ports 80->8012, 443->8112
svc/app - kourier-internal.knative-serving-ingress.svc.cluster.local

svc/app-00001-private - <...> ports 80->8012, 443->8112, 9090->http-autometric, 9091->http-usermetric, 8022, 8012
  deployment/app-00001-deployment deploys image-registry.openshift-image-registry.svc:5000/fd-demonstration/app@sha256:<...>,registry.redhat.io/openshift-serverless-1/serving-queue-rhel8@sha256:<...>
    deployment #1 running for 22 minutes

bc/app source builds uploaded code on istag/openjdk-17:latest
  -> istag/app:1.0.0-SNAPSHOT
  build #1 succeeded 22 minutes ago


1 info identified, use 'oc status --suggest' to see details.

mvn clean verify -Dit.test=ServerlessExtensionOpenShiftHttpMinimumReactiveIT -Popenshift -Dts.openshift.delete.project.after.all=false -Dts.openshift.ephemeral.namespaces.enabled=false -Doc.reruns=0

from http/http-minimum-reactive

Copy link
Contributor

Choose a reason for hiding this comment

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

When I tried this, with @fedinskiy 's command, the pod were present in the openshift while tests were searching for logs and remained there for quite a while even after the test failed. Logs from the pod were manually readable no problem and contained all the lines tests are searching for.

IDK why tests were not able to read those. But I agree that for now it is best to disable it investigate the issue with logs reading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mocenas was it "build pod" or "app pod"? They can be distinguished by "Completed" status (or lack thereof). I have only seen the former.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mocenas was it "build pod" or "app pod"? They can be distinguished by "Completed" status (or lack thereof). I have only seen the former.

I was app pod. It was in state Running. (also build pod had "build" in name).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, now I see temporary pod, which lives for roughly 80 seconds, but the last line in its logs contains "11:26:56,497 Installed features: [cdi, kubernetes, rest, rest-jackson, smallrye-context-propagation, vertx]", so no notification about interception there.

Maybe it is a bug in Quarkus interception, but for now we cann't check it automatically anyway

Copy link
Member

@michalvavrik michalvavrik Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Let's consider that you really can't inspect Knative serving function logs (which makes it basically unusable in production, I suppose they just export it automatically to grafana or something like that we don't have) - you still have running application that returns response. Can you set quarkus.log.file.enable and quarkus.log.file.path and read it from the file system inside Quarkus application resource method? Here you have operator resource that you call https://github.com/quarkus-qe/quarkus-test-suite/blob/main/http/http-minimum-reactive/src/test/java/io/quarkus/ts/http/minimum/reactive/HttpMinimumReactiveIT.java#L99, is there something that stops you from adding a new method that returns logs for certain category (class - so that it is short)?

Interestingly, this can be implemented as a FW feature as well when at least one extension is based on Vert.x HTTP (so 99 of 100 deployments), but I suspect that there is proper solution possible and it just needs a time that we don't have.

void interceptedMethodFound() {
}
}
Loading