wmedvede commented on code in PR #558:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/558#discussion_r1823118314
##########
internal/controller/platform/k8s.go:
##########
@@ -359,18 +359,24 @@ func createOrUpdateKnativeResources(ctx context.Context,
client client.Client, p
return nil
})
if err != nil {
- return err
+ return nil, err
}
kSinkInjected, err := psh.CheckKSinkInjected()
if err != nil {
- return err
+ return nil, err
}
if !kSinkInjected {
- return fmt.Errorf("waiting for K_SINK injection
for %s to complete", psh.GetServiceName())
+ msg := fmt.Sprintf("waiting for K_SINK
injection for %s to complete", psh.GetServiceName())
Review Comment:
This is good, we can see the service name in the event.
##########
internal/controller/platform/services/services.go:
##########
@@ -582,18 +583,23 @@ func (d *DataIndexHandler) newTrigger(labels
map[string]string, brokerName, name
},
}
}
-func (d *DataIndexHandler) GenerateKnativeResources(platform
*operatorapi.SonataFlowPlatform, lbl map[string]string) ([]client.Object,
error) {
+func (d *DataIndexHandler) GenerateKnativeResources(platform
*operatorapi.SonataFlowPlatform, lbl map[string]string) ([]client.Object,
*corev1.Event, error) {
broker := d.GetSourceBroker()
if broker == nil || len(broker.Ref.Name) == 0 {
- return nil, nil // Nothing to do
+ return nil, nil, nil // Nothing to do
}
brokerName := broker.Ref.Name
namespace := broker.Ref.Namespace
if len(namespace) == 0 {
namespace = platform.Namespace
}
if err := knative.ValidateBroker(brokerName, namespace); err != nil {
- return nil, err
+ event := &corev1.Event{
+ Type: corev1.EventTypeWarning,
+ Reason: WaitingKnativeEventing,
+ Message: err.Error(),
Review Comment:
For this error message it would be nice if we can see the service name as
part of the message.
I have forced the use case to happen, and in the events I can see:
5m47s Warning WaitingKnativeEventing
sonataflowplatform/sonataflow-platform broker
sonataflow-broker in namespace case1-kn-eventing does not exist
13m Warning Failed
sonataflowplatform/sonataflow-platform Failed to update
SonataFlowPlaform: broker sonataflow-broker in namespace case1-kn-
So, it's difficult to distinguish for which service we have the missing
broker.
If it's not too much work, it would include the we could include the name of
the service as part of the message.
I think we can just grab the ValidatBroker message and concatenate "service"
+ d.GetServiceName()
and we can see:
sonataflow-broker in namespace case1-kn-eventing does not exist for
**service: sonataflow-platform-data-index**
Considering that the configuration alternatives admits different Brokers for
DI and JS, I think it'll be very helpful in situations where users needs to
query the events to find the cause of the issue
--
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]
