Added configMaps section in Kubernetes #6646

Anumita · 2018-03-09T06:02:44Z

jikuma · 2018-03-09T08:02:58Z

Tasks/Kubernetes/task.loc.json

@@ -13,7 +13,7 @@
  "version": {
    "Major": 0,
    "Minor": 1,
-    "Patch": 15
+    "Patch": 53


jikuma · 2018-03-12T08:15:03Z

Tasks/Kubernetes/task.json

+            "label": "Force update configmap",
+            "defaultValue": "true",
+            "helpMarkDown": "Delete the configmap if it exists and create a new one with updated values.",
+            "groupName": "configMaps"


From the spec, I can see that Secrets and Config map is combined. Did you talk to Atul.

jikuma · 2018-03-12T08:19:04Z

Tasks/Kubernetes/src/kubernetes.ts

+                executeCreateConfigMapCommand(clusterConnection, configMapName);
+            }
+            else {
+                executeKubectlCommand(clusterConnection);


in case of secret, then config map will command be executed?

jikuma · 2018-03-12T08:19:42Z

Tasks/Kubernetes/src/kubernetes.ts

        }, function failure(err) {
            tl.setResult(tl.TaskResult.Failed, err.message);
        }).done();
    }
+    else if(configMapName) {
+        executeCreateConfigMapCommand(clusterConnection, configMapName); 


will command be executed after creating config map?

jikuma · 2018-03-12T08:21:13Z

Tasks/Kubernetes/src/kubernetes.ts

+    kubectlConfigMap.run(clusterConnection, configMapName).fin(function cleanup(){
+        clusterConnection.close();
+    }).then(function success() {
+        executeKubectlCommand(clusterConnection);


the function name is executeCreateConfigMapCommand, it should not run any other kubectl command. You need to refactor the code.

jikuma · 2018-03-12T08:25:41Z

Tasks/Kubernetes/Tests/L0.ts

@@ -311,6 +315,84 @@ describe('Kubernetes Suite', function() {
        console.log(tr.stderr);
        done();
    });
+
+    it('Runs successfully for kubectl create configMap from file or directory with forceUpdate', (done:MochaDone) => {


Add test cases to validate following scenerios.

create secret -> create configmap -> kubectl commad execution. All three commands are getting executed.

create configmap -> kubectl commad. These two commands are independently getting executed.

create secret -> kubectl commad. These two commands are independently getting executed.

jikuma · 2018-03-13T09:06:32Z

Please check the error scenarios also.

jikuma · 2018-03-13T09:08:47Z

Tasks/Kubernetes/src/kubernetes.ts

            clusterConnection.close();
-        }).then(function success() {
-            executeKubectlCommand(clusterConnection);
        }, function failure(err) {
            tl.setResult(tl.TaskResult.Failed, err.message);


you can use try catch here. clusterConnection is not cleaned while exception occurs.

jikuma · 2018-03-14T04:42:11Z

Tasks/Kubernetes/Strings/resources.resjson/en-US/resources.resjson

+  "loc.input.label.configMapFile": "ConfigMap File",
+  "loc.input.help.configMapFile": "Specify a file or directory that contains the configMaps",
+  "loc.input.label.configMapArguments": "Arguments",
+  "loc.input.help.configMapArguments": "Specify keys and literal values to insert in configMap.For example, --from-literal=key1=value1 --from-literal=key2=\"top secret\". Please use double quotes to specify any literals that have spaces.",


Use not 'Please Use'. Get these messages reviewed by Atul.

jikuma · 2018-03-14T04:43:51Z

Tasks/Kubernetes/src/kubernetes.ts

+       connection.close();
+    });
+}
+catch (error)


Why was this try catch needed? catch at the end of promise should be enough, right?

That catches exceptions only in the run function and not in the open function

jikuma · 2018-03-14T04:45:28Z

Tasks/Kubernetes/src/kubernetes.ts

 }

 // execute kubectl command
 function executeKubectlCommand(clusterConnection: ClusterConnection) : any {
-
    var command = tl.getInput("command", true);
    var result = "";
    var ouputVariableName =  tl.getInput("kubectlOutput", false);  
    kubectl.run(clusterConnection, command, (data) => result += data)


I think you have to return back the promise from kubectl.run

bansalaseem · 2018-03-20T11:24:19Z

Tasks/Kubernetes/src/kubernetesconfigmap.ts

+
+export function run(connection: ClusterConnection, configMapName: string): any {
+    if(tl.getBoolInput("forceUpdateConfigMap") == true) {
+        return deleteConfigMap(connection, configMapName).fin(() =>{


Is deletion of config map ok?

bansalaseem · 2018-03-20T11:33:53Z

Tasks/Kubernetes/src/kubernetes.ts

@@ -5,6 +5,7 @@ import path = require('path');

 import ClusterConnection from "./clusterconnection";


As you mentioned, make sure that it works in a re-runnable scenario.

bansalaseem · 2018-03-26T05:52:38Z

Tasks/Kubernetes/src/kubernetesconfigmap.ts

+    {
+        return executeKubetclGetConfigmapCommand(connection, configMapName).then(function success() {
+            tl.debug(tl.loc('ConfigMapExists', configMapName));
+        }, function failure() {


will it be failure?

bansalaseem · 2018-03-26T05:53:09Z

Tasks/Kubernetes/src/kubernetesconfigmap.ts

+        }
+
+        return configMapFromFromFileArgument;
+    } else {


why it is else here?

Added configMaps section in Kubernetes

d8d5b69

Anumita requested review from azooinmyluggage, bansalaseem and jikuma March 9, 2018 06:59

jikuma reviewed Mar 9, 2018

View reviewed changes

Tasks/Kubernetes/task.loc.json Outdated

@@ -13,7 +13,7 @@

"version": {

"Major": 0,

"Minor": 1,

"Patch": 15

"Patch": 53

Copy link

Contributor

jikuma Mar 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

53?

Correct task.loc.json for kubernetes

5ff536e

jikuma reviewed Mar 12, 2018

View reviewed changes

jikuma requested changes Mar 12, 2018

View reviewed changes

kmkumaran assigned bansalaseem Mar 13, 2018

kmkumaran added the Area: Release label Mar 13, 2018

Refactored code to run secret and configmap command and added more tests

a53fb4c

jikuma approved these changes Mar 13, 2018

View reviewed changes

jikuma reviewed Mar 13, 2018

View reviewed changes

Removed the unnecessary close connection calls

17cecf4

jikuma reviewed Mar 14, 2018

View reviewed changes

Changed secrets and configMaps section help text

c2705b5

bansalaseem reviewed Mar 20, 2018

View reviewed changes

Changed the forceUpdate functionality in configMaps

0669d4e

bansalaseem reviewed Mar 26, 2018

View reviewed changes

Tasks/Kubernetes/src/kubernetesconfigmap.ts Outdated

}

return configMapFromFromFileArgument;

} else {

Copy link

Contributor

bansalaseem Mar 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it is else here?

Disabled use file option for configMap

3aebae2

Anumita merged commit 8b888d3 into master Mar 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added configMaps section in Kubernetes #6646

Added configMaps section in Kubernetes #6646

Anumita commented Mar 9, 2018

jikuma Mar 9, 2018

jikuma Mar 12, 2018 •

edited

Loading

jikuma Mar 12, 2018

jikuma Mar 12, 2018

jikuma Mar 12, 2018

jikuma Mar 12, 2018

jikuma commented Mar 13, 2018

jikuma Mar 13, 2018

jikuma Mar 14, 2018

jikuma Mar 14, 2018

Anumita Mar 15, 2018

jikuma Mar 14, 2018

bansalaseem Mar 20, 2018

bansalaseem Mar 20, 2018

bansalaseem Mar 26, 2018

bansalaseem Mar 26, 2018

		@@ -5,6 +5,7 @@ import path = require('path');

		import ClusterConnection from "./clusterconnection";

Added configMaps section in Kubernetes #6646

Added configMaps section in Kubernetes #6646

Conversation

Anumita commented Mar 9, 2018

Choose a reason for hiding this comment

jikuma Mar 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jikuma commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jikuma Mar 12, 2018 •

edited

Loading