Merge pull request #564 from Shopify/revert-563-add-some-run-refactors

Revert "Remove `kubernetes-run` and slightly refactor `krane run`"

CHANGELOG.md

@@ -3,10 +3,6 @@
 *Important!*
 - The next release will be 1.0.0, which means that master will contain breaking changes.
 
-*Other*
-- **[Breaking change]** Simplify how the user passes environment variables to the `krane run` `--env-vars` argument, "`--env-vars KEY:value FOO:bar`" ([#563](https://github.com/Shopify/kubernetes-deploy/pull/563))
-- **[Breaking change]** Remove `kuberenetes-run`. Use `krane run` instead ([#563](https://github.com/Shopify/kubernetes-deploy/pull/563))
-
 ## 0.28.0
 
 *Enhancements*

exe/kubernetes-run

@@ -0,0 +1,42 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require 'kubernetes-deploy/runner_task'
+require 'kubernetes-deploy/options_helper'
+require 'optparse'
+
+template = "task-runner-template"
+entrypoint = nil
+env_vars = []
+verify_result = true
+max_watch_seconds = nil
+
+ARGV.options do |opts|
+  opts.on("--skip-wait", "Skip verification of pod success") { verify_result = false }
+  opts.on("--max-watch-seconds=seconds",
+    "Timeout error is raised if the pod runs for longer than the specified number of seconds") do |t|
+    max_watch_seconds = t.to_i
+  end
+  opts.on("--template=TEMPLATE") { |n| template = n }
+  opts.on("--env-vars=ENV_VARS") { |vars| env_vars = vars.split(",") }
+  opts.on("--entrypoint=ENTRYPOINT") { |c| entrypoint = [c] }
+  opts.parse!
+end
+
+namespace = ARGV[0]
+context = ARGV[1]
+
+runner = KubernetesDeploy::RunnerTask.new(
+  namespace: namespace,
+  context: context,
+  max_watch_seconds: max_watch_seconds
+)
+
+success = runner.run(
+  verify_result: verify_result,
+  task_template: template,
+  entrypoint: entrypoint,
+  args: ARGV[2..-1],
+  env_vars: env_vars
+)
+exit(1) unless success

lib/krane/cli/krane.rb

@@ -61,12 +61,6 @@ def rescue_and_exit
         exit(TIMEOUT_EXIT_CODE)
       rescue KubernetesDeploy::FatalDeploymentError
         exit(FAILURE_EXIT_CODE)
-      rescue KubernetesDeploy::DurationParser::ParsingError => e
-        STDERR.puts(<<~ERROR_MESSAGE)
-          Error parsing duration
-          #{e.message}. Duration must be a full ISO8601 duration or time value (e.g. 300s, 10m, 1h)
-        ERROR_MESSAGE
-        exit(FAILURE_EXIT_CODE)
       end
     end
   end

lib/krane/cli/run_command.rb

@@ -25,10 +25,10 @@ class RunCommand
           default: 'task-runner-template',
         },
         "env-vars" => {
-          type: :hash,
-          banner: "VAR:val FOO:bar",
-          desc: "A space-separated list of environment variables written as e.g. PORT:8000",
-          default: {},
+          type: :string,
+          banner: "VAR=val,FOO=bar",
+          desc: "A Comma-separated list of env vars",
+          default: '',
         },
       }
 
@@ -43,9 +43,9 @@ def self.from_options(namespace, context, options)
         runner.run!(
           verify_result: options['verify-result'],
           task_template: options['template'],
-          command: options['command'],
+          entrypoint: options['command'],
           args: options['arguments']&.split(" "),
-          env_vars: options['env-vars'],
+          env_vars: options['env-vars'].split(','),
         )
       end
     end

lib/kubernetes-deploy/runner_task.rb

@@ -31,7 +31,7 @@ def run(*args)
       false
     end
 
-    def run!(task_template:, command:, args:, env_vars: {}, verify_result: true)
+    def run!(task_template:, entrypoint:, args:, env_vars: [], verify_result: true)
       start = Time.now.utc
       @logger.reset
 
@@ -41,7 +41,7 @@ def run!(task_template:, command:, args:, env_vars: {}, verify_result: true)
       verify_config!(task_template, args)
       @logger.info("Using namespace '#{@namespace}' in context '#{@context}'")
 
-      pod = build_pod(task_template, command, args, env_vars, verify_result)
+      pod = build_pod(task_template, entrypoint, args, env_vars, verify_result)
       validate_pod(pod)
 
       @logger.phase_heading("Running pod")
@@ -79,11 +79,11 @@ def create_pod(pod)
       raise FatalDeploymentError, msg
     end
 
-    def build_pod(template_name, command, args, env_vars, verify_result)
+    def build_pod(template_name, entrypoint, args, env_vars, verify_result)
       task_template = get_template(template_name)
       @logger.info("Using template '#{template_name}'")
       pod_template = build_pod_definition(task_template)
-      set_container_overrides!(pod_template, command, args, env_vars)
+      set_container_overrides!(pod_template, entrypoint, args, env_vars)
       ensure_valid_restart_policy!(pod_template, verify_result)
       Pod.new(namespace: @namespace, context: @context, logger: @logger, stream_logs: true,
                     definition: pod_template.to_hash.deep_stringify_keys, statsd_tags: [])
@@ -146,18 +146,19 @@ def build_pod_definition(base_template)
       pod_definition
     end
 
-    def set_container_overrides!(pod_definition, command, args, env_vars)
+    def set_container_overrides!(pod_definition, entrypoint, args, env_vars)
       container = pod_definition.spec.containers.find { |cont| cont.name == 'task-runner' }
       if container.nil?
         message = "Pod spec does not contain a template container called 'task-runner'"
         @logger.summary.add_paragraph(message)
         raise TaskConfigurationError, message
       end
 
-      container.command = command if command
+      container.command = entrypoint if entrypoint
       container.args = args if args
 
-      env_args = env_vars.map do |key, value|
+      env_args = env_vars.map do |env|
+        key, value = env.split('=', 2)
         { name: key, value: value }
       end
       container.env ||= []

test/exe/run_test.rb

@@ -23,7 +23,7 @@ def test_run_parses_verify_result
   end
 
   def test_run_parses_command
-    set_krane_run_expectations(run_args: { command: %w(/bin/sh) })
+    set_krane_run_expectations(run_args: { entrypoint: %w(/bin/sh) })
     krane_run!(flags: '--command /bin/sh')
   end
 
@@ -38,8 +38,8 @@ def test_run_parses_template
   end
 
   def test_run_parses_env_vars
-    set_krane_run_expectations(run_args: { env_vars: { 'SOMETHING' => '8000', 'FOO' => 'bar' } })
-    krane_run!(flags: '--env-vars SOMETHING:8000 FOO:bar')
+    set_krane_run_expectations(run_args: { env_vars: %w(SOMETHING=8000 FOO=bar) })
+    krane_run!(flags: '--env-vars SOMETHING=8000,FOO=bar')
   end
 
   def test_run_failure_with_not_enough_arguments_as_black_box
@@ -49,20 +49,13 @@ def test_run_failure_with_not_enough_arguments_as_black_box
     assert_match("ERROR", err)
   end
 
-  def test_run_failure_with_too_many_args_as_black_box
+  def test_run_failure_with_too_many_args
     out, err, status = krane_black_box('run', 'ns ctx some_extra_arg')
     assert_equal(1, status.exitstatus)
     assert_empty(out)
     assert_match("ERROR", err)
   end
 
-  def test_run_failure_with_bad_timeout_as_black_box
-    out, err, status = krane_black_box('run', 'ns ctx --global-timeout=mittens')
-    assert_equal(1, status.exitstatus)
-    assert_empty(out)
-    assert_match("Error parsing duration", err)
-  end
-
   private
 
   def set_krane_run_expectations(new_args: {}, run_args: {})
@@ -94,9 +87,9 @@ def default_options(new_args = {}, run_args = {})
       run_args: {
         verify_result: true,
         task_template: 'task-runner-template',
-        command: nil,
+        entrypoint: nil,
         args: nil,
-        env_vars: {},
+        env_vars: [],
       }.merge(run_args),
     }
   end

test/helpers/task_runner_test_helper.rb

@@ -22,7 +22,7 @@ def build_task_runner(context: KubeclientHelper::TEST_CONTEXT, ns: @namespace, m
   def run_params(log_lines: 5, log_interval: 0.1, verify_result: true)
     {
       task_template: 'hello-cloud-template-runner',
-      command: ['/bin/sh', '-c'],
+      entrypoint: ['/bin/sh', '-c'],
       args: [
         "i=1; " \
         "while [ $i -le #{log_lines} ]; do " \

test/integration/runner_task_test.rb

@@ -207,9 +207,9 @@ def test_run_fails_if_namespace_is_missing
   def test_run_fails_if_task_template_is_blank
     task_runner = build_task_runner
     result = task_runner.run(task_template: '',
-      command: ['/bin/sh', '-c'],
+      entrypoint: ['/bin/sh', '-c'],
       args: nil,
-      env_vars: { MY_CUSTOM_VARIABLE: "MITTENS" })
+      env_vars: ["MY_CUSTOM_VARIABLE=MITTENS"])
     assert_task_run_failure(result)
 
     assert_logs_match_all([
@@ -225,9 +225,9 @@ def test_run_bang_fails_if_task_template_is_invalid
     task_runner = build_task_runner
     assert_raises(KubernetesDeploy::TaskConfigurationError) do
       task_runner.run!(task_template: '',
-        command: ['/bin/sh', '-c'],
+        entrypoint: ['/bin/sh', '-c'],
         args: nil,
-        env_vars: { MY_CUSTOM_VARIABLE: "MITTENS" })
+        env_vars: ["MY_CUSTOM_VARIABLE=MITTENS"])
     end
   end
 
@@ -272,9 +272,9 @@ def test_run_adds_env_vars_provided_to_the_task_container
     task_runner = build_task_runner
     result = task_runner.run(
       task_template: 'hello-cloud-template-runner',
-      command: ['/bin/sh', '-c'],
+      entrypoint: ['/bin/sh', '-c'],
       args: ['echo "The value is: $MY_CUSTOM_VARIABLE"'],
-      env_vars: { MY_CUSTOM_VARIABLE: "MITTENS", SOME_VARIABLE: "GLOVES" }
+      env_vars: ["MY_CUSTOM_VARIABLE=MITTENS"]
     )
     assert_task_run_success(result)