Refactoring OpenFuncAsync runtime definition about openfunction

[benjaminhuo]

it can also be:

// for keda
scaleOptions:
  keda:
    minReplicaCount:  10
    maxReplicaCount:  100

// for knative
scaleOptions:
  knative:
    minReplicaCount:  10
    maxReplicaCount:  100

A function can only use one runtime, so setting min/maxReplicaCount just once is ok

[tpiperatgod]

Knative's autoscaling options are put into annotations which is map I think. But we can take a look at the KnativeServing crd of knative operator

https://github.com/knative/serving-operator/blob/master/pkg/apis/serving/v1alpha1/knativeserving_types.go#L99

Looks better to just use a map

example:

type ScaleOptions struct {
	Max     int32              `json:"max,omitempty"`
	Min     int32              `json:"min,omitempty"`
	Keda    *KedaScaleOptions  `json:"keda,omitempty"`
	Knative *map[string]string `json:"knative,omitempty"`
}

type KedaScaleOptions struct {
	// +optional
	ScaledObject *KedaScaledObject `json:"scaledObject,omitempty"`
	// +optional
	ScaledJob *KedaScaledJob `json:"scaledJob,omitempty"`
}

[tpiperatgod]

@tpiperatgod Do we need to change functions framework and builders besides the CRD and controller changes?

The logic of the CRD( and the controller) part is associated with the processing of the function-framework part through the OpenFunction Context.

The two parts can be considered decoupled unless the contextual structure changes.

[tpiperatgod]

Maybe we can simply remove the dapr and keda and use bindings and scaleOptions instead?
What do you think @wanjunlei @tpiperatgod ？

I agree with adding scaleOptions, which would solve the problem in #173. I have suggested follows in the proposal https://hackmd.io/A-r2frbVTpiDhpfNc7ac2w:

I would prefer to add a new property to spec.serving in function crd to support this feature, as it is a feature of the serving phase.

the property can be spec.serving.scaleOptions

For bindings, since we also support pubsub type input and output sources, I'd like to know if spec.serving.bindings refers to both bindings and pubsub type input and output sources or just bindings type input and output sources?

[benjaminhuo]

OK, you mean the scaleOptions could be applied to both the sync and async runtime

      scaleOptions:
        minReplicaCount:  10                               # 
        maxReplicaCount:  100 
        knative:
          xx
        keda:
          scaledObject:

And yes spec.serving.bindings refers to both dapr bindings and dapr pubsub
The dapr binding docs said "Using bindings, you can trigger your app with events coming in from external systems, or interface with external systems", so dapr bindings and dapr pubsub are used for the same purpose in OpenFuncAsync runtime

The bindings could be moved up to the serving level as well because knative and the upcoming OpenFuncSync runtime could use these components as well.

[tpiperatgod]

OK, you mean the scaleOptions could be applied to both the sync and async runtime

      scaleOptions:
        minReplicaCount:  10                               # 
        maxReplicaCount:  100 
        knative:
          xx
        keda:
          scaledObject:

it can also be:

// for keda
scaleOptions:
  keda:
    minReplicaCount:  10
    maxReplicaCount:  100

// for knative
scaleOptions:
  knative:
    minReplicaCount:  10
    maxReplicaCount:  100

[benjaminhuo]

So the CRD looks like below now:

  serving:
    runtime: "OpenFuncAsync"
    bindings:
      dapr:
        annotations:
          dapr.io/log-level: "debug"
        components:
          kafka-receiver:
            type: bindings.kafka
            version: v1
            metadata:
              - name: brokers
                value: "kafka-logs-receiver-kafka-brokers:9092"
              - name: authRequired
                value: "false"
              - name: publishTopic
                value: "logs"
              - name: topics
                value: "logs"
              - name: consumerGroup
                value: "logs-handler"
          notification-manager:
            type: bindings.http
            version: v1
            metadata:
              - name: url
                value: http://notification-manager-svc.kubesphere-monitoring-system.svc.cluster.local:19093/api/v2/alerts
    scaleOptions:
      minReplicaCount: 1
      maxReplicaCount:  10
      keda:
        scaledObject:
          pollingInterval: 15
          minReplicaCount: 0 // will overwrite the min/max params above
          maxReplicaCount: 10 // will overwrite the min/max params above
          cooldownPeriod: 30
          triggers:
            - type: kafka
              metadata:
                topic: logs
                bootstrapServers: kafka-logs-receiver-kafka-brokers.default.svc.cluster.local:9092
                consumerGroup: logs-handler
                lagThreshold: "10"
    openFuncAsync:
      inputs:
        - name: kafka
          component: kafka-receiver
          type: bindings
      outputs:
        - name: notify
          type: bindings
          component: notification-manager
          operation: "post"

