ricardozanini commented on code in PR #367:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/367#discussion_r1468036883
##########
controllers/profiles/dev/states_dev.go:
##########
@@ -62,23 +62,26 @@ func (e *ensureRunningWorkflowState) CanReconcile(workflow
*operatorapi.SonataFl
func (e *ensureRunningWorkflowState) Do(ctx context.Context, workflow
*operatorapi.SonataFlow) (ctrl.Result, []client.Object, error) {
var objs []client.Object
- flowDefCM, _, err := e.ensurers.definitionConfigMap.Ensure(ctx,
workflow, ensureWorkflowDefConfigMapMutator(workflow))
- if err != nil {
- return ctrl.Result{Requeue: false}, objs, err
- }
- objs = append(objs, flowDefCM)
-
devBaseContainerImage := workflowdef.GetDefaultWorkflowDevModeImageTag()
// check if the Platform available
pl, err := platform.GetActivePlatform(ctx, e.C, workflow.Namespace)
if err == nil && len(pl.Spec.DevMode.BaseImage) > 0 {
devBaseContainerImage = pl.Spec.DevMode.BaseImage
}
- propsCM, _, err := e.ensurers.propertiesConfigMap.Ensure(ctx, workflow,
common.WorkflowPropertiesMutateVisitor(ctx, e.StateSupport.Catalog, workflow,
pl))
+ flowDefCM, _, err := e.ensurers.definitionConfigMap.Ensure(ctx,
workflow, pl, ensureWorkflowDefConfigMapMutator(workflow, pl))
+ if err != nil {
+ return ctrl.Result{Requeue: false}, objs, err
+ }
+ objs = append(objs, flowDefCM)
+ userPropsCM, _, err := e.ensurers.userPropsConfigMap.Ensure(ctx,
workflow, pl)
+ if err != nil {
+ return ctrl.Result{Requeue: false}, objs, err
+ }
+ managedPropsCM, _, err := e.ensurers.managedPropsConfigMap.Ensure(ctx,
workflow, pl, common.UserPropertiesMutateVisitor(ctx, e.StateSupport.Catalog,
workflow, pl, userPropsCM.(*corev1.ConfigMap)))
Review Comment:
I see, but maybe we can do in a different function? Not sure if I followed
the code exactly but it seems confusing having user and managed properties
being merged in the function responsible by creating the CM. Maybe a
`calculateHash(user, managed)` function?
##########
controllers/profiles/common/ensurer.go:
##########
@@ -62,10 +62,10 @@ type defaultObjectEnsurer struct {
creator ObjectCreator
}
-func (d *defaultObjectEnsurer) Ensure(ctx context.Context, workflow
*operatorapi.SonataFlow, visitors ...MutateVisitor) (client.Object,
controllerutil.OperationResult, error) {
+func (d *defaultObjectEnsurer) Ensure(ctx context.Context, workflow
*operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, visitors
...MutateVisitor) (client.Object, controllerutil.OperationResult, error) {
result := controllerutil.OperationResultNone
- object, err := d.creator(workflow)
+ object, err := d.creator(workflow, platform)
Review Comment:
@dmartinol we can instead introduce `ObjectCreatorWithPlatform` and then
implement it to a new ensurer:
```go
type ObjectCreatorWithPlatform func(workflow *operatorapi.SonataFlow,
platform *operatorapi.SonataFlowPlatform) (client.Object, error)
type ensurerWithPlatform struct {
c client.Client
creator ObjectCreatorWithPlatform
pl SonataFlowPlatform
}
```
Then we add a new constructor to `ObjectEnsurer`:
```go
func NewObjectEnsurerWithPlaform(client client.Client, creator
ObjectCreatorWithPlatform, pl *SonataFlowPlatform) ObjectEnsurer {
return &ensurerWithPlatform{
c: client,
creator: creator,
pl: pl
}
}
```
We can then review the states and modify there:
```go
func newObjectEnsurers(support *common.StateSupport) *objectEnsurers {
return &objectEnsurers{
deployment: common.NewObjectEnsurer(support.C,
deploymentCreator),
service: common.NewObjectEnsurer(support.C,
serviceCreator),
network: common.NewNoopObjectEnsurer(),
definitionConfigMap: common.NewObjectEnsurer(support.C,
workflowDefConfigMapCreator),
propertiesConfigMap:
common.NewObjectEnsurerWithPlaform(support.C,
common.WorkflowPropsConfigMapCreator),
}
}
```
wdyt?
--
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]
