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

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

Closed
porcelli opened this issue May 21, 2024 · 13 comments
Closed
Assignees

Comments

@porcelli
Copy link
Member

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:

  • SonataFlow :: Quarkus Serverless Workflow Extension :: Integration Tests
  • jBPM with Drools :: Quarkus Extension :: Integration Tests (Maven devmode)
  • jBPM :: Quarkus Extension :: Integration Tests (Hot Reload)

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

@porcelli porcelli converted this from a draft issue May 21, 2024
@porcelli
Copy link
Member Author

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
Copy link

fjtirado commented Jun 13, 2024

@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
Copy link

fjtirado commented Jun 14, 2024

@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
Copy link
Member Author

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

@fjtirado
Copy link

fjtirado commented Jun 14, 2024

@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
Copy link
Member Author

@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
Copy link

fjtirado commented Jun 14, 2024

@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
Copy link
Member Author

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

@fjtirado
Copy link

fjtirado commented Jun 14, 2024

@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
Copy link

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

@porcelli
Copy link
Member Author

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
Copy link

@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
Copy link
Member Author

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

@github-project-automation github-project-automation bot moved this from 📋 Backlog to 🎯 Done in 🦉 KIE Podling Board Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎯 Done
Development

No branches or pull requests

2 participants