wmedvede commented on code in PR #311:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/311#discussion_r1410551982
##########
controllers/profiles/common/app_properties.go:
##########
@@ -44,28 +45,67 @@ const (
dataIndexServiceUrlProperty =
"mp.messaging.outgoing.kogito-processinstances-events.url"
kafkaSmallRyeHealthProperty =
"quarkus.smallrye-health.check.\"io.quarkus.kafka.client.health.KafkaHealthCheck\".enabled"
dataIndexServiceUrlProtocol = "http"
-
- DataIndexImageBase = "quay.io/kiegroup/kogito-data-index-"
- DataIndexName = "data-index-service"
-
- PersistenceTypeEphemeral = "ephemeral"
- PersistenceTypePostgressql = "postgresql"
-
+ jobServiceURLProperty =
"mp.messaging.outgoing.kogito-job-service-job-request-events.url"
+ jobServiceURLProtocol = "http"
+
+ imageNamePrefix = "quay.io/kiegroup/kogito"
+ DataIndexServiceName = "data-index-service"
+ DataIndexName = "data-index"
+ JobServiceName = "jobs-service"
+ PersistenceTypeEphemeral = "ephemeral"
+ PersistenceTypePostgressql = "postgresql"
microprofileServiceCatalogPropertyPrefix =
"org.kie.kogito.addons.discovery."
discoveryLikePropertyPattern =
"^\\${(kubernetes|knative|openshift):(.*)}$"
)
-var immutableApplicationProperties = "quarkus.http.port=" +
DefaultHTTPWorkflowPortIntStr.String() + "\n" +
- "quarkus.http.host=0.0.0.0\n" +
- // We disable the Knative health checks to not block the dev pod to run
if Knative objects are not available
- // See:
https://kiegroup.github.io/kogito-docs/serverlessworkflow/latest/eventing/consume-produce-events-with-knative-eventing.html#ref-knative-eventing-add-on-source-configuration
- "org.kie.kogito.addons.knative.eventing.health-enabled=false\n" +
- "quarkus.devservices.enabled=false\n" +
- "quarkus.kogito.devservices.enabled=false\n"
+var (
+ immutableApplicationProperties = "quarkus.http.port=" +
DefaultHTTPWorkflowPortIntStr.String() + "\n" +
+ "quarkus.http.host=0.0.0.0\n" +
+ // We disable the Knative health checks to not block the dev
pod to run if Knative objects are not available
+ // See:
https://kiegroup.github.io/kogito-docs/serverlessworkflow/latest/eventing/consume-produce-events-with-knative-eventing.html#ref-knative-eventing-add-on-source-configuration
+ "org.kie.kogito.addons.knative.eventing.health-enabled=false\n"
+
+ "quarkus.devservices.enabled=false\n" +
+ "quarkus.kogito.devservices.enabled=false\n"
+
+ discoveryLikePropertyExpr =
regexp.MustCompile(discoveryLikePropertyPattern)
+ _ AppPropertyHandler = &appPropertyHandler{}
+)
-var discoveryLikePropertyExpr =
regexp.MustCompile(discoveryLikePropertyPattern)
+type ServiceType int
-var _ AppPropertyHandler = &appPropertyHandler{}
+const (
+ DataIndexService ServiceType = iota
+ JobService
+)
+
+// GetContainerName returns the name of the service's container in the
deployment.
+func GetContainerName(stype ServiceType) string {
+ switch stype {
+ case DataIndexService:
+ return DataIndexServiceName
+ case JobService:
+ return JobServiceName
+ }
+ return ""
+}
+
+// GetServiceImageName returns the image name of the service's container. It
takes in the service and persistence types and returns a string
+// that contains the FQDN of the image, including the tag.
+func GetServiceImageName(persistenceName string, stype ServiceType) string {
Review Comment:
Let me add guys a general comment regarding the app_properties.go. I think
that here, we originally started to manage the application.properties related
to the workflows, but the application.properties related to the services are a
different stuff. Now I think this is starting to become a sort of all in
one.... Not sure it's good.
now we have things like this:
https://github.com/apache/incubator-kie-kogito-serverless-operator/blob/9a1e05d3e67081a9e2a713316abc7f60f3736a3d/controllers/profiles/common/app_properties.go#L256
with a common.NewServiceAppPropertyHandler that uses the original handler
:= &appPropertyHandler{
platform: platform,
isService: true,
}
and that we use when we need to create the applicaton.properties for a
service, in places like this:
https://github.com/apache/incubator-kie-kogito-serverless-operator/blob/b017c4b41431772dbc398a51ddc12119cc11e4df/controllers/platform/services.go#L404
IDK, just thinking aloud, but maybe this deserves a piece of refactoring.
##########
controllers/platform/services.go:
##########
@@ -58,30 +60,72 @@ func (action *serviceAction) Handle(ctx context.Context,
platform *operatorapi.S
return nil, err
}
- if err := createDataIndexComponents(ctx, action.client, platform); err
!= nil {
- return nil, err
+ if platform.Spec.Services.DataIndex != nil {
Review Comment:
I have question here, maybe I'm missing something or not understanding well,
but my understanding is that when we execute this Handle, is because the
platform.Status.IsReady().
So, when the platform is ready, we start to create the services. All good,
but on the other hand, if the platform is ready, the workflows might have been
started the creation procedure too.
if worklfows and services re being created at the same time, what happens
if a workflow needs a service?
does it make sense?
return platform.Status.IsReady()
##########
controllers/profiles/common/app_properties.go:
##########
@@ -156,7 +194,7 @@ func (a *appPropertyHandler) withDataIndexServiceUrl()
AppPropertyHandler {
if profiles.IsProdProfile(a.workflow) && dataIndexEnabled(a.platform) {
a.addDefaultMutableProperty(
dataIndexServiceUrlProperty,
- fmt.Sprintf("%s://%s.%s/processes",
dataIndexServiceUrlProtocol, GetDataIndexName(a.platform),
a.platform.Namespace),
+ fmt.Sprintf("%s://%s.%s/processes",
dataIndexServiceUrlProtocol, GetServiceName(a.platform.Name, DataIndexService),
a.platform.Namespace),
Review Comment:
yes, this mp.messaging.outgoing.kogito-job-service-job-request-events.url
should be enough
##########
controllers/platform/services.go:
##########
@@ -99,7 +143,7 @@ func createDataIndexDeployment(ctx context.Context, client
client.Client, platfo
liveProbe := readyProbe.DeepCopy()
liveProbe.ProbeHandler.HTTPGet.Path = common.QuarkusHealthPathLive
dataDeployContainer := &corev1.Container{
- Image: common.DataIndexImageBase +
common.PersistenceTypeEphemeral,
+ Image:
common.GetServiceImageName(common.PersistenceTypeEphemeral, serviceType),
Env: []corev1.EnvVar{
{
Review Comment:
This KOGITO_DATA_INDEX_QUARKUS_PROFILE is specific for the data-index, I
think that here we should probably generate different env vars depending on the
service type.
--
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]
