Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

[KOGITO-1294] Adding support for binary builds #201

Merged
merged 1 commit into from
Mar 12, 2020

Conversation

ricardozanini
Copy link
Member

@ricardozanini ricardozanini commented Mar 6, 2020

See:
https://issues.redhat.com/browse/KOGITO-1294

@radtriste @DuncanDoyle this is a preliminary version for the Binary Builds support. I still have some work to do:

  • Review why Build Configurations are triggering too much reconciliation requests
  • Introduce Kogito CLI kogito create-service <my-kogito-service>
  • Write documentation

To test it, you will need:

  1. Update the KogitoApp CRD on your cluster with the one from this PR
  2. Install the operator from this image: quay.io/ricardozanini/kogito-cloud-operator:0.9.0-rc1 (use the yaml files from deploy\)
  3. Use this example to create your own app:
apiVersion: app.kiegroup.org/v1alpha1
kind: KogitoApp
metadata:
  name: example-quarkus-flash
spec:
  build:
    imageVersion: 0.8.0-rc3
    imageRuntimeTag: quay.io/ricardozanini/kogito-quarkus-jvm-ubi8:0.9.0-rc1
  1. Compile any project and upload it to the cluster with:
oc start-build example-quarkus-flash-binary --from-dir=target

Tomorrow I will work in your feedback and remaining tasks to turn this into a green PR.

Thank you!

Many thanks for submiting your Pull Request ❤️!

Please make sure that your PR meets the following requirements:

  • You have read the contributors guide
  • Pull Request title is properly formatted: [KOGITO-XYZ] Subject
  • Pull Request contains link to the JIRA issue
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket
  • Your feature/bug fix has a unit test that verifies it
  • You've tested the new feature/bug fix in an actual OpenShift cluster

@ricardozanini ricardozanini added the WIP 👷‍♂️ Pull Request is on work in progress label Mar 6, 2020
@ricardozanini ricardozanini added this to the v0.9.0 milestone Mar 6, 2020
@sutaakar
Copy link
Contributor

So far looks ok. Will wait for final version for final review.

@ricardozanini ricardozanini added needs review 🔍 Pull Request that needs reviewers and removed WIP 👷‍♂️ Pull Request is on work in progress labels Mar 10, 2020
@ricardozanini ricardozanini marked this pull request as ready for review March 10, 2020 19:16
Copy link
Contributor

@radtriste radtriste left a comment

Choose a reason for hiding this comment

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

lgtm

  • OK for simple check CR & CLI creation & deployment
  • OK with infinispan
  • KO with events

I get
2020-03-11 10:15:33,258 ERROR [io.sma.rea.mes.imp.ConfiguredChannelFactory] (main) Unable to create the publisher or subscriber during initialization: java.util.NoSuchElementException: Cannot find attribute bootstrap.serversfor channelkogito-processinstances-events. Has been tried: mp.messaging.outgoing.kogito-processinstances-events.bootstrap.servers and mp.messaging.connector.smallrye-kafka.bootstrap.servers
and in environment there is KAFKA_BOOTSTRAP_SERVERS=kogito-kafka-kafka-bootstrap:9092
so seems weird ... (or a change in Quarkus ?)

Still need also to run the full tests on it

cmd/kogito/command/deploy/deploy_service.go Outdated Show resolved Hide resolved
@radtriste
Copy link
Contributor

Forgot, here are my commands for events failing:

  • Install crds and operator
  • kogito install data-index
  • kogito deploy-service example-quarkus --image-runtime quay.io/ricardozanini/kogito-quarkus-jvm-ubi8:0.8.0-rc4 --image-version 0.8.0-rc3 --install-infinispan Always --install-kafka Always
  • (in jbpm-quarkus-example) mvn clean package -Ppersistence,events -DskipTests
  • oc start-build example-quarkus-binary --from-dir=target

I get a CrashloopBackoff on example-quarkus pod with above error.

@ricardozanini
Copy link
Member Author

@radtriste I'll test this scenario locally, thanks for the input!

Copy link
Contributor

@sutaakar sutaakar left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@ricardozanini
Copy link
Member Author

ricardozanini commented Mar 11, 2020

@radtriste I believe that this error has no relation with this PR. I've reproduced it locally and make it work by manually adding these properties to the CR (can be blank, the operator set their value automatically):

  - name: MP_MESSAGING_OUTGOING_KOGITO_PROCESSINSTANCES_EVENTS_BOOTSTRAP_SERVERS
    value: kogito-kafka-kafka-bootstrap:9092
  - name: MP_MESSAGING_OUTGOING_KOGITO_USERTASKINSTANCES_EVENTS_BOOTSTRAP_SERVERS
    value: kogito-kafka-kafka-bootstrap:9092

The error should be the same for the traditional s2i builds as well. I've filed this bug report: quarkusio/quarkus#7791

This JIRA should help us get around this:
https://issues.redhat.com/browse/KOGITO-1193

The application.properties file should be mounted in a ConfigMap file and have the connection properties (Kafka/Infinispan) filled by the Operator.

A quick and dirty workaround is injecting those variables every time we have Kafka enabled for the App.

A good proposition would be having a property in the CR like enableDataIndex (or something like that), and then set those variables and ensure that Kafka is configured for the service. We could elaborate more in a JIRA. wdyt? Let's discuss this approach tomorrow.

Copy link
Contributor

@radtriste radtriste left a comment

Choose a reason for hiding this comment

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

Indeed, this is a Quarkus bug ...
The Cucumber test is passing as we set those env on KogitoApp CR:

  env:
    - name: MP_MESSAGING_OUTGOING_KOGITO_PROCESSINSTANCES_EVENTS_BOOTSTRAP_SERVERS
    - name: MP_MESSAGING_OUTGOING_KOGITO_USERTASKINSTANCES_EVENTS_BOOTSTRAP_SERVERS

other tests are running fine

Copy link
Member

@spolti spolti left a comment

Choose a reason for hiding this comment

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

great work!

@ricardozanini ricardozanini added ready 🚀 PR is ready to be merged and removed needs review 🔍 Pull Request that needs reviewers labels Mar 12, 2020
@ricardozanini
Copy link
Member Author

ricardozanini commented Mar 12, 2020

Depends on:
#209

@ricardozanini ricardozanini merged commit 1eb23b3 into apache:master Mar 12, 2020
@ricardozanini ricardozanini deleted the kogito-1294 branch March 12, 2020 18:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready 🚀 PR is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants