From bf1f6fad8db0d1cfa7b5b7b31ea31b484b2d7e06 Mon Sep 17 00:00:00 2001
From: Iulian Titiriga <titiriga@adobe.com>
Date: Wed, 16 Oct 2019 18:47:43 +0300
Subject: [PATCH 1/3] Add concurrency option for helmfile commands

---
 src/ops/cli/helmfile.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/ops/cli/helmfile.py b/src/ops/cli/helmfile.py
index 77737698..bb29ebae 100644
--- a/src/ops/cli/helmfile.py
+++ b/src/ops/cli/helmfile.py
@@ -52,6 +52,8 @@ def get_epilog(self):
             ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile sync
             # Run helmfile sync for a single chart
             ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile sync -- --selector chart=nginx-controller
+            # Run helmfile sync with concurrency flag
+            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile --concurrency=1 sync -- --selector chart=nginx-controller
         '''
 
 
@@ -151,5 +153,10 @@ def generate_helmfile_config(self, path, args):
                                                      print_data=True)
 
     def get_helmfile_command(self, args):
-        cmd = ' '.join(args.extra_args + [args.subcommand])
-        return "cd {} && helmfile {}".format(args.helmfile_path, cmd)
+        extra_args = ' '.join(args.extra_args)
+        helm_concurent_proc = '--concurrency={}'.format(args.concurrency) if args.concurrency else ''
+        return "cd {helmfile_path} && helmfile {extra_args} {subcommand} {concurrency}".format(
+            helmfile_path=args.helmfile_path,
+            extra_args=extra_args,
+            subcommand=args.subcommand,
+            concurrency=helm_concurent_proc)

From ab3e9f4695aee3ce70feafa173aeea77dbc9452d Mon Sep 17 00:00:00 2001
From: Iulian Titiriga <titiriga@adobe.com>
Date: Thu, 17 Oct 2019 18:32:01 +0300
Subject: [PATCH 2/3] Address reviews

---
 src/ops/cli/helmfile.py | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/src/ops/cli/helmfile.py b/src/ops/cli/helmfile.py
index bb29ebae..99603cd2 100644
--- a/src/ops/cli/helmfile.py
+++ b/src/ops/cli/helmfile.py
@@ -30,14 +30,10 @@ def get_help(self):
 
     def configure(self, parser):
         parser.add_argument(
-            'subcommand',
-            help='plan | sync | apply | template',
-            type=str)
-        parser.add_argument(
-            'extra_args',
+            'helmfile_args',
             type=str,
             nargs='*',
-            help='Extra args')
+            help='Full subcommand of helmfile including params')
         parser.add_argument(
             '--helmfile-path',
             type=str,
@@ -51,9 +47,9 @@ def get_epilog(self):
             # Run helmfile sync
             ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile sync
             # Run helmfile sync for a single chart
-            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile sync -- --selector chart=nginx-controller
+            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile -- --selector chart=nginx-controller sync
             # Run helmfile sync with concurrency flag
-            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile --concurrency=1 sync -- --selector chart=nginx-controller
+            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile -- --selector chart=nginx-controller sync --concurrency=1
         '''
 
 
@@ -153,10 +149,7 @@ def generate_helmfile_config(self, path, args):
                                                      print_data=True)
 
     def get_helmfile_command(self, args):
-        extra_args = ' '.join(args.extra_args)
-        helm_concurent_proc = '--concurrency={}'.format(args.concurrency) if args.concurrency else ''
-        return "cd {helmfile_path} && helmfile {extra_args} {subcommand} {concurrency}".format(
+        helmfile_args = ' '.join(args.helmfile_args)
+        return "cd {helmfile_path} && helmfile {helmfile_args}".format(
             helmfile_path=args.helmfile_path,
-            extra_args=extra_args,
-            subcommand=args.subcommand,
-            concurrency=helm_concurent_proc)
+            helmfile_args=helmfile_args)