[benjaminhuo]

We'd better investigate scale options of knative, keda, and keda-http to make the scaleoptions more common:

• knative https://knative.dev/docs/serving/autoscaling/
• keda: https://keda.sh/docs/2.5/concepts/scaling-deployments/
  https://keda.sh/docs/2.5/concepts/scaling-jobs/
• keda-http: https://github.com/kedacore/http-add-on/blob/main/examples/v0.2.0/httpscaledobject.yaml

And we needn't to follow keda's definition to minReplicaCount and maxReplicaCount, maybe we can just call it minReplicas/maxReplicas
@tpiperatgod @wanjunlei

[tpiperatgod]

I have made some adjustments to spec.serving.scaleOptions and spec.serving.runtime:

apiVersion: core.openfunction.io/v1beta1
kind: Function
metadata:
  name: function-sample
spec:
  serving:
    scaleOptions:
      min: 1
      max: 10
      keda:
        scaledJob:
          pollingInterval: 30
          maxReplicaCount: 10
        scaledObject:
          pollingInterval: 30
          cooldownPeriod: 30
          minReplicaCount: 0
          maxReplicaCount: 10
      knative:
        autoscaling:
          target: 200
          max-scale: 3
          min-scale: 1
    runtime:
      async:
        inputs:
          - name: kafka
            component: kafka-receiver
            type: bindings
        outputs:
          - name: nats
            component: nats-eventbus
            type: pubsub
      sync:
        outputs:
          - name: kafka
            component: kafka-receiver
            type: bindings
      knative:
        outputs:
          - name: kafka
            component: kafka-receiver
            type: bindings
    bindings:
      annotations:
        dapr.io/log-level: "debug"
      components:
        kafka-receiver:
          type: bindings.kafka
          version: v1
          metadata:
            - name: brokers
              value: "kafka-logs-receiver-kafka-brokers:9092"
            - name: authRequired
              value: "false"
            - name: publishTopic
              value: "logs"
            - name: topics
              value: "logs"
            - name: consumerGroup
              value: "logs-handler"
    pubsub:
      annotations:
        dapr.io/log-level: "debug"
      components:
        kafka-receiver:
          type: pubsub.natsstreaming
          version: v1
          metadata:
            - name: brokers
              value: "nats:8222"
            - name: authRequired
              value: "false"
            - name: publishTopic
              value: "eventbus"
            - name: topics
              value: "eventbus"
            - name: consumerGroup
              value: "eventbus"
    template:
      containers:
        - name: function
          imagePullPolicy: Always

[benjaminhuo]

Looks better now!
And maybe we can add dapr state store to store state between functions as well.

[tpiperatgod]

Should we use a structure in spec.serving.scaleOptions.knative corresponding to the Knative serving autoscaling property or a generic map to hold the relevant parameters?

[benjaminhuo]

Knative's autoscaling options are put into annotations which is map I think.
But we can take a look at the KnativeServing crd of knative operator

[tpiperatgod]

And should we integrate spec.serving.pubsub and spec.serving.bindings to spec.serving.components?

like this:

    components:
      annotations:
        dapr.io/log-level: "debug"
      spec:
      - kafka-receiver:
          type: bindings.kafka
          version: v1
          metadata:
            - name: brokers
              value: "kafka-logs-receiver-kafka-brokers:9092"
            - name: authRequired
              value: "false"
            - name: publishTopic
              value: "logs"
            - name: topics
              value: "logs"
            - name: consumerGroup
              value: "logs-handler"
      - nats-eventbus:
          type: pubsub.natsstreaming
          version: v1
          metadata:
            - name: brokers
              value: "nats:8222"
            - name: authRequired
              value: "false"
            - name: publishTopic
              value: "eventbus"
            - name: topics
              value: "eventbus"
            - name: consumerGroup
              value: "eventbus"

[benjaminhuo]

The new proposal can be found here