ricardozanini commented on code in PR #457:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/457#discussion_r1593971370
##########
controllers/cfg/controllers_cfg.go:
##########
@@ -93,3 +101,11 @@ func GetCfg() *ControllersCfg {
}
return controllersCfg
}
+
+func (g *GAV) GroupAndArtifact() string {
+ return fmt.Sprintf("%s:%s", g.GroupId, g.ArtifactId)
+}
+
+func (g *GAV) String() string {
Review Comment:
nitpick; you can move these functions near to the `GAV` struct.
##########
controllers/builder/openshiftbuilder.go:
##########
@@ -186,16 +186,21 @@ func (o *openshiftBuilderManager)
newDefaultBuildConfig(build *operatorapi.Sonat
}
func (o *openshiftBuilderManager) addExternalResources(config
*buildv1.BuildConfig, workflow *operatorapi.SonataFlow) error {
- if len(workflow.Spec.Resources.ConfigMaps) == 0 {
- return nil
- }
var configMapSources []buildv1.ConfigMapBuildSource
for _, workflowRes := range workflow.Spec.Resources.ConfigMaps {
configMapSources = append(configMapSources,
buildv1.ConfigMapBuildSource{
ConfigMap: workflowRes.ConfigMap,
DestinationDir: workflowRes.WorkflowPath,
})
}
+ //make the workflow properties available to the OpenShift build config.
Review Comment:
This is gold
##########
controllers/cfg/controllers_cfg.go:
##########
@@ -40,18 +41,25 @@ var defaultControllersCfg = &ControllersCfg{
BuilderConfigMapName: "sonataflow-operator-builder-config",
}
+type GAV struct {
+ GroupId string `yaml:"GroupId,omitempty"`
+ ArtifactId string `yaml:"ArtifactId,omitempty"`
+ Version string `yaml:"Version,omitempty"`
Review Comment:
```suggestion
GroupId string `yaml:"groupId,omitempty"`
ArtifactId string `yaml:"artifactId,omitempty"`
Version string `yaml:"version,omitempty"`
```
For ubiquity with the other properties.
##########
controllers/builder/kogitoserverlessbuild_manager_test.go:
##########
@@ -0,0 +1,185 @@
+// 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 builder
+
+import (
+ "testing"
+
+ operatorapi
"github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08"
+
"github.com/apache/incubator-kie-kogito-serverless-operator/controllers/cfg"
+
"github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/persistence"
+ "github.com/apache/incubator-kie-kogito-serverless-operator/test"
+ "github.com/stretchr/testify/assert"
+ v1 "k8s.io/api/core/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestSonataFlowBuildManager_GetOrCreateBuildWithWorkflowPersistence(t
*testing.T) {
+ // Current platform with no persistence
+ currentPlatform := operatorapi.SonataFlowPlatform{
+ ObjectMeta: metav1.ObjectMeta{Name: "current-platform"},
+ Spec: operatorapi.SonataFlowPlatformSpec{},
+ Status: operatorapi.SonataFlowPlatformStatus{},
+ }
+ // Persistence is configured in the workflow
+ workflow := operatorapi.SonataFlow{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-workflow",
+ },
+ Spec: operatorapi.SonataFlowSpec{
+ Persistence: &operatorapi.PersistenceOptionsSpec{
+ PostgreSQL:
&operatorapi.PersistencePostgreSQL{},
+ },
+ },
+ Status: operatorapi.SonataFlowStatus{},
+ }
+ testGetOrCreateBuildWithPersistence(t, ¤tPlatform, &workflow)
+}
+
+func TestSonataFlowBuildManager_GetOrCreateBuildWithPlatformPersistence(t
*testing.T) {
+ // Persistence is configured in the platform
+ currentPlatform := operatorapi.SonataFlowPlatform{
+ ObjectMeta: metav1.ObjectMeta{Name: "current-platform"},
+ Spec: operatorapi.SonataFlowPlatformSpec{
+ Persistence:
&operatorapi.PlatformPersistenceOptionsSpec{
+ PostgreSQL:
&operatorapi.PlatformPersistencePostgreSQL{},
+ },
+ },
+ Status: operatorapi.SonataFlowPlatformStatus{},
+ }
+ // Workflow with no persistence
+ workflow := operatorapi.SonataFlow{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-workflow",
+ },
+ Status: operatorapi.SonataFlowStatus{},
+ }
+ testGetOrCreateBuildWithPersistence(t, ¤tPlatform, &workflow)
+}
+
+func TestSonataFlowBuildManager_GetOrCreateBuildWithNoPersistence(t
*testing.T) {
+ // Platform has no persistence
+ currentPlatform := operatorapi.SonataFlowPlatform{
+ ObjectMeta: metav1.ObjectMeta{Name: "current-platform"},
+ Spec: operatorapi.SonataFlowPlatformSpec{},
+ Status: operatorapi.SonataFlowPlatformStatus{},
+ }
+ // Workflow has no persistence
+ workflow := operatorapi.SonataFlow{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-workflow",
+ },
+ Status: operatorapi.SonataFlowStatus{},
+ }
+ buildManager := prepareGetOrCreateBuildTest(t, ¤tPlatform)
+ build, _ := buildManager.GetOrCreateBuild(&workflow)
+ assert.Equal(t, 0, len(build.Spec.BuildArgs))
+ restoreControllersConfig(t)
+}
+
+func testGetOrCreateBuildWithPersistence(t *testing.T, currentPlatform
*operatorapi.SonataFlowPlatform, workflow *operatorapi.SonataFlow) {
+ buildManager := prepareGetOrCreateBuildTest(t, currentPlatform)
+ build, _ := buildManager.GetOrCreateBuild(workflow)
+ assert.NotNil(t, build)
+ assert.Equal(t, 1, len(build.Spec.BuildArgs))
+ assertContainsPersistence(t, build.Spec.BuildArgs, 0)
+ restoreControllersConfig(t)
+}
+
+func prepareGetOrCreateBuildTest(t *testing.T, currentPlatform
*operatorapi.SonataFlowPlatform) sonataFlowBuildManager {
+ initializeControllersConfig(t)
+ platforms := operatorapi.NewSonataFlowPlatformList()
+ platforms.Items = []operatorapi.SonataFlowPlatform{*currentPlatform}
+ cli :=
test.NewSonataFlowClientBuilder().WithRuntimeObjects(&platforms).Build()
+ buildManager := sonataFlowBuildManager{
+ client: cli,
+ }
+ return buildManager
+}
+
+func Test_addPersistenceExtensionsWithEmptyArgs(t *testing.T) {
+ initializeControllersConfig(t)
+ buildTemplate := &operatorapi.BuildTemplate{}
+ addPersistenceExtensions(buildTemplate)
+ assert.Equal(t, 1, len(buildTemplate.BuildArgs))
+ assertContainsPersistence(t, buildTemplate.BuildArgs, 0)
+ restoreControllersConfig(t)
+}
+
+func Test_addPersistenceExtensionsWithNoQuarkusExtensionsArg(t *testing.T) {
+ initializeControllersConfig(t)
+ buildTemplate := &operatorapi.BuildTemplate{
+ BuildArgs: []v1.EnvVar{
+ {Name: "VAR1"},
+ },
+ }
+ addPersistenceExtensions(buildTemplate)
+ assert.Equal(t, 2, len(buildTemplate.BuildArgs))
+ assertContainsPersistence(t, buildTemplate.BuildArgs, 1)
+ restoreControllersConfig(t)
+}
+
+func
Test_addPersistenceExtensionsWithQuarkusExtensionsArgAndNoPersistenceExtensions(t
*testing.T) {
+ initializeControllersConfig(t)
+ buildTemplate := &operatorapi.BuildTemplate{
+ BuildArgs: []v1.EnvVar{
+ {Name: "VAR1"},
+ {Name: "QUARKUS_EXTENSIONS", Value:
"org.acme:org.acme.library:1.0.0"},
+ },
+ }
+ addPersistenceExtensions(buildTemplate)
+ assert.Equal(t, 2, len(buildTemplate.BuildArgs))
+ assertContainsPersistence(t, buildTemplate.BuildArgs, 1)
+ restoreControllersConfig(t)
+}
+
+func
Test_addPersistenceExtensionsWithQuarkusExtensionsArgAndPersistenceExtensions(t
*testing.T) {
+ initializeControllersConfig(t)
+ buildTemplate := &operatorapi.BuildTemplate{
+ BuildArgs: []v1.EnvVar{
+ {Name: "VAR1", Value: "VALUE1"},
+ {Name: "QUARKUS_EXTENSIONS", Value:
"org.acme:org.acme.library:1.0.0,io.quarkus:quarkus-jdbc-postgresql:8.8.0.Final"},
+ },
+ }
+ addPersistenceExtensions(buildTemplate)
+ assert.Equal(t, 2, len(buildTemplate.BuildArgs))
+ assert.Equal(t, v1.EnvVar{Name: "VAR1", Value: "VALUE1"},
buildTemplate.BuildArgs[0])
+ assert.Equal(t, v1.EnvVar{Name: "QUARKUS_EXTENSIONS", Value:
"org.acme:org.acme.library:1.0.0,io.quarkus:quarkus-jdbc-postgresql:8.8.0.Final"},
buildTemplate.BuildArgs[1])
+ restoreControllersConfig(t)
+}
+
+func initializeControllersConfig(t *testing.T) {
+ // emulate the controllers config initialization
+ cfg, err :=
cfg.InitializeControllersCfgAt("../cfg/testdata/controllers-cfg-test.yaml")
+ assert.NoError(t, err)
+ assert.NotNil(t, cfg)
+ assert.Equal(t, 3, len(cfg.PostgreSQLPersistenceExtensions))
+}
+
+func restoreControllersConfig(t *testing.T) {
Review Comment:
Mind moving this function to a global util test file? I think I had
something like this when I first implemented the configuration feature. So we
keep it in one place to reuse across the code base.
##########
controllers/platform/services/services.go:
##########
@@ -408,6 +408,7 @@ func (j JobServiceHandler)
ConfigurePersistence(containerSpec *corev1.Container)
c.Env = append(c.Env,
persistence.ConfigurePostgreSQLEnv(p.PostgreSQL, j.GetServiceName(),
j.platform.Namespace)...)
// Specific to Job Service
c.Env = append(c.Env, corev1.EnvVar{Name:
"QUARKUS_FLYWAY_MIGRATE_AT_START", Value: "true"})
+ c.Env = append(c.Env, corev1.EnvVar{Name:
"KOGITO_JOBS_SERVICE_LOADJOBERRORSTRATEGY", Value: "FAIL_SERVICE"})
Review Comment:
What's this new env? How does that relate to persistence? Is it something
that we missed in previous PRs? 🤔
--
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]
