apache / incubator-kie-issues

Apache License 2.0
12 stars 1 forks source link

Move few integrations tests that depends on external image to kogito-apps #1241

Closed porcelli closed 3 months ago

porcelli commented 4 months ago

A new circular dependency has been identified in kogito-runtimes repository referencing an image that is build only on kogito-images.

A way to solve this issue is to move the following modules to kogito-apps repo:

In kogito-apps an image is build internally that can replace the quay.io image reference

porcelli commented 4 months ago

this can be postponed to after 10;

actually this is just surface, there are a lot of more code making ghost references to job-services and data-index in the runtimes repos than just those.

fjtirado commented 3 months ago

@porcelli Sonata Workflow Quarkus extension IT should not be referencing any kogito-apps image. It if is doing so, its an unexpected situation (very likely a bug that needs to be fixed). Do you remember which was the image getting downloaded, so we can take a look?

fjtirado commented 3 months ago

@porcelli If when running any runtimes quarkus IT test you see kogto apps images being downloaded, since runtimes test should not need them by desing, you can disable the downloading of the image by adding quarkus.kogito.devservices.enabled=false to application.properties in that module. Therefore there is no need to move code around at all. If there were some module in runtimes that has a dependency with apps (which is not the case for sonataflow), thats a bug and needs to be fixed.

porcelli commented 3 months ago

good point @fjtirado - we just need to make sure that we do it for the whole codebase

fjtirado commented 3 months ago

@porcelli In sonataflow automatic it test there was not need so far to add that property (test do not rely on job/dataindex). I think the inclusion of this property should be a reactive exercise, if there are failing test because docker is trying to download a kogito image that is not there, then this property will become handy. Since there is not really any dependency between Sonataflow runtimes and job/dataindex services, I was assuming that you what you saw is that during manual testing using quarkus:dev, docker was trying to download some kogito app images (very likely dataindex ephemeral). This is intentional for development environmment (so developers can use tooling in examples while not having to include any explicit dependency, you can discuss with @krisv about that), but it can be disable adding that property if user is not interested in using runtime tooling. But maybe you see something else, any extra information would be more than welcome.

porcelli commented 3 months ago

@fjtirado the build at this point should be failing as this container does not exists:

https://github.com/apache/incubator-kie-kogito-runtimes/blob/main/quarkus/extensions/kogito-quarkus-workflow-extension-common/kogito-quarkus-workflow-common-deployment/pom.xml#L37

I don't think we should be reactive, we should be proactive and identify all the places that needs to be fixed so we don't waste resources on unnecessary builds that we know will break.

fjtirado commented 3 months ago

@porcelli When I say reactive I meant fix all places that failed, not to blindly add the property everywhere. I assumed that image was only used in dev mode (as I said, that should be discussed with @krisv ), not in test mode (where it is not needed). If it is being used in test mode (I assumed it is not because test for runtimes sonataflow IT are working in my local and in GHA), then we need to add quarkus.kogito.devservices.enabled=false to application.properties in the test module

porcelli commented 3 months ago

@fjtirado when I tried to build with mvn clean install -Dfull - the build failed in several places due the image not found.

fjtirado commented 3 months ago

@porcelli Im trying to reproduce in my env in the meantime, can you try (that will be quick) running mvn clean install on quarkus/extensions/kogito-quarkus-serverless-workflow-extension/kogito-quarkus-serverless-workflow-integration-test at your end? This module depends on that pom you linked so the test result log will tell us if the image is being donwloaded (maybe I have it in the cache and thats why Im not able to see it getting downloaded, although I see it failing when running examples on dev mode)

fjtirado commented 3 months ago

@porcelli Are you running with main latest or another branch? (Maybe we are running different things)

porcelli commented 3 months ago

i was running main - it has been 2 weeks that I don't run... i'll try to get back to this today, or monday.

fjtirado commented 3 months ago

@porcelli Ok, right now in my local, these are the docker images before and after running (with no test failed) sonata flow integration tests.

ftirados@localhost:~/git/kogito-runtimes$ docker images
REPOSITORY   TAG       IMAGE ID   CREATED   SIZE
ftirados@localhost:~/git/kogito-runtimes$ docker images
REPOSITORY            TAG        IMAGE ID       CREATED        SIZE
postgres              14         fc37c3d673d5   5 weeks ago    422MB
testcontainers/ryuk   0.6.0      71009a3edde7   5 months ago   14.9MB
vectorized/redpanda   v21.11.8   50a97aff8f46   2 years ago    302MB
ftirados@localhost:~/git/kogito-runtimes$ 
porcelli commented 3 months ago

you are right, I don't know what has been changed, but seems that this problem is not present anymore.