Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break the current operator's configuration into custom and managed properties #367

Merged
merged 19 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
82abeb1
Initial commit
dmartinol Jan 25, 2024
0a62a8b
Merge remote-tracking branch 'upstream/main' into SRVLOGIC-195-props
dmartinol Jan 29, 2024
54f460a
integrating comments: introducing ObjectEnsurerWithPlatform and Objec…
dmartinol Jan 29, 2024
d77fe18
Removing fewe more unneeded references to platform
dmartinol Jan 29, 2024
d481813
reviewed workflowProjectHandler. removed user props from managed props
dmartinol Jan 29, 2024
fea41a0
fixed unit tests
dmartinol Jan 29, 2024
0bbd57b
workarond for failed discovery options
dmartinol Jan 29, 2024
239c2e0
fixed broken unit tests
dmartinol Jan 29, 2024
821fd3e
fixed mutator for user props
dmartinol Jan 30, 2024
4ba85a7
reviewed hashing function
dmartinol Jan 30, 2024
5941bf2
Anticipating deactivation of broken e2e test
dmartinol Jan 31, 2024
fe96451
Merge remote-tracking branch 'upstream/main' into SRVLOGIC-195-props
dmartinol Jan 31, 2024
63ed134
integrating comments: removing unneeded comment
dmartinol Jan 31, 2024
4d0afe8
Merge remote-tracking branch 'upstream/main' into SRVLOGIC-195-props
dmartinol Feb 1, 2024
612766f
adding discovered value to properties whose value mathes the service …
dmartinol Feb 2, 2024
4a0c759
Merge remote-tracking branch 'upstream/main' into SRVLOGIC-195-props
dmartinol Feb 2, 2024
4a743bd
removed unused package common_test
dmartinol Feb 2, 2024
c842feb
Renamed managed props visitor and reviewed description of NewAppPrope…
dmartinol Feb 2, 2024
69713ce
Merge remote-tracking branch 'upstream/main' into SRVLOGIC-195-props
dmartinol Feb 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions controllers/profiles/common/ensurer.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var _ ObjectEnsurer = &defaultObjectEnsurer{}
var _ ObjectEnsurer = &noopObjectEnsurer{}

type ObjectEnsurer interface {
Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error)
Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error)
ricardozanini marked this conversation as resolved.
Show resolved Hide resolved
}

// MutateVisitor is a visitor function that mutates the given object before performing any updates in the cluster.
Expand Down Expand Up @@ -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)
ricardozanini marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, result, err
}
Expand Down Expand Up @@ -93,7 +93,7 @@ func NewNoopObjectEnsurer() ObjectEnsurer {
type noopObjectEnsurer struct {
}

func (d *noopObjectEnsurer) Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) {
func (d *noopObjectEnsurer) Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) {
result := controllerutil.OperationResultNone
return nil, result, nil
}
37 changes: 18 additions & 19 deletions controllers/profiles/common/mutate_visitors.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ func ImageDeploymentMutateVisitor(workflow *operatorapi.SonataFlow, image string
}

