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

Add section to configure when operator full knative #247

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions bundle/manifests/rhdh.redhat.com_orchestrators.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,16 @@ spec:
Job Service container image to be used instead of the provided
one by SonataFlow
type: string
broker:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

do we want to match the SFP structure instead, i.e:

eventing:
  description: Eventing related properties to set in the SonataFlow Platform
  properties:
    broker:
      description: Broker contains the name and namespace of the broker to use if using Knative eventing for components communications 
      properties:
        name:
          description: name describes the name of the broker to use
          type: string
        namespace:
          description: namespace describes the namespace on which the broker to use is deployed
          type: string
      type: object
type: object

Copy link
Collaborator

Choose a reason for hiding this comment

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

have you considered adding these fields to the sonataflowOperator structure? I'm aware that as it is here keeps consistency with the other fields (dataindex and job service image resource limits), but I find it strange that these fields reside under the orchestrator struct, even though they are specific to sonataflow. If they belong to both orchestrator and sonataflow, what do you think of the broker being its own high level entity, sibling to the orchestrator?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

For consistency sake, I think we can leave it at the same level with dataindex and job service images...(children of sonataflowPlatform) under the orchestrator.

Perhaps, for the go-operator, we split the sonataflowOperator (now serverlessLogicOperator) as we did for rhdh, and have the sonataflowPlatform configurations (dataindex, job service, broker etc) as a child since these are all specific to sonataflow. E.g

