Specify any params structured

With this commit we get rid of all string-based representation of more complex
parameters. This affects track-params and car-params specifically which will
now be passed to Rally as JSON string in any case. This allows to merge those
parameters in more complex scenarios where we need to source them from multiple
places.

Closes elastic#82
Relates elastic#83

Author: danielmitterdorfer

night_rally/night_rally.py

@@ -8,6 +8,7 @@
 import shlex
 import socket
 import time
+import json
 
 ROOT = os.path.dirname(os.path.realpath(__file__))
 RALLY_BINARY = "rally --skip-update"
@@ -263,9 +264,10 @@ def __init__(self, params, distribution_version):
         if int(self.distribution_version[0]) < 6:
             # 5.x needs additional settings as we removed this from Rally in c805ccda0ea05f15bdae22a1eac601bb33a66eae
             docker_params.append(
-                ConstantParam("car-params", "{\\\"additional_cluster_settings\\\": {\\\"xpack.security.enabled\\\": \\\"false\\\", "
-                                            "\\\"xpack.ml.enabled\\\": \\\"false\\\", \\\"xpack.monitoring.enabled\\\": \\\"false\\\", "
-                                            "\\\"xpack.watcher.enabled\\\": \\\"false\\\"}}")
+                ConstantParam("car-params", {"additional_cluster_settings": {"xpack.security.enabled": "false",
+                                                                             "xpack.ml.enabled": "false",
+                                                                             "xpack.monitoring.enabled": "false",
+                                                                             "xpack.watcher.enabled": "false"}})
             )
 
         self.params = ParamsFormatter(params=params + docker_params)
@@ -301,15 +303,24 @@ def command_line(self, race_config):
         for p in self.params:
             for k, v in p(race_config).items():
                 if k in cmd_line_params:
-                    # treat as array first, then join them later
-                    cmd_line_params[k] = cmd_line_params[k] + v
+                    if isinstance(v, dict):
+                        cmd_line_params[k].update(v)
+                    else:
+                        # treat as array first, then join them later
+                        cmd_line_params[k] = cmd_line_params[k] + v
                 else:
-                    cmd_line_params[k] = v
+                    if isinstance(v, dict):
+                        cmd_line_params[k] = collections.OrderedDict()
+                        cmd_line_params[k].update(v)
+                    else:
+                        cmd_line_params[k] = v
 
         cmd = RALLY_BINARY
         for k, v in cmd_line_params.items():
             if isinstance(v, list):
                 cmd += " --{}=\"{}\"".format(k, join_nullables(*v))
+            elif isinstance(v, dict):
+                cmd += " --{}=\"{}\"".format(k, json.dumps(v).replace('"', '\\"'))
             elif v is None:
                 cmd += " --{}".format(k)
             else:
@@ -370,7 +381,7 @@ def __call__(self, race_config):
             "effective-start-date": self.effective_start_date,
             "track": race_config.track,
             "challenge": race_config.challenge,
-            "car": [race_config.car],
+            "car": race_config.car,
             "user-tag": self.format_tag(additional_tags={"name": race_config.name})
         }
         add_if_present(params, "runtime-jdk", self.runtime_jdk)
@@ -456,7 +467,11 @@ def challenge(self):
 
     @property
     def car(self):
-        return self.configuration["car"]
+        c = self.configuration["car"]
+        if isinstance(c, str):
+            return [c]
+        else:
+            return c
 
     @property
     def car_params(self):

night_rally/resources/tracks.json

@@ -8,7 +8,9 @@
         "label": "add-4g-3nodes",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:1",
+        "track-params": {
+          "number_of_replicas": 1
+        },
         "car": "4gheap",
         "node-count": 3
       },
@@ -32,7 +34,9 @@
         "label": "add-4g",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:0",
+        "track-params": {
+          "number_of_replicas": 0
+        },
         "car": "4gheap"
       },
       {
@@ -54,9 +58,13 @@
         "#COMMENT": "Actually, we should not produce graphs for this one because indexing throughput is not that useful here.",
         "charts": [],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:0",
+        "track-params": {
+          "number_of_replicas": 0
+        },
         "car": "4gheap",
-        "car-params": "verbose_iw_logging_enabled:true"
+        "car-params": {
+          "verbose_iw_logging_enabled": "true"
+        }
       }
     ]
   },
