ricardozanini commented on code in PR #350: URL: https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/350#discussion_r1455824070
########## controllers/profiles/common/knative.go: ########## @@ -0,0 +1,156 @@ +// Copyright 2024 Apache Software Foundation (ASF) +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package common + +import ( + "context" + + "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/discovery" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "github.com/apache/incubator-kie-kogito-serverless-operator/api" + operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" + "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/constants" + "github.com/apache/incubator-kie-kogito-serverless-operator/log" + "k8s.io/klog/v2" + eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" + sourcesv1 "knative.dev/eventing/pkg/apis/sources/v1" + "knative.dev/pkg/apis" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type knativeObjectEnsurers struct { + sinkBinding ObjectEnsurer + trigger ObjectsEnsurer +} + +func newKnativeObjectEnsurers(support *StateSupport) *knativeObjectEnsurers { + return &knativeObjectEnsurers{ + sinkBinding: NewObjectEnsurer(support.C, SinkBindingCreator), + trigger: NewObjectsEnsurer(support.C, TriggersCreator), + } +} + +type FollowWorkflowKnativeState struct { + *StateSupport +} + +// CanReconcileNext to create deployment & knative asynchronously +func (f FollowWorkflowKnativeState) CanReconcileNext() bool { + return true +} + +func (f FollowWorkflowKnativeState) CanReconcile(workflow *operatorapi.SonataFlow) bool { + if workflow.Spec.Flow.Events == nil { + // skip if no event is found + workflow.Status.Manager().MarkTrueWithReason(api.KnativeResourcesConditionType, + api.KnativeSkippedReason, + "no need to create Knative eventing resources since no event definition is found") + } else if workflow.Spec.Sink == nil { + // mark false if spec.sink is not found + workflow.Status.Manager().MarkFalse(api.KnativeResourcesConditionType, + api.KnativeFailureReason, + "Spec.Sink is not provided") + } else if f.Catalog != nil && f.Catalog.(*discovery.SonataFlowServiceCatalog).KnativeCatalog == nil { + workflow.Status.Manager().MarkFalse(api.KnativeResourcesConditionType, + api.KnativeFailureReason, + "Knative Client is not installed") Review Comment: ```suggestion "Knative Eventing is not installed") ``` ########## controllers/profiles/profile.go: ########## @@ -71,6 +71,8 @@ type ReconciliationState interface { Do(ctx context.Context, workflow *operatorapi.SonataFlow) (ctrl.Result, []client.Object, error) // PostReconcile performs the actions to perform after the reconciliation that are not mandatory PostReconcile(ctx context.Context, workflow *operatorapi.SonataFlow) error + // CanReconcileNext check if next state can be reconciled + CanReconcileNext() bool Review Comment: Why can't we wait for the Knative reconciliation state to finish to proceed to others? Not sure if we have to change this API for this feature. I'd rather keep it simpler. I think we can keep it the way it was and each reconciliation cycle be deterministic on the state it supposed to run. Actually, not sure why we need a Knative state. That can happen within the deployment handler. Not even the Knative conditions we need, sounds off. We can instead post the Knative objects status to the events API. The more conditions and states we add, the more complex it gets. Now adding a new API to the core reconcile might make things more difficult to maintain. Not sure if I was clear in the last review, but within the existing state where we create everything, we add a new `KnativeEventingHandler` as we have here: https://github.com/apache/incubator-kie-kogito-serverless-operator/blob/main/controllers/profiles/prod/deployment_handler.go#L50 So the code from the Knative state goes to a handler like this in the `knative.go` file. You reuse the whole states we already have in both profiles. For example, in dev mode you add here: https://github.com/apache/incubator-kie-kogito-serverless-operator/blob/main/controllers/profiles/dev/states_dev.go#L111C28-L111C28 Something like: ```go knative_objs := common.KnativeHandler().Ensure() ``` In prod, you use the `deployment_handler.go`: https://github.com/apache/incubator-kie-kogito-serverless-operator/blob/main/controllers/profiles/prod/deployment_handler.go#L78 ########## controllers/profiles/common/object_creators.go: ########## @@ -197,6 +213,90 @@ func ServiceCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { return service, nil } +// SinkBindingCreator is an ObjectsCreator for SinkBinding. +// It will create v1.SinkBinding based on events defined in workflow. +func SinkBindingCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { + lbl := workflowproj.GetDefaultLabels(workflow) + + // skip if no produced event is found + if workflow.Spec.Sink == nil || !workflowdef.ContainsEventKind(workflow, cncfmodel.EventKindProduced) { + return nil, nil + } + + sink := workflow.Spec.Sink + + // subject must be deployment to inject K_SINK, service won't work + sinkBinding := &sourcesv1.SinkBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: strings.ToLower(fmt.Sprintf("sb-%s", workflow.Name)), + Namespace: workflow.Namespace, + Labels: lbl, + }, + Spec: sourcesv1.SinkBindingSpec{ + SourceSpec: duckv1.SourceSpec{ + Sink: *sink, + }, + BindingSpec: duckv1.BindingSpec{ + Subject: tracker.Reference{ + Name: workflow.Name, + Namespace: workflow.Namespace, + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + }, + } + return sinkBinding, nil +} + +// TriggersCreator is an ObjectsCreator for Triggers. +// It will create a list of eventingv1.Trigger based on events defined in workflow. +func TriggersCreator(workflow *operatorapi.SonataFlow) ([]client.Object, error) { + var resultObjects []client.Object + lbl := workflowproj.GetDefaultLabels(workflow) + + //consumed + events := workflow.Spec.Flow.Events + for _, event := range events { + // filter out produce events + if event.Kind == cncfmodel.EventKindProduced { + continue + } + sink := workflow.Spec.Sink + + //TODO: user should have the flexibility to config trigger source broker + if sink.Ref.Kind != "Broker" || sink.Ref.APIVersion != "eventing.knative.dev/v1" { + return nil, errors.New("Trigger source must be a broker!") + } Review Comment: You can remove this code and hardcode to the trigger's broker to `default` for now. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