"serverlessLogic": {
  "installOperator": true
  "resources":
      "limits":
        "cpu": "cpu"
        "memory": "memory"
      "requests":
        "cpu": "cpu"
        "memory": "memory"
  "eventing":
    "broker":
      "name": "name"
      "namespace": "namespace"
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So I should introduce the eventing level here as I suggested, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes please, that's sounds fine to me.

description: Broker contains the name and namespace of the broker to use if using Knative eventing for components communications
properties:
name:
description: name describes the name of the broker to use
type: string
namespace:
description: namespace describes the namespace on which the broker to use is deployed
type: string
type: object
resources:
description: Resources contains the requests and limit of
CPU and memory resources for the pod instance
Expand Down
11 changes: 11 additions & 0 deletions config/crd/bases/rhdh.redhat.com_orchestrators.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,17 @@ spec:
description: This field contains the location of a custom Job Service container image to be used instead of the provided one by SonataFlow
default:
type: string
broker:
description: Broker contains the name and namespace of the broker to use if using Knative eventing for components communications
properties:
name:
description: name describes the name of the broker to use
type: string
namespace:
description: namespace describes the namespace on which the broker to use is deployed
type: string
type: object

type: object
type: object
tekton:
Expand Down
7 changes: 7 additions & 0 deletions docs/main/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,13 @@ Note that as of November 6, 2023, OpenShift Serverless Operator is based on RHEL
oc apply -n orchestrator -f https://raw.githubusercontent.com/parodos-dev/orchestrator-helm-operator/refs/heads/main/config/samples/_v1alpha1_orchestrator.yaml
```

### Using Knative kafka broker
If you want to use a Knative broker for communication between the different componenets (Data Index, Job Service and Workflows), you should use a reliable broker, i.e: not in-memory.

Kafka perfectly fullfills this reliability need.

Follow these [instructions](https://raw.githubusercontent.com/parodos-dev/orchestrator-helm-operator/refs/heads/main/docs/main/kafka-knative-broker/README.md) to setup the a kafka broker.

## Additional information

### Additional Workflow Namespaces
Expand Down
99 changes: 99 additions & 0 deletions docs/main/kafka-knative-broker/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# Using Knative kafka broker
If you want to use a Knative broker for communication between the different componenets (Data Index, Job Service and Workflows), you should use a reliable broker, i.e: not in-memory.

Kafka perfectly fullfills this reliability need.

## Pre-requisites

1. A Kafka cluster running, see https://strimzi.io/quickstarts/ for a quickstart setup

## Installation steps

1. Configure and enable kafka broker feature in Knative: https://knative.dev/docs/eventing/brokers/broker-types/kafka-broker/
* i.e:
Copy link
Collaborator

Choose a reason for hiding this comment

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

i.e meaning in essence? I'm curious why use the abbreviation in a statement that has no other sentences. What do you think of adding more context to this bullet point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i.e means that is, so, to configure knative with kafka, that is the commands you need to run.
I could have used e.g as the configuration may change and may differ https://www.merriam-webster.com/grammar/ie-vs-eg-abbreviation-meaning-usage-difference

```console
oc apply --filename https://github.com/knative-extensions/eventing-kafka-broker/releases/download/knative-v1.14.5/eventing-kafka-controller.yaml
oc apply --filename https://github.com/knative-extensions/eventing-kafka-broker/releases/download/knative-v1.14.5/eventing-kafka-broker.yaml
```
> [!NOTE]
> We are using the version `knative-v1.14.5` in this example, this may change, please refer to [the official documentation link](https://knative.dev/docs/eventing/brokers/broker-types/kafka-broker/)
* Wait for the `kafka-broker-receiver` resource to be ready:
```console
oc wait --for condition=ready=true pod -l app=kafka-broker-receiver -n knative-eventing --timeout=60s
```
* Review the `scc` to be granted to the `knative-kafka-broker-data-plane` service account used by the `kafka-broker-receiver` deployment:
```console
oc get deployments.apps -n knative-eventing kafka-broker-receiver -oyaml | oc adm policy scc-subject-review --filename -
```
* i.e:
```console
oc -n knative-eventing adm policy add-scc-to-user nonroot-v2 -z knative-kafka-broker-data-plane
```
Make sure the `replication.factor` of your Kafka cluster match the one of the `kafka-broker-config` ConfigMap. With the Strimzi quickstart, this value is `1`:
```console
oc patch cm kafka-broker-config -n knative-eventing \
--type merge \
-p '
{
"data": {
"default.topic.replication.factor": "1"
}
}'
```
2. Create kafka broker (Knative `sink`): see https://docs.openshift.com/serverless/1.33/eventing/brokers/kafka-broker.html for more details:
```Console
echo "apiVersion: eventing.knative.dev/v1
kind: Broker
metadata:
annotations:
# case-sensitive
eventing.knative.dev/broker.class: Kafka
name: kafka-broker
spec:
# Configuration specific to this broker.
config:
apiVersion: v1
kind: ConfigMap
name: kafka-broker-config
namespace: knative-eventing" | oc apply -f -
```
3. Configure the `sonataflowplatforms.sonataflow.org`: given that the `Orchestrator` is named `orchestrator-sample` and was created under the `orchestrator` namespace:
```console
oc -n orchestrator patch sorchestrators.rhdh.redhat.com orchestrator-sample --type merge \
-p '
{
"spec": {
"orchestrator": {
"sonataflowPlatform": {
"broker": {
"name": "<BROKER NAME>",
"namespace": "<BROKER NAMESPACE>"
}
}
}
}
}
```

You should have `sinkbinding` and `trigger` created:
```
$ oc -n sonataflow-infra get sinkbindings.sources.knative.dev
NAME SINK READY REASON
sonataflow-platform-jobs-service-sb http://kafka-broker-ingress.knative-eventing.svc.cluster.local/orchestrator/kafka-broker True

$ oc -n sonataflow-infra get trigger
NAME BROKER SUBSCRIBER_URI READY REASON
data-index-jobs-2ac1baab-d856-40bc-bcec-c6dd50951419 kafka-broker http://sonataflow-platform-data-index-service.orchestrator.svc.cluster.local/jobs True
data-index-process-definition-634c6f230b6364cdda8272f98c5d58722 kafka-broker http://sonataflow-platform-data-index-service.orchestrator.svc.cluster.local/definitions True
data-index-process-error-2ac1baab-d856-40bc-bcec-c6dd50951419 kafka-broker http://sonataflow-platform-data-index-service.orchestrator.svc.cluster.local/processes True
data-index-process-node-2ac1baab-d856-40bc-bcec-c6dd50951419 kafka-broker http://sonataflow-platform-data-index-service.orchestrator.svc.cluster.local/processes True
data-index-process-sla-2ac1baab-d856-40bc-bcec-c6dd50951419 kafka-broker http://sonataflow-platform-data-index-service.orchestrator.svc.cluster.local/processes True
data-index-process-state-2ac1baab-d856-40bc-bcec-c6dd50951419 kafka-broker http://sonataflow-platform-data-index-service.orchestrator.svc.cluster.local/processes True
data-index-process-variable-6f721bf111e75efc394000bca9884ae22ac kafka-broker http://sonataflow-platform-data-index-service.orchestrator.svc.cluster.local/processes True
jobs-service-create-job-2ac1baab-d856-40bc-bcec-c6dd50951419 kafka-broker http://sonataflow-platform-jobs-service.orchestrator.svc.cluster.local/v2/jobs/events True
jobs-service-delete-job-2ac1baab-d856-40bc-bcec-c6dd50951419 kafka-broker http://sonataflow-platform-jobs-service.orchestrator.svc.cluster.local/v2/jobs/events True
```

For each workflows deployed:
* A `sinkbinding` resource will be created: it will inject the `K_SINK` environment variable into the `deployment` resource. See https://knative.dev/docs/eventing/custom-event-source/sinkbinding/ for more information about`sinkbinding`.
* A `trigger` resource will be created for each event consumed by the workflow. See https://knative.dev/docs/eventing/triggers/ for more information about `trigger` and their usage.
9 changes: 9 additions & 0 deletions helm-charts/orchestrator/templates/sonataflows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ spec:
limits:
memory: {{ .Values.orchestrator.sonataflowPlatform.resources.limits.memory }}
cpu: {{ .Values.orchestrator.sonataflowPlatform.resources.limits.cpu }}
{{- if (and (.Values.orchestrator.sonataflowPlatform.broker.name) (.Values.orchestrator.sonataflowPlatform.broker.namespace)) }}
eventing:
broker:
ref:
apiVersion: eventing.knative.dev/v1
kind: Broker
name: {{ .Values.orchestrator.sonataflowPlatform.broker.name }}
namespace: {{ .Values.orchestrator.sonataflowPlatform.broker.namespace }}
{{- end }}
services:
dataIndex:
enabled: true
Expand Down