ricardozanini commented on code in PR #345: URL: https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/345#discussion_r1455753166
########## controllers/clusterplatform/initialize.go: ########## @@ -0,0 +1,118 @@ +/* + * 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 clusterplatform + +import ( + "context" + "fmt" + + "github.com/apache/incubator-kie-kogito-serverless-operator/api" + "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" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/klog/v2" +) + +// NewInitializeAction returns an action that initializes the platform configuration when not provided by the user. +func NewInitializeAction() Action { + return &initializeAction{} +} + +type initializeAction struct { + baseAction +} + +func (action *initializeAction) Name() string { + return "initialize" +} + +func (action *initializeAction) CanHandle(ctx context.Context, cPlatform *operatorapi.SonataFlowClusterPlatform) bool { + return !cPlatform.Status.IsDuplicated() || allDuplicatedClusterPlatforms(ctx, action.client) +} + +func (action *initializeAction) Handle(ctx context.Context, cPlatform *operatorapi.SonataFlowClusterPlatform) error { + duplicate, err := action.isPrimaryDuplicate(ctx, cPlatform) + if err != nil { + return err + } + if duplicate { + // another cluster platform already present + if !cPlatform.Status.IsDuplicated() { + cPlatform.Status.Manager().MarkFalse(api.SucceedConditionType, operatorapi.PlatformDuplicatedReason, "") + } + return nil + } + cPlatform.Status.Version = metadata.SpecVersion + platformRef := cPlatform.Spec.PlatformRef + + // Check referenced platform status + platform := &operatorapi.SonataFlowPlatform{} + err = action.client.Get(ctx, types.NamespacedName{Namespace: platformRef.Namespace, Name: platformRef.Name}, platform) + if err != nil { + if k8serrors.IsNotFound(err) { + klog.V(log.D).InfoS("%s platform does not exist in %s namespace.", platformRef.Name, platformRef.Namespace) + cPlatform.Status.Manager().MarkFalse(api.SucceedConditionType, operatorapi.PlatformNotFoundReason, + fmt.Sprintf("%s platform does not exist in %s namespace.", platformRef.Name, platformRef.Namespace)) Review Comment: If the platform is not found, how do we update it in the reconciliation cycle? Maybe we should skip setting the status? ########## controllers/platform/services/services.go: ########## @@ -67,6 +67,22 @@ type Platform interface { GenerateWorkflowProperties() (*properties.Properties, error) // GenerateServiceProperties returns a property object that contains the application properties required by the service deployment GenerateServiceProperties() (*properties.Properties, error) + + // ServiceSetInSpec returns true if the service is set in the spec. + ServiceSetInSpec() bool + // ServiceEnabledInSpec returns true if the service is enabled in the spec. + ServiceEnabledInSpec() bool + // ServiceBaseUrl returns the base url of the service + ServiceBaseUrl() string + // GetServiceBaseUrl returns the base url of the service, based on whether using local or cluster-scoped service. + GetServiceBaseUrl() string + // GetServiceUrl returns the service url, based on whether using local or cluster-scoped service. + GetServiceUrl() string + // ServiceEnabled returns true if the service is enabled in either the spec or the status.clusterPlatformRef. + ServiceEnabled() bool Review Comment: Can we change this naming to `Is...` or `Get...`? (add the prefix). ########## controllers/profiles/common/app_properties_test.go.removed: ########## @@ -127,7 +127,7 @@ func Test_appPropertyHandler_WithServicesWithUserOverrides(t *testing.T) { platform := test.GetBasePlatform() platform.Namespace = ns platform.Spec = v1alpha08.SonataFlowPlatformSpec{ - Services: v1alpha08.ServicesPlatformSpec{ + Services: &v1alpha08.ServicesPlatformSpec{ Review Comment: This file should be deleted, it's a leftover. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