@@ -107,15 +115,19 @@
         "label": "add-4g",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:0",
+        "track-params": {
+          "number_of_replicas": 0
+        },
         "car": "4gheap"
       },
       {
         "name": "geopoint-append-4g-3nodes",
         "label": "add-4g-3nodes",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:1",
+        "track-params": {
+          "number_of_replicas": 1
+        },
         "car": "4gheap",
         "node-count": 3
       },
@@ -136,7 +148,9 @@
         "label": "add-defaults",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:0",
+        "track-params": {
+          "number_of_replicas": 0
+        },
         "car": "defaults"
       },
       {
@@ -159,7 +173,9 @@
         "label": "add-4g-3nodes",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts",
-        "track-params": "number_of_replicas:1",
+        "track-params": {
+          "number_of_replicas": 1
+        },
         "car": "4gheap",
         "node-count": 3
       },
@@ -168,9 +184,16 @@
         "label": "nio-4g-3nodes",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts",
-        "track-params": "number_of_replicas:1",
-        "car": "4gheap,unpooled",
-        "car-params": "{\\\"additional_cluster_settings\\\": {\\\"cache.recycler.page.limit.heap\\\": \\\"13.5%\\\",\\\"cache.recycler.page.weight.bytes\\\": 2}}",
+        "track-params": {
+          "number_of_replicas": 1
+        },
+        "car": ["4gheap", "unpooled"],
+        "car-params": {
+          "additional_cluster_settings": {
+            "cache.recycler.page.limit.heap": "13.5%",
+            "cache.recycler.page.weight.bytes": 2
+          }
+        },
         "plugins": "transport-nio:transport+http",
         "node-count": 3
       },
@@ -213,7 +236,9 @@
         "label": "add-sorted-4g",
         "charts": ["indexing"],
         "challenge": "append-sorted-no-conflicts-index-only",
-        "track-params": "number_of_replicas:0",
+        "track-params": {
+          "number_of_replicas": 0
+        },
         "car": "4gheap"
       },
       {
@@ -222,7 +247,12 @@
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
         "#COMMENT": "We need to be a bit more explicit here because nyc_taxis sets a bunch of other index settings as well which we don't want",
-        "track-params": "{\\\"index_settings\\\": {\\\"index.number_of_replicas\\\": 1,\\\"index.number_of_shards\\\": 5}}",
+        "track-params": {
+          "index_settings": {
+            "index.number_of_replicas": 1,
+            "index.number_of_shards": 5
+          }
+        },
         "car": "4gheap",
         "node-count": 3
       }
@@ -236,7 +266,9 @@
         "label": "defaults",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:0",
+        "track-params": {
+          "number_of_replicas": 0
+        },
         "car": "defaults"
       },
       {
@@ -251,7 +283,9 @@
         "label": "no-src-4g",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "source_enabled:false",
+        "track-params": {
+          "source_enabled": false
+        },
         "car": "4gheap"
       },
       {
@@ -267,7 +301,9 @@
         "label": "3nodes-4g",
         "charts": ["indexing"],
         "challenge": "append-no-conflicts-index-only",
-        "track-params": "number_of_replicas:1",
+        "track-params": {
+          "number_of_replicas": 1
+        },
         "car": "4gheap",
         "node-count": 3
       },
@@ -283,7 +319,10 @@
         "label": "no-src-4g-grok",
         "charts": ["indexing"],
         "challenge": "append-index-only-with-ingest-pipeline",
-        "track-params": "source_enabled:false,ingest_pipeline:'grok'",
+        "track-params": {
+          "source_enabled": false,
+          "ingest_pipeline": "grok"
+        },
         "car": "4gheap"
       }
     ]

tests/night_rally_test.py

@@ -104,7 +104,9 @@ def test_run_two_challenges_successfully(self, mocked_wait_until_port_is_free):
                         "name": "geonames-defaults",
                         "challenge": "append-no-conflicts",
                         "car": "defaults",
