Add code review changes

tharindu1st · Dec 4, 2019 · 094f396 · 094f396
1 parent b9e48d9
commit 094f396
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 53 deletions.
diff --git a/deployment/src/main/resources/kubedeployment_editor.groovy b/deployment/src/main/resources/kubedeployment_editor.groovy
@@ -22,10 +22,17 @@
 @Grapes(
         @Grab(group='org.json', module='json', version='20180813')
 )
+
 import org.yaml.snakeyaml.Yaml
 import org.json.JSONObject
 import org.json.JSONArray
 
+/**
+ * This function formats the Elastic Search Endpoint to support logstash by filtering out any https endpoints
+ *
+ * @param esEndpoint
+ * @return
+ */
 def static formatESURL (String esEndpoint) {
     if(esEndpoint.startsWith("https://")){
         return (esEndpoint.split("https://")[1]).split(":")[0]
@@ -34,13 +41,13 @@ def static formatESURL (String esEndpoint) {
     }
     return "error"
 }
+
 /**
- * Formats the user provided inputs to a an absolute File Path that can be used by the Side car
+ * Formats the user provided inputs to a an absolute File Path that can be used by the sidecar
  *
- * @param logLocation JSONArray containing the log locations
+ * @param logLocation String containing the log locations
  * @return
  */
-
 def static formatFilePaths(String logLocation){
 
     String absoluteFilePath
@@ -73,14 +80,13 @@ def static deriveConfFile(JSONObject logOptions){
  * the deployment
  *
  * @param logPathDetailsYamlLoc - Path to yaml file which is to store Log Path details
- * @param paramsJSONFilePath - File path to the json file containing input Parameters
- * @param depType - Type of the deployment ( Helm or K8S )
+ * @param depPropPath- File path to the .properties file containing input Parameters
+ * @param depType - Type of the deployment (Helm or K8S)
  *
  */
-def confLogCapabilities(String logPathDetailsYamlLoc, String depPropPath, String depType ){
+def static confLogCapabilities(String logPathDetailsYamlLoc, String depPropPath, String depType ){
+    String logRequirementDetails = "false"
     try{
-        // Read json file
-
         InputStream depPropsFileReader = new FileInputStream(depPropPath)
         Properties depProps = new Properties()
         depProps.load(depPropsFileReader)
@@ -94,10 +100,10 @@ def confLogCapabilities(String logPathDetailsYamlLoc, String depPropPath, String
 
         String logRequirement = logOptions.getString("logRequirement")
 
-        if (logRequirement == "Sidecar_Required") {
+        if (logRequirement.equalsIgnoreCase("Sidecar_Required") ) {
             // If sidecar is required create logPathDetails.yaml file with all information
             JSONArray logLocations = logOptions.getJSONArray("logFileLocations")
-            Yaml yaml = new Yaml()
+            Yaml logpathDetailsYaml = new Yaml()
 
             FileWriter logPathFileWriter = new FileWriter(logPathDetailsYamlLoc)
 
@@ -111,15 +117,14 @@ def confLogCapabilities(String logPathDetailsYamlLoc, String depPropPath, String
                             logLocation.getString("containerName") , "path" : absoluteFilePath]
                     logPathConf.add( entry )
                 }
-                String logConfFile = deriveConfFile(logOptions)
                 Map logConfiguration = ["onlyVars":false, "logPaths": logPathConf]
-                yaml.dump(logConfiguration,logPathFileWriter)
-                println("SidecarReq ".concat(logConfFile))
+                logpathDetailsYaml.dump(logConfiguration,logPathFileWriter)
+                logRequirementDetails = "SidecarReq default.conf"
             } else {
-                println("False")
+                logRequirementDetails = "False"
             }
             logPathFileWriter.close()
-        } else if ( logRequirement == "log_endPoints_Required" ) {
+        } else if ( logRequirement.equalsIgnoreCase("log_endpoints_required")  ) {
             if (depType == "helm" ) {
                 // If only ES endpoint is required access values.yaml file and edit the appropriate value
                 String valYamlFormat = logOptions.getString("valuesYamlLocation")
@@ -130,65 +135,63 @@ def confLogCapabilities(String logPathDetailsYamlLoc, String depPropPath, String
 
                 JSONArray replaceableValues = logOptions.getJSONArray("replaceableVals")
                 InputStream valuesYamlInputStream = new FileInputStream(valuesYamlLoc)
-
-                Yaml yaml = new Yaml()
-                Map valuesYaml = yaml.load(valuesYamlInputStream)
+                Yaml logpathDetailsYaml = new Yaml()
+                Map valuesYaml = logpathDetailsYaml.load(valuesYamlInputStream)
 
                 String elasticsearchURL = formatESURL(elasticsearchEndPoint)
                 String elasticsearchPort = elasticsearchEndPoint.split(":")[2]
-
-                for ( int j=0 ; j<replaceableValues.length() ; j++  ){
+                for (int j = 0; j < replaceableValues.length(); j++){
                     JSONObject replaceableObj = replaceableValues.getJSONObject(j)
                     Map editedValuesMap = valuesYaml
                     String replaceableObjLoc = replaceableObj.getString("location")
                     List<String> pathToRepObj = replaceableObjLoc.split("&")
                     String key
-                    for (int i = 0 ; i < pathToRepObj.size() -1 ; i++ ) {
-                        key = pathToRepObj[i]
+                    for (int i = 0; i < pathToRepObj.size()-1; i++) {
+                        key = pathToRepObj.get(i)
                         editedValuesMap = (Map) editedValuesMap[key]
                     }
-                    // add more ifs for other vars
+                    // add more if blocks for other variables
                     if (replaceableObj.getString("type") == "elasticsearchEndPoint" && elasticsearchURL != "error") {
-                        editedValuesMap[pathToRepObj[pathToRepObj.size()-1]] = elasticsearchURL
+                        editedValuesMap[pathToRepObj.get(pathToRepObj.size()-1)] = elasticsearchURL
                     } else if (replaceableObj.getString("type") == "elasticsearchPort" && elasticsearchURL != "error") {
-                        editedValuesMap[pathToRepObj[pathToRepObj.size()-1]] = elasticsearchPort
+                        editedValuesMap[pathToRepObj.get(pathToRepObj.size()-1)] = elasticsearchPort
                     }
                 }
                 valuesYamlInputStream.close()
                 FileWriter valuesYamlOutputStream = new FileWriter(valuesYamlLoc)
-                yaml.dump(valuesYaml,valuesYamlOutputStream)
+                logpathDetailsYaml.dump(valuesYaml,valuesYamlOutputStream)
                 valuesYamlOutputStream.close()
-                println("False")
-            } else if ( depType == "k8s" ) {
+                logRequirementDetails = "False"
+            } else if (depType == "k8s") {
                 // If k8s inject a env Var to the deployments which stores the elastic search endpoint
                 FileWriter logPathFileWriter = new FileWriter(logPathDetailsYamlLoc)
                 Yaml yaml = new Yaml()
                 List envVars = []
                 String formattedElasticsearchURL = elasticsearchEndPoint.replace("https://","http://")
                 JSONArray injectableValues = logOptions.getJSONArray("injectableVals")
-                for ( int i=0 ; i<injectableValues.length() ; i++ ){
+                for (int i=0 ; i<injectableValues.length() ; i++){
                     // add more ifs for other vars
                     JSONObject injectableObj = injectableValues.getJSONObject(i)
                     if (injectableObj.getString("type") == "elasticsearchEndpoint") {
                         envVars.add(["name": injectableObj.getString("name"), "value" : formattedElasticsearchURL])
                     }
                 }
                 if(envVars.size() > 0 ){
-                    Map logConf = ["onlyVars":true, "envVars": envVars ]
+                    Map logConf = ["onlyVars": true, "envVars": envVars ]
                     yaml.dump(logConf,logPathFileWriter)
-                    logPathFileWriter.close()
-                    println("onlyEnvVars null")
+                    logRequirementDetails = "onlyEnvVars null"
                 } else {
-                    logPathFileWriter.close()
-                    println("False")
+                    logRequirementDetails = "False"
                 }
+                logPathFileWriter.close()
             }
         } else if ( logRequirement == "None" ) {
-            println("False")
+            logRequirementDetails = "False"
         }
     }catch(RuntimeException e){
-        println(e.printStackTrace())
+        logRequirementDetails = "False"
     }
+    println(logRequirementDetails)
 }
 
 /**

diff --git a/deployment/src/main/resources/kubernetes_deploy.sh b/deployment/src/main/resources/kubernetes_deploy.sh
@@ -19,12 +19,13 @@
 set -o xtrace
 
 #
-#This class is used for the deployment of resources into the namespace using kubectl commands
-#The created resources will be exposed using an Ingress to the external usage
-#
+# This script is used for the deployment of resources into the namespace using kubectl commands.
+# The created resources will be exposed using an Ingress to the external usage.
+function edit_deployment() {
+  details=$(groovy kubedeployment_editor.groovy \
+  "${infra_props["depRepoLocation"]}/testgrid-sidecar/deployment/logpath-details.yaml" \
+  "${INPUT_DIR}/infrastructure.properties" k8s)
 
-function edit_deployments() {
-  details=$(groovy kubedeployment_editor.groovy "${infra_props["depRepoLocation"]}/testgrid-sidecar/deployment/logpath-details.yaml" "${OUTPUT_DIR}/infrastructure.properties" k8s)
   read -ra detailsArr <<< $details
   sidecarReq=${detailsArr[0]}
   filename=${detailsArr[1]}
@@ -51,7 +52,6 @@ function create_k8s_resources() {
       exposed."
     fi
 
-
     if [[ -z ${loadBalancerHostName} ]]; then
         echo WARN: loadBalancerHostName not found in deployment.properties. Generating a random name under \
         *.gke.wso2testgrid.com CN
@@ -80,10 +80,8 @@ kubectl create secret tls ${tlskeySecret} \
     --cert ${INPUT_DIR}/testgrid-certs-v2.crt  \
     --key ${INPUT_DIR}/testgrid-certs-v2.key -n ${namespace}
 
-
 echo "# public key to access the endpoints using the Ingress is available in $OUTPUT_DIR" >> $OUTPUT_DIR/deployment.properties
 
-
     cat > ${ingressName}.yaml << EOF
 apiVersion: extensions/v1beta1
 kind: Ingress
@@ -191,7 +189,7 @@ function readinesss_services(){
 
 }
 
-#This function is used to add paths to etc/host fils
+# This function is used to add paths to etc/host files
 function addhost() {
     ip_address=$1
     hostname=$2
@@ -321,14 +319,13 @@ namespace=${infra_props["namespace"]}
 yamlFilesLocation=${infra_props["yamlFilesLocation"]}
 loadBalancerHostName=${deploy_props["loadBalancerHostName"]}
 logOptions=${infra_props["logOptions"]}
-
 testgrid_env=${infra_props["environment"]}
 testgrid_pass=${infra_props["password"]}
 
 if [ -z "$logOptions" ]; then
     echo "[INFO] No Logging Options are set"
 else
-    edit_deployments
+    edit_deployment
 fi
 
 create_k8s_resources

diff --git a/deployment/src/main/resources/kubernetes_helmDeploy.sh b/deployment/src/main/resources/kubernetes_helmDeploy.sh
@@ -26,7 +26,9 @@ alias unzip='unzip -q'
 #
 
 function edit_deployments() {
-  details=$(groovy kubedeployment_editor.groovy "${infra_props["depRepoLocation"]}/testgrid-sidecar/deployment/logpath-details.yaml" "${OUTPUT_DIR}/infrastructure.properties" helm)
+  details=$(groovy kubedeployment_editor.groovy \
+  "${infra_props["depRepoLocation"]}/testgrid-sidecar/deployment/logpath-details.yaml" \
+  "${OUTPUT_DIR}/infrastructure.properties" helm)
   read -ra detailsArr <<< $details
   sidecarReq=${detailsArr[0]}
   filename=${detailsArr[1]}
@@ -52,11 +54,8 @@ function create_k8s_resources() {
       exit 1
     fi
 
-
-
     #deploy the helm configurations into the kubernetes cluster
     source $helmDeployScript
-
     readiness_deployments
 
     if [[ -z ${loadBalancerHostName} ]]; then
@@ -190,8 +189,7 @@ function readinesss_services(){
 
 }
 
-#This function is used to add paths to etc/host fils
-#This function is used to add paths to etc/host fils
+#This function is used to add paths to etc/host files
 function addhost() {
     ip_address=$1
     hostname=$2