From 82abeb1f65b5a435df3501166da71f9c72ff56c8 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Thu, 25 Jan 2024 21:08:00 +0100 Subject: [PATCH 01/14] Initial commit --- controllers/profiles/common/ensurer.go | 8 +- .../profiles/common/mutate_visitors.go | 37 ++++---- .../profiles/common/object_creators.go | 21 +++-- .../profiles/common/object_creators_test.go | 60 +++++++------ controllers/profiles/common/test/common.go | 84 +++++++++++++++++++ .../profiles/dev/object_creators_dev.go | 23 ++--- .../profiles/dev/object_creators_dev_test.go | 4 +- controllers/profiles/dev/profile_dev.go | 33 ++++---- controllers/profiles/dev/profile_dev_test.go | 24 ++++-- controllers/profiles/dev/states_dev.go | 29 ++++--- .../profiles/dev/status_enricher_dev_test.go | 9 +- .../profiles/prod/deployment_handler.go | 35 +++++--- .../profiles/prod/deployment_handler_test.go | 52 +++++++----- .../profiles/prod/object_creators_prod.go | 11 ++- controllers/profiles/prod/profile_prod.go | 14 ++-- utils/kubernetes/deployment.go | 20 +++-- workflowproj/operator.go | 50 ++++++++--- workflowproj/workflowproj.go | 4 +- 18 files changed, 349 insertions(+), 169 deletions(-) create mode 100644 controllers/profiles/common/test/common.go diff --git a/controllers/profiles/common/ensurer.go b/controllers/profiles/common/ensurer.go index 0d9cb472e..d4bc0db0d 100644 --- a/controllers/profiles/common/ensurer.go +++ b/controllers/profiles/common/ensurer.go @@ -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) } // MutateVisitor is a visitor function that mutates the given object before performing any updates in the cluster. @@ -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) if err != nil { return nil, result, err } @@ -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 } diff --git a/controllers/profiles/common/mutate_visitors.go b/controllers/profiles/common/mutate_visitors.go index cffda66af..666992f5a 100644 --- a/controllers/profiles/common/mutate_visitors.go +++ b/controllers/profiles/common/mutate_visitors.go @@ -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 } @@ -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 } @@ -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, + 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). WithServiceDiscovery(ctx, catalog). Build() return nil @@ -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 } } diff --git a/controllers/profiles/common/object_creators.go b/controllers/profiles/common/object_creators.go index 55c95d8af..2334e0125 100644 --- a/controllers/profiles/common/object_creators.go +++ b/controllers/profiles/common/object_creators.go @@ -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 @@ -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{ @@ -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{ @@ -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 } diff --git a/controllers/profiles/common/object_creators_test.go b/controllers/profiles/common/object_creators_test.go index f942a56e2..ff6ae7a32 100644 --- a/controllers/profiles/common/object_creators_test.go +++ b/controllers/profiles/common/object_creators_test.go @@ -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" @@ -39,28 +38,32 @@ 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", "")) @@ -68,25 +71,27 @@ func Test_ensureWorkflowPropertiesConfigMapMutator(t *testing.T) { 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 @@ -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) @@ -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 @@ -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) diff --git a/controllers/profiles/common/test/common.go b/controllers/profiles/common/test/common.go new file mode 100644 index 000000000..a4f2783d1 --- /dev/null +++ b/controllers/profiles/common/test/common.go @@ -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 ( + 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 +} diff --git a/controllers/profiles/dev/object_creators_dev.go b/controllers/profiles/dev/object_creators_dev.go index 1cbb42e4c..28419bed5 100644 --- a/controllers/profiles/dev/object_creators_dev.go +++ b/controllers/profiles/dev/object_creators_dev.go @@ -44,8 +44,8 @@ const ( // aiming a vanilla Kubernetes Deployment. // It maps the default HTTP port (80) to the target Java application webserver on port 8080. // It configures the Service as a NodePort type service, in this way it will be easier for a developer access the service -func serviceCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { - object, _ := common.ServiceCreator(workflow) +func serviceCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { + object, _ := common.ServiceCreator(workflow, platform) service := object.(*corev1.Service) // Let's double-check that the workflow is using the Dev Profile we would like to expose it via NodePort if profiles.IsDevProfile(workflow) { @@ -54,8 +54,8 @@ func serviceCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { return service, nil } -func deploymentCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { - obj, err := common.DeploymentCreator(workflow) +func deploymentCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { + obj, err := common.DeploymentCreator(workflow, platform) if err != nil { return nil, err } @@ -74,7 +74,7 @@ func deploymentCreator(workflow *operatorapi.SonataFlow) (client.Object, error) } // workflowDefConfigMapCreator creates a new ConfigMap that holds the definition of a workflow specification. -func workflowDefConfigMapCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { +func workflowDefConfigMapCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { configMap, err := workflowdef.CreateNewConfigMap(workflow) if err != nil { return nil, err @@ -84,13 +84,13 @@ func workflowDefConfigMapCreator(workflow *operatorapi.SonataFlow) (client.Objec } // deploymentMutateVisitor guarantees the state of the default Deployment object -func deploymentMutateVisitor(workflow *operatorapi.SonataFlow) common.MutateVisitor { +func deploymentMutateVisitor(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) common.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 } @@ -100,13 +100,13 @@ func deploymentMutateVisitor(workflow *operatorapi.SonataFlow) common.MutateVisi } } -func ensureWorkflowDefConfigMapMutator(workflow *operatorapi.SonataFlow) common.MutateVisitor { +func ensureWorkflowDefConfigMapMutator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) common.MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { if kubeutil.IsObjectNew(object) { return nil } - original, err := workflowDefConfigMapCreator(workflow) + original, err := workflowDefConfigMapCreator(workflow, platform) if err != nil { return err } @@ -118,7 +118,7 @@ func ensureWorkflowDefConfigMapMutator(workflow *operatorapi.SonataFlow) common. } // mountDevConfigMapsMutateVisitor mounts the required configMaps in the Workflow Dev Deployment -func mountDevConfigMapsMutateVisitor(flowDefCM, propsCM *corev1.ConfigMap, workflowResCMs []operatorapi.ConfigMapWorkflowResource) common.MutateVisitor { +func mountDevConfigMapsMutateVisitor(workflow *operatorapi.SonataFlow, flowDefCM, userPropsCM, managedPropsCM *corev1.ConfigMap, workflowResCMs []operatorapi.ConfigMapWorkflowResource) common.MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { deployment := object.(*appsv1.Deployment) @@ -129,7 +129,8 @@ func mountDevConfigMapsMutateVisitor(flowDefCM, propsCM *corev1.ConfigMap, workf // defaultResourcesVolume holds every ConfigMap mount required on src/main/resources defaultResourcesVolume := corev1.Volume{Name: configMapResourcesVolumeName, VolumeSource: corev1.VolumeSource{Projected: &corev1.ProjectedVolumeSource{}}} - kubeutil.VolumeProjectionAddConfigMap(defaultResourcesVolume.Projected, propsCM.Name, corev1.KeyToPath{Key: workflowproj.ApplicationPropertiesFileName, Path: workflowproj.ApplicationPropertiesFileName}) + kubeutil.VolumeProjectionAddConfigMap(defaultResourcesVolume.Projected, userPropsCM.Name, corev1.KeyToPath{Key: workflowproj.ApplicationPropertiesFileName, Path: workflowproj.ApplicationPropertiesFileName}) + kubeutil.VolumeProjectionAddConfigMap(defaultResourcesVolume.Projected, managedPropsCM.Name, corev1.KeyToPath{Key: workflowproj.GetManagedPropertiesFileName(workflow), Path: workflowproj.GetManagedPropertiesFileName(workflow)}) kubeutil.VolumeProjectionAddConfigMap(defaultResourcesVolume.Projected, flowDefCM.Name) // resourceVolumes holds every resource that needs to be mounted on src/main/resources/ diff --git a/controllers/profiles/dev/object_creators_dev_test.go b/controllers/profiles/dev/object_creators_dev_test.go index 37adaf8a4..12881abe5 100644 --- a/controllers/profiles/dev/object_creators_dev_test.go +++ b/controllers/profiles/dev/object_creators_dev_test.go @@ -30,9 +30,9 @@ import ( func Test_ensureWorkflowDevServiceIsExposed(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) - + platform := test.GetBasePlatform() //On Kubernetes we want the service exposed in Dev with NodePort - service, _ := serviceCreator(workflow) + service, _ := serviceCreator(workflow, platform) service.SetUID("1") service.SetResourceVersion("1") diff --git a/controllers/profiles/dev/profile_dev.go b/controllers/profiles/dev/profile_dev.go index dd0ae728d..ffafc6a51 100644 --- a/controllers/profiles/dev/profile_dev.go +++ b/controllers/profiles/dev/profile_dev.go @@ -75,21 +75,23 @@ func NewProfileReconciler(client client.Client, cfg *rest.Config, recorder recor 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.NewObjectEnsurer(support.C, common.WorkflowPropsConfigMapCreator), + deployment: common.NewObjectEnsurer(support.C, deploymentCreator), + service: common.NewObjectEnsurer(support.C, serviceCreator), + network: common.NewNoopObjectEnsurer(), + definitionConfigMap: common.NewObjectEnsurer(support.C, workflowDefConfigMapCreator), + userPropsConfigMap: common.NewObjectEnsurer(support.C, common.UserPropsConfigMapCreator), + managedPropsConfigMap: common.NewObjectEnsurer(support.C, common.ManagedPropsConfigMapCreator), } } func newObjectEnsurersOpenShift(support *common.StateSupport) *objectEnsurers { return &objectEnsurers{ - deployment: common.NewObjectEnsurer(support.C, deploymentCreator), - service: common.NewObjectEnsurer(support.C, serviceCreator), - network: common.NewObjectEnsurer(support.C, common.OpenShiftRouteCreator), - definitionConfigMap: common.NewObjectEnsurer(support.C, workflowDefConfigMapCreator), - propertiesConfigMap: common.NewObjectEnsurer(support.C, common.WorkflowPropsConfigMapCreator), + deployment: common.NewObjectEnsurer(support.C, deploymentCreator), + service: common.NewObjectEnsurer(support.C, serviceCreator), + network: common.NewObjectEnsurer(support.C, common.OpenShiftRouteCreator), + definitionConfigMap: common.NewObjectEnsurer(support.C, workflowDefConfigMapCreator), + userPropsConfigMap: common.NewObjectEnsurer(support.C, common.UserPropsConfigMapCreator), + managedPropsConfigMap: common.NewObjectEnsurer(support.C, common.ManagedPropsConfigMapCreator), } } @@ -106,11 +108,12 @@ func newStatusEnrichersOpenShift(support *common.StateSupport) *statusEnrichers } type objectEnsurers struct { - deployment common.ObjectEnsurer - service common.ObjectEnsurer - network common.ObjectEnsurer - definitionConfigMap common.ObjectEnsurer - propertiesConfigMap common.ObjectEnsurer + deployment common.ObjectEnsurer + service common.ObjectEnsurer + network common.ObjectEnsurer + definitionConfigMap common.ObjectEnsurer + userPropsConfigMap common.ObjectEnsurer + managedPropsConfigMap common.ObjectEnsurer } type statusEnrichers struct { diff --git a/controllers/profiles/dev/profile_dev_test.go b/controllers/profiles/dev/profile_dev_test.go index 5c2591d0b..1c3478ea0 100644 --- a/controllers/profiles/dev/profile_dev_test.go +++ b/controllers/profiles/dev/profile_dev_test.go @@ -145,12 +145,16 @@ func Test_newDevProfile(t *testing.T) { assert.Equal(t, quarkusDevConfigMountPath, deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath) assert.Equal(t, "", deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].SubPath) //https://kubernetes.io/docs/concepts/configuration/configmap/#mounted-configmaps-are-updated-automatically - propCM := &corev1.ConfigMap{} - _ = client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflowproj.GetWorkflowPropertiesConfigMapName(workflow)}, propCM) - assert.NotEmpty(t, propCM.Data[workflowproj.ApplicationPropertiesFileName]) + userPropsCM := &corev1.ConfigMap{} + _ = client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflowproj.GetWorkflowUserPropertiesConfigMapName(workflow)}, userPropsCM) + assert.Empty(t, userPropsCM.Data[workflowproj.ApplicationPropertiesFileName]) + + managedPropsCM := &corev1.ConfigMap{} + _ = client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflowproj.GetWorkflowManagedPropertiesConfigMapName(workflow)}, managedPropsCM) + assert.NotEmpty(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)]) assert.Equal(t, quarkusDevConfigMountPath, deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath) assert.Equal(t, "", deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].SubPath) //https://kubernetes.io/docs/concepts/configuration/configmap/#mounted-configmaps-are-updated-automatically - assert.Contains(t, propCM.Data[workflowproj.ApplicationPropertiesFileName], "quarkus.http.port") + assert.Contains(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)], "quarkus.http.port") service := test.MustGetService(t, client, workflow) assert.Equal(t, int32(constants.DefaultHTTPWorkflowPortInt), service.Spec.Ports[0].TargetPort.IntVal) @@ -179,10 +183,14 @@ func Test_newDevProfile(t *testing.T) { err = client.Update(context.TODO(), deployment) assert.NoError(t, err) - propCM = &corev1.ConfigMap{} - _ = client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflowproj.GetWorkflowPropertiesConfigMapName(workflow)}, propCM) - assert.NotEmpty(t, propCM.Data[workflowproj.ApplicationPropertiesFileName]) - assert.Contains(t, propCM.Data[workflowproj.ApplicationPropertiesFileName], "quarkus.http.port") + userPropsCM = &corev1.ConfigMap{} + _ = client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflowproj.GetWorkflowUserPropertiesConfigMapName(workflow)}, userPropsCM) + assert.Empty(t, userPropsCM.Data[workflowproj.ApplicationPropertiesFileName]) + + managedPropsCM = &corev1.ConfigMap{} + _ = client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflowproj.GetWorkflowManagedPropertiesConfigMapName(workflow)}, managedPropsCM) + assert.NotEmpty(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)]) + assert.Contains(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)], "quarkus.http.port") // reconcile workflow.Status.Manager().MarkTrue(api.RunningConditionType) diff --git a/controllers/profiles/dev/states_dev.go b/controllers/profiles/dev/states_dev.go index e965547ef..c33551ebb 100644 --- a/controllers/profiles/dev/states_dev.go +++ b/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))) if err != nil { return ctrl.Result{Requeue: false}, objs, err } - objs = append(objs, propsCM) + objs = append(objs, managedPropsCM) externalCM, err := workflowdef.FetchExternalResourcesConfigMapsRef(e.C, workflow) if err != nil { @@ -89,22 +92,22 @@ func (e *ensureRunningWorkflowState) Do(ctx context.Context, workflow *operatora return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, nil } - deployment, _, err := e.ensurers.deployment.Ensure(ctx, workflow, - deploymentMutateVisitor(workflow), + deployment, _, err := e.ensurers.deployment.Ensure(ctx, workflow, pl, + deploymentMutateVisitor(workflow, pl), common.ImageDeploymentMutateVisitor(workflow, devBaseContainerImage), - mountDevConfigMapsMutateVisitor(flowDefCM.(*corev1.ConfigMap), propsCM.(*corev1.ConfigMap), externalCM)) + mountDevConfigMapsMutateVisitor(workflow, flowDefCM.(*corev1.ConfigMap), userPropsCM.(*corev1.ConfigMap), managedPropsCM.(*corev1.ConfigMap), externalCM)) if err != nil { return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, err } objs = append(objs, deployment) - service, _, err := e.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow)) + service, _, err := e.ensurers.service.Ensure(ctx, workflow, pl, common.ServiceMutateVisitor(workflow, pl)) if err != nil { return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, err } objs = append(objs, service) - route, _, err := e.ensurers.network.Ensure(ctx, workflow) + route, _, err := e.ensurers.network.Ensure(ctx, workflow, pl) if err != nil { return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, err } diff --git a/controllers/profiles/dev/status_enricher_dev_test.go b/controllers/profiles/dev/status_enricher_dev_test.go index 39e4570df..534c0d7c2 100644 --- a/controllers/profiles/dev/status_enricher_dev_test.go +++ b/controllers/profiles/dev/status_enricher_dev_test.go @@ -38,8 +38,9 @@ func Test_enrichmentStatusOnK8s(t *testing.T) { t.Run("verify that the service URL is returned with the default cluster name on default namespace", func(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) + platform := test.GetBasePlatform() workflow.Namespace = toK8SNamespace(t.Name()) - service, _ := common.ServiceCreator(workflow) + service, _ := common.ServiceCreator(workflow, platform) client := test.NewSonataFlowClientBuilder().WithRuntimeObjects(workflow, service).Build() obj, err := statusEnricher(context.TODO(), client, workflow) @@ -54,8 +55,9 @@ func Test_enrichmentStatusOnK8s(t *testing.T) { t.Run("verify that the service URL won't be generated if an invalid namespace is used", func(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) + platform := test.GetBasePlatform() workflow.Namespace = t.Name() - service, _ := serviceCreator(workflow) + service, _ := serviceCreator(workflow, platform) client := test.NewSonataFlowClientBuilder().WithRuntimeObjects(workflow, service).Build() _, err := statusEnricher(context.TODO(), client, workflow) assert.Error(t, err) @@ -66,8 +68,9 @@ func Test_enrichmentStatusOnK8s(t *testing.T) { func Test_enrichmentStatusOnOCP(t *testing.T) { t.Run("verify that the service URL is returned with the default cluster name on default namespace", func(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) + platform := test.GetBasePlatform() workflow.Namespace = toK8SNamespace(t.Name()) - service, _ := serviceCreator(workflow) + service, _ := serviceCreator(workflow, platform) route := &openshiftv1.Route{} route.Name = workflow.Name route.Namespace = workflow.Namespace diff --git a/controllers/profiles/prod/deployment_handler.go b/controllers/profiles/prod/deployment_handler.go index a5459a873..a7cfafced 100644 --- a/controllers/profiles/prod/deployment_handler.go +++ b/controllers/profiles/prod/deployment_handler.go @@ -49,9 +49,16 @@ func (d *deploymentReconciler) reconcile(ctx context.Context, workflow *operator func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, workflow *operatorapi.SonataFlow, image string) (reconcile.Result, []client.Object, error) { pl, _ := platform.GetActivePlatform(ctx, d.C, workflow.Namespace) - propsCM, _, err := d.ensurers.propertiesConfigMap.Ensure(ctx, workflow, common.WorkflowPropertiesMutateVisitor(ctx, d.StateSupport.Catalog, workflow, pl)) + userPropsCM, _, err := d.ensurers.userPropsConfigMap.Ensure(ctx, workflow, pl) if err != nil { - workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.ExternalResourcesNotFoundReason, "Unable to retrieve the properties config map") + workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.ExternalResourcesNotFoundReason, "Unable to retrieve the user properties config map") + _, err = d.PerformStatusUpdate(ctx, workflow) + return ctrl.Result{}, nil, err + } + managedPropsCM, _, err := d.ensurers.managedPropsConfigMap.Ensure(ctx, workflow, pl, + common.UserPropertiesMutateVisitor(ctx, d.StateSupport.Catalog, workflow, pl, userPropsCM.(*v1.ConfigMap))) + if err != nil { + workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.ExternalResourcesNotFoundReason, "Unable to retrieve the managed properties config map") _, err = d.PerformStatusUpdate(ctx, workflow) return ctrl.Result{}, nil, err } @@ -59,8 +66,8 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work deployment, deploymentOp, err := d.ensurers.deployment.Ensure( ctx, - workflow, - d.getDeploymentMutateVisitors(workflow, image, propsCM.(*v1.ConfigMap))..., + workflow, pl, + d.getDeploymentMutateVisitors(workflow, pl, image, userPropsCM.(*v1.ConfigMap), managedPropsCM.(*v1.ConfigMap))..., ) if err != nil { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentUnavailableReason, "Unable to perform the deploy due to ", err) @@ -68,14 +75,14 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work return reconcile.Result{}, nil, err } - service, _, err := d.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow)) + service, _, err := d.ensurers.service.Ensure(ctx, workflow, pl, common.ServiceMutateVisitor(workflow, pl)) if err != nil { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentUnavailableReason, "Unable to make the service available due to ", err) _, err = d.PerformStatusUpdate(ctx, workflow) return reconcile.Result{}, nil, err } - objs := []client.Object{deployment, service, propsCM} + objs := []client.Object{deployment, service, managedPropsCM} if deploymentOp == controllerutil.OperationResultCreated { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.WaitingForDeploymentReason, "") @@ -99,18 +106,20 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work func (d *deploymentReconciler) getDeploymentMutateVisitors( workflow *operatorapi.SonataFlow, + platform *operatorapi.SonataFlowPlatform, image string, - configMap *v1.ConfigMap) []common.MutateVisitor { + userPropsCM *v1.ConfigMap, + managedPropsCM *v1.ConfigMap) []common.MutateVisitor { if utils.IsOpenShift() { - return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow), - mountProdConfigMapsMutateVisitor(configMap), + return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow, platform), + mountProdConfigMapsMutateVisitor(workflow, userPropsCM, managedPropsCM), addOpenShiftImageTriggerDeploymentMutateVisitor(workflow, image), common.ImageDeploymentMutateVisitor(workflow, image), - common.RolloutDeploymentIfCMChangedMutateVisitor(configMap), + common.RolloutDeploymentIfCMChangedMutateVisitor(workflow, userPropsCM, managedPropsCM), } } - return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow), + return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow, platform), common.ImageDeploymentMutateVisitor(workflow, image), - mountProdConfigMapsMutateVisitor(configMap), - common.RolloutDeploymentIfCMChangedMutateVisitor(configMap)} + mountProdConfigMapsMutateVisitor(workflow, userPropsCM, managedPropsCM), + common.RolloutDeploymentIfCMChangedMutateVisitor(workflow, userPropsCM, managedPropsCM)} } diff --git a/controllers/profiles/prod/deployment_handler_test.go b/controllers/profiles/prod/deployment_handler_test.go index 5adfae686..a133b9635 100644 --- a/controllers/profiles/prod/deployment_handler_test.go +++ b/controllers/profiles/prod/deployment_handler_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" ) @@ -77,8 +78,12 @@ func Test_CheckDeploymentRolloutAfterCMChange(t *testing.T) { assert.NotEmpty(t, objects) assert.True(t, result.Requeue) + userPropsCM := &corev1.ConfigMap{} + err = client.Get(context.TODO(), types.NamespacedName{Name: workflowproj.GetWorkflowUserPropertiesConfigMapName(workflow), Namespace: t.Name()}, userPropsCM) + assert.NoError(t, err) + // Second reconciliation, we do change the configmap and that must rollout the deployment - var cm *corev1.ConfigMap + var managedPropsCM *corev1.ConfigMap var checksum string for _, o := range objects { if _, ok := o.(*v1.Deployment); ok { @@ -89,16 +94,20 @@ func Test_CheckDeploymentRolloutAfterCMChange(t *testing.T) { assert.NotContains(t, deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt) } if _, ok := o.(*corev1.ConfigMap); ok { - cm = o.(*corev1.ConfigMap) - currentProps := cm.Data[workflowproj.ApplicationPropertiesFileName] - props, err := properties.LoadString(currentProps) - assert.Nil(t, err) - props.MustSet("test.property", "test.value") - cm.Data[workflowproj.ApplicationPropertiesFileName] = props.String() + cm := o.(*corev1.ConfigMap) + if cm.Name == workflowproj.GetWorkflowManagedPropertiesConfigMapName(workflow) { + managedPropsCM = cm + } } } - assert.NotNil(t, cm) - utilruntime.Must(client.Update(context.TODO(), cm)) + assert.NotNil(t, managedPropsCM) + + currentProps := userPropsCM.Data[workflowproj.ApplicationPropertiesFileName] + props, err := properties.LoadString(currentProps) + assert.Nil(t, err) + props.MustSet("test.property", "test.value") + userPropsCM.Data[workflowproj.ApplicationPropertiesFileName] = props.String() + utilruntime.Must(client.Update(context.TODO(), userPropsCM)) result, objects, err = handler.reconcile(context.TODO(), workflow) assert.NoError(t, err) assert.NotEmpty(t, objects) @@ -131,9 +140,13 @@ func Test_CheckDeploymentUnchangedAfterCMChangeOtherKeys(t *testing.T) { assert.NotEmpty(t, objects) assert.True(t, result.Requeue) + userPropsCM := &corev1.ConfigMap{} + err = client.Get(context.TODO(), types.NamespacedName{Name: workflowproj.GetWorkflowUserPropertiesConfigMapName(workflow), Namespace: t.Name()}, userPropsCM) + assert.NoError(t, err) + // Second reconciliation, we do change the configmap and that must not rollout the deployment // because we're not updating the application.properties key - var cm *corev1.ConfigMap + var managedPropsCM *corev1.ConfigMap var checksum string for _, o := range objects { if _, ok := o.(*v1.Deployment); ok { @@ -144,12 +157,16 @@ func Test_CheckDeploymentUnchangedAfterCMChangeOtherKeys(t *testing.T) { assert.NotContains(t, deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt) } if _, ok := o.(*corev1.ConfigMap); ok { - cm = o.(*corev1.ConfigMap) - cm.Data["other.key"] = "useless.key = value" + cm := o.(*corev1.ConfigMap) + if cm.Name == workflowproj.GetWorkflowManagedPropertiesConfigMapName(workflow) { + managedPropsCM = cm + } } } - assert.NotNil(t, cm) - utilruntime.Must(client.Update(context.TODO(), cm)) + assert.NotNil(t, managedPropsCM) + + userPropsCM.Data["other.key"] = "useless.key = value" + utilruntime.Must(client.Update(context.TODO(), userPropsCM)) result, objects, err = handler.reconcile(context.TODO(), workflow) assert.NoError(t, err) assert.NotEmpty(t, objects) @@ -157,13 +174,10 @@ func Test_CheckDeploymentUnchangedAfterCMChangeOtherKeys(t *testing.T) { for _, o := range objects { if _, ok := o.(*v1.Deployment); ok { deployment := o.(*v1.Deployment) - // Commented while waiting for SRVLOGIC-195 to be addressed - // assert.NotContains(t, deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt) - assert.Contains(t, deployment.Spec.Template.ObjectMeta.Annotations, metadata.Checksum) + assert.NotContains(t, deployment.Spec.Template.ObjectMeta.Annotations, metadata.RestartedAt) newChecksum := deployment.Spec.Template.ObjectMeta.Annotations[metadata.Checksum] assert.NotEmpty(t, newChecksum) - // Change to asssert.Equal when SRVLOGIC-195 is addressed - assert.NotEqual(t, newChecksum, checksum) + assert.Equal(t, newChecksum, checksum) break } } diff --git a/controllers/profiles/prod/object_creators_prod.go b/controllers/profiles/prod/object_creators_prod.go index 34d1447ec..02328e9b4 100644 --- a/controllers/profiles/prod/object_creators_prod.go +++ b/controllers/profiles/prod/object_creators_prod.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" + operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common" "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/constants" @@ -64,7 +65,7 @@ func addOpenShiftImageTriggerDeploymentMutateVisitor(workflow *v1alpha08.SonataF } // mountDevConfigMapsMutateVisitor mounts the required configMaps in the Workflow Dev Deployment -func mountProdConfigMapsMutateVisitor(propsCM *v1.ConfigMap) common.MutateVisitor { +func mountProdConfigMapsMutateVisitor(workflow *operatorapi.SonataFlow, userPropsCM *v1.ConfigMap, managedPropsCM *v1.ConfigMap) common.MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { deployment := object.(*appsv1.Deployment) @@ -77,12 +78,14 @@ func mountProdConfigMapsMutateVisitor(propsCM *v1.ConfigMap) common.MutateVisito deployment.Spec.Template.Spec.Containers[idx].VolumeMounts = make([]v1.VolumeMount, 0, 1) } - kubeutil.AddOrReplaceVolume(&deployment.Spec.Template.Spec, - kubeutil.VolumeConfigMap(constants.ConfigMapWorkflowPropsVolumeName, propsCM.Name, v1.KeyToPath{Key: workflowproj.ApplicationPropertiesFileName, Path: workflowproj.ApplicationPropertiesFileName})) + defaultResourcesVolume := v1.Volume{Name: constants.ConfigMapWorkflowPropsVolumeName, VolumeSource: v1.VolumeSource{Projected: &v1.ProjectedVolumeSource{}}} + kubeutil.VolumeProjectionAddConfigMap(defaultResourcesVolume.Projected, userPropsCM.Name, v1.KeyToPath{Key: workflowproj.ApplicationPropertiesFileName, Path: workflowproj.ApplicationPropertiesFileName}) + kubeutil.VolumeProjectionAddConfigMap(defaultResourcesVolume.Projected, managedPropsCM.Name, v1.KeyToPath{Key: workflowproj.GetManagedPropertiesFileName(workflow), Path: workflowproj.GetManagedPropertiesFileName(workflow)}) + kubeutil.AddOrReplaceVolume(&deployment.Spec.Template.Spec, defaultResourcesVolume) kubeutil.AddOrReplaceVolumeMount(idx, &deployment.Spec.Template.Spec, kubeutil.VolumeMount(constants.ConfigMapWorkflowPropsVolumeName, true, quarkusProdConfigMountPath)) - kubeutil.AnnotateDeploymentConfigChecksum(deployment, propsCM) + kubeutil.AnnotateDeploymentConfigChecksum(workflow, deployment, userPropsCM, managedPropsCM) return nil } } diff --git a/controllers/profiles/prod/profile_prod.go b/controllers/profiles/prod/profile_prod.go index 062359ee1..ce3a89b6b 100644 --- a/controllers/profiles/prod/profile_prod.go +++ b/controllers/profiles/prod/profile_prod.go @@ -52,16 +52,18 @@ const ( // ReconciliationState that needs access to it must include this struct as an attribute and initialize it in the profile builder. // Use newObjectEnsurers to facilitate building this struct type objectEnsurers struct { - deployment common.ObjectEnsurer - service common.ObjectEnsurer - propertiesConfigMap common.ObjectEnsurer + deployment common.ObjectEnsurer + service common.ObjectEnsurer + userPropsConfigMap common.ObjectEnsurer + managedPropsConfigMap common.ObjectEnsurer } func newObjectEnsurers(support *common.StateSupport) *objectEnsurers { return &objectEnsurers{ - deployment: common.NewObjectEnsurer(support.C, common.DeploymentCreator), - service: common.NewObjectEnsurer(support.C, common.ServiceCreator), - propertiesConfigMap: common.NewObjectEnsurer(support.C, common.WorkflowPropsConfigMapCreator), + deployment: common.NewObjectEnsurer(support.C, common.DeploymentCreator), + service: common.NewObjectEnsurer(support.C, common.ServiceCreator), + userPropsConfigMap: common.NewObjectEnsurer(support.C, common.UserPropsConfigMapCreator), + managedPropsConfigMap: common.NewObjectEnsurer(support.C, common.ManagedPropsConfigMapCreator), } } diff --git a/utils/kubernetes/deployment.go b/utils/kubernetes/deployment.go index ab8dd9409..ec9b0aa79 100644 --- a/utils/kubernetes/deployment.go +++ b/utils/kubernetes/deployment.go @@ -24,9 +24,11 @@ import ( "encoding/hex" "errors" "fmt" + "strings" "time" "github.com/apache/incubator-kie-kogito-serverless-operator/api/metadata" + operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" "github.com/apache/incubator-kie-kogito-serverless-operator/log" "github.com/apache/incubator-kie-kogito-serverless-operator/workflowproj" appsv1 "k8s.io/api/apps/v1" @@ -112,7 +114,7 @@ func MarkDeploymentToRollout(deployment *appsv1.Deployment) error { // AnnotateDeploymentConfigChecksum adds the checksum/config annotation to the template annotations of the Deployment to set the current configuration. // If the checksum has changed from the previous value, the restartedAt annotation is also added and a new rollout is started. // Code adapted from here: https://github.com/kubernetes/kubectl/blob/release-1.26/pkg/polymorphichelpers/objectrestarter.go#L44 -func AnnotateDeploymentConfigChecksum(deployment *appsv1.Deployment, cm *v1.ConfigMap) error { +func AnnotateDeploymentConfigChecksum(workflow *operatorapi.SonataFlow, deployment *appsv1.Deployment, userPropsCM *v1.ConfigMap, managedPropsCM *v1.ConfigMap) error { if deployment.Spec.Paused { return errors.New("can't restart paused deployment (run rollout resume first)") } @@ -124,7 +126,9 @@ func AnnotateDeploymentConfigChecksum(deployment *appsv1.Deployment, cm *v1.Conf if !ok { currentChecksum = "" } - newChecksum, err := configMapChecksum(cm) + newChecksum, err := configMapChecksum( + dataFromCM(userPropsCM, workflowproj.ApplicationPropertiesFileName), + dataFromCM(managedPropsCM, workflowproj.GetManagedPropertiesFileName(workflow))) if err != nil { return err } @@ -141,14 +145,18 @@ func AnnotateDeploymentConfigChecksum(deployment *appsv1.Deployment, cm *v1.Conf return nil } -func configMapChecksum(cm *v1.ConfigMap) (string, error) { - props, hasKey := cm.Data[workflowproj.ApplicationPropertiesFileName] +func dataFromCM(cm *v1.ConfigMap, key string) string { + data, hasKey := cm.Data[key] if !hasKey { - props = "" + return "" } + return data +} +func configMapChecksum(props ...string) (string, error) { + aggregatedProps := strings.Join(props, ",") hash := sha256.New() - _, err := hash.Write([]byte(props)) + _, err := hash.Write([]byte(aggregatedProps)) if err != nil { return "", err } diff --git a/workflowproj/operator.go b/workflowproj/operator.go index 7821c36ae..33b4ca672 100644 --- a/workflowproj/operator.go +++ b/workflowproj/operator.go @@ -20,18 +20,22 @@ package workflowproj import ( + "fmt" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "github.com/apache/incubator-kie-kogito-serverless-operator/api/metadata" operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" + "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles" ) const ( - workflowConfigMapNameSuffix = "-props" - // ApplicationPropertiesFileName is the default application properties file name - ApplicationPropertiesFileName = "application.properties" + workflowUserConfigMapNameSuffix = "-props" + // ApplicationPropertiesFileName is the default application properties file name holding user properties + ApplicationPropertiesFileName = "application.properties" + workflowManagedConfigMapNameSuffix = "-managed-props" // LabelApp key to use among object selectors, "app" is used among k8s applications to group objects in some UI consoles LabelApp = "app" // LabelService key to use among object selectors @@ -60,9 +64,23 @@ func SetTypeToObject(obj runtime.Object, s *runtime.Scheme) error { return nil } -// GetWorkflowPropertiesConfigMapName gets the default ConfigMap name that holds the application property for the given workflow -func GetWorkflowPropertiesConfigMapName(workflow *operatorapi.SonataFlow) string { - return workflow.Name + workflowConfigMapNameSuffix +// GetWorkflowUserPropertiesConfigMapName gets the default ConfigMap name that holds the user application property for the given workflow +func GetWorkflowUserPropertiesConfigMapName(workflow *operatorapi.SonataFlow) string { + return workflow.Name + workflowUserConfigMapNameSuffix +} + +// GetWorkflowManagedPropertiesConfigMapName gets the default ConfigMap name that holds the managed application property for the given workflow +func GetWorkflowManagedPropertiesConfigMapName(workflow *operatorapi.SonataFlow) string { + return workflow.Name + workflowManagedConfigMapNameSuffix +} + +// GetWorkflowManagedPropertiesConfigMapName gets the default ConfigMap name that holds the managed application property for the given workflow +func GetManagedPropertiesFileName(workflow *operatorapi.SonataFlow) string { + profile := metadata.ProdProfile + if profiles.IsDevProfile(workflow) { + profile = metadata.DevProfile + } + return fmt.Sprintf("application-%s.properties", profile) } // SetDefaultLabels adds the default workflow application labels to the given object. @@ -80,15 +98,27 @@ func GetDefaultLabels(workflow *operatorapi.SonataFlow) map[string]string { } } -// CreateNewAppPropsConfigMap creates a new ConfigMap object to hold the workflow application properties. -func CreateNewAppPropsConfigMap(workflow *operatorapi.SonataFlow, properties string) *corev1.ConfigMap { +// CreateNewUserPropsConfigMap creates a new empty ConfigMap object to hold the user application properties of the workflow. +func CreateNewUserPropsConfigMap(workflow *operatorapi.SonataFlow) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: GetWorkflowUserPropertiesConfigMapName(workflow), + Namespace: workflow.Namespace, + Labels: GetDefaultLabels(workflow), + }, + Data: map[string]string{ApplicationPropertiesFileName: ""}, + } +} + +// CreateNewManagedPropsConfigMap creates a new ConfigMap object to hold the managed application properties of the workflos. +func CreateNewManagedPropsConfigMap(workflow *operatorapi.SonataFlow, properties string) *corev1.ConfigMap { return &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: GetWorkflowPropertiesConfigMapName(workflow), + Name: GetWorkflowManagedPropertiesConfigMapName(workflow), Namespace: workflow.Namespace, Labels: GetDefaultLabels(workflow), }, - Data: map[string]string{ApplicationPropertiesFileName: properties}, + Data: map[string]string{GetManagedPropertiesFileName(workflow): properties}, } } diff --git a/workflowproj/workflowproj.go b/workflowproj/workflowproj.go index 3b46f8c95..11b781f7d 100644 --- a/workflowproj/workflowproj.go +++ b/workflowproj/workflowproj.go @@ -38,6 +38,8 @@ import ( operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" ) +// TODO: should we delete this whole file? seems used only the the associated test functions + var _ WorkflowProjectHandler = &workflowProjectHandler{} // defaultResourcePath is the default resource path to add to the generated ConfigMaps @@ -247,7 +249,7 @@ func (w *workflowProjectHandler) parseRawAppProperties() error { if err != nil { return err } - w.project.Properties = CreateNewAppPropsConfigMap(w.project.Workflow, string(appPropsContent)) + w.project.Properties = CreateNewManagedPropsConfigMap(w.project.Workflow, string(appPropsContent)) if err = SetTypeToObject(w.project.Properties, w.scheme); err != nil { return err } From 54f460a4b37734ab079d9f3f668feb377c8ad310 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Mon, 29 Jan 2024 08:55:03 +0100 Subject: [PATCH 02/14] integrating comments: introducing ObjectEnsurerWithPlatform and ObjectCreatorWithPlatform --- controllers/profiles/common/ensurer.go | 45 +++++++++++++++++-- .../profiles/common/mutate_visitors.go | 4 +- .../profiles/common/object_creators.go | 14 +++--- .../profiles/common/object_creators_test.go | 10 ++--- .../profiles/dev/object_creators_dev.go | 18 ++++---- .../profiles/dev/object_creators_dev_test.go | 3 +- controllers/profiles/dev/profile_dev.go | 6 +-- controllers/profiles/dev/states_dev.go | 12 ++--- .../profiles/dev/status_enricher_dev_test.go | 9 ++-- .../profiles/prod/deployment_handler.go | 6 +-- controllers/profiles/prod/profile_prod.go | 4 +- 11 files changed, 84 insertions(+), 47 deletions(-) diff --git a/controllers/profiles/common/ensurer.go b/controllers/profiles/common/ensurer.go index d4bc0db0d..7d258dbb3 100644 --- a/controllers/profiles/common/ensurer.go +++ b/controllers/profiles/common/ensurer.go @@ -34,6 +34,9 @@ var _ ObjectEnsurer = &defaultObjectEnsurer{} var _ ObjectEnsurer = &noopObjectEnsurer{} type ObjectEnsurer interface { + Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) +} +type ObjectEnsurerWithPlatform interface { Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) } @@ -56,16 +59,52 @@ func NewObjectEnsurer(client client.Client, creator ObjectCreator) ObjectEnsurer } } +// NewObjectEnsurerWithPlatform see defaultObjectEnsurerWithPLatform +func NewObjectEnsurerWithPlatform(client client.Client, creator ObjectCreatorWithPlatform) ObjectEnsurerWithPlatform { + return &defaultObjectEnsurerWithPlatform{ + c: client, + creator: creator, + } +} + // defaultObjectEnsurer provides the engine for a ReconciliationState that needs to create or update a given Kubernetes object during the reconciliation cycle. type defaultObjectEnsurer struct { c client.Client creator ObjectCreator } -func (d *defaultObjectEnsurer) Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) { +func (d *defaultObjectEnsurer) Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) { + result := controllerutil.OperationResultNone + + object, err := d.creator(workflow) + if err != nil { + return nil, result, err + } + if result, err = controllerutil.CreateOrPatch(ctx, d.c, object, + func() error { + for _, v := range visitors { + if visitorErr := v(object)(); visitorErr != nil { + return visitorErr + } + } + return controllerutil.SetControllerReference(workflow, object, d.c.Scheme()) + }); err != nil { + return nil, result, err + } + klog.V(log.I).InfoS("Object operation finalized", "result", result, "kind", object.GetObjectKind().GroupVersionKind().String(), "name", object.GetName(), "namespace", object.GetNamespace()) + return object, result, nil +} + +// defaultObjectEnsurerWithPlatform is the equivalent of defaultObjectEnsurer for resources that require a reference to the SonataFlowPlatform +type defaultObjectEnsurerWithPlatform struct { + c client.Client + creator ObjectCreatorWithPlatform +} + +func (d *defaultObjectEnsurerWithPlatform) Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, pl *operatorapi.SonataFlowPlatform, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) { result := controllerutil.OperationResultNone - object, err := d.creator(workflow, platform) + object, err := d.creator(workflow, pl) if err != nil { return nil, result, err } @@ -93,7 +132,7 @@ func NewNoopObjectEnsurer() ObjectEnsurer { type noopObjectEnsurer struct { } -func (d *noopObjectEnsurer) Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) { +func (d *noopObjectEnsurer) Ensure(ctx context.Context, workflow *operatorapi.SonataFlow, visitors ...MutateVisitor) (client.Object, controllerutil.OperationResult, error) { result := controllerutil.OperationResultNone return nil, result, nil } diff --git a/controllers/profiles/common/mutate_visitors.go b/controllers/profiles/common/mutate_visitors.go index 06236408b..b34b6c7d4 100644 --- a/controllers/profiles/common/mutate_visitors.go +++ b/controllers/profiles/common/mutate_visitors.go @@ -63,7 +63,7 @@ func DeploymentMutateVisitor(workflow *operatorapi.SonataFlow, platform *operato if kubeutil.IsObjectNew(object) { return nil } - original, err := DeploymentCreator(workflow, platform) + original, err := DeploymentCreator(workflow) if err != nil { return err } @@ -94,7 +94,7 @@ func ServiceMutateVisitor(workflow *operatorapi.SonataFlow, platform *operatorap if kubeutil.IsObjectNew(object) { return nil } - original, err := ServiceCreator(workflow, platform) + original, err := ServiceCreator(workflow) if err != nil { return err } diff --git a/controllers/profiles/common/object_creators.go b/controllers/profiles/common/object_creators.go index 664c6fab5..0e2d6f209 100644 --- a/controllers/profiles/common/object_creators.go +++ b/controllers/profiles/common/object_creators.go @@ -39,7 +39,11 @@ 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, platform *operatorapi.SonataFlowPlatform) (client.Object, error) +type ObjectCreator func(workflow *operatorapi.SonataFlow) (client.Object, error) + +// ObjectCreatorWithPlatform is the func equivalent to ObjectCreator to use when the resource being created needs a reference to the +// SonataFlowPlatform +type ObjectCreatorWithPlatform func(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) const ( defaultHTTPServicePort = 80 @@ -67,7 +71,7 @@ var ( // 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, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { +func DeploymentCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { lbl := workflowproj.GetDefaultLabels(workflow) deployment := &appsv1.Deployment{ @@ -179,7 +183,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, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { +func ServiceCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { lbl := workflowproj.GetDefaultLabels(workflow) service := &corev1.Service{ @@ -204,13 +208,13 @@ func ServiceCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.Sona // 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, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { +func OpenShiftRouteCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { route, err := openshift.RouteForWorkflow(workflow) return route, err } // UserPropsConfigMapCreator creates an empty ConfigMap to hold the user application properties -func UserPropsConfigMapCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { +func UserPropsConfigMapCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { return workflowproj.CreateNewUserPropsConfigMap(workflow), nil } diff --git a/controllers/profiles/common/object_creators_test.go b/controllers/profiles/common/object_creators_test.go index 4c0194794..f15fd6479 100644 --- a/controllers/profiles/common/object_creators_test.go +++ b/controllers/profiles/common/object_creators_test.go @@ -45,7 +45,7 @@ func Test_ensureWorkflowPropertiesConfigMapMutator(t *testing.T) { managedProps.SetResourceVersion("1") managedPropsCM := managedProps.(*corev1.ConfigMap) - userProps, _ := UserPropsConfigMapCreator(workflow, platform) + userProps, _ := UserPropsConfigMapCreator(workflow) userPropsCM := userProps.(*corev1.ConfigMap) visitor := UserPropertiesMutateVisitor(context.TODO(), nil, workflow, nil, userPropsCM) mutateFn := visitor(managedProps) @@ -78,7 +78,7 @@ func Test_ensureWorkflowPropertiesConfigMapMutator_DollarReplacement(t *testing. managedProps.SetUID("0000-0001-0002-0003") managedPropsCM := managedProps.(*corev1.ConfigMap) - userProps, _ := UserPropsConfigMapCreator(workflow, platform) + userProps, _ := UserPropsConfigMapCreator(workflow) userPropsCM := userProps.(*corev1.ConfigMap) userPropsCM.Data[workflowproj.ApplicationPropertiesFileName] = "mp.messaging.outgoing.kogito_outgoing_stream.url=${kubernetes:services.v1/event-listener}" @@ -91,7 +91,6 @@ func Test_ensureWorkflowPropertiesConfigMapMutator_DollarReplacement(t *testing. 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 @@ -128,7 +127,7 @@ func TestMergePodSpec(t *testing.T) { }, } - object, err := DeploymentCreator(workflow, platform) + object, err := DeploymentCreator(workflow) assert.NoError(t, err) deployment := object.(*appsv1.Deployment) @@ -145,7 +144,6 @@ 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 @@ -164,7 +162,7 @@ func TestMergePodSpec_OverrideContainers(t *testing.T) { }, } - object, err := DeploymentCreator(workflow, platform) + object, err := DeploymentCreator(workflow) assert.NoError(t, err) deployment := object.(*appsv1.Deployment) diff --git a/controllers/profiles/dev/object_creators_dev.go b/controllers/profiles/dev/object_creators_dev.go index 28419bed5..03b857502 100644 --- a/controllers/profiles/dev/object_creators_dev.go +++ b/controllers/profiles/dev/object_creators_dev.go @@ -44,8 +44,8 @@ const ( // aiming a vanilla Kubernetes Deployment. // It maps the default HTTP port (80) to the target Java application webserver on port 8080. // It configures the Service as a NodePort type service, in this way it will be easier for a developer access the service -func serviceCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { - object, _ := common.ServiceCreator(workflow, platform) +func serviceCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { + object, _ := common.ServiceCreator(workflow) service := object.(*corev1.Service) // Let's double-check that the workflow is using the Dev Profile we would like to expose it via NodePort if profiles.IsDevProfile(workflow) { @@ -54,8 +54,8 @@ func serviceCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.Sona return service, nil } -func deploymentCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { - obj, err := common.DeploymentCreator(workflow, platform) +func deploymentCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { + obj, err := common.DeploymentCreator(workflow) if err != nil { return nil, err } @@ -74,7 +74,7 @@ func deploymentCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.S } // workflowDefConfigMapCreator creates a new ConfigMap that holds the definition of a workflow specification. -func workflowDefConfigMapCreator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (client.Object, error) { +func workflowDefConfigMapCreator(workflow *operatorapi.SonataFlow) (client.Object, error) { configMap, err := workflowdef.CreateNewConfigMap(workflow) if err != nil { return nil, err @@ -84,13 +84,13 @@ func workflowDefConfigMapCreator(workflow *operatorapi.SonataFlow, platform *ope } // deploymentMutateVisitor guarantees the state of the default Deployment object -func deploymentMutateVisitor(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) common.MutateVisitor { +func deploymentMutateVisitor(workflow *operatorapi.SonataFlow) common.MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { if kubeutil.IsObjectNew(object) { return nil } - original, err := deploymentCreator(workflow, platform) + original, err := deploymentCreator(workflow) if err != nil { return err } @@ -100,13 +100,13 @@ func deploymentMutateVisitor(workflow *operatorapi.SonataFlow, platform *operato } } -func ensureWorkflowDefConfigMapMutator(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) common.MutateVisitor { +func ensureWorkflowDefConfigMapMutator(workflow *operatorapi.SonataFlow) common.MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { if kubeutil.IsObjectNew(object) { return nil } - original, err := workflowDefConfigMapCreator(workflow, platform) + original, err := workflowDefConfigMapCreator(workflow) if err != nil { return err } diff --git a/controllers/profiles/dev/object_creators_dev_test.go b/controllers/profiles/dev/object_creators_dev_test.go index 12881abe5..8209940a2 100644 --- a/controllers/profiles/dev/object_creators_dev_test.go +++ b/controllers/profiles/dev/object_creators_dev_test.go @@ -30,9 +30,8 @@ import ( func Test_ensureWorkflowDevServiceIsExposed(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) - platform := test.GetBasePlatform() //On Kubernetes we want the service exposed in Dev with NodePort - service, _ := serviceCreator(workflow, platform) + service, _ := serviceCreator(workflow) service.SetUID("1") service.SetResourceVersion("1") diff --git a/controllers/profiles/dev/profile_dev.go b/controllers/profiles/dev/profile_dev.go index ffafc6a51..42b9151ab 100644 --- a/controllers/profiles/dev/profile_dev.go +++ b/controllers/profiles/dev/profile_dev.go @@ -80,7 +80,7 @@ func newObjectEnsurers(support *common.StateSupport) *objectEnsurers { network: common.NewNoopObjectEnsurer(), definitionConfigMap: common.NewObjectEnsurer(support.C, workflowDefConfigMapCreator), userPropsConfigMap: common.NewObjectEnsurer(support.C, common.UserPropsConfigMapCreator), - managedPropsConfigMap: common.NewObjectEnsurer(support.C, common.ManagedPropsConfigMapCreator), + managedPropsConfigMap: common.NewObjectEnsurerWithPlatform(support.C, common.ManagedPropsConfigMapCreator), } } @@ -91,7 +91,7 @@ func newObjectEnsurersOpenShift(support *common.StateSupport) *objectEnsurers { network: common.NewObjectEnsurer(support.C, common.OpenShiftRouteCreator), definitionConfigMap: common.NewObjectEnsurer(support.C, workflowDefConfigMapCreator), userPropsConfigMap: common.NewObjectEnsurer(support.C, common.UserPropsConfigMapCreator), - managedPropsConfigMap: common.NewObjectEnsurer(support.C, common.ManagedPropsConfigMapCreator), + managedPropsConfigMap: common.NewObjectEnsurerWithPlatform(support.C, common.ManagedPropsConfigMapCreator), } } @@ -113,7 +113,7 @@ type objectEnsurers struct { network common.ObjectEnsurer definitionConfigMap common.ObjectEnsurer userPropsConfigMap common.ObjectEnsurer - managedPropsConfigMap common.ObjectEnsurer + managedPropsConfigMap common.ObjectEnsurerWithPlatform } type statusEnrichers struct { diff --git a/controllers/profiles/dev/states_dev.go b/controllers/profiles/dev/states_dev.go index c33551ebb..da239dd92 100644 --- a/controllers/profiles/dev/states_dev.go +++ b/controllers/profiles/dev/states_dev.go @@ -68,12 +68,12 @@ func (e *ensureRunningWorkflowState) Do(ctx context.Context, workflow *operatora if err == nil && len(pl.Spec.DevMode.BaseImage) > 0 { devBaseContainerImage = pl.Spec.DevMode.BaseImage } - flowDefCM, _, err := e.ensurers.definitionConfigMap.Ensure(ctx, workflow, pl, ensureWorkflowDefConfigMapMutator(workflow, pl)) + flowDefCM, _, err := e.ensurers.definitionConfigMap.Ensure(ctx, workflow, ensureWorkflowDefConfigMapMutator(workflow)) if err != nil { return ctrl.Result{Requeue: false}, objs, err } objs = append(objs, flowDefCM) - userPropsCM, _, err := e.ensurers.userPropsConfigMap.Ensure(ctx, workflow, pl) + userPropsCM, _, err := e.ensurers.userPropsConfigMap.Ensure(ctx, workflow) if err != nil { return ctrl.Result{Requeue: false}, objs, err } @@ -92,8 +92,8 @@ func (e *ensureRunningWorkflowState) Do(ctx context.Context, workflow *operatora return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, nil } - deployment, _, err := e.ensurers.deployment.Ensure(ctx, workflow, pl, - deploymentMutateVisitor(workflow, pl), + deployment, _, err := e.ensurers.deployment.Ensure(ctx, workflow, + deploymentMutateVisitor(workflow), common.ImageDeploymentMutateVisitor(workflow, devBaseContainerImage), mountDevConfigMapsMutateVisitor(workflow, flowDefCM.(*corev1.ConfigMap), userPropsCM.(*corev1.ConfigMap), managedPropsCM.(*corev1.ConfigMap), externalCM)) if err != nil { @@ -101,13 +101,13 @@ func (e *ensureRunningWorkflowState) Do(ctx context.Context, workflow *operatora } objs = append(objs, deployment) - service, _, err := e.ensurers.service.Ensure(ctx, workflow, pl, common.ServiceMutateVisitor(workflow, pl)) + service, _, err := e.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow, pl)) if err != nil { return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, err } objs = append(objs, service) - route, _, err := e.ensurers.network.Ensure(ctx, workflow, pl) + route, _, err := e.ensurers.network.Ensure(ctx, workflow) if err != nil { return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, err } diff --git a/controllers/profiles/dev/status_enricher_dev_test.go b/controllers/profiles/dev/status_enricher_dev_test.go index 534c0d7c2..39e4570df 100644 --- a/controllers/profiles/dev/status_enricher_dev_test.go +++ b/controllers/profiles/dev/status_enricher_dev_test.go @@ -38,9 +38,8 @@ func Test_enrichmentStatusOnK8s(t *testing.T) { t.Run("verify that the service URL is returned with the default cluster name on default namespace", func(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) - platform := test.GetBasePlatform() workflow.Namespace = toK8SNamespace(t.Name()) - service, _ := common.ServiceCreator(workflow, platform) + service, _ := common.ServiceCreator(workflow) client := test.NewSonataFlowClientBuilder().WithRuntimeObjects(workflow, service).Build() obj, err := statusEnricher(context.TODO(), client, workflow) @@ -55,9 +54,8 @@ func Test_enrichmentStatusOnK8s(t *testing.T) { t.Run("verify that the service URL won't be generated if an invalid namespace is used", func(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) - platform := test.GetBasePlatform() workflow.Namespace = t.Name() - service, _ := serviceCreator(workflow, platform) + service, _ := serviceCreator(workflow) client := test.NewSonataFlowClientBuilder().WithRuntimeObjects(workflow, service).Build() _, err := statusEnricher(context.TODO(), client, workflow) assert.Error(t, err) @@ -68,9 +66,8 @@ func Test_enrichmentStatusOnK8s(t *testing.T) { func Test_enrichmentStatusOnOCP(t *testing.T) { t.Run("verify that the service URL is returned with the default cluster name on default namespace", func(t *testing.T) { workflow := test.GetBaseSonataFlowWithDevProfile(t.Name()) - platform := test.GetBasePlatform() workflow.Namespace = toK8SNamespace(t.Name()) - service, _ := serviceCreator(workflow, platform) + service, _ := serviceCreator(workflow) route := &openshiftv1.Route{} route.Name = workflow.Name route.Namespace = workflow.Namespace diff --git a/controllers/profiles/prod/deployment_handler.go b/controllers/profiles/prod/deployment_handler.go index a7cfafced..a20fdec8b 100644 --- a/controllers/profiles/prod/deployment_handler.go +++ b/controllers/profiles/prod/deployment_handler.go @@ -49,7 +49,7 @@ func (d *deploymentReconciler) reconcile(ctx context.Context, workflow *operator func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, workflow *operatorapi.SonataFlow, image string) (reconcile.Result, []client.Object, error) { pl, _ := platform.GetActivePlatform(ctx, d.C, workflow.Namespace) - userPropsCM, _, err := d.ensurers.userPropsConfigMap.Ensure(ctx, workflow, pl) + userPropsCM, _, err := d.ensurers.userPropsConfigMap.Ensure(ctx, workflow) if err != nil { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.ExternalResourcesNotFoundReason, "Unable to retrieve the user properties config map") _, err = d.PerformStatusUpdate(ctx, workflow) @@ -66,7 +66,7 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work deployment, deploymentOp, err := d.ensurers.deployment.Ensure( ctx, - workflow, pl, + workflow, d.getDeploymentMutateVisitors(workflow, pl, image, userPropsCM.(*v1.ConfigMap), managedPropsCM.(*v1.ConfigMap))..., ) if err != nil { @@ -75,7 +75,7 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work return reconcile.Result{}, nil, err } - service, _, err := d.ensurers.service.Ensure(ctx, workflow, pl, common.ServiceMutateVisitor(workflow, pl)) + service, _, err := d.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow, pl)) if err != nil { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentUnavailableReason, "Unable to make the service available due to ", err) _, err = d.PerformStatusUpdate(ctx, workflow) diff --git a/controllers/profiles/prod/profile_prod.go b/controllers/profiles/prod/profile_prod.go index ce3a89b6b..f5046d05f 100644 --- a/controllers/profiles/prod/profile_prod.go +++ b/controllers/profiles/prod/profile_prod.go @@ -55,7 +55,7 @@ type objectEnsurers struct { deployment common.ObjectEnsurer service common.ObjectEnsurer userPropsConfigMap common.ObjectEnsurer - managedPropsConfigMap common.ObjectEnsurer + managedPropsConfigMap common.ObjectEnsurerWithPlatform } func newObjectEnsurers(support *common.StateSupport) *objectEnsurers { @@ -63,7 +63,7 @@ func newObjectEnsurers(support *common.StateSupport) *objectEnsurers { deployment: common.NewObjectEnsurer(support.C, common.DeploymentCreator), service: common.NewObjectEnsurer(support.C, common.ServiceCreator), userPropsConfigMap: common.NewObjectEnsurer(support.C, common.UserPropsConfigMapCreator), - managedPropsConfigMap: common.NewObjectEnsurer(support.C, common.ManagedPropsConfigMapCreator), + managedPropsConfigMap: common.NewObjectEnsurerWithPlatform(support.C, common.ManagedPropsConfigMapCreator), } } From d77fe1827504e1380be22e0109cbb8e77ee85966 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Mon, 29 Jan 2024 09:04:44 +0100 Subject: [PATCH 03/14] Removing fewe more unneeded references to platform --- controllers/profiles/common/mutate_visitors.go | 4 ++-- controllers/profiles/dev/states_dev.go | 2 +- controllers/profiles/prod/deployment_handler.go | 9 ++++----- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/controllers/profiles/common/mutate_visitors.go b/controllers/profiles/common/mutate_visitors.go index b34b6c7d4..38703cf74 100644 --- a/controllers/profiles/common/mutate_visitors.go +++ b/controllers/profiles/common/mutate_visitors.go @@ -57,7 +57,7 @@ func ImageDeploymentMutateVisitor(workflow *operatorapi.SonataFlow, image string } // DeploymentMutateVisitor guarantees the state of the default Deployment object -func DeploymentMutateVisitor(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) MutateVisitor { +func DeploymentMutateVisitor(workflow *operatorapi.SonataFlow) MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { if kubeutil.IsObjectNew(object) { @@ -88,7 +88,7 @@ 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, platform *operatorapi.SonataFlowPlatform) MutateVisitor { +func ServiceMutateVisitor(workflow *operatorapi.SonataFlow) MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { if kubeutil.IsObjectNew(object) { diff --git a/controllers/profiles/dev/states_dev.go b/controllers/profiles/dev/states_dev.go index da239dd92..34b445b7a 100644 --- a/controllers/profiles/dev/states_dev.go +++ b/controllers/profiles/dev/states_dev.go @@ -101,7 +101,7 @@ func (e *ensureRunningWorkflowState) Do(ctx context.Context, workflow *operatora } objs = append(objs, deployment) - service, _, err := e.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow, pl)) + service, _, err := e.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow)) if err != nil { return ctrl.Result{RequeueAfter: constants.RequeueAfterFailure}, objs, err } diff --git a/controllers/profiles/prod/deployment_handler.go b/controllers/profiles/prod/deployment_handler.go index a20fdec8b..e075543f0 100644 --- a/controllers/profiles/prod/deployment_handler.go +++ b/controllers/profiles/prod/deployment_handler.go @@ -67,7 +67,7 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work d.ensurers.deployment.Ensure( ctx, workflow, - d.getDeploymentMutateVisitors(workflow, pl, image, userPropsCM.(*v1.ConfigMap), managedPropsCM.(*v1.ConfigMap))..., + d.getDeploymentMutateVisitors(workflow, image, userPropsCM.(*v1.ConfigMap), managedPropsCM.(*v1.ConfigMap))..., ) if err != nil { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentUnavailableReason, "Unable to perform the deploy due to ", err) @@ -75,7 +75,7 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work return reconcile.Result{}, nil, err } - service, _, err := d.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow, pl)) + service, _, err := d.ensurers.service.Ensure(ctx, workflow, common.ServiceMutateVisitor(workflow)) if err != nil { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentUnavailableReason, "Unable to make the service available due to ", err) _, err = d.PerformStatusUpdate(ctx, workflow) @@ -106,19 +106,18 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work func (d *deploymentReconciler) getDeploymentMutateVisitors( workflow *operatorapi.SonataFlow, - platform *operatorapi.SonataFlowPlatform, image string, userPropsCM *v1.ConfigMap, managedPropsCM *v1.ConfigMap) []common.MutateVisitor { if utils.IsOpenShift() { - return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow, platform), + return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow), mountProdConfigMapsMutateVisitor(workflow, userPropsCM, managedPropsCM), addOpenShiftImageTriggerDeploymentMutateVisitor(workflow, image), common.ImageDeploymentMutateVisitor(workflow, image), common.RolloutDeploymentIfCMChangedMutateVisitor(workflow, userPropsCM, managedPropsCM), } } - return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow, platform), + return []common.MutateVisitor{common.DeploymentMutateVisitor(workflow), common.ImageDeploymentMutateVisitor(workflow, image), mountProdConfigMapsMutateVisitor(workflow, userPropsCM, managedPropsCM), common.RolloutDeploymentIfCMChangedMutateVisitor(workflow, userPropsCM, managedPropsCM)} From d481813ad7eae57f5885b3cba4ba0048ede203b4 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Mon, 29 Jan 2024 12:08:54 +0100 Subject: [PATCH 04/14] reviewed workflowProjectHandler. removed user props from managed props --- .../profiles/common/properties/application.go | 24 +++++++++---------- workflowproj/workflowproj.go | 3 ++- workflowproj/workflowproj_test.go | 1 + 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/controllers/profiles/common/properties/application.go b/controllers/profiles/common/properties/application.go index fda4ec1e4..ce6fb89a5 100644 --- a/controllers/profiles/common/properties/application.go +++ b/controllers/profiles/common/properties/application.go @@ -75,36 +75,34 @@ func (a *appPropertyHandler) WithServiceDiscovery(ctx context.Context, catalog d } func (a *appPropertyHandler) Build() string { - var props *properties.Properties + var userProps *properties.Properties var propErr error = nil if len(a.userProperties) == 0 { - props = properties.NewProperties() + userProps = properties.NewProperties() } else { - props, propErr = properties.LoadString(a.userProperties) + userProps, propErr = properties.LoadString(a.userProperties) } if propErr != nil { klog.V(log.D).InfoS("Can't load user's property", "workflow", a.workflow.Name, "namespace", a.workflow.Namespace, "properties", a.userProperties) - props = properties.NewProperties() + userProps = properties.NewProperties() } // Disable expansions since it's not our responsibility // Property expansion means resolving ${} within the properties and environment context. Quarkus will do that in runtime. - props.DisableExpansion = true + userProps.DisableExpansion = true - removeDiscoveryProperties(props) + removeDiscoveryProperties(userProps) + discoveryProps := properties.NewProperties() if a.requireServiceDiscovery() { // produce the MicroProfileConfigServiceCatalog properties for the service discovery property values if any. - discoveryProperties := generateDiscoveryProperties(a.ctx, a.catalog, props, a.workflow) - if discoveryProperties.Len() > 0 { - props.Merge(discoveryProperties) - } + discoveryProps.Merge(generateDiscoveryProperties(a.ctx, a.catalog, userProps, a.workflow)) } - props = utils.NewApplicationPropertiesBuilder(). - WithInitialProperties(props). + userProps = utils.NewApplicationPropertiesBuilder(). + WithInitialProperties(discoveryProps). WithImmutableProperties(properties.MustLoadString(immutableApplicationProperties)). WithDefaultMutableProperties(a.defaultMutableProperties). Build() - return props.String() + return userProps.String() } // withKogitoServiceUrl adds the property kogitoServiceUrlProperty to the application properties. diff --git a/workflowproj/workflowproj.go b/workflowproj/workflowproj.go index 11b781f7d..77118d259 100644 --- a/workflowproj/workflowproj.go +++ b/workflowproj/workflowproj.go @@ -249,7 +249,8 @@ func (w *workflowProjectHandler) parseRawAppProperties() error { if err != nil { return err } - w.project.Properties = CreateNewManagedPropsConfigMap(w.project.Workflow, string(appPropsContent)) + w.project.Properties = CreateNewUserPropsConfigMap(w.project.Workflow) + w.project.Properties.Data[ApplicationPropertiesFileName] = string(appPropsContent) if err = SetTypeToObject(w.project.Properties, w.scheme); err != nil { return err } diff --git a/workflowproj/workflowproj_test.go b/workflowproj/workflowproj_test.go index 0bc9679cf..c1207527f 100644 --- a/workflowproj/workflowproj_test.go +++ b/workflowproj/workflowproj_test.go @@ -61,6 +61,7 @@ func Test_Handler_WorkflowMinimalAndProps(t *testing.T) { assert.NotNil(t, proj.Properties) assert.Equal(t, "minimal", proj.Workflow.Name) assert.Equal(t, "minimal-props", proj.Properties.Name) + assert.NotEmpty(t, proj.Properties.Data["application.properties"]) assert.Equal(t, string(metadata.ProdProfile), proj.Workflow.Annotations[metadata.Profile]) assert.NotEmpty(t, proj.Properties.Data) } From fea41a090cfc6ef066e511f359afbc216a515795 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Mon, 29 Jan 2024 14:44:44 +0100 Subject: [PATCH 05/14] fixed unit tests --- .../profiles/common/object_creators_test.go | 5 ++- .../common/properties/application_test.go | 45 +++++++++++-------- controllers/profiles/dev/states_dev.go | 11 ++--- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/controllers/profiles/common/object_creators_test.go b/controllers/profiles/common/object_creators_test.go index f15fd6479..1d2b26c4e 100644 --- a/controllers/profiles/common/object_creators_test.go +++ b/controllers/profiles/common/object_creators_test.go @@ -66,7 +66,7 @@ func Test_ensureWorkflowPropertiesConfigMapMutator(t *testing.T) { 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", "")) + assert.NotContains(t, "my.new.prop", props.Keys()) } func Test_ensureWorkflowPropertiesConfigMapMutator_DollarReplacement(t *testing.T) { @@ -86,7 +86,8 @@ func Test_ensureWorkflowPropertiesConfigMapMutator_DollarReplacement(t *testing. err := mutateVisitorFn(managedPropsCM)() assert.NoError(t, err) - assert.Contains(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)], "${kubernetes:services.v1/event-listener}") + assert.NotContains(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)], "mp.messaging.outgoing.kogito_outgoing_stream.url") + // assert.Contains(t, managedPropsCM.Data[workflowproj.GetManagedPropertiesFileName(workflow)], "${kubernetes:services.v1/event-listener}") } func TestMergePodSpec(t *testing.T) { diff --git a/controllers/profiles/common/properties/application_test.go b/controllers/profiles/common/properties/application_test.go index a4c5dcf33..d42f8a7fd 100644 --- a/controllers/profiles/common/properties/application_test.go +++ b/controllers/profiles/common/properties/application_test.go @@ -124,9 +124,9 @@ func Test_appPropertyHandler_WithUserPropertiesWithNoUserOverrides(t *testing.T) assert.NoError(t, err) generatedProps, propsErr := properties.LoadString(props.WithUserProperties(userProperties).Build()) assert.NoError(t, propsErr) - assert.Equal(t, 9, len(generatedProps.Keys())) - assert.Equal(t, "value1", generatedProps.GetString("property1", "")) - assert.Equal(t, "value2", generatedProps.GetString("property2", "")) + assert.Equal(t, 7, len(generatedProps.Keys())) + assert.NotContains(t, "property1", generatedProps.Keys()) + assert.NotContains(t, "property2", generatedProps.Keys()) assert.Equal(t, "http://greeting.default", generatedProps.GetString("kogito.service.url", "")) assert.Equal(t, "8080", generatedProps.GetString("quarkus.http.port", "")) assert.Equal(t, "0.0.0.0", generatedProps.GetString("quarkus.http.host", "")) @@ -157,13 +157,16 @@ func Test_appPropertyHandler_WithUserPropertiesWithServiceDiscovery(t *testing.T Build()) generatedProps.DisableExpansion = true assert.NoError(t, propsErr) - assert.Equal(t, 23, len(generatedProps.Keys())) - assertHasProperty(t, generatedProps, "property1", "value1") - assertHasProperty(t, generatedProps, "property2", "value2") - - assertHasProperty(t, generatedProps, "service1", "${kubernetes:services.v1/namespace1/my-service1}") - assertHasProperty(t, generatedProps, "service2", "${kubernetes:services.v1/my-service2}") - assertHasProperty(t, generatedProps, "service3", "${knative:namespace1/my-kn-service1}") + assert.Equal(t, 14, len(generatedProps.Keys())) + assert.NotContains(t, "property1", generatedProps.Keys()) + assert.NotContains(t, "property2", generatedProps.Keys()) + assert.NotContains(t, "service1", generatedProps.Keys()) + assert.NotContains(t, "service2", generatedProps.Keys()) + assert.NotContains(t, "service3", generatedProps.Keys()) + assert.NotContains(t, "service4", generatedProps.Keys()) + assert.NotContains(t, "service5", generatedProps.Keys()) + assert.NotContains(t, "broker1", generatedProps.Keys()) + assert.NotContains(t, "broker2", generatedProps.Keys()) //org.kie.kogito.addons.discovery.kubernetes\:services.v1\/usecase1º/my-service1 below we use the unescaped vale because the properties.LoadString removes them. assertHasProperty(t, generatedProps, "org.kie.kogito.addons.discovery.kubernetes:services.v1/namespace1/my-service1", myService1Address) @@ -214,12 +217,12 @@ func Test_appPropertyHandler_WithServicesWithUserOverrides(t *testing.T) { assert.NoError(t, err) generatedProps, propsErr := properties.LoadString(props.WithUserProperties(userProperties).Build()) assert.NoError(t, propsErr) - assert.Equal(t, 13, len(generatedProps.Keys())) - assert.Equal(t, "value1", generatedProps.GetString("property1", "")) - assert.Equal(t, "value2", generatedProps.GetString("property2", "")) + assert.Equal(t, 11, len(generatedProps.Keys())) + assert.NotContains(t, "property1", generatedProps.Keys()) + assert.NotContains(t, "property2", generatedProps.Keys()) - //kogito.service.url takes the user provided value since it's a default mutable property. - assert.Equal(t, "http://myUrl.override.com", generatedProps.GetString("kogito.service.url", "")) + //kogito.service.url is a default immutable property. + assert.Equal(t, "http://greeting.default", generatedProps.GetString("kogito.service.url", "")) //quarkus.http.port remains with the default value since it's immutable. assert.Equal(t, "8080", generatedProps.GetString("quarkus.http.port", "")) assert.Equal(t, "0.0.0.0", generatedProps.GetString("quarkus.http.host", "")) @@ -239,7 +242,9 @@ func Test_appPropertyHandler_WithServicesWithUserOverrides(t *testing.T) { assert.NoError(t, err) generatedProps, propsErr = properties.LoadString(props.WithUserProperties(userProperties).Build()) assert.NoError(t, propsErr) - assert.Equal(t, 15, len(generatedProps.Keys())) + assert.Equal(t, 13, len(generatedProps.Keys())) + assert.NotContains(t, "property1", generatedProps.Keys()) + assert.NotContains(t, "property2", generatedProps.Keys()) assert.Equal(t, "http://"+platform.Name+"-"+constants.DataIndexServiceName+"."+platform.Namespace+"/definitions", generatedProps.GetString(constants.KogitoProcessDefinitionsEventsURL, "")) assert.Equal(t, "true", generatedProps.GetString(constants.KogitoProcessDefinitionsEventsEnabled, "")) assert.Equal(t, "http://"+platform.Name+"-"+constants.DataIndexServiceName+"."+platform.Namespace+"/processes", generatedProps.GetString(constants.KogitoProcessInstancesEventsURL, "")) @@ -256,7 +261,9 @@ func Test_appPropertyHandler_WithServicesWithUserOverrides(t *testing.T) { assert.NoError(t, err) generatedProps, propsErr = properties.LoadString(props.WithUserProperties(userProperties).Build()) assert.NoError(t, propsErr) - assert.Equal(t, 13, len(generatedProps.Keys())) + assert.Equal(t, 11, len(generatedProps.Keys())) + assert.NotContains(t, "property1", generatedProps.Keys()) + assert.NotContains(t, "property2", generatedProps.Keys()) assert.Equal(t, "", generatedProps.GetString(constants.KogitoProcessDefinitionsEventsURL, "")) assert.Equal(t, "false", generatedProps.GetString(constants.KogitoProcessDefinitionsEventsEnabled, "")) assert.Equal(t, "", generatedProps.GetString(constants.KogitoProcessInstancesEventsURL, "")) @@ -272,7 +279,9 @@ func Test_appPropertyHandler_WithServicesWithUserOverrides(t *testing.T) { assert.NoError(t, err) generatedProps, propsErr = properties.LoadString(props.WithUserProperties(userProperties).Build()) assert.NoError(t, propsErr) - assert.Equal(t, 13, len(generatedProps.Keys())) + assert.Equal(t, 11, len(generatedProps.Keys())) + assert.NotContains(t, "property1", generatedProps.Keys()) + assert.NotContains(t, "property2", generatedProps.Keys()) assert.Equal(t, "", generatedProps.GetString(constants.KogitoProcessDefinitionsEventsURL, "")) assert.Equal(t, "false", generatedProps.GetString(constants.KogitoProcessDefinitionsEventsEnabled, "")) assert.Equal(t, "", generatedProps.GetString(constants.KogitoProcessInstancesEventsURL, "")) diff --git a/controllers/profiles/dev/states_dev.go b/controllers/profiles/dev/states_dev.go index 34b445b7a..9c568814f 100644 --- a/controllers/profiles/dev/states_dev.go +++ b/controllers/profiles/dev/states_dev.go @@ -62,17 +62,18 @@ 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 } - flowDefCM, _, err := e.ensurers.definitionConfigMap.Ensure(ctx, workflow, ensureWorkflowDefConfigMapMutator(workflow)) - if err != nil { - return ctrl.Result{Requeue: false}, objs, err - } - objs = append(objs, flowDefCM) userPropsCM, _, err := e.ensurers.userPropsConfigMap.Ensure(ctx, workflow) if err != nil { return ctrl.Result{Requeue: false}, objs, err From 0bbd57b3f5b56c0919f6e1e5be980235b7998bc0 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Mon, 29 Jan 2024 21:45:36 +0100 Subject: [PATCH 06/14] workarond for failed discovery options --- controllers/profiles/common/properties/discovery.go | 1 + 1 file changed, 1 insertion(+) diff --git a/controllers/profiles/common/properties/discovery.go b/controllers/profiles/common/properties/discovery.go index a7663d419..19796ef92 100644 --- a/controllers/profiles/common/properties/discovery.go +++ b/controllers/profiles/common/properties/discovery.go @@ -99,6 +99,7 @@ func generateDiscoveryProperties(ctx context.Context, catalog discovery.ServiceC mpProperty := generateMicroprofileServiceCatalogProperty(plainUri) klog.V(log.I).Infof("Generating microprofile service catalog property %s=%s.", mpProperty, address) result.MustSet(mpProperty, address) + result.MustSet(k, "http://localhost:8080") } } } From 239c2e01858e653ac6fc532bc64e7f7fd40cc23a Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Mon, 29 Jan 2024 21:59:44 +0100 Subject: [PATCH 07/14] fixed broken unit tests --- .../common/properties/application_test.go | 17 +++++++++-------- .../common/properties/discovery_test.go | 8 +++++++- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/controllers/profiles/common/properties/application_test.go b/controllers/profiles/common/properties/application_test.go index d42f8a7fd..520b860ce 100644 --- a/controllers/profiles/common/properties/application_test.go +++ b/controllers/profiles/common/properties/application_test.go @@ -157,16 +157,17 @@ func Test_appPropertyHandler_WithUserPropertiesWithServiceDiscovery(t *testing.T Build()) generatedProps.DisableExpansion = true assert.NoError(t, propsErr) - assert.Equal(t, 14, len(generatedProps.Keys())) + assert.Equal(t, 21, len(generatedProps.Keys())) assert.NotContains(t, "property1", generatedProps.Keys()) assert.NotContains(t, "property2", generatedProps.Keys()) - assert.NotContains(t, "service1", generatedProps.Keys()) - assert.NotContains(t, "service2", generatedProps.Keys()) - assert.NotContains(t, "service3", generatedProps.Keys()) - assert.NotContains(t, "service4", generatedProps.Keys()) - assert.NotContains(t, "service5", generatedProps.Keys()) - assert.NotContains(t, "broker1", generatedProps.Keys()) - assert.NotContains(t, "broker2", generatedProps.Keys()) + defaultService := "http://localhost:8080" + assertHasProperty(t, generatedProps, "service1", defaultService) + assertHasProperty(t, generatedProps, "service2", defaultService) + assertHasProperty(t, generatedProps, "service3", defaultService) + assertHasProperty(t, generatedProps, "service4", defaultService) + assertHasProperty(t, generatedProps, "service5", defaultService) + assertHasProperty(t, generatedProps, "broker1", defaultService) + assertHasProperty(t, generatedProps, "broker2", defaultService) //org.kie.kogito.addons.discovery.kubernetes\:services.v1\/usecase1º/my-service1 below we use the unescaped vale because the properties.LoadString removes them. assertHasProperty(t, generatedProps, "org.kie.kogito.addons.discovery.kubernetes:services.v1/namespace1/my-service1", myService1Address) diff --git a/controllers/profiles/common/properties/discovery_test.go b/controllers/profiles/common/properties/discovery_test.go index 4555c6c38..5a79fb1a0 100644 --- a/controllers/profiles/common/properties/discovery_test.go +++ b/controllers/profiles/common/properties/discovery_test.go @@ -62,7 +62,13 @@ func Test_generateDiscoveryProperties(t *testing.T) { Spec: v1alpha08.SonataFlowSpec{Flow: workflow}, }) - assert.Equal(t, result.Len(), 5) + assert.Equal(t, 8, result.Len()) + defaultService := "http://localhost:8080" + assertHasProperty(t, result, "service1", defaultService) + assertHasProperty(t, result, "service2", defaultService) + assertHasProperty(t, result, "service3", defaultService) + assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/namespace1\\/my-service1", myService1Address) + assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/namespace1\\/my-service1", myService1Address) assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/namespace1\\/my-service1", myService1Address) assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/my-service2", myService2Address) assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/my-service3?port\\=http-port", myService3Address) From 821fd3eaea984f73181a658484ab818585435a81 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Tue, 30 Jan 2024 09:58:44 +0100 Subject: [PATCH 08/14] fixed mutator for user props --- controllers/profiles/common/mutate_visitors.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/controllers/profiles/common/mutate_visitors.go b/controllers/profiles/common/mutate_visitors.go index 38703cf74..28833227b 100644 --- a/controllers/profiles/common/mutate_visitors.go +++ b/controllers/profiles/common/mutate_visitors.go @@ -109,9 +109,6 @@ func UserPropertiesMutateVisitor(ctx context.Context, catalog discovery.ServiceC 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 - } managedProps := object.(*corev1.ConfigMap) managedProps.Labels = workflow.GetLabels() _, hasKey := managedProps.Data[workflowproj.GetManagedPropertiesFileName(workflow)] From 4ba85a7daab8753d2dcc2f97c0210f9651e07e59 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Tue, 30 Jan 2024 15:57:36 +0100 Subject: [PATCH 09/14] reviewed hashing function --- utils/kubernetes/deployment.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/utils/kubernetes/deployment.go b/utils/kubernetes/deployment.go index ec9b0aa79..676c49924 100644 --- a/utils/kubernetes/deployment.go +++ b/utils/kubernetes/deployment.go @@ -24,7 +24,6 @@ import ( "encoding/hex" "errors" "fmt" - "strings" "time" "github.com/apache/incubator-kie-kogito-serverless-operator/api/metadata" @@ -126,9 +125,7 @@ func AnnotateDeploymentConfigChecksum(workflow *operatorapi.SonataFlow, deployme if !ok { currentChecksum = "" } - newChecksum, err := configMapChecksum( - dataFromCM(userPropsCM, workflowproj.ApplicationPropertiesFileName), - dataFromCM(managedPropsCM, workflowproj.GetManagedPropertiesFileName(workflow))) + newChecksum, err := calculateHash(userPropsCM, managedPropsCM, workflow) if err != nil { return err } @@ -153,8 +150,9 @@ func dataFromCM(cm *v1.ConfigMap, key string) string { return data } -func configMapChecksum(props ...string) (string, error) { - aggregatedProps := strings.Join(props, ",") +func calculateHash(userPropsCM, managedPropsCM *v1.ConfigMap, workflow *operatorapi.SonataFlow) (string, error) { + aggregatedProps := fmt.Sprintf("%s,%s", dataFromCM(userPropsCM, workflowproj.ApplicationPropertiesFileName), + dataFromCM(managedPropsCM, workflowproj.GetManagedPropertiesFileName(workflow))) hash := sha256.New() _, err := hash.Write([]byte(aggregatedProps)) if err != nil { From 5941bf2b751893c393cead2fe7a2396ccefa8b50 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:26:58 +0100 Subject: [PATCH 10/14] Anticipating deactivation of broken e2e test --- test/e2e/platform_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/platform_test.go b/test/e2e/platform_test.go index 24115567a..a19c961c9 100644 --- a/test/e2e/platform_test.go +++ b/test/e2e/platform_test.go @@ -117,7 +117,7 @@ var _ = Describe("Validate the persistence", Ordered, func() { } }, Entry("with both Job Service and Data Index and ephemeral persistence and the workflow in a dev profile", test.GetSonataFlowE2EPlatformServicesDirectory(), dev, ephemeral), - Entry("with both Job Service and Data Index and ephemeral persistence and the workflow in a production profile", test.GetSonataFlowE2EPlatformServicesDirectory(), production, ephemeral), + XEntry("with both Job Service and Data Index and ephemeral persistence and the workflow in a production profile", test.GetSonataFlowE2EPlatformServicesDirectory(), production, ephemeral), Entry("with both Job Service and Data Index and postgreSQL persistence and the workflow in a dev profile", test.GetSonataFlowE2EPlatformServicesDirectory(), dev, postgreSQL), Entry("with both Job Service and Data Index and postgreSQL persistence and the workflow in a production profile", test.GetSonataFlowE2EPlatformServicesDirectory(), production, postgreSQL), ) From 63ed134719af9fbdcbfcf5ff6a4092851d94ce8d Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Wed, 31 Jan 2024 16:23:49 +0100 Subject: [PATCH 11/14] integrating comments: removing unneeded comment --- workflowproj/workflowproj.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/workflowproj/workflowproj.go b/workflowproj/workflowproj.go index 77118d259..afabd7e5c 100644 --- a/workflowproj/workflowproj.go +++ b/workflowproj/workflowproj.go @@ -38,8 +38,6 @@ import ( operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" ) -// TODO: should we delete this whole file? seems used only the the associated test functions - var _ WorkflowProjectHandler = &workflowProjectHandler{} // defaultResourcePath is the default resource path to add to the generated ConfigMaps From 612766f39c149c0bc984c9d653c7ebe508431d09 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Fri, 2 Feb 2024 11:13:53 +0100 Subject: [PATCH 12/14] adding discovered value to properties whose value mathes the service discovery pattern --- .../common/properties/application_test.go | 15 +++++++-------- .../profiles/common/properties/discovery.go | 3 ++- .../profiles/common/properties/discovery_test.go | 7 +++---- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/controllers/profiles/common/properties/application_test.go b/controllers/profiles/common/properties/application_test.go index f49503f5f..ca0de4537 100644 --- a/controllers/profiles/common/properties/application_test.go +++ b/controllers/profiles/common/properties/application_test.go @@ -160,14 +160,13 @@ func Test_appPropertyHandler_WithUserPropertiesWithServiceDiscovery(t *testing.T assert.Equal(t, 21, len(generatedProps.Keys())) assert.NotContains(t, "property1", generatedProps.Keys()) assert.NotContains(t, "property2", generatedProps.Keys()) - defaultService := "http://localhost:8080" - assertHasProperty(t, generatedProps, "service1", defaultService) - assertHasProperty(t, generatedProps, "service2", defaultService) - assertHasProperty(t, generatedProps, "service3", defaultService) - assertHasProperty(t, generatedProps, "service4", defaultService) - assertHasProperty(t, generatedProps, "service5", defaultService) - assertHasProperty(t, generatedProps, "broker1", defaultService) - assertHasProperty(t, generatedProps, "broker2", defaultService) + assertHasProperty(t, generatedProps, "service1", myService1Address) + assertHasProperty(t, generatedProps, "service2", myService2Address) + assertHasProperty(t, generatedProps, "service3", myKnService1Address) + assertHasProperty(t, generatedProps, "service4", myKnService2Address) + assertHasProperty(t, generatedProps, "service5", myKnService3Address) + assertHasProperty(t, generatedProps, "broker1", myKnBroker1Address) + assertHasProperty(t, generatedProps, "broker2", myKnBroker2Address) //org.kie.kogito.addons.discovery.kubernetes\:services.v1\/usecase1º/my-service1 below we use the unescaped vale because the properties.LoadString removes them. assertHasProperty(t, generatedProps, "org.kie.kogito.addons.discovery.kubernetes:services.v1/namespace1/my-service1", myService1Address) diff --git a/controllers/profiles/common/properties/discovery.go b/controllers/profiles/common/properties/discovery.go index 19796ef92..6d2ac71bb 100644 --- a/controllers/profiles/common/properties/discovery.go +++ b/controllers/profiles/common/properties/discovery.go @@ -99,7 +99,8 @@ func generateDiscoveryProperties(ctx context.Context, catalog discovery.ServiceC mpProperty := generateMicroprofileServiceCatalogProperty(plainUri) klog.V(log.I).Infof("Generating microprofile service catalog property %s=%s.", mpProperty, address) result.MustSet(mpProperty, address) - result.MustSet(k, "http://localhost:8080") + klog.V(log.I).Infof("Overriding the discoverable value as the managed property %s=%s.", k, address) + result.MustSet(k, address) } } } diff --git a/controllers/profiles/common/properties/discovery_test.go b/controllers/profiles/common/properties/discovery_test.go index 5a79fb1a0..dd502d184 100644 --- a/controllers/profiles/common/properties/discovery_test.go +++ b/controllers/profiles/common/properties/discovery_test.go @@ -63,10 +63,9 @@ func Test_generateDiscoveryProperties(t *testing.T) { }) assert.Equal(t, 8, result.Len()) - defaultService := "http://localhost:8080" - assertHasProperty(t, result, "service1", defaultService) - assertHasProperty(t, result, "service2", defaultService) - assertHasProperty(t, result, "service3", defaultService) + assertHasProperty(t, result, "service1", myService1Address) + assertHasProperty(t, result, "service2", myService2Address) + assertHasProperty(t, result, "service3", myService3Address) assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/namespace1\\/my-service1", myService1Address) assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/namespace1\\/my-service1", myService1Address) assertHasProperty(t, result, "org.kie.kogito.addons.discovery.kubernetes\\:services.v1\\/namespace1\\/my-service1", myService1Address) From 4a743bd3f10a16035070da205d9b038699d8b55d Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Fri, 2 Feb 2024 18:05:12 +0100 Subject: [PATCH 13/14] removed unused package common_test --- controllers/profiles/common/test/common.go | 84 ---------------------- 1 file changed, 84 deletions(-) delete mode 100644 controllers/profiles/common/test/common.go diff --git a/controllers/profiles/common/test/common.go b/controllers/profiles/common/test/common.go deleted file mode 100644 index a4f2783d1..000000000 --- a/controllers/profiles/common/test/common.go +++ /dev/null @@ -1,84 +0,0 @@ -/* - * 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 ( - 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 -} From c842feba2c9402a244da50f0b871eb68d49067a7 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Fri, 2 Feb 2024 18:14:00 +0100 Subject: [PATCH 14/14] Renamed managed props visitor and reviewed description of NewAppPropertyHandler --- controllers/profiles/common/mutate_visitors.go | 2 +- controllers/profiles/common/object_creators_test.go | 4 ++-- controllers/profiles/common/properties/application.go | 9 +++++---- controllers/profiles/dev/states_dev.go | 2 +- controllers/profiles/prod/deployment_handler.go | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/controllers/profiles/common/mutate_visitors.go b/controllers/profiles/common/mutate_visitors.go index 28833227b..8ccbcab35 100644 --- a/controllers/profiles/common/mutate_visitors.go +++ b/controllers/profiles/common/mutate_visitors.go @@ -105,7 +105,7 @@ func ServiceMutateVisitor(workflow *operatorapi.SonataFlow) MutateVisitor { } } -func UserPropertiesMutateVisitor(ctx context.Context, catalog discovery.ServiceCatalog, +func ManagedPropertiesMutateVisitor(ctx context.Context, catalog discovery.ServiceCatalog, workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform, userProps *corev1.ConfigMap) MutateVisitor { return func(object client.Object) controllerutil.MutateFn { return func() error { diff --git a/controllers/profiles/common/object_creators_test.go b/controllers/profiles/common/object_creators_test.go index 1d2b26c4e..ae252248e 100644 --- a/controllers/profiles/common/object_creators_test.go +++ b/controllers/profiles/common/object_creators_test.go @@ -47,7 +47,7 @@ func Test_ensureWorkflowPropertiesConfigMapMutator(t *testing.T) { userProps, _ := UserPropsConfigMapCreator(workflow) userPropsCM := userProps.(*corev1.ConfigMap) - visitor := UserPropertiesMutateVisitor(context.TODO(), nil, workflow, nil, userPropsCM) + visitor := ManagedPropertiesMutateVisitor(context.TODO(), nil, workflow, nil, userPropsCM) mutateFn := visitor(managedProps) assert.NoError(t, mutateFn()) @@ -82,7 +82,7 @@ func Test_ensureWorkflowPropertiesConfigMapMutator_DollarReplacement(t *testing. 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) + mutateVisitorFn := ManagedPropertiesMutateVisitor(context.TODO(), nil, workflow, nil, userPropsCM) err := mutateVisitorFn(managedPropsCM)() assert.NoError(t, err) diff --git a/controllers/profiles/common/properties/application.go b/controllers/profiles/common/properties/application.go index ce6fb89a5..414891405 100644 --- a/controllers/profiles/common/properties/application.go +++ b/controllers/profiles/common/properties/application.go @@ -133,13 +133,14 @@ func (a *appPropertyHandler) addDefaultMutableProperty(name string, value string } // NewAppPropertyHandler creates a property handler for a given workflow to execute in the provided platform. -// This handler is intended to build the application properties required by the workflow to execute properly, note that -// the produced properties might vary depending on the platfom, for example, if the job service managed by the platform +// This handler is intended to build the managed application properties required by the workflow to execute properly together with +// the user properties defined in the user-managed ConfigMap. +// Note that the produced properties might vary depending on the platfom, for example, if the job service managed by the platform // a particular set of properties will be added, etc. // By default, the following properties are incorporated: // The set of immutable properties provided by the operator. (user can never change) -// The set of defaultMutableProperties that are provided by the operator, and that the user might overwrite if it changes -// the workflow ConfigMap. This set includes for example the required properties to connect with the data index and the +// The set of defaultMutableProperties that are provided by the operator, and that the user cannot overwrite even if it changes +// the user-managed ConfigMap. This set includes for example the required properties to connect with the data index and the // job service when any of these services are managed by the platform. func NewAppPropertyHandler(workflow *operatorapi.SonataFlow, platform *operatorapi.SonataFlowPlatform) (AppPropertyHandler, error) { handler := &appPropertyHandler{ diff --git a/controllers/profiles/dev/states_dev.go b/controllers/profiles/dev/states_dev.go index 9c568814f..f98e03eb2 100644 --- a/controllers/profiles/dev/states_dev.go +++ b/controllers/profiles/dev/states_dev.go @@ -78,7 +78,7 @@ func (e *ensureRunningWorkflowState) Do(ctx context.Context, workflow *operatora 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))) + managedPropsCM, _, err := e.ensurers.managedPropsConfigMap.Ensure(ctx, workflow, pl, common.ManagedPropertiesMutateVisitor(ctx, e.StateSupport.Catalog, workflow, pl, userPropsCM.(*corev1.ConfigMap))) if err != nil { return ctrl.Result{Requeue: false}, objs, err } diff --git a/controllers/profiles/prod/deployment_handler.go b/controllers/profiles/prod/deployment_handler.go index e075543f0..3b693429d 100644 --- a/controllers/profiles/prod/deployment_handler.go +++ b/controllers/profiles/prod/deployment_handler.go @@ -56,7 +56,7 @@ func (d *deploymentReconciler) reconcileWithBuiltImage(ctx context.Context, work return ctrl.Result{}, nil, err } managedPropsCM, _, err := d.ensurers.managedPropsConfigMap.Ensure(ctx, workflow, pl, - common.UserPropertiesMutateVisitor(ctx, d.StateSupport.Catalog, workflow, pl, userPropsCM.(*v1.ConfigMap))) + common.ManagedPropertiesMutateVisitor(ctx, d.StateSupport.Catalog, workflow, pl, userPropsCM.(*v1.ConfigMap))) if err != nil { workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.ExternalResourcesNotFoundReason, "Unable to retrieve the managed properties config map") _, err = d.PerformStatusUpdate(ctx, workflow)