-                        "car-params": "verbose_iw_logging_enabled:true"
+                        "car-params": {
+                            "verbose_iw_logging_enabled": "true"
+                        }
                     },
                     {
                         "name": "geonames-4g",
@@ -124,7 +126,7 @@ def test_run_two_challenges_successfully(self, mocked_wait_until_port_is_free):
                 "rally --skip-update --configuration-name=\"nightly\" --quiet --target-host=\"localhost:39200\" "
                 "--effective-start-date=\"2016-01-01 00:00:00\" --track=\"geonames\" --challenge=\"append-no-conflicts\" "
                 "--car=\"defaults\" --user-tag=\"env:bare,name:geonames-defaults\" --runtime-jdk=\"8\" "
-                "--car-params=\"verbose_iw_logging_enabled:true\" --pipeline=\"from-sources-complete\" "
+                "--car-params=\"{\\\"verbose_iw_logging_enabled\\\": \\\"true\\\"}\" --pipeline=\"from-sources-complete\" "
                 "--revision=\"@2016-01-01T00:00:00Z\"",
 
                 "rally --skip-update --configuration-name=\"nightly\" --quiet --target-host=\"localhost:39200\" "
@@ -398,7 +400,7 @@ def test_run_release_benchmark_with_transport_nio(self, mocked_wait_until_port_i
                     {
                         "name": "geonames-defaults",
                         "challenge": "append-no-conflicts",
-                        "car": "defaults",
+                        "car": ["defaults", "unpooled"],
                         "plugins": "transport-nio:transport+http"
                     }
                 ]
@@ -414,7 +416,7 @@ def test_run_release_benchmark_with_transport_nio(self, mocked_wait_until_port_i
             [
                 "rally --skip-update --configuration-name=\"release\" --quiet --target-host=\"localhost:39200\" "
                 "--effective-start-date=\"2016-01-01 00:00:00\" --track=\"geonames\" --challenge=\"append-no-conflicts\" "
-                "--car=\"defaults\" --user-tag=\"env:bare,name:geonames-defaults\" --runtime-jdk=\"8\" "
+                "--car=\"defaults,unpooled\" --user-tag=\"env:bare,name:geonames-defaults\" --runtime-jdk=\"8\" "
                 "--elasticsearch-plugins=\"transport-nio:transport+http\" --distribution-version=\"7.0.0\" "
                 "--pipeline=\"from-distribution\""
             ]
@@ -487,7 +489,10 @@ def test_run_docker_5x_benchmark(self, mocked_wait_until_port_is_free):
                     {
                         "name": "geonames-4g",
                         "challenge": "append-no-conflicts",
-                        "car": "4gheap"
+                        "car": "4gheap",
+                        "car-params": {
+                            "verbose_iw_logging_enabled": "true"
+                        }
                     }
                 ]
             }
@@ -510,9 +515,10 @@ def test_run_docker_5x_benchmark(self, mocked_wait_until_port_is_free):
                 "rally --skip-update --configuration-name=\"release\" --quiet --target-host=\"localhost:39200\" "
                 "--effective-start-date=\"2016-01-01 00:00:00\" --track=\"geonames\" --challenge=\"append-no-conflicts\" "
                 "--car=\"4gheap\" --user-tag=\"env:docker,name:geonames-4g\" --runtime-jdk=\"8\" "
-                "--distribution-version=\"5.6.0\" --pipeline=\"docker\" --car-params=\"{\\\"additional_cluster_settings\\\": "
+                "--car-params=\"{\\\"verbose_iw_logging_enabled\\\": \\\"true\\\", \\\"additional_cluster_settings\\\": "
                 "{\\\"xpack.security.enabled\\\": \\\"false\\\", \\\"xpack.ml.enabled\\\": \\\"false\\\", "
-                "\\\"xpack.monitoring.enabled\\\": \\\"false\\\", \\\"xpack.watcher.enabled\\\": \\\"false\\\"}}\""
+                "\\\"xpack.monitoring.enabled\\\": \\\"false\\\", \\\"xpack.watcher.enabled\\\": \\\"false\\\"}}\" "
+                "--distribution-version=\"5.6.0\" --pipeline=\"docker\""
             ]
             ,
             system_call.calls