Make task arguments match the new flag names

1.0-Upgrade.md

@@ -15,14 +15,85 @@ custom resources. See [blacklist](https://github.com/Shopify/kubernetes-deploy/b
 
 ## Public API changes
 
-Important changes:
-- Removed `KubernetesDeploy#*`: use the appropriate `Krane::*` class instead.
-- Removed `template_dir` from `DeployTask#initialize`: use `template_paths` instead.
-- Removed `allow_protected_ns` from `DeployTask#run`: use `protected_namespaces` instead.
-- Removed `template_dir` from `RenderTask#initialize` and `only_filenames` from `RenderTask#run`: use `template_paths` instead.
-- Removed `allow_globals` from `DeployTask`: use `GlobalDeployTask` instead.
-
-If you want to see the current state of this API, check the comment-based docs on these classes or the [rendered documentation at RubyGems.org](https://www.rubydoc.info/gems/kubernetes-deploy/1.0.0/KubernetesDeploy/DeployTask).
+Besides the renaming of the `KubernetesDeploy` namespace to `Krane`, we made the public interfaces of the major public classes (`DeployTask`, `RenderTask`, `RunnerTask`, and `RestartTask`) match the CLI flag names more closely.
+
+If you're curious about this API, check the comment-based docs on these classes or the [rendered documentation at RubyGems.org](https://www.rubydoc.info/gems/kubernetes-deploy/1.0.0/KubernetesDeploy/DeployTask).
+
+#### `DeployTask#new`
+
+Old name | New name | Comments
+--- | --- | ---
+template_paths | filenames |
+max_watch_seconds | global_timeout | This is now a duration that can be expressed in a more user-friendly language (e.g. `30s` or `1h`)
+template_dir | [none] | Removed in favour of `filenames`
+allow_globals | [none] | Removed. Use `GlobalDeployTask` instead
+
+#### `DeployTask#run`
+
+Old name | New name | Comments
+--- | --- | ---
+allow_protected_ns | [none] | This is now an implicit argument derived from the usage of `protected_namespaces` instead
+
+#### `RestartTask#new`
+
+Old name | New name | Comments
+--- | --- | ---
+max_watch_seconds | global_timeout | This is now a duration that can be expressed in a more user-friendly language (e.g. `30s` or `1h`)
+
+#### `RestartTask#run`
+
+Method signature changed from this:
+
+```ruby
+def run(deployments, selector:)
+```
+
+... to this, to maintain the convention of using keyword arguments for all the public interfaces:
+
+```ruby
+def run(deployments:, selector:)
+```
+
+#### `RunnerTask#new`
+
+Old name | New name | Comments
+--- | --- | ---
+max_watch_seconds | global_timeout | This is now a duration that can be expressed in a more user-friendly language (e.g. `30s` or `1h`)
+
+#### `RunnerTask#run`
+
+Old name | New name | Comments
+--- | --- | ---
+task_template | filename |
+entrypoint | command |
+args | arguments |
+
+
+#### `RenderTask#new`
+
+Old name | New name | Comments
+--- | --- | ---
+template_paths | filenames |
+max_watch_seconds | global_timeout | This is now a duration that can be expressed in a more user-friendly language (e.g. `30s` or `1h`)
+template_dir | [none] | Removed in favour of `filenames`
+
+#### `RenderTask#run`
+
+Old name | New name | Comments
+--- | --- | ---
+only_filenames | [none] | Removed in favour of `filenames` in `RenderTask#new`
+
+Method signature changed from this:
+
+```ruby
+def run(stream, only_filenames = [])
+```
+
+... to this, to maintain the convention of using keyword arguments for all the public interfaces:
+
+```ruby
+def run(stream:)
+```
 
 ## Command-line interface changes
 

lib/krane/cli/deploy_command.rb

@@ -68,10 +68,10 @@ def self.from_options(namespace, context, options)
             namespace: namespace,
             context: context,
             current_sha: options['current-sha'],
-            template_paths: paths,
+            filenames: paths,
             bindings: bindings_parser.parse,
             logger: logger,
-            max_watch_seconds: ::Krane::DurationParser.new(options["global-timeout"]).parse!.to_i,
+            global_timeout: ::Krane::DurationParser.new(options["global-timeout"]).parse!.to_i,
             selector: selector,
             protected_namespaces: protected_namespaces,
           )

lib/krane/cli/render_command.rb

@@ -27,12 +27,12 @@ def self.from_options(options)
         end
 
         ::Krane::OptionsHelper.with_processed_template_paths(filenames) do |paths|
-          runner = ::Krane::RenderTask.new(
+          renderer = ::Krane::RenderTask.new(
             current_sha: options['current-sha'],
-            template_paths: paths,
+            filenames: paths,
             bindings: bindings_parser.parse,
           )
-          runner.run!(STDOUT)
+          renderer.run!(stream: STDOUT)
         end
       end
     end

lib/krane/cli/restart_command.rb

@@ -21,10 +21,10 @@ def self.from_options(namespace, context, options)
         restart = ::Krane::RestartTask.new(
           namespace: namespace,
           context: context,
-          max_watch_seconds: ::Krane::DurationParser.new(options["global-timeout"]).parse!.to_i,
+          global_timeout: ::Krane::DurationParser.new(options["global-timeout"]).parse!.to_i,
         )
         restart.run!(
-          options[:deployments],
+          deployments: options[:deployments],
           selector: selector,
           verify_result: options["verify-result"]
         )

lib/krane/cli/run_command.rb

@@ -23,6 +23,7 @@ class RunCommand
           type: :string,
           desc: "The template file you'll be rendering",
           required: true,
+          aliases: :f,
         },
         "env-vars" => {
           type: :string,
@@ -37,14 +38,14 @@ def self.from_options(namespace, context, options)
         runner = ::Krane::RunnerTask.new(
           namespace: namespace,
           context: context,
-          max_watch_seconds: ::Krane::DurationParser.new(options["global-timeout"]).parse!.to_i,
+          global_timeout: ::Krane::DurationParser.new(options["global-timeout"]).parse!.to_i,
         )
 
         runner.run!(
           verify_result: options['verify-result'],
-          task_template: options['template'],
-          entrypoint: options['command'],
-          args: options['arguments']&.split(" "),
+          template: options['template'],
+          command: options['command'],
+          arguments: options['arguments']&.split(" "),
           env_vars: options['env-vars'].split(','),
         )
       end

lib/krane/deploy_task.rb

@@ -84,33 +84,31 @@ def server_version
 
     # Initializes the deploy task
     #
-    # @param namespace [String] Kubernetes namespace
-    # @param context [String] Kubernetes context
+    # @param namespace [String] Kubernetes namespace (*required*)
+    # @param context [String] Kubernetes context (*required*)
     # @param current_sha [String] The SHA of the commit
     # @param logger [Object] Logger object (defaults to an instance of Krane::FormattedLogger)
     # @param kubectl_instance [Kubectl] Kubectl instance
     # @param bindings [Hash] Bindings parsed by Krane::BindingsParser
-    # @param max_watch_seconds [Integer] Timeout in seconds
+    # @param global_timeout [Integer] Timeout in seconds
     # @param selector [Hash] Selector(s) parsed by Krane::LabelSelector
-    # @param template_paths [Array<String>] An array of template paths
+    # @param filenames [Array<String>] An array of filenames and/or directories containing templates (*required*)
     # @param protected_namespaces [Array<String>] Array of protected Kubernetes namespaces (defaults
     #   to Krane::DeployTask::PROTECTED_NAMESPACES)
     # @param render_erb [Boolean] Enable ERB rendering
     def initialize(namespace:, context:, current_sha:, logger: nil, kubectl_instance: nil, bindings: {},
-      max_watch_seconds: nil, selector: nil, template_paths: [], protected_namespaces: nil,
+      global_timeout: nil, selector: nil, filenames: [], protected_namespaces: nil,
       render_erb: true)
-      template_paths = (template_paths.map { |path| File.expand_path(path) }).compact
-
       @logger = logger || Krane::FormattedLogger.build(namespace, context)
-      @template_sets = TemplateSets.from_dirs_and_files(paths: template_paths, logger: @logger)
+      @template_sets = TemplateSets.from_dirs_and_files(paths: filenames, logger: @logger)
       @task_config = Krane::TaskConfig.new(context, namespace, @logger)
       @bindings = bindings
       @namespace = namespace
       @namespace_tags = []
       @context = context
       @current_sha = current_sha
       @kubectl = kubectl_instance
-      @max_watch_seconds = max_watch_seconds
+      @global_timeout = global_timeout
       @selector = selector
       @protected_namespaces = protected_namespaces || PROTECTED_NAMESPACES
       @render_erb = render_erb
@@ -185,7 +183,7 @@ def run!(verify_result: true, prune: true)
 
     def resource_deployer
       @resource_deployer ||= Krane::ResourceDeployer.new(task_config: @task_config,
-        prune_whitelist: prune_whitelist, max_watch_seconds: @max_watch_seconds,
+        prune_whitelist: prune_whitelist, global_timeout: @global_timeout,
         selector: @selector, statsd_tags: statsd_tags, current_sha: @current_sha)
     end
 

lib/krane/global_deploy_task.rb

@@ -29,10 +29,10 @@ class GlobalDeployTask
 
     # Initializes the deploy task
     #
-    # @param context [String] Kubernetes context
+    # @param context [String] Kubernetes context (*required*)
     # @param global_timeout [Integer] Timeout in seconds
-    # @param selector [Hash] Selector(s) parsed by Krane::LabelSelector
-    # @param template_paths [Array<String>] An array of template paths
+    # @param selector [Hash] Selector(s) parsed by Krane::LabelSelector (*required*)
+    # @param filenames [Array<String>] An array of filenames and/or directories containing templates (*required*)
     def initialize(context:, global_timeout: nil, selector: nil, filenames: [], logger: nil)
       template_paths = filenames.map { |path| File.expand_path(path) }
 
@@ -104,7 +104,7 @@ def run!(verify_result: true, prune: true)
 
     def deploy!(resources, verify_result, prune)
       resource_deployer = ResourceDeployer.new(task_config: @task_config,
-        prune_whitelist: prune_whitelist, max_watch_seconds: @global_timeout,
+        prune_whitelist: prune_whitelist, global_timeout: @global_timeout,
         selector: @selector, statsd_tags: statsd_tags)
       resource_deployer.deploy!(resources, verify_result, prune)
     end

lib/krane/render_task.rb

@@ -12,11 +12,11 @@ class RenderTask
     #
     # @param logger [Object] Logger object (defaults to an instance of Krane::FormattedLogger)
     # @param current_sha [String] The SHA of the commit
-    # @param template_paths [Array<String>] An array of template paths to render
+    # @param filenames [Array<String>] An array of filenames and/or directories containing templates (*required*)
     # @param bindings [Hash] Bindings parsed by Krane::BindingsParser
-    def initialize(logger: nil, current_sha:, template_paths: [], bindings:)
+    def initialize(logger: nil, current_sha:, filenames: [], bindings:)
       @logger = logger || Krane::FormattedLogger.build
-      @template_paths = template_paths.map { |path| File.expand_path(path) }
+      @filenames = filenames.map { |path| File.expand_path(path) }
       @bindings = bindings
       @current_sha = current_sha
     end
@@ -36,11 +36,11 @@ def run(*args)
     # @param stream [IO] Place to stream the output to
     #
     # @return [nil]
-    def run!(stream)
+    def run!(stream:)
       @logger.reset
       @logger.phase_heading("Initializing render task")
 
-      ts = TemplateSets.from_dirs_and_files(paths: @template_paths, logger: @logger)
+      ts = TemplateSets.from_dirs_and_files(paths: @filenames, logger: @logger)
 
       validate_configuration(ts)
       count = render_templates(stream, ts)
@@ -88,8 +88,8 @@ def write_to_stream(rendered_content, filename, stream)
     def validate_configuration(template_sets)
       @logger.info("Validating configuration")
       errors = []
-      if @template_paths.blank?
-        errors << "template_paths must be set"
+      if @filenames.blank?
+        errors << "filenames must be set"
       end
       errors += template_sets.validate
 

lib/krane/resource_deployer.rb

@@ -11,10 +11,10 @@ class ResourceDeployer
     delegate :logger, to: :@task_config
     attr_reader :statsd_tags
 
-    def initialize(task_config:, prune_whitelist:, max_watch_seconds:, current_sha: nil, selector:, statsd_tags:)
+    def initialize(task_config:, prune_whitelist:, global_timeout:, current_sha: nil, selector:, statsd_tags:)
       @task_config = task_config
       @prune_whitelist = prune_whitelist
-      @max_watch_seconds = max_watch_seconds
+      @global_timeout = global_timeout
       @current_sha = current_sha
       @selector = selector
       @statsd_tags = statsd_tags
@@ -119,7 +119,7 @@ def deploy_resources(resources, prune: false, verify:, record_summary: true)
 
       if verify
         watcher = Krane::ResourceWatcher.new(resources: resources, deploy_started_at: deploy_started_at,
-          timeout: @max_watch_seconds, task_config: @task_config, sha: @current_sha)
+          timeout: @global_timeout, task_config: @task_config, sha: @current_sha)
         watcher.run(record_summary: record_summary)
       end
     end

lib/krane/restart_task.rb

@@ -24,16 +24,16 @@ def initialize(deployment_name, response)
 
     # Initializes the restart task
     #
-    # @param context [String] Kubernetes context / cluster
-    # @param namespace [String] Kubernetes namespace
+    # @param context [String] Kubernetes context / cluster (*required*)
+    # @param namespace [String] Kubernetes namespace (*required*)
     # @param logger [Object] Logger object (defaults to an instance of Krane::FormattedLogger)
-    # @param max_watch_seconds [Integer] Timeout in seconds
-    def initialize(context:, namespace:, logger: nil, max_watch_seconds: nil)
+    # @param global_timeout [Integer] Timeout in seconds
+    def initialize(context:, namespace:, logger: nil, global_timeout: nil)
       @logger = logger || Krane::FormattedLogger.build(namespace, context)
       @task_config = Krane::TaskConfig.new(context, namespace, @logger)
       @context = context
       @namespace = namespace
-      @max_watch_seconds = max_watch_seconds
+      @global_timeout = global_timeout
     end
 
     # Runs the task, returning a boolean representing success or failure
@@ -49,18 +49,18 @@ def run(*args)
 
     # Runs the task, raising exceptions in case of issues
     #
-    # @param deployments_names [Array<String>] Array of workload names to restart
+    # @param deployments [Array<String>] Array of workload names to restart
     # @param selector [Hash] Selector(s) parsed by Krane::LabelSelector
     # @param verify_result [Boolean] Wait for completion and verify success
     #
     # @return [nil]
-    def run!(deployments_names = nil, selector: nil, verify_result: true)
+    def run!(deployments: nil, selector: nil, verify_result: true)
       start = Time.now.utc
       @logger.reset
 
       @logger.phase_heading("Initializing restart")
       verify_config!
-      deployments = identify_target_deployments(deployments_names, selector: selector)
+      deployments = identify_target_deployments(deployments, selector: selector)
 
       @logger.phase_heading("Triggering restart by touching ENV[RESTARTED_AT]")
       patch_kubeclient_deployments(deployments)
@@ -187,7 +187,7 @@ def build_patch_payload(deployment)
 
     def verify_restart(resources)
       ResourceWatcher.new(resources: resources, operation_name: "restart",
-        timeout: @max_watch_seconds, task_config: @task_config).run
+        timeout: @global_timeout, task_config: @task_config).run
       failed_resources = resources.reject(&:deploy_succeeded?)
       success = failed_resources.empty?
       if !success && failed_resources.all?(&:deploy_timed_out?)