From 27d02302598d4d979dfabf556c0d6ee1c043fc39 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Fri, 12 Jan 2018 13:39:43 -0800 Subject: [PATCH 1/7] Fast exit from debug_message --- lib/kubernetes-deploy/kubernetes_resource.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/kubernetes-deploy/kubernetes_resource.rb b/lib/kubernetes-deploy/kubernetes_resource.rb index 163db86ee..33bad9707 100644 --- a/lib/kubernetes-deploy/kubernetes_resource.rb +++ b/lib/kubernetes-deploy/kubernetes_resource.rb @@ -147,8 +147,6 @@ def sync_debug_info end def debug_message - sync_debug_info unless @debug_info_synced - helpful_info = [] if deploy_failed? helpful_info << ColorizedString.new("#{id}: FAILED").red @@ -164,6 +162,9 @@ def debug_message end helpful_info << " - Final status: #{status}" + return helpful_info.join("\n") if ENV['NO_DEBUG_MESSAGES'] + sync_debug_info unless @debug_info_synced + if @events.present? helpful_info << " - Events (common success events excluded):" @events.each do |identifier, event_hashes| From 55b4df2794565c62c02a24d74f1476e2f9083cba Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Fri, 12 Jan 2018 14:09:51 -0800 Subject: [PATCH 2/7] restructure --- lib/kubernetes-deploy/kubernetes_resource.rb | 52 +++++++++++--------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/lib/kubernetes-deploy/kubernetes_resource.rb b/lib/kubernetes-deploy/kubernetes_resource.rb index 33bad9707..11ad0d9e5 100644 --- a/lib/kubernetes-deploy/kubernetes_resource.rb +++ b/lib/kubernetes-deploy/kubernetes_resource.rb @@ -12,6 +12,7 @@ class KubernetesResource TIMEOUT = 5.minutes LOG_LINE_COUNT = 250 + NO_DEBUG_INFO_MESSAGE = "Event and log collection is disabled by the NO_DEBUG_INFO env var." DEBUG_RESOURCE_NOT_FOUND_MESSAGE = "None found. Please check your usual logging service (e.g. Splunk)." UNUSUAL_FAILURE_MESSAGE = <<~MSG It is very unusual for this resource type to fail to deploy. Please try the deploy again. @@ -141,12 +142,15 @@ def deploy_method end def sync_debug_info + return if ENV['NO_DEBUG_INFO'] @events = fetch_events @logs = fetch_logs if supports_logs? @debug_info_synced = true end def debug_message + sync_debug_info unless @debug_info_synced + helpful_info = [] if deploy_failed? helpful_info << ColorizedString.new("#{id}: FAILED").red @@ -162,37 +166,37 @@ def debug_message end helpful_info << " - Final status: #{status}" - return helpful_info.join("\n") if ENV['NO_DEBUG_MESSAGES'] - sync_debug_info unless @debug_info_synced - - if @events.present? - helpful_info << " - Events (common success events excluded):" - @events.each do |identifier, event_hashes| - event_hashes.each { |event| helpful_info << " [#{identifier}]\t#{event}" } - end + if ENV['NO_DEBUG_INFO'] + helpful_info << NO_DEBUG_INFO_MESSAGE else - helpful_info << " - Events: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" - end - - if supports_logs? - if @logs.blank? || @logs.values.all?(&:blank?) - helpful_info << " - Logs: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" + if @events.present? + helpful_info << " - Events (common success events excluded):" + @events.each do |identifier, event_hashes| + event_hashes.each { |event| helpful_info << " [#{identifier}]\t#{event}" } + end else - sorted_logs = @logs.sort_by { |_, log_lines| log_lines.length } - sorted_logs.each do |identifier, log_lines| - if log_lines.empty? - helpful_info << " - Logs from container '#{identifier}': #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" - next - end + helpful_info << " - Events: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" + end - helpful_info << " - Logs from container '#{identifier}' (last #{LOG_LINE_COUNT} lines shown):" - log_lines.each do |line| - helpful_info << " #{line}" + if supports_logs? + elsif @logs.blank? || @logs.values.all?(&:blank?) + helpful_info << " - Logs: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" + else + sorted_logs = @logs.sort_by { |_, log_lines| log_lines.length } + sorted_logs.each do |identifier, log_lines| + if log_lines.empty? + helpful_info << " - Logs from container '#{identifier}': #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" + next + end + + helpful_info << " - Logs from container '#{identifier}' (last #{LOG_LINE_COUNT} lines shown):" + log_lines.each do |line| + helpful_info << " #{line}" + end end end end end - helpful_info.join("\n") end From 5997abcb27edd053e2ad65a3151cf75656dec563 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Fri, 12 Jan 2018 14:46:22 -0800 Subject: [PATCH 3/7] Fix formatting and add a test --- lib/kubernetes-deploy/kubernetes_resource.rb | 46 ++++++++++--------- .../kubernetes_resource_test.rb | 12 +++++ 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/kubernetes-deploy/kubernetes_resource.rb b/lib/kubernetes-deploy/kubernetes_resource.rb index 11ad0d9e5..bb25a520b 100644 --- a/lib/kubernetes-deploy/kubernetes_resource.rb +++ b/lib/kubernetes-deploy/kubernetes_resource.rb @@ -168,35 +168,37 @@ def debug_message if ENV['NO_DEBUG_INFO'] helpful_info << NO_DEBUG_INFO_MESSAGE + return helpful_info.join("\n") + end + + if @events.present? + helpful_info << " - Events (common success events excluded):" + @events.each do |identifier, event_hashes| + event_hashes.each { |event| helpful_info << " [#{identifier}]\t#{event}" } + end else - if @events.present? - helpful_info << " - Events (common success events excluded):" - @events.each do |identifier, event_hashes| - event_hashes.each { |event| helpful_info << " [#{identifier}]\t#{event}" } - end + helpful_info << " - Events: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" + end + + if supports_logs? + if @logs.blank? || @logs.values.all?(&:blank?) + helpful_info << " - Logs: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" else - helpful_info << " - Events: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" - end + sorted_logs = @logs.sort_by { |_, log_lines| log_lines.length } + sorted_logs.each do |identifier, log_lines| + if log_lines.empty? + helpful_info << " - Logs from container '#{identifier}': #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" + next + end - if supports_logs? - elsif @logs.blank? || @logs.values.all?(&:blank?) - helpful_info << " - Logs: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" - else - sorted_logs = @logs.sort_by { |_, log_lines| log_lines.length } - sorted_logs.each do |identifier, log_lines| - if log_lines.empty? - helpful_info << " - Logs from container '#{identifier}': #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" - next - end - - helpful_info << " - Logs from container '#{identifier}' (last #{LOG_LINE_COUNT} lines shown):" - log_lines.each do |line| - helpful_info << " #{line}" - end + helpful_info << " - Logs from container '#{identifier}' (last #{LOG_LINE_COUNT} lines shown):" + log_lines.each do |line| + helpful_info << " #{line}" end end end end + helpful_info.join("\n") end diff --git a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb index d28467aa6..460935f18 100644 --- a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb +++ b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb @@ -183,6 +183,18 @@ def test_deploy_timed_out_respects_annotation_based_timeouts end end + def test_debug_message_with_no_debug_info + ENV['NO_DEBUG_INFO'] = 'true' + dummy = DummyResource.new + dummy.expects(:deploy_failed?).returns(true) + + assert_match dummy.debug_message, + "DummyResource/test: FAILED\n - Final status: Unknown\n" + + KubernetesDeploy::KubernetesResource::NO_DEBUG_INFO_MESSAGE + ensure + ENV['NO_DEBUG_INFO'] = nil + end + private def timeout_override_err_prefix From d61e3d826436feb2a2dfb2067fe8a20007797ee8 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Fri, 12 Jan 2018 15:01:03 -0800 Subject: [PATCH 4/7] make policial happy --- test/unit/kubernetes-deploy/kubernetes_resource_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb index 460935f18..02fd5e086 100644 --- a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb +++ b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb @@ -189,8 +189,8 @@ def test_debug_message_with_no_debug_info dummy.expects(:deploy_failed?).returns(true) assert_match dummy.debug_message, - "DummyResource/test: FAILED\n - Final status: Unknown\n" + - KubernetesDeploy::KubernetesResource::NO_DEBUG_INFO_MESSAGE + "DummyResource/test: FAILED\n - Final status: Unknown\n" + + KubernetesDeploy::KubernetesResource::NO_DEBUG_INFO_MESSAGE ensure ENV['NO_DEBUG_INFO'] = nil end From 06661fb7e93fabcdd2d4116e920a134a9bb854b1 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Wed, 17 Jan 2018 10:51:21 -0600 Subject: [PATCH 5/7] Disable logs and events seperately --- lib/kubernetes-deploy/kubernetes_resource.rb | 21 +++---- .../kubernetes_resource_test.rb | 56 +++++++++++++++---- 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/lib/kubernetes-deploy/kubernetes_resource.rb b/lib/kubernetes-deploy/kubernetes_resource.rb index bb25a520b..df29eb463 100644 --- a/lib/kubernetes-deploy/kubernetes_resource.rb +++ b/lib/kubernetes-deploy/kubernetes_resource.rb @@ -12,7 +12,10 @@ class KubernetesResource TIMEOUT = 5.minutes LOG_LINE_COUNT = 250 - NO_DEBUG_INFO_MESSAGE = "Event and log collection is disabled by the NO_DEBUG_INFO env var." + DISABLE_FETCHING_LOG_INFO = 'DISABLE_FETCHING_LOG_INFO' + DISABLE_FETCHING_EVENT_INFO = 'DISABLE_FETCHING_EVENT_INFO' + DISABLED_LOG_INFO_MESSAGE = "Log collection is disabled by the #{DISABLE_FETCHING_LOG_INFO} env var." + DISABLED_EVENT_INFO_MESSAGE = "Event collection is disabled by the #{DISABLE_FETCHING_EVENT_INFO} env var." DEBUG_RESOURCE_NOT_FOUND_MESSAGE = "None found. Please check your usual logging service (e.g. Splunk)." UNUSUAL_FAILURE_MESSAGE = <<~MSG It is very unusual for this resource type to fail to deploy. Please try the deploy again. @@ -142,9 +145,8 @@ def deploy_method end def sync_debug_info - return if ENV['NO_DEBUG_INFO'] - @events = fetch_events - @logs = fetch_logs if supports_logs? + @events = fetch_events unless ENV[DISABLE_FETCHING_EVENT_INFO] + @logs = fetch_logs if supports_logs? && !ENV[DISABLE_FETCHING_EVENT_INFO] @debug_info_synced = true end @@ -166,22 +168,21 @@ def debug_message end helpful_info << " - Final status: #{status}" - if ENV['NO_DEBUG_INFO'] - helpful_info << NO_DEBUG_INFO_MESSAGE - return helpful_info.join("\n") - end - if @events.present? helpful_info << " - Events (common success events excluded):" @events.each do |identifier, event_hashes| event_hashes.each { |event| helpful_info << " [#{identifier}]\t#{event}" } end + elsif ENV[DISABLE_FETCHING_EVENT_INFO] + helpful_info << DISABLED_EVENT_INFO_MESSAGE else helpful_info << " - Events: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" end if supports_logs? - if @logs.blank? || @logs.values.all?(&:blank?) + if ENV[DISABLE_FETCHING_LOG_INFO] + helpful_info << DISABLED_LOG_INFO_MESSAGE + elsif @logs.blank? || @logs.values.all?(&:blank?) helpful_info << " - Logs: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" else sorted_logs = @logs.sort_by { |_, log_lines| log_lines.length } diff --git a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb index 02fd5e086..727f76d6b 100644 --- a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb +++ b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb @@ -3,7 +3,7 @@ class KubernetesResourceTest < KubernetesDeploy::TestCase class DummyResource < KubernetesDeploy::KubernetesResource - attr_writer :succeeded + attr_writer :succeeded, :deploy_failed def initialize(definition_extras: {}) definition = { "kind" => "DummyResource", "metadata" => { "name" => "test" } }.merge(definition_extras) @@ -15,6 +15,18 @@ def exists? true end + def deploy_failed? + @deploy_failed + end + + def supports_logs? + true + end + + def fetch_logs + [] + end + def deploy_succeeded? @succeeded end @@ -183,20 +195,44 @@ def test_deploy_timed_out_respects_annotation_based_timeouts end end - def test_debug_message_with_no_debug_info - ENV['NO_DEBUG_INFO'] = 'true' - dummy = DummyResource.new - dummy.expects(:deploy_failed?).returns(true) + def test_debug_message_with_no_log_info + with_env(KubernetesDeploy::KubernetesResource::DISABLE_FETCHING_LOG_INFO, 'true') do + dummy = DummyResource.new + dummy.deploy_failed = true - assert_match dummy.debug_message, - "DummyResource/test: FAILED\n - Final status: Unknown\n" + - KubernetesDeploy::KubernetesResource::NO_DEBUG_INFO_MESSAGE - ensure - ENV['NO_DEBUG_INFO'] = nil + assert_includes dummy.debug_message, "DummyResource/test: FAILED\n - Final status: Unknown\n" + assert_includes dummy.debug_message, KubernetesDeploy::KubernetesResource::DISABLED_LOG_INFO_MESSAGE + refute_includes dummy.debug_message, "- Logs" + end + end + + def test_debug_message_with_no_event_info + with_env(KubernetesDeploy::KubernetesResource::DISABLE_FETCHING_EVENT_INFO, 'true') do + dummy = DummyResource.new + dummy.deploy_failed = true + + assert_includes dummy.debug_message, "DummyResource/test: FAILED\n - Final status: Unknown\n" + assert_includes dummy.debug_message, KubernetesDeploy::KubernetesResource::DISABLED_EVENT_INFO_MESSAGE + refute_includes dummy.debug_message, "- Events" + end end private + def with_env(key, value) + old_env_id = ENV[key] + + if value.nil? + ENV.delete(key) + else + ENV[key] = value.to_s + end + + yield + ensure + ENV[key] = old_env_id + end + def timeout_override_err_prefix "kubernetes-deploy.shopify.io/timeout-override annotation is invalid" end From fdb5ca38937f216309c2f703c87a52d3a83979df Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Thu, 18 Jan 2018 09:04:51 -0600 Subject: [PATCH 6/7] Make error message formatting match other messages --- lib/kubernetes-deploy/kubernetes_resource.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/kubernetes-deploy/kubernetes_resource.rb b/lib/kubernetes-deploy/kubernetes_resource.rb index df29eb463..6e2248a5f 100644 --- a/lib/kubernetes-deploy/kubernetes_resource.rb +++ b/lib/kubernetes-deploy/kubernetes_resource.rb @@ -14,8 +14,8 @@ class KubernetesResource DISABLE_FETCHING_LOG_INFO = 'DISABLE_FETCHING_LOG_INFO' DISABLE_FETCHING_EVENT_INFO = 'DISABLE_FETCHING_EVENT_INFO' - DISABLED_LOG_INFO_MESSAGE = "Log collection is disabled by the #{DISABLE_FETCHING_LOG_INFO} env var." - DISABLED_EVENT_INFO_MESSAGE = "Event collection is disabled by the #{DISABLE_FETCHING_EVENT_INFO} env var." + DISABLED_LOG_INFO_MESSAGE = "collection is disabled by the #{DISABLE_FETCHING_LOG_INFO} env var." + DISABLED_EVENT_INFO_MESSAGE = "collection is disabled by the #{DISABLE_FETCHING_EVENT_INFO} env var." DEBUG_RESOURCE_NOT_FOUND_MESSAGE = "None found. Please check your usual logging service (e.g. Splunk)." UNUSUAL_FAILURE_MESSAGE = <<~MSG It is very unusual for this resource type to fail to deploy. Please try the deploy again. @@ -174,14 +174,14 @@ def debug_message event_hashes.each { |event| helpful_info << " [#{identifier}]\t#{event}" } end elsif ENV[DISABLE_FETCHING_EVENT_INFO] - helpful_info << DISABLED_EVENT_INFO_MESSAGE + helpful_info << " - Events: #{DISABLED_EVENT_INFO_MESSAGE}" else helpful_info << " - Events: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" end if supports_logs? if ENV[DISABLE_FETCHING_LOG_INFO] - helpful_info << DISABLED_LOG_INFO_MESSAGE + helpful_info << " - Logs: #{DISABLED_LOG_INFO_MESSAGE}" elsif @logs.blank? || @logs.values.all?(&:blank?) helpful_info << " - Logs: #{DEBUG_RESOURCE_NOT_FOUND_MESSAGE}" else From ffe52a8c5129ba65a8275d93b30d0f1fe75d652d Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Thu, 18 Jan 2018 09:21:30 -0600 Subject: [PATCH 7/7] Fix tests --- test/unit/kubernetes-deploy/kubernetes_resource_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb index 727f76d6b..c6f06223d 100644 --- a/test/unit/kubernetes-deploy/kubernetes_resource_test.rb +++ b/test/unit/kubernetes-deploy/kubernetes_resource_test.rb @@ -202,7 +202,6 @@ def test_debug_message_with_no_log_info assert_includes dummy.debug_message, "DummyResource/test: FAILED\n - Final status: Unknown\n" assert_includes dummy.debug_message, KubernetesDeploy::KubernetesResource::DISABLED_LOG_INFO_MESSAGE - refute_includes dummy.debug_message, "- Logs" end end @@ -213,7 +212,6 @@ def test_debug_message_with_no_event_info assert_includes dummy.debug_message, "DummyResource/test: FAILED\n - Final status: Unknown\n" assert_includes dummy.debug_message, KubernetesDeploy::KubernetesResource::DISABLED_EVENT_INFO_MESSAGE - refute_includes dummy.debug_message, "- Events" end end