From 1ae441ee59b86e4a0399b2b57943ab5a9b396f00 Mon Sep 17 00:00:00 2001 From: timothysmith0609 Date: Wed, 20 Feb 2019 09:18:38 -0500 Subject: [PATCH] Different approach with yielded deploy dir prevent hanging waiting for stream. Tidyup renames More flexible args handling remove debug gem OptionsHelper as singleton class cop most important method at top of file Tests, OptionsError, refactors Automatic corrections for lib/kubernetes-deploy/options_helper.rb Automatic corrections for test/unit/kubernetes-deploy/options_helper_test.rb fix test police Don't use temp file if only using a single template dir and no additional input tidier --- exe/kubernetes-deploy | 49 +++++----- exe/kubernetes-render | 37 +++----- lib/kubernetes-deploy/options_helper.rb | 68 ++++++++----- .../kubernetes-deploy/options_helper_test.rb | 95 +++++++++++++++++++ 4 files changed, 173 insertions(+), 76 deletions(-) create mode 100644 test/unit/kubernetes-deploy/options_helper_test.rb diff --git a/exe/kubernetes-deploy b/exe/kubernetes-deploy index 4ccbc5a23..8161e1b9e 100755 --- a/exe/kubernetes-deploy +++ b/exe/kubernetes-deploy @@ -3,12 +3,9 @@ require 'kubernetes-deploy' require 'optparse' -require 'tmpdir' -require 'fileutils' skip_wait = false -use_stdin = false -template_dir = nil +template_dirs = [] allow_protected_ns = false prune = true bindings = {} @@ -25,8 +22,11 @@ ARGV.options do |opts| prot_ns = KubernetesDeploy::DeployTask::PROTECTED_NAMESPACES.join(', ') opts.on("--allow-protected-ns", "Enable deploys to #{prot_ns}; requires --no-prune") { allow_protected_ns = true } opts.on("--no-prune", "Disable deletion of resources that do not appear in the template dir") { prune = false } - opts.on("--template-dir=DIR", "Set the template dir (default: config/deploy/$ENVIRONMENT)") { |v| template_dir = v } - opts.on("--use-stdin", "Use $stdin instead of a template dir") { use_stdin = true } + opts.on("--template-dir=DIR", + "Set the template dir (default: config/deploy/$ENVIRONMENT). " \ + "Set this flag multiple times to use templates from different directories" ) do |d| + template_dirs << d + end opts.on("--verbose-log-prefix", "Add [context][namespace] to the log prefix") { verbose_log_prefix = true } opts.on("--max-watch-seconds=seconds", "Timeout error is raised if it takes longer than the specified number of seconds") do |t| @@ -44,19 +44,12 @@ ARGV.options do |opts| opts.parse! end -if use_stdin && template_dir - puts "The flags --use-stdin and --template-dir cannot be combined" - exit(1) -end - -namespace = ARGV[0] -context = ARGV[1] +namespace = ARGV.shift +context = ARGV.shift revision = KubernetesDeploy::OptionsHelper.revision_from_environment logger = KubernetesDeploy::FormattedLogger.build(namespace, context, verbose_prefix: verbose_log_prefix) -begin - template_dir = KubernetesDeploy::OptionsHelper.default_and_check_template_dir(template_dir, use_stdin: use_stdin) - +KubernetesDeploy::OptionsHelper.with_consolidated_template_dir(template_dirs) do |template_dir| runner = KubernetesDeploy::DeployTask.new( namespace: namespace, context: context, @@ -67,15 +60,17 @@ begin max_watch_seconds: max_watch_seconds ) - runner.run!( - verify_result: !skip_wait, - allow_protected_ns: allow_protected_ns, - prune: prune - ) -rescue KubernetesDeploy::DeploymentTimeoutError - exit(70) -rescue KubernetesDeploy::FatalDeploymentError - exit(1) -ensure - FileUtils.rm_r(Dir.glob(File.expand_path(template_dir))) if use_stdin + begin + runner.run!( + verify_result: !skip_wait, + allow_protected_ns: allow_protected_ns, + prune: prune + ) + rescue KubernetesDeploy::DeploymentTimeoutError + exit(70) + rescue KubernetesDeploy::FatalDeploymentError + exit(1) + rescue KubernetesDeploy::OptionsHelper::OptionsError + exit(1) + end end diff --git a/exe/kubernetes-render b/exe/kubernetes-render index 76a4d4fed..c77a1c5a5 100755 --- a/exe/kubernetes-render +++ b/exe/kubernetes-render @@ -4,12 +4,8 @@ require 'kubernetes-deploy' require 'kubernetes-deploy/render_task' require 'optparse' -require 'tmpdir' -require 'fileutils' template_dir = nil -templates = [] -use_stdin = false bindings = {} ARGV.options do |opts| @@ -18,30 +14,19 @@ ARGV.options do |opts| bindings.merge!(KubernetesDeploy::BindingsParser.parse(binds)) end opts.on("--template-dir=DIR", "Set the template dir (default: config/deploy/$ENVIRONMENT)") { |v| template_dir = v } - opts.on("--use-stdin", "Use $stdin instead of a template dir") { use_stdin = true } opts.parse! end -if template_dir && use_stdin - puts "The flags --use-stdin and --template-dir cannot be combined" - exit(1) -end - -begin - template_dir = KubernetesDeploy::OptionsHelper.default_and_check_template_dir(template_dir, use_stdin: use_stdin) - templates = ARGV if template_dir || !use_stdin +templates = ARGV +logger = KubernetesDeploy::FormattedLogger.build(verbose_prefix: false) +revision = KubernetesDeploy::OptionsHelper.revision_from_environment - logger = KubernetesDeploy::FormattedLogger.build(verbose_prefix: false) - revision = KubernetesDeploy::OptionsHelper.revision_from_environment +runner = KubernetesDeploy::RenderTask.new( + logger: logger, + current_sha: revision, + template_dir: template_dir, + bindings: bindings, +) - runner = KubernetesDeploy::RenderTask.new( - logger: logger, - current_sha: revision, - template_dir: template_dir, - bindings: bindings, - ) - success = runner.run(STDOUT, templates) - exit(1) unless success -ensure - FileUtils.rm_r(Dir.glob(File.expand_path(template_dir))) if use_stdin -end +success = runner.run(STDOUT, templates) +exit(1) unless success diff --git a/lib/kubernetes-deploy/options_helper.rb b/lib/kubernetes-deploy/options_helper.rb index d7db340df..be0f3c376 100644 --- a/lib/kubernetes-deploy/options_helper.rb +++ b/lib/kubernetes-deploy/options_helper.rb @@ -2,36 +2,58 @@ module KubernetesDeploy module OptionsHelper - def self.default_and_check_template_dir(template_dir, use_stdin: false) - if use_stdin - if $stdin.tty? - puts "Nothing to read from $stdin" - exit(1) - end + class OptionsError < StandardError; end - template_dir = Dir.mktmpdir("kubernetes-deploy") - input = $stdin.readlines.join("") - File.open("#{File.expand_path(template_dir)}/templates.yml.erb", 'w+') { |f| f.print(input) } - return template_dir + STDIN_TEMP_FILE = "from_stdin.yml.erb" + class << self + def with_consolidated_template_dir(template_dirs) + if template_dirs.length <= 1 && ARGV.empty? && $stdin.tty? + yield default_template_dir(template_dirs.first) + else + Dir.mktmpdir("kubernetes-deploy") do |dir| + populate_temp_dir(temp_dir: dir, template_dirs: template_dirs) + yield dir + end + end end - if !template_dir && ENV.key?("ENVIRONMENT") - template_dir = "config/deploy/#{ENV['ENVIRONMENT']}" + def revision_from_environment + ENV.fetch('REVISION') do + raise OptionsError, "ENV['REVISION'] is missing. Please specify the commit SHA" + end end - if !template_dir || template_dir.empty? - puts "Template directory is unknown. " \ - "Either specify --template-dir argument or set $ENVIRONMENT to use config/deploy/$ENVIRONMENT " \ - + "as a default path." - exit(1) + + private + + def default_template_dir(template_dir) + if ENV.key?("ENVIRONMENT") + template_dir = "config/deploy/#{ENV['ENVIRONMENT']}" + end + + if !template_dir || template_dir.empty? + raise OptionsError, "Template directory is unknown. " \ + "Either specify --template-dir argument or set $ENVIRONMENT to use config/deploy/$ENVIRONMENT " \ + "as a default path." + end + + template_dir end - template_dir - end + def populate_temp_dir(temp_dir:, template_dirs:) + File.open("#{temp_dir}/#{STDIN_TEMP_FILE}", 'w+') { |f| f.print(ARGF.read) } unless $stdin.tty? - def self.revision_from_environment - ENV.fetch('REVISION') do - puts "ENV['REVISION'] is missing. Please specify the commit SHA" - exit 1 + template_dirs.each do |template_dir| + template_dir = File.expand_path(template_dir) + templates = Dir.entries(template_dir).reject { |f| f =~ /^\.{1,2}$/ } + templates.each do |template| + next if File.directory?(File.expand_path("#{template_dir}/#{template}")) + File.open("#{temp_dir}/#{template_dir.tr('/', '_')}_#{template}", 'w+') do |f| + f.print(File.read("#{template_dir}/#{template}")) + end + end + end + rescue IOError, Errno::ENOENT => e + raise OptionsError, e.message end end end diff --git a/test/unit/kubernetes-deploy/options_helper_test.rb b/test/unit/kubernetes-deploy/options_helper_test.rb new file mode 100644 index 000000000..b7592d2fc --- /dev/null +++ b/test/unit/kubernetes-deploy/options_helper_test.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true +require 'test_helper' +require 'tempfile' + +class OptionsHelperTest < KubernetesDeploy::TestCase + def test_missing_template_dir_no_extra_input + assert_raises(KubernetesDeploy::OptionsHelper::OptionsError) do + KubernetesDeploy::OptionsHelper.with_consolidated_template_dir([]) do + end + end + end + + def test_no_template_dir_with_stdin + old_stdin = $stdin + + input = Tempfile.open("kubernetes_deploy_test") + fixture_path_entries = Dir.glob("#{fixture_path('hello-cloud')}/*.{yml,yaml}*") + fixture_path_entries.each do |filename| + File.open(filename, 'r') { |f| input.print(f.read) } + end + input.rewind + $stdin = input + + KubernetesDeploy::OptionsHelper.with_consolidated_template_dir([]) do |template_dir| + split_templates = File.read( + File.join(template_dir, KubernetesDeploy::OptionsHelper::STDIN_TEMP_FILE) + ).split("\n---\n") + refute split_templates.empty? + split_templates.each do |template| + assert(YAML.safe_load(template)) + end + end + ensure + $stdin = old_stdin + end + + def test_template_dir_with_stdin + old_stdin = $stdin + + input = Tempfile.open("kubernetes_deploy_test") + fixture_path_entries = Dir.glob("#{fixture_path('hello-cloud')}/*.{yml,yaml}*") + fixture_path_entries.each do |filename| + File.open(filename, 'r') { |f| input.print(f.read) } + end + input.rewind + $stdin = input + + KubernetesDeploy::OptionsHelper.with_consolidated_template_dir([fixture_path('hello-cloud')]) do |template_dir| + split_templates = File.read( + File.join(template_dir, KubernetesDeploy::OptionsHelper::STDIN_TEMP_FILE) + ).split("\n---\n") + refute split_templates.empty? + split_templates.each do |template| + assert(YAML.safe_load(template)) + end + + fixture_path_entries = Dir.entries(fixture_path('hello-cloud')).reject { |f| f =~ /^\.{1,2}$/ } + template_dir_entries = Dir.entries(template_dir).reject do |f| + f == KubernetesDeploy::OptionsHelper::STDIN_TEMP_FILE || f =~ /^\.{1,2}$/ + end + + refute(template_dir_entries.empty?) + assert_equal(template_dir_entries.length, fixture_path_entries.length) + fixture_path_entries.each do |fixture| + template = template_dir_entries.find { |t| t.include?(fixture) } + assert(template) + assert_equal( + YAML.safe_load(File.read(File.join(template_dir, template))), + YAML.safe_load(File.read(File.join(fixture_path('hello-cloud'), fixture))) + ) + end + end + ensure + $stdin = old_stdin + end + + def test_single_template_dir_only + KubernetesDeploy::OptionsHelper.with_consolidated_template_dir([fixture_path('hello-cloud')]) do |template_dir| + assert_equal(fixture_path('hello-cloud'), template_dir) + end + end + + def test_multiple_template_dirs + template_dirs = [fixture_path('hello-cloud'), fixture_path('partials')] + KubernetesDeploy::OptionsHelper.with_consolidated_template_dir(template_dirs) do |template_dir| + fixture_path_entries = template_dirs.collect { |dir| Dir.entries(dir) }.flatten.uniq + template_dir_entries = Dir.entries(template_dir) + assert_equal(fixture_path_entries.length, template_dir_entries.length) + fixture_path_entries.each do |fixture| + next if fixture =~ /^\.{1,2}$/ + assert template_dir_entries.index { |s| s.include?(fixture) } + end + end + end +end