// DeploymentMutateVisitor guarantees the state of the default Deployment object
func DeploymentMutateVisitor(workflow *operatorapi.SonataFlow) MutateVisitor {
func DeploymentMutateVisitor(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) MutateVisitor {
return func(object client.Object) controllerutil.MutateFn {
return func() error {
if kubeutil.IsObjectNew(object) {
return nil
}
original, err := DeploymentCreator(workflow)
original, err := DeploymentCreator(workflow, platform)
if err != nil {
return err
}
Expand All @@ -87,13 +87,13 @@ func EnsureDeployment(original *appsv1.Deployment, object *appsv1.Deployment) er
return mergo.Merge(&object.Spec.Template.Spec, original.Spec.Template.Spec, mergo.WithOverride)
}

func ServiceMutateVisitor(workflow *operatorapi.SonataFlow) MutateVisitor {
func ServiceMutateVisitor(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) MutateVisitor {
return func(object client.Object) controllerutil.MutateFn {
return func() error {
if kubeutil.IsObjectNew(object) {
return nil
}
original, err := ServiceCreator(workflow)
original, err := ServiceCreator(workflow, platform)
if err != nil {
return err
}
Expand All @@ -104,32 +104,31 @@ func ServiceMutateVisitor(workflow *operatorapi.SonataFlow) MutateVisitor {
}
}

func WorkflowPropertiesMutateVisitor(ctx context.Context, catalog discovery.ServiceCatalog,
workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) MutateVisitor {
func UserPropertiesMutateVisitor(ctx context.Context, catalog discovery.ServiceCatalog,
ricardozanini marked this conversation as resolved.
Show resolved Hide resolved
workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, userProps *corev1.ConfigMap) MutateVisitor {
return func(object client.Object) controllerutil.MutateFn {
return func() error {
if kubeutil.IsObjectNew(object) {
return nil
}
cm := object.(*corev1.ConfigMap)
cm.Labels = workflow.GetLabels()
_, hasKey := cm.Data[workflowproj.ApplicationPropertiesFileName]
managedProps := object.(*corev1.ConfigMap)
managedProps.Labels = workflow.GetLabels()
_, hasKey := managedProps.Data[workflowproj.GetManagedPropertiesFileName(workflow)]
if !hasKey {
cm.Data = make(map[string]string, 1)
props, err := properties.ImmutableApplicationProperties(workflow, platform)
if err != nil {
return err
}
cm.Data[workflowproj.ApplicationPropertiesFileName] = props
return nil
managedProps.Data = make(map[string]string, 1)
managedProps.Data[workflowproj.GetManagedPropertiesFileName(workflow)] = ""
}

userProperties, hasKey := userProps.Data[workflowproj.ApplicationPropertiesFileName]
if !hasKey {
userProperties = ""
}
// In the future, if this needs change, instead we can receive an AppPropertyHandler in this mutator
props, err := properties.NewAppPropertyHandler(workflow, platform)
if err != nil {
return err
}
cm.Data[workflowproj.ApplicationPropertiesFileName] = props.WithUserProperties(cm.Data[workflowproj.ApplicationPropertiesFileName]).
managedProps.Data[workflowproj.GetManagedPropertiesFileName(workflow)] = props.WithUserProperties(userProperties).
ricardozanini marked this conversation as resolved.
Show resolved Hide resolved
WithServiceDiscovery(ctx, catalog).
Build()
return nil
Expand All @@ -141,11 +140,11 @@ func WorkflowPropertiesMutateVisitor(ctx context.Context, catalog discovery.Serv
// This method can be used as an alternative to the Kubernetes ConfigMap refresher.
//
// See: https://kubernetes.io/docs/concepts/configuration/configmap/#mounted-configmaps-are-updated-automatically
func RolloutDeploymentIfCMChangedMutateVisitor(cm *v1.ConfigMap) MutateVisitor {
func RolloutDeploymentIfCMChangedMutateVisitor(workflow *operatorapi.SonataFlow, userPropsCM *v1.ConfigMap, managedPropsCM *v1.ConfigMap) MutateVisitor {
return func(object client.Object) controllerutil.MutateFn {
return func() error {
deployment := object.(*appsv1.Deployment)
err := kubeutil.AnnotateDeploymentConfigChecksum(deployment, cm)
err := kubeutil.AnnotateDeploymentConfigChecksum(workflow, deployment, userPropsCM, managedPropsCM)
return err
}
}
Expand Down
21 changes: 13 additions & 8 deletions controllers/profiles/common/object_creators.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (

// ObjectCreator is the func that creates the initial reference object, if the object doesn't exist in the cluster, this one is created.
// Can be used as a reference to keep the object immutable
type ObjectCreator func(workflow *operatorapi.SonataFlow) (client.Object, error)
type ObjectCreator func(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error)

const (
defaultHTTPServicePort = 80
Expand All @@ -63,7 +63,7 @@ var DefaultHTTPWorkflowPortIntStr = intstr.FromInt(constants.DefaultHTTPWorkflow

// DeploymentCreator is an objectCreator for a base Kubernetes Deployments for profiles that need to deploy the workflow on a vanilla deployment.
// It serves as a basis for a basic Quarkus Java application, expected to listen on http 8080.
func DeploymentCreator(workflow *operatorapi.SonataFlow) (client.Object, error) {
func DeploymentCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) {
lbl := workflowproj.GetDefaultLabels(workflow)

deployment := &appsv1.Deployment{
Expand Down Expand Up @@ -175,7 +175,7 @@ func defaultContainer(workflow *operatorapi.SonataFlow) (*corev1.Container, erro

// ServiceCreator is an objectCreator for a basic Service aiming a vanilla Kubernetes Deployment.
// It maps the default HTTP port (80) to the target Java application webserver on port 8080.
func ServiceCreator(workflow *operatorapi.SonataFlow) (client.Object, error) {
func ServiceCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) {
lbl := workflowproj.GetDefaultLabels(workflow)

service := &corev1.Service{
Expand All @@ -200,16 +200,21 @@ func ServiceCreator(workflow *operatorapi.SonataFlow) (client.Object, error) {
// OpenShiftRouteCreator is an ObjectCreator for a basic Route for a workflow running on OpenShift.
// It enables the exposition of the service using an OpenShift Route.
// See: https://github.com/openshift/api/blob/d170fcdc0fa638b664e4f35f2daf753cb4afe36b/route/v1/route.crd.yaml
func OpenShiftRouteCreator(workflow *operatorapi.SonataFlow) (client.Object, error) {
func OpenShiftRouteCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) {
route, err := openshift.RouteForWorkflow(workflow)
return route, err
}

// WorkflowPropsConfigMapCreator creates a ConfigMap to hold the external application properties
func WorkflowPropsConfigMapCreator(workflow *operatorapi.SonataFlow) (client.Object, error) {
props, err := properties.ImmutableApplicationProperties(workflow, nil)
// UserPropsConfigMapCreator creates an empty ConfigMap to hold the user application properties
func UserPropsConfigMapCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) {
return workflowproj.CreateNewUserPropsConfigMap(workflow), nil
}

// ManagedPropsConfigMapCreator creates an empty ConfigMap to hold the external application properties
func ManagedPropsConfigMapCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) {
props, err := properties.ImmutableApplicationProperties(workflow, platform)
if err != nil {
return nil, err
}
return workflowproj.CreateNewAppPropsConfigMap(workflow, props), nil
return workflowproj.CreateNewManagedPropsConfigMap(workflow, props), nil
}
60 changes: 33 additions & 27 deletions controllers/profiles/common/object_creators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/apache/incubator-kie-kogito-serverless-operator/utils"
kubeutil "github.com/apache/incubator-kie-kogito-serverless-operator/utils/kubernetes"
Expand All @@ -39,54 +38,60 @@ import (

func Test_ensureWorkflowPropertiesConfigMapMutator(t *testing.T) {
workflow := test.GetBaseSonataFlowWithDevProfile(t.Name())
platform := test.GetBasePlatform()
// can't be new
cm, _ := WorkflowPropsConfigMapCreator(workflow)
cm.SetUID("1")
cm.SetResourceVersion("1")
reflectCm := cm.(*corev1.ConfigMap)
managedProps, _ := ManagedPropsConfigMapCreator(workflow, platform)
managedProps.SetUID("1")
managedProps.SetResourceVersion("1")
managedPropsCM := managedProps.(*corev1.ConfigMap)

visitor := WorkflowPropertiesMutateVisitor(context.TODO(), nil, workflow, nil)
mutateFn := visitor(cm)
userProps, _ := UserPropsConfigMapCreator(workflow, platform)
userPropsCM := userProps.(*corev1.ConfigMap)
visitor := UserPropertiesMutateVisitor(context.TODO(), nil, workflow, nil, userPropsCM)
mutateFn := visitor(managedProps)

assert.NoError(t, mutateFn())
assert.NotEmpty(t, reflectCm.Data[workflowproj.ApplicationPropertiesFileName])
assert.Empty(t, managedPropsCM.Data[workflowproj.ApplicationPropertiesFileName])
assert.NotEmpty(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)])

props := properties.MustLoadString(reflectCm.Data[workflowproj.ApplicationPropertiesFileName])
props := properties.MustLoadString(managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)])
assert.Equal(t, "8080", props.GetString("quarkus.http.port", ""))

// we change the properties to something different, we add ours and change the default
reflectCm.Data[workflowproj.ApplicationPropertiesFileName] = "quarkus.http.port=9090\nmy.new.prop=1"
visitor(reflectCm)
userPropsCM.Data[workflowproj.ApplicationPropertiesFileName] = "quarkus.http.port=9090\nmy.new.prop=1"
visitor(managedPropsCM)
assert.NoError(t, mutateFn())

// we should preserve the default, and still got ours
props = properties.MustLoadString(reflectCm.Data[workflowproj.ApplicationPropertiesFileName])
props = properties.MustLoadString(managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)])
assert.Equal(t, "8080", props.GetString("quarkus.http.port", ""))
assert.Equal(t, "0.0.0.0", props.GetString("quarkus.http.host", ""))
assert.Equal(t, "1", props.GetString("my.new.prop", ""))
}

func Test_ensureWorkflowPropertiesConfigMapMutator_DollarReplacement(t *testing.T) {
workflow := test.GetBaseSonataFlowWithDevProfile(t.Name())
existingCM := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: workflow.Name,
Namespace: workflow.Namespace,
UID: "0000-0001-0002-0003",
},
Data: map[string]string{
workflowproj.ApplicationPropertiesFileName: "mp.messaging.outgoing.kogito_outgoing_stream.url=${kubernetes:services.v1/event-listener}",
},
}
mutateVisitorFn := WorkflowPropertiesMutateVisitor(context.TODO(), nil, workflow, nil)
platform := test.GetBasePlatform()
managedProps, _ := ManagedPropsConfigMapCreator(workflow, platform)
managedProps.SetName(workflow.Name)
managedProps.SetNamespace(workflow.Namespace)
managedProps.SetUID("0000-0001-0002-0003")
managedPropsCM := managedProps.(*corev1.ConfigMap)

userProps, _ := UserPropsConfigMapCreator(workflow, platform)
userPropsCM := userProps.(*corev1.ConfigMap)
userPropsCM.Data[workflowproj.ApplicationPropertiesFileName] = "mp.messaging.outgoing.kogito_outgoing_stream.url=${kubernetes:services.v1/event-listener}"

mutateVisitorFn := UserPropertiesMutateVisitor(context.TODO(), nil, workflow, nil, userPropsCM)

err := mutateVisitorFn(existingCM)()
err := mutateVisitorFn(managedPropsCM)()
assert.NoError(t, err)
assert.Contains(t, existingCM.Data[workflowproj.ApplicationPropertiesFileName], "${kubernetes:services.v1/event-listener}")
assert.Contains(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)], "${kubernetes:services.v1/event-listener}")
}

func TestMergePodSpec(t *testing.T) {
workflow := test.GetBaseSonataFlow(t.Name())
platform := test.GetBasePlatform()
workflow.Spec.PodTemplate = v1alpha08.PodTemplateSpec{
Container: v1alpha08.ContainerSpec{
// this one we can override
Expand Down Expand Up @@ -123,7 +128,7 @@ func TestMergePodSpec(t *testing.T) {
},
}

object, err := DeploymentCreator(workflow)
object, err := DeploymentCreator(workflow, platform)
assert.NoError(t, err)

deployment := object.(*appsv1.Deployment)
Expand All @@ -140,6 +145,7 @@ func TestMergePodSpec(t *testing.T) {

func TestMergePodSpec_OverrideContainers(t *testing.T) {
workflow := test.GetBaseSonataFlow(t.Name())
platform := test.GetBasePlatform()
workflow.Spec.PodTemplate = v1alpha08.PodTemplateSpec{
PodSpec: v1alpha08.PodSpec{
// Try to override the workflow container via the podspec
Expand All @@ -158,7 +164,7 @@ func TestMergePodSpec_OverrideContainers(t *testing.T) {
},
}

object, err := DeploymentCreator(workflow)
object, err := DeploymentCreator(workflow, platform)
assert.NoError(t, err)

deployment := object.(*appsv1.Deployment)
Expand Down
84 changes: 84 additions & 0 deletions controllers/profiles/common/test/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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_test

import (
"context"

"github.com/apache/incubator-kie-kogito-serverless-operator/controllers/discovery"
)

const (
ricardozanini marked this conversation as resolved.
Show resolved Hide resolved
DefaultNamespace = "default-namespace"
namespace1 = "namespace1"
myService1 = "my-service1"
MyService1Address = "http://10.110.90.1:80"
myService2 = "my-service2"
MyService2Address = "http://10.110.90.2:80"
myService3 = "my-service3"
MyService3Address = "http://10.110.90.3:80"

myKnService1 = "my-kn-service1"
MyKnService1Address = "http://my-kn-service1.namespace1.svc.cluster.local"

myKnService2 = "my-kn-service2"
MyKnService2Address = "http://my-kn-service2.namespace1.svc.cluster.local"

myKnService3 = "my-kn-service3"
MyKnService3Address = "http://my-kn-service3.default-namespace.svc.cluster.local"

myKnBroker1 = "my-kn-broker1"
MyKnBroker1Address = "http://broker-ingress.knative-eventing.svc.cluster.local/namespace1/my-kn-broker1"

myKnBroker2 = "my-kn-broker2"
MyKnBroker2Address = "http://broker-ingress.knative-eventing.svc.cluster.local/default-namespace/my-kn-broker2"
)

type MockCatalogService struct {
}

func (c *MockCatalogService) Query(ctx context.Context, uri discovery.ResourceUri, outputFormat string) (string, error) {
if uri.Scheme == discovery.KubernetesScheme && uri.Namespace == namespace1 && uri.Name == myService1 {
return MyService1Address, nil
}
if uri.Scheme == discovery.KubernetesScheme && uri.Name == myService2 && uri.Namespace == DefaultNamespace {
return MyService2Address, nil
}
if uri.Scheme == discovery.KubernetesScheme && uri.Name == myService3 && uri.Namespace == DefaultNamespace && uri.GetPort() == "http-port" {
return MyService3Address, nil
}
if uri.Scheme == discovery.KnativeScheme && uri.Name == myKnService1 && uri.Namespace == namespace1 {
return MyKnService1Address, nil
}
if uri.Scheme == discovery.KnativeScheme && uri.Name == myKnService2 && uri.Namespace == namespace1 {
return MyKnService2Address, nil
}
if uri.Scheme == discovery.KnativeScheme && uri.Name == myKnService3 && uri.Namespace == DefaultNamespace {
return MyKnService3Address, nil
}
if uri.Scheme == discovery.KnativeScheme && uri.Name == myKnBroker1 && uri.Namespace == namespace1 {
return MyKnBroker1Address, nil
}
if uri.Scheme == discovery.KnativeScheme && uri.Name == myKnBroker2 && uri.Namespace == DefaultNamespace {
return MyKnBroker2Address, nil
}

return "", nil
}
Loading
Loading