Merge pull request #104 from reactiveops/refactor-helm-args

WIP Refactored the helm arguments to implement spaces

.circleci/config.yml

@@ -229,9 +229,10 @@ jobs:
             reckoner plot --only nginx-ingress end_to_end_testing/basic.yml
             helm list --output json | jq -e '.Releases[]|select(.Name == "nginx-ingress")|.Status == "DEPLOYED"'
 
-            reckoner plot --only nginx-ingress --only redis-env end_to_end_testing/basic.yml
+            test_environ_var=somevalue reckoner plot --only nginx-ingress --only redis-env end_to_end_testing/basic.yml
             helm list --output json | jq -e '.Releases[]|select(.Name == "nginx-ingress")|.Status == "DEPLOYED"'
             helm list --output json | jq -e '.Releases[]|select(.Name == "redis-env")|.Status == "DEPLOYED"'
+            helm get values redis-env --output json | jq -e '.test_environ_var == "somevalue"'
   publish-binaries:
     executor: python-3-7
     steps:

CHANGELOG.md

@@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [1.2.0]
 - Support helm wrapper plugins such as [Helm Secrets](https://github.com/futuresimple/helm-secrets)
+- POTENTIAL BREAKING CHANGE: Refactored all helm commands to use `--arg value` instead of `--arg=value`. (This helps with poor param support with how helm plugin wrappers work)
 
 ## [1.1.6]
 - Skipped versions to kickstart pipelines

reckoner/chart.py

@@ -268,31 +268,37 @@ def install(self, namespace=None, context=None):
         if self._deprecation_messages:
             [logging.warning(msg) for msg in self._deprecation_messages]
 
+    def _append_arg(self, arg_string):
+        for item in arg_string.split(" ", 1):
+            self.args.append(item)
+
     def build_helm_arguments_for_chart(self):
         """
         This method builds all the arguments we'll pass along to the helm
         client once we need to run the install
         """
 
+        # always start off with empty args list
+        self.args = []
+
         # Set Default args (release name and chart path)
-        self.args = [
-            '{}'.format(self._release_name),
-            self.repository.chart_path,
-        ]
+        self._append_arg('{}'.format(self._release_name))
+        self._append_arg(self.repository.chart_path)
 
         # Add namespace to args
-        self.args.append('--namespace={}'.format(self.namespace))
+        self._append_arg('--namespace {}'.format(self.namespace))
 
         # Add kubecfg context
         if self.context is not None:
-            self.args.append('--kube-context={}'.format(self.context))
+            self._append_arg('--kube-context {}'.format(self.context))
 
         # Add debug arguments
-        self.args.extend(self.debug_args)
+        for debug_arg in self.debug_args:
+            self._append_arg(debug_arg)
 
         # Add the version arguments
         if self.version:
-            self.args.append('--version={}'.format(self.version))
+            self._append_arg('--version {}'.format(self.version))
 
         # HACK: This is in place until we can fully deprecate the usage of set
         #       in place of values. Currently values: gets translated into
@@ -335,7 +341,7 @@ def build_files_list(self):
         files specified in the course.yml
         """
         for values_file in self.files:
-            self.args.append("-f={}".format(values_file))
+            self._append_arg("-f {}".format(values_file))
 
     def build_set_string_arguments(self):
         """
@@ -347,7 +353,7 @@ def build_set_string_arguments(self):
         """
         for key, value in self.values_strings.items():
             for k, v in self._format_set(key, value):
-                self.args.append("--set-string={}={}".format(k, v))
+                self._append_arg("--set-string {}={}".format(k, v))
 
     def build_set_arguments(self):
         """
@@ -359,7 +365,7 @@ def build_set_arguments(self):
         """
         for key, value in self.set_values.items():
             for k, v in self._format_set(key, value):
-                self.args.append("--set={}={}".format(k, v))
+                self._append_arg("--set {}={}".format(k, v))
 
     def _format_set(self, key, value):
         """

reckoner/tests/test_chart.py

@@ -231,7 +231,7 @@ def test_chart_install(self, repositoryMock):
         chart.install()
         helm_client_mock.upgrade.assert_called_once()
         upgrade_call = helm_client_mock.upgrade.call_args
-        self.assertEqual(upgrade_call[0][0], ['nameofchart', '', '--namespace=fakenamespace'])
+        self.assertEqual(upgrade_call[0][0], ['nameofchart', '', '--namespace', 'fakenamespace'])
 
     @mock.patch('reckoner.chart.Repository')
     def test_chart_install_with_plugin(self, repositoryMock):
@@ -246,5 +246,5 @@ def test_chart_install_with_plugin(self, repositoryMock):
         chart.install()
         helm_client_mock.upgrade.assert_called_once()
         upgrade_call = helm_client_mock.upgrade.call_args
-        self.assertEqual(upgrade_call[0][0], ['nameofchart', '', '--namespace=fakenamespace'])
+        self.assertEqual(upgrade_call[0][0], ['nameofchart', '', '--namespace', 'fakenamespace'])
         self.assertEqual(upgrade_call[1], {'plugin': 'someplugin'})

tests/test_reckoner.py

@@ -397,7 +397,6 @@ def test_chart_install(self):
                     'upgrade',
                     '--recreate-pods',
                     '--install',
-                    # '--namespace={}'.format(chart.namespace),
                     chart.release_name,
                     chart.repository.chart_path,
                 ]
@@ -413,8 +412,10 @@ def test_chart_install(self):
                         '--install',
                         chart.release_name,
                         chart.repository.chart_path,
-                        '--namespace={}'.format(chart.namespace),
-                        '--set={}={}'.format(test_environ_var_name, test_environ_var)]
+                        '--namespace',
+                        '{}'.format(chart.namespace),
+                        '--set',
+                        '{}={}'.format(test_environ_var_name, test_environ_var)]
                 )
             if chart.release_name == test_values_strings_chart:
                 self.assertEqual(
@@ -426,11 +427,16 @@ def test_chart_install(self):
                         '--install',
                         chart.release_name,
                         chart.repository.chart_path,
-                        '--namespace={}'.format(chart.namespace),
-                        '--version=0.1.0',
-                        '--set-string=string=string',
-                        '--set-string=integer=10',
-                        '--set-string=boolean=True'
+                        '--namespace',
+                        '{}'.format(chart.namespace),
+                        '--version',
+                        '0.1.0',
+                        '--set-string',
+                        'string=string',
+                        '--set-string',
+                        'integer=10',
+                        '--set-string',
+                        'boolean=True'
                     ]
                 )