Merge pull request #123 from BuddhiWathsala/master

Refactor the code and address minor fixes
siddhi-io · Dec 13, 2019 · f46165a · f46165a
2 parents f35e80d + 5a2fe20
commit f46165a
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 53 deletions.
diff --git a/pkg/controller/siddhiprocess/artifact/artifacts.go b/pkg/controller/siddhiprocess/artifact/artifacts.go
@@ -21,6 +21,7 @@ package artifacts
 import (
 	"context"
 	"errors"
+	"path"
 	"reflect"
 	"strconv"
 	"strings"
@@ -86,8 +87,7 @@ func (k *KubeClient) CreateOrUpdateIngress(
 
 	var ingressPaths []extensionsv1beta1.HTTPIngressPath
 	for _, port := range containerPorts {
-		path := "/" + strings.ToLower(serviceName) +
-			"/" + strconv.Itoa(int(port.ContainerPort)) + "(/|$)(.*)"
+		path := path.Join("/", strings.ToLower(serviceName), (strconv.Itoa(int(port.ContainerPort)) + "(/|$)(.*)"))
 		ingressPath := extensionsv1beta1.HTTPIngressPath{
 			Path: path,
 			Backend: extensionsv1beta1.IngressBackend{
@@ -615,8 +615,7 @@ func IngressMutateFunc(
 		ingress := obj.(*extensionsv1beta1.Ingress)
 		var ingressPaths []extensionsv1beta1.HTTPIngressPath
 		for _, port := range containerPorts {
-			path := "/" + strings.ToLower(serviceName) +
-				"/" + strconv.Itoa(int(port.ContainerPort)) + "(/|$)(.*)"
+			path := path.Join("/", strings.ToLower(serviceName), (strconv.Itoa(int(port.ContainerPort)) + "(/|$)(.*)"))
 			ingressPath := extensionsv1beta1.HTTPIngressPath{
 				Path: path,
 				Backend: extensionsv1beta1.IngressBackend{

diff --git a/pkg/controller/siddhiprocess/artifact/artifacts_test.go b/pkg/controller/siddhiprocess/artifact/artifacts_test.go
@@ -20,7 +20,7 @@ package artifacts
 
 import (
 	"context"
-	"strconv"
+	"fmt"
 	"testing"
 
 	natsv1alpha2 "github.com/siddhi-io/siddhi-operator/pkg/apis/nats/v1alpha2"
@@ -31,15 +31,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"k8s.io/apimachinery/pkg/api/resource"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 var kubeClient = KubeClient{
 	Scheme: scheme.Scheme,
-	Client: fake.NewFakeClient([]runtime.Object{}...),
+	Client: fake.NewFakeClient(),
 }
 
 // apps to run tests
@@ -115,7 +114,7 @@ func TestCreateOrUpdateIngress(t *testing.T) {
 		t.Error(err)
 	}
 	if len(ingress.Spec.Rules[0].HTTP.Paths) != 2 {
-		t.Error("Ingress update error. Expected entries 2, but found " + strconv.Itoa(len(ingress.Spec.Rules)))
+		t.Error(fmt.Sprint("Ingress update error. Expected entries 2, but found", len(ingress.Spec.Rules)))
 	}
 }
 

diff --git a/pkg/controller/siddhiprocess/deploymanager/application.go b/pkg/controller/siddhiprocess/deploymanager/application.go
@@ -115,12 +115,11 @@ func (d *DeployManager) Deploy() (operationResult controllerutil.OperationResult
 				d.SiddhiProcess,
 			)
 			if err != nil {
-				return
+				return operationResult, err
 			}
-			mountPath := ""
-			mountPath, err = populateMountPath(d.SiddhiProcess, d.Image.Home, d.Image.Profile)
+			mountPath, err := populateMountPath(d.SiddhiProcess, d.Image.Home, d.Image.Profile)
 			if err != nil {
-				return
+				return operationResult, err
 			}
 			volume, volumeMount := createPVCVolumes(pvcName, mountPath)
 			volumes = append(volumes, volume)
@@ -136,7 +135,7 @@ func (d *DeployManager) Deploy() (operationResult controllerutil.OperationResult
 		}
 		err = d.KubeClient.CreateOrUpdateCM(deployYAMLCMName, d.SiddhiProcess.Namespace, data, d.SiddhiProcess)
 		if err != nil {
-			return
+			return operationResult, err
 		}
 		mountPath := d.Image.Home + DepConfMountPath
 		volume, volumeMount := createCMVolumes(deployYAMLCMName, mountPath)
@@ -150,7 +149,7 @@ func (d *DeployManager) Deploy() (operationResult controllerutil.OperationResult
 		}
 		err = d.KubeClient.CreateOrUpdateCM(deployYAMLCMName, d.SiddhiProcess.Namespace, data, d.SiddhiProcess)
 		if err != nil {
-			return
+			return operationResult, err
 		}
 		mountPath := d.Image.Home + DepConfMountPath
 		volume, volumeMount := createCMVolumes(deployYAMLCMName, mountPath)
@@ -184,7 +183,7 @@ func (d *DeployManager) Deploy() (operationResult controllerutil.OperationResult
 		}
 		err = d.KubeClient.CreateOrUpdateCM(appsCMName, d.SiddhiProcess.Namespace, appsMap, d.SiddhiProcess)
 		if err != nil {
-			return
+			return operationResult, err
 		}
 		appsPath := d.Image.Home + SiddhiFilesDir
 		volume, volumeMount := createCMVolumes(appsCMName, appsPath)
@@ -205,7 +204,7 @@ func (d *DeployManager) Deploy() (operationResult controllerutil.OperationResult
 		ContainerName,
 		[]string{Shell},
 		[]string{
-			d.Image.Home + SiddhiBin + "/" + d.Image.Profile + ".sh",
+			filepath.Join(d.Image.Home, SiddhiBin, (d.Image.Profile + ".sh")),
 			appParameter,
 			configParameter,
 		},
@@ -219,15 +218,14 @@ func (d *DeployManager) Deploy() (operationResult controllerutil.OperationResult
 		depStrategy,
 		d.SiddhiProcess,
 	)
-	return
+	return operationResult, err
 }
 
 // createLocalObjectReference creates a local object reference secret to download docker images from private registries.
 func createLocalObjectReference(secret string) (localObjectRef corev1.LocalObjectReference) {
-	localObjectRef = corev1.LocalObjectReference{
+	return corev1.LocalObjectReference{
 		Name: secret,
 	}
-	return
 }
 
 // populateMountPath reads the runner configs given by the user.
@@ -240,11 +238,11 @@ func populateMountPath(sp *siddhiv1alpha2.SiddhiProcess, home string, profile st
 	if err != nil {
 		return
 	}
-	mountPath = home + WSO2Dir + "/" + profile + "/" + FilePersistentDir
+	mountPath = filepath.Join(home, WSO2Dir, profile, FilePersistentDir)
 	if config.StatePersistence.SPConfig.Location != "" && filepath.IsAbs(config.StatePersistence.SPConfig.Location) {
 		mountPath = config.StatePersistence.SPConfig.Location
 	} else if config.StatePersistence.SPConfig.Location != "" {
-		mountPath = home + WSO2Dir + "/" + profile + "/" + config.StatePersistence.SPConfig.Location
+		mountPath = filepath.Join(home, WSO2Dir, profile, config.StatePersistence.SPConfig.Location)
 	}
 	return
 }

diff --git a/pkg/controller/siddhiprocess/parser/parser.go b/pkg/controller/siddhiprocess/parser/parser.go
@@ -135,6 +135,7 @@ func (p *Parser) Parse() (applications []deploymanager.Application, err error) {
 }
 
 func (p *Parser) deploy() (err error) {
+	generatedParserName := p.Name + ParserExtension
 	containerPorts := []corev1.ContainerPort{
 		corev1.ContainerPort{
 			Name:          ParserName,
@@ -143,7 +144,7 @@ func (p *Parser) deploy() (err error) {
 		},
 	}
 	application := deploymanager.Application{
-		Name:           p.Name + ParserExtension,
+		Name:           generatedParserName,
 		ContainerPorts: containerPorts,
 		ServiceEnabled: true,
 		Replicas:       ParserReplicas,
@@ -160,7 +161,7 @@ func (p *Parser) deploy() (err error) {
 		return
 	}
 	_, err = p.KubeClient.CreateOrUpdateService(
-		p.Name+ParserExtension,
+		generatedParserName,
 		p.SiddhiProcess.Namespace,
 		containerPorts,
 		deployManeger.Labels,
@@ -170,7 +171,7 @@ func (p *Parser) deploy() (err error) {
 		return
 	}
 
-	url := ParserHTTP + p.Name + ParserExtension + "." + p.SiddhiProcess.Namespace + ParserHealth
+	url := ParserHTTP + generatedParserName + "." + p.SiddhiProcess.Namespace + ParserHealth
 	p.Logger.Info("Waiting for parser", "deployment", p.Name)
 	err = waitForParser(url)
 	if err != nil {

diff --git a/pkg/controller/siddhiprocess/siddhicontroller/siddhicontroller.go b/pkg/controller/siddhiprocess/siddhicontroller/siddhicontroller.go
@@ -182,8 +182,8 @@ func (sc *SiddhiController) CreateArtifacts(applications []deploymanager.Applica
 
 	sc.UpdatePartialAppStatus(applications)
 	for _, application := range applications {
-		if (eventType == controllerutil.OperationResultCreated) ||
-			(eventType == controllerutil.OperationResultUpdated) {
+		if eventType == controllerutil.OperationResultCreated ||
+			eventType == controllerutil.OperationResultUpdated {
 			deployManeger := deploymanager.DeployManager{
 				Application:   application,
 				KubeClient:    sc.KubeClient,
@@ -196,14 +196,14 @@ func (sc *SiddhiController) CreateArtifacts(applications []deploymanager.Applica
 				sc.UpdateErrorStatus("AppDeploymentError", err)
 				continue
 			}
-			if (eventType == controllerutil.OperationResultCreated) &&
-				(operationResult == controllerutil.OperationResultCreated) {
+			if eventType == controllerutil.OperationResultCreated &&
+				operationResult == controllerutil.OperationResultCreated {
 				sc.UpdateRunningStatus(
 					"DeploymentCreated",
 					(application.Name + " deployment created successfully"),
 				)
-			} else if (eventType == controllerutil.OperationResultUpdated) &&
-				(operationResult == controllerutil.OperationResultUpdated) {
+			} else if eventType == controllerutil.OperationResultUpdated &&
+				operationResult == controllerutil.OperationResultUpdated {
 				sc.UpdateRunningStatus(
 					"DeploymentUpdated",
 					(application.Name + " deployment updated successfully"),
@@ -222,14 +222,14 @@ func (sc *SiddhiController) CreateArtifacts(applications []deploymanager.Applica
 					sc.UpdateErrorStatus("ServiceCreationError", err)
 					continue
 				}
-				if (eventType == controllerutil.OperationResultCreated) &&
-					(operationResult == controllerutil.OperationResultCreated) {
+				if eventType == controllerutil.OperationResultCreated &&
+					operationResult == controllerutil.OperationResultCreated {
 					sc.UpdateRunningStatus(
 						"ServiceCreated",
 						(application.Name + " service created successfully"),
 					)
-				} else if (eventType == controllerutil.OperationResultUpdated) &&
-					(operationResult == controllerutil.OperationResultUpdated) {
+				} else if eventType == controllerutil.OperationResultUpdated &&
+					operationResult == controllerutil.OperationResultUpdated {
 					sc.UpdateRunningStatus(
 						"ServiceUpdated",
 						(application.Name + " service updated successfully"),
@@ -247,20 +247,19 @@ func (sc *SiddhiController) CreateArtifacts(applications []deploymanager.Applica
 						sc.UpdateErrorStatus("IngressCreationError", err)
 						continue
 					}
-					if (eventType == controllerutil.OperationResultCreated) &&
-						(operationResult == controllerutil.OperationResultCreated) {
+					if eventType == controllerutil.OperationResultCreated &&
+						operationResult == controllerutil.OperationResultCreated {
 						sc.Logger.Info("Ingress created", "Ingress.Name", artifact.IngressName)
-					} else if (eventType == controllerutil.OperationResultUpdated) &&
-						(operationResult == controllerutil.OperationResultUpdated) {
+					} else if eventType == controllerutil.OperationResultUpdated &&
+						operationResult == controllerutil.OperationResultUpdated {
 						sc.Logger.Info("Ingress changed", "Ingress.Name", artifact.IngressName)
 					}
 				}
 			}
 		}
 	}
 	sc.SyncVersion()
-	if (eventType == controllerutil.OperationResultCreated) ||
-		(eventType == controllerutil.OperationResultUpdated) {
+	if eventType == controllerutil.OperationResultCreated || eventType == controllerutil.OperationResultUpdated {
 		sc.UpdateReadyDeployments(0, needDep)
 	}
 	return
@@ -433,32 +432,32 @@ func (sc *SiddhiController) UpdateDefaultConfigs() {
 		configMap,
 	)
 	if err == nil {
-		if configMap.Data["siddhiHome"] != "" {
-			sc.Image.Home = configMap.Data["siddhiHome"]
+		if v, ok := configMap.Data["siddhiHome"]; ok {
+			sc.Image.Home = v
 		}
 
-		if configMap.Data["siddhiImage"] != "" {
-			sc.Image.Name = configMap.Data["siddhiImage"]
+		if v, ok := configMap.Data["siddhiImage"]; ok {
+			sc.Image.Name = v
 		}
 
-		if configMap.Data["siddhiImageSecret"] != "" {
-			sc.Image.Secret = configMap.Data["siddhiImageSecret"]
+		if v, ok := configMap.Data["siddhiImageSecret"]; ok {
+			sc.Image.Secret = v
 		}
 
-		if configMap.Data["siddhiProfile"] != "" {
-			sc.Image.Profile = configMap.Data["siddhiProfile"]
+		if v, ok := configMap.Data["siddhiProfile"]; ok {
+			sc.Image.Profile = v
 		}
 
-		if configMap.Data["autoIngressCreation"] != "" {
-			if configMap.Data["autoIngressCreation"] == "true" {
+		if v, ok := configMap.Data["autoIngressCreation"]; ok {
+			if v == "true" {
 				sc.AutoCreateIngress = true
 			} else {
 				sc.AutoCreateIngress = false
 			}
 		}
 
-		if configMap.Data["ingressTLS"] != "" {
-			sc.TLS = configMap.Data["ingressTLS"]
+		if v, ok := configMap.Data["ingressTLS"]; ok {
+			sc.TLS = v
 		}
 
 	}
@@ -489,7 +488,7 @@ func (sc *SiddhiController) SetDefaultPendingState() {
 		sc.SiddhiProcess.Status.CurrentVersion,
 		sc.SiddhiProcess.Status.PreviousVersion,
 	)
-	if (eventType == controllerutil.OperationResultCreated) && (sc.SiddhiProcess.Status.Status == "") {
+	if eventType == controllerutil.OperationResultCreated && sc.SiddhiProcess.Status.Status == "" {
 		sc.UpdatePendingStatus()
 	}
 }