From 51c4e94b318a51bbd3242be60437dc9f9655336f Mon Sep 17 00:00:00 2001
From: Iulian Titiriga <titiriga@adobe.com>
Date: Fri, 18 Oct 2019 17:11:36 +0300
Subject: [PATCH 3/3] Add functionality for parsing unknown args

---
 src/ops/cli/config_generator.py |  4 +++-
 src/ops/cli/helmfile.py         | 17 ++++++-----------
 src/ops/cli/inventory.py        |  5 ++++-
 src/ops/cli/packer.py           |  5 ++++-
 src/ops/cli/parser.py           |  4 ++++
 src/ops/cli/playbook.py         |  5 ++++-
 src/ops/cli/run.py              |  5 ++++-
 src/ops/cli/ssh.py              |  5 ++++-
 src/ops/cli/sync.py             |  5 ++++-
 src/ops/cli/terraform.py        |  4 ++--
 src/ops/main.py                 |  7 ++++---
 11 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/src/ops/cli/config_generator.py b/src/ops/cli/config_generator.py
index 7492b6d1..d761bf77 100644
--- a/src/ops/cli/config_generator.py
+++ b/src/ops/cli/config_generator.py
@@ -12,6 +12,7 @@
 from himl.main import ConfigRunner
 from ops.cli.parser import SubParserConfig
 
+logger = logging.getLogger(__name__)
 
 class ConfigGeneratorParserConfig(SubParserConfig):
     def get_name(self):
@@ -35,7 +36,8 @@ class ConfigGeneratorRunner(object):
     def __init__(self, cluster_config_path):
         self.cluster_config_path = cluster_config_path
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         logging.basicConfig(level=logging.INFO)
         args.path = self.cluster_config_path
         if args.output_file is None:
diff --git a/src/ops/cli/helmfile.py b/src/ops/cli/helmfile.py
index 99603cd2..65881240 100644
--- a/src/ops/cli/helmfile.py
+++ b/src/ops/cli/helmfile.py
@@ -29,11 +29,6 @@ def get_help(self):
         return 'Wrap common helmfile tasks using hierarchical configuration support'
 
     def configure(self, parser):
