Merge pull request #258 from Shopify/fix_summary_actions

Fix action summaries with failures/timeouts

.rubocop.yml

@@ -3,3 +3,9 @@ inherit_from:
 
 AllCops:
   TargetRubyVersion: 2.3
+
+Style/TrailingCommaInArrayLiteral:
+  Enabled: false
+
+Style/TrailingCommaInHashLiteral:
+  Enabled: false

lib/kubernetes-deploy/deploy_task.rb

@@ -158,7 +158,7 @@ def run!(verify_result: true, allow_protected_ns: false, prune: true)
         failed_resources = resources.reject(&:deploy_succeeded?)
         success = failed_resources.empty?
         if !success && failed_resources.all?(&:deploy_timed_out?)
-          raise DeploymentTimeoutError, failed_resources
+          raise DeploymentTimeoutError
         end
         raise FatalDeploymentError unless success
       else
@@ -172,13 +172,12 @@ def run!(verify_result: true, allow_protected_ns: false, prune: true)
       end
       ::StatsD.measure('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:success")
       @logger.print_summary(:success)
-    rescue DeploymentTimeoutError => error
-      @logger.summary.add_action(error.message)
+    rescue DeploymentTimeoutError
       @logger.print_summary(:timed_out)
       ::StatsD.measure('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:timeout")
       raise
     rescue FatalDeploymentError => error
-      @logger.summary.add_action(error.message) if error.message.present?
+      @logger.summary.add_action(error.message) if error.message != error.class.to_s
       @logger.print_summary(:failure)
       ::StatsD.measure('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:failed")
       raise

lib/kubernetes-deploy/errors.rb

@@ -9,13 +9,7 @@ def initialize(name, context)
     end
   end
 
-  class DeploymentTimeoutError < FatalDeploymentError
-    attr_reader :resources
-    def initialize(resources)
-      @resources = resources
-      super("Timed out waiting for #{@resources.count} resources.")
-    end
-  end
+  class DeploymentTimeoutError < FatalDeploymentError; end
 
   module Errors
     extend self

lib/kubernetes-deploy/resource_watcher.rb

@@ -81,7 +81,18 @@ def record_statuses_for_summary(resources)
       end
 
       if fail_count > 0
-        @logger.summary.add_action("failed to #{@operation_name} #{fail_count} #{'resource'.pluralize(fail_count)}")
+        timeouts, failures = failed_resources.partition(&:deploy_timed_out?)
+        if timeouts.present?
+          @logger.summary.add_action(
+            "timed out waiting for #{timeouts.length} #{'resource'.pluralize(timeouts.length)} to #{@operation_name}"
+          )
+        end
+
+        if failures.present?
+          @logger.summary.add_action(
+            "failed to #{@operation_name} #{failures.length} #{'resource'.pluralize(failures.length)}"
+          )
+        end
         KubernetesDeploy::Concurrency.split_across_threads(failed_resources, &:sync_debug_info)
         failed_resources.each { |r| @logger.summary.add_paragraph(r.debug_message) }
       end

lib/kubernetes-deploy/restart_task.rb

@@ -52,19 +52,18 @@ def perform!(deployments_names = nil)
       failed_resources = resources.reject(&:deploy_succeeded?)
       success = failed_resources.empty?
       if !success && failed_resources.all?(&:deploy_timed_out?)
-        raise DeploymentTimeoutError, failed_resources
+        raise DeploymentTimeoutError
       end
       raise FatalDeploymentError unless success
       ::StatsD.measure('restart.duration', StatsD.duration(start), tags: tags('success', deployments))
       @logger.print_summary(:success)
-    rescue DeploymentTimeoutError => error
+    rescue DeploymentTimeoutError
       ::StatsD.measure('restart.duration', StatsD.duration(start), tags: tags('timeout', deployments))
-      @logger.summary.add_action(error.message)
       @logger.print_summary(:timed_out)
       raise
     rescue FatalDeploymentError => error
       ::StatsD.measure('restart.duration', StatsD.duration(start), tags: tags('failure', deployments))
-      @logger.summary.add_action(error.message) if error.message.present?
+      @logger.summary.add_action(error.message) if error.message != error.class.to_s
       @logger.print_summary(:failure)
       raise
     end

test/integration/kubernetes_deploy_test.rb

@@ -91,7 +91,10 @@ def test_refuses_deploy_to_protected_namespace_with_override_if_pruning_enabled
     generated_ns = @namespace
     @namespace = 'default'
     assert_deploy_failure(deploy_fixtures("hello-cloud", allow_protected_ns: true, prune: true))
-    assert_logs_match(/Refusing to deploy to protected namespace 'default' with pruning enabled/)
+    assert_logs_match_all([
+      "Configuration invalid",
+      "- Refusing to deploy to protected namespace 'default' with pruning enabled"
+    ], in_order: true)
   ensure
     @namespace = generated_ns
   end
@@ -100,7 +103,10 @@ def test_refuses_deploy_to_protected_namespace_without_override
     generated_ns = @namespace
     @namespace = 'default'
     assert_deploy_failure(deploy_fixtures("hello-cloud", prune: false))
-    assert_logs_match(/Refusing to deploy to protected namespace/)
+    assert_logs_match_all([
+      "Configuration invalid",
+      "- Refusing to deploy to protected namespace"
+    ], in_order: true)
   ensure
     @namespace = generated_ns
   end
@@ -301,6 +307,7 @@ def test_deployment_container_mounting_secret_that_does_not_exist_as_env_var_fai
     assert_deploy_failure(result)
 
     assert_logs_match_all([
+      "Failed to deploy 1 resource",
       "Deployment/web: FAILED",
       "The following containers are in a state that is unlikely to be recoverable:",
       "app: Failed to generate container configuration: secrets \"monitoring-token\" not found",
@@ -318,6 +325,7 @@ def test_bad_container_image_on_deployment_pod_fails_quickly
     assert_deploy_failure(result)
 
     assert_logs_match_all([
+      "Failed to deploy 1 resource",
       "Deployment/cannot-run: FAILED",
       "The following containers are in a state that is unlikely to be recoverable:",
       "container-cannot-run: Failed to pull image some-invalid-image:badtag.",
@@ -339,6 +347,7 @@ def test_crashing_container_on_deployment_fails_quickly
     assert_deploy_failure(deploy_fixtures("invalid", subset: ["crash_loop.yml"]))
 
     assert_logs_match_all([
+      "Failed to deploy 1 resource",
       "Deployment/crash-loop: FAILED",
       "The following containers are in a state that is unlikely to be recoverable:",
       "crash-loop-back-off: Crashing repeatedly (exit 1). See logs for more information.",
@@ -350,6 +359,7 @@ def test_unrunnable_container_on_deployment_pod_fails_quickly
     assert_deploy_failure(deploy_fixtures("invalid", subset: ["cannot_run.yml"]))
 
     assert_logs_match_all([
+      "Failed to deploy 1 resource",
       "Deployment/cannot-run: FAILED",
       "The following containers are in a state that is unlikely to be recoverable:",
       %r{container-cannot-run: Failed to start \(exit 127\): .*/some/bad/path},
@@ -385,6 +395,7 @@ def test_deployment_with_progress_times_out_for_short_duration
     assert_deploy_failure(result, :timed_out)
 
     assert_logs_match_all([
+      "Successfully deployed 1 resource and timed out waiting for 1 resource to deploy",
       'Deployment/undying: TIMED OUT (progress deadline: 10s)',
       'Timeout reason: ProgressDeadlineExceeded'
     ])
@@ -442,7 +453,7 @@ def test_create_ejson_secrets_with_malformed_secret_data
       fixtures["secrets.ejson"]["kubernetes_secrets"]["monitoring-token"]["data"] = malformed
     end
     assert_deploy_failure(result)
-    assert_logs_match(/data for secret monitoring-token was invalid/)
+    assert_logs_match("Creation of kubernetes secrets from ejson failed: data for secret monitoring-token was invalid")
   end
 
   def test_pruning_of_secrets_created_from_ejson
@@ -489,8 +500,20 @@ def test_pruning_of_existing_managed_secrets_when_ejson_file_has_been_deleted
   end
 
   def test_deploy_result_logging_for_mixed_result_deploy
-    result = deploy_fixtures("invalid", subset: ["bad_probe.yml", "init_crash.yml", "missing_volumes.yml"])
+    result = deploy_fixtures("invalid", subset: ["bad_probe.yml", "init_crash.yml", "missing_volumes.yml"]) do |f|
+      f["bad_probe.yml"]["ConfigMap"] = {
+        "apiVersion" => "v1",
+        "kind" => "ConfigMap",
+        "metadata" => { "name" => "test" },
+        "data" => { "datapoint1" => "value1" }
+      }
+    end
     assert_deploy_failure(result)
+    assert_logs_match_all([
+      "Successfully deployed 1 resource, timed out waiting for 2 resources to deploy, and failed to deploy 1 resource",
+      "Successful resources",
+      %r{ConfigMap/test\s+Available}
+    ], in_order: true)
 
     # Debug info for bad probe timeout
     assert_logs_match_all([
@@ -706,6 +729,7 @@ def test_output_when_unmanaged_pod_preexists
   def test_bad_container_on_daemon_sets_fails
     assert_deploy_failure(deploy_fixtures("invalid", subset: ["crash_loop_daemon_set.yml"]))
     assert_logs_match_all([
+      "Failed to deploy 1 resource",
       "DaemonSet/crash-loop: FAILED",
       "crash-loop-back-off: Crashing repeatedly (exit 1). See logs for more information.",
       "Final status: 1 currentNumberScheduled, 1 desiredNumberScheduled, 0 numberReady",
@@ -728,6 +752,7 @@ def test_bad_container_on_stateful_sets_fails_with_rolling_update
 
     assert_deploy_failure(result)
     assert_logs_match_all([
+      "Failed to deploy 1 resource",
       "StatefulSet/stateful-busybox: FAILED",
       "app: Crashing repeatedly (exit 1). See logs for more information.",
       "Events (common success events excluded):",
@@ -770,6 +795,7 @@ def test_resource_quotas_are_deployed_first
       "Predeploying priority resources",
       "Deploying ResourceQuota/resource-quotas (timeout: 30s)",
       "Deployment/web rollout timed out",
+      "Successfully deployed 1 resource and timed out waiting for 1 resource to deploy",
       "Successful resources",
       "ResourceQuota/resource-quotas",
       %r{Deployment/web: TIMED OUT \(progress deadline: \d+s\)},

test/integration/restart_task_test.rb

@@ -180,7 +180,7 @@ def test_restart_failure
       "Triggered `web` restart",
       "Deployment/web rollout timed out",
       "Result: TIMED OUT",
-      "Failed to restart 1 resource",
+      "Timed out waiting for 1 resource to restart",
       "Deployment/web: TIMED OUT",
       "The following containers have not passed their readiness probes",
       "app must exit 0 from the following command",