-        parser.add_argument(
-            'helmfile_args',
-            type=str,
-            nargs='*',
-            help='Full subcommand of helmfile including params')
         parser.add_argument(
             '--helmfile-path',
             type=str,
@@ -47,9 +42,9 @@ def get_epilog(self):
             # Run helmfile sync
             ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile sync
             # Run helmfile sync for a single chart
-            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile -- --selector chart=nginx-controller sync
+            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile --selector chart=nginx-controller sync
             # Run helmfile sync with concurrency flag
-            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile -- --selector chart=nginx-controller sync --concurrency=1
+            ops data/env=dev/region=va6/project=ee/cluster=experiments/composition=helmfiles helmfile --selector chart=nginx-controller sync --concurrency=1
         '''
 
 
@@ -61,7 +56,7 @@ def __init__(self, ops_config, cluster_config_path, execute):
         self.cluster_config_path = cluster_config_path
         self.execute = execute
 
-    def run(self, args):
+    def run(self, args, extra_args):
         config_path_prefix = os.path.join(self.cluster_config_path, '')
         default_helmfiles = '../ee-k8s-infra/compositions/helmfiles'
         args.helmfile_path = default_helmfiles if args.helmfile_path is None else os.path.join(
@@ -77,7 +72,7 @@ def run(self, args):
         data = self.generate_helmfile_config(conf_path, args)
         self.setup_kube_config(data)
 
-        command = self.get_helmfile_command(args)
+        command = self.get_helmfile_command(args, extra_args)
         return dict(command=command)
 
     def setup_kube_config(self, data):
@@ -148,8 +143,8 @@ def generate_helmfile_config(self, path, args):
                                                      output_file=output_file,
                                                      print_data=True)
 
-    def get_helmfile_command(self, args):
-        helmfile_args = ' '.join(args.helmfile_args)
+    def get_helmfile_command(self, args, extra_args):
+        helmfile_args = ' '.join(extra_args)
         return "cd {helmfile_path} && helmfile {helmfile_args}".format(
             helmfile_path=args.helmfile_path,
             helmfile_args=helmfile_args)
diff --git a/src/ops/cli/inventory.py b/src/ops/cli/inventory.py
index 67d22588..28d77e5b 100644
--- a/src/ops/cli/inventory.py
+++ b/src/ops/cli/inventory.py
@@ -9,12 +9,14 @@
 # governing permissions and limitations under the License.
 
 import yaml
+import logging
 
 from ansible.parsing.yaml.dumper import AnsibleDumper
 from ansible.utils.color import stringc
 from . import display
 from .parser import configure_common_arguments, SubParserConfig
 
+logger = logging.getLogger(__name__)
 
 class InventoryParserConfig(SubParserConfig):
     def get_name(self):
@@ -45,7 +47,8 @@ def __init__(self, ansible_inventory, cluster_name):
         self.ansible_inventory = ansible_inventory
         self.cluster_name = cluster_name
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         for host in self.get_inventory_hosts(args):
             group_names = [group.name for group in host.get_groups()]
             group_names = sorted(group_names)
diff --git a/src/ops/cli/packer.py b/src/ops/cli/packer.py
index b38b7357..d21735b5 100644
--- a/src/ops/cli/packer.py
+++ b/src/ops/cli/packer.py
@@ -8,9 +8,11 @@
 # OF ANY KIND, either express or implied. See the License for the specific language
 # governing permissions and limitations under the License.
 
+import logging
 from ops.cli.parser import SubParserConfig
 from . import aws
 
+logger = logging.getLogger(__name__)
 
 class PackerParserConfig(SubParserConfig):
     def get_name(self):
@@ -39,7 +41,8 @@ def __init__(self, root_dir, cluster_config):
         self.cluster_config = cluster_config
         self.root_dir = root_dir
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         config_all = self.cluster_config.all()
 
         packer_variables = config_all['packer']['variables']
diff --git a/src/ops/cli/parser.py b/src/ops/cli/parser.py
index 4f616e32..6e42cb11 100644
--- a/src/ops/cli/parser.py
+++ b/src/ops/cli/parser.py
@@ -75,6 +75,10 @@ def parse_args(self, args=None):
         RootParser._check_args_for_unicode(args)
         return self._get_parser().parse_args(args)
 
+    def parse_known_args(self, args=None):
+        RootParser._check_args_for_unicode(args)
+        return self._get_parser().parse_known_args(args)
+
 
 class SubParserConfig(object):
     def get_name(self):
diff --git a/src/ops/cli/playbook.py b/src/ops/cli/playbook.py
index 08000926..7bda27cb 100644
--- a/src/ops/cli/playbook.py
+++ b/src/ops/cli/playbook.py
@@ -11,7 +11,9 @@
 from .parser import SubParserConfig
 from .parser import configure_common_ansible_args, configure_common_arguments
 import getpass
+import logging
 
+logger = logging.getLogger(__name__)
 
 class PlaybookParserConfig(SubParserConfig):
     def get_name(self):
@@ -66,7 +68,8 @@ def __init__(self, ops_config, root_dir, inventory_generator,
         self.cluster_config_path = cluster_config_path
         self.cluster_config = cluster_config
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         inventory_path, ssh_config_path = self.inventory_generator.generate()
 
         ssh_config = "ANSIBLE_SSH_ARGS='-F %s'" % ssh_config_path
diff --git a/src/ops/cli/run.py b/src/ops/cli/run.py
index cd791b0b..b7888308 100644
--- a/src/ops/cli/run.py
+++ b/src/ops/cli/run.py
@@ -8,8 +8,10 @@
 # OF ANY KIND, either express or implied. See the License for the specific language
 # governing permissions and limitations under the License.
 
+import logging
 from .parser import configure_common_ansible_args, SubParserConfig
 
+logger = logging.getLogger(__name__)
 
 class CommandParserConfig(SubParserConfig):
     def get_epilog(self):
@@ -61,7 +63,8 @@ def __init__(self, ops_config, root_dir, inventory_generator,
         self.cluster_config_path = cluster_config_path
         self.cluster_config = cluster_config
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         inventory_path, ssh_config_path = self.inventory_generator.generate()
         limit = args.host_pattern
 
diff --git a/src/ops/cli/ssh.py b/src/ops/cli/ssh.py
index d3b49b89..f1db8080 100644
--- a/src/ops/cli/ssh.py
+++ b/src/ops/cli/ssh.py
@@ -20,7 +20,9 @@
 import getpass
 import re
 import os
+import logging
 
+logger = logging.getLogger(__name__)
 IP_HOST_REG_EX = re.compile(r'^((\d+)\.(\d+)\.(\d+)\.(\d+):)?(\d+)$')
 
 
@@ -126,7 +128,8 @@ def __init__(self, cluster_config_path, cluster_config,
         self.cluster_config = cluster_config
         self.ansible_inventory = ansible_inventory
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         if args.keygen:
             if self.cluster_config.has_ssh_keys:
                 err('Cluster already has ssh keys, refusing to overwrite')
diff --git a/src/ops/cli/sync.py b/src/ops/cli/sync.py
index 3089b02e..9a1b6ac1 100644
--- a/src/ops/cli/sync.py
+++ b/src/ops/cli/sync.py
@@ -8,12 +8,14 @@
 # OF ANY KIND, either express or implied. See the License for the specific language
 # governing permissions and limitations under the License.
 
+import logging
 import getpass
 import subprocess
 
 from .parser import SubParserConfig
 from . import *
 
+logger = logging.getLogger(__name__)
 
 class SyncParserConfig(SubParserConfig):
     def configure(self, parser):
@@ -64,7 +66,8 @@ def __init__(self, cluster_config, root_dir,
         self.cluster_config = cluster_config
         self.ops_config = ops_config
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         inventory_path, ssh_config_path = self.inventory_generator.generate()
         src = PathExpr(args.src)
         dest = PathExpr(args.dest)
diff --git a/src/ops/cli/terraform.py b/src/ops/cli/terraform.py
index 8fec6287..69adf825 100644
--- a/src/ops/cli/terraform.py
+++ b/src/ops/cli/terraform.py
@@ -20,7 +20,6 @@
 
 logger = logging.getLogger(__name__)
 
-
 class TerraformParserConfig(SubParserConfig):
     def get_name(self):
         return 'terraform'
@@ -171,7 +170,8 @@ def check_ops_version(self):
                     self.cluster_config.conf["terraform"]["ops_min_version"])
                 validate_ops_version(ops_min_version)
 
-    def run(self, args):
+    def run(self, args, extra_args):
+        logger.info("Found extra_args %s", extra_args)
         self.check_ops_version()
         terraform_config_path = os.environ.get(
                                                 "TF_CLI_CONFIG_FILE",
diff --git a/src/ops/main.py b/src/ops/main.py
index 00f5efa6..3ecf2e86 100644
--- a/src/ops/main.py
+++ b/src/ops/main.py
@@ -113,13 +113,14 @@ def configure_inventory(self):
         self.inventory_plugins = inventory_plugins
 
     def configure(self):
-        args = self.root_parser.parse_args(self.argv)
+        args, extra_args = self.root_parser.parse_known_args(self.argv)
         configure_logging(args)
 
-        logger.debug('cli args: %s', args)
+        logger.debug('cli args: %s, extra_args: %s', args, extra_args)
 
         # Bind some very useful dependencies
         self.console_args = cache(instance(args))
+        self.console_extra_args = cache(instance(extra_args))
         self.command = lambda c: self.console_args.command
         self.cluster_config_path = cache(
             lambda c: get_cluster_config_path(
@@ -144,7 +145,7 @@ def run(self):
         command_name = '%s_runner' % self.console_args.command
         runner_instance = self.get_instance(command_name)
 
-        return runner_instance.run(self.console_args)
+        return runner_instance.run(self.console_args, self.console_extra_args)
 
 
 def run(args=None):