PR Feedback 2

lib/kubernetes-deploy/errors.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 module KubernetesDeploy
   class FatalDeploymentError < StandardError; end
+  class FatalKubeAPIError < FatalDeploymentError; end
   class KubectlError < StandardError; end
 
   class InvalidTemplateError < FatalDeploymentError

lib/kubernetes-deploy/runner_task.rb

@@ -141,10 +141,10 @@ def get_template(template_name)
     rescue KubeException => error
       if error.error_code == 404
         msg = "Pod template `#{template_name}` not found in namespace `#{@namespace}`, context `#{@context}`"
-        @logger.summary.add_paragraph msg
+        @logger.summary.add_paragraph(msg)
         raise TaskTemplateMissingError, msg
       else
-        raise TaskConfigurationError, "Error communicating with the API server"
+        raise FatalKubeAPIError, "Error retrieving pod template: #{error.class.name}: #{error.message}"
       end
     end
 

test/helpers/env_test_helper.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 module EnvTestHelper
   def with_env(key, value)
-    old_env_id = ENV[key]
+    old_env_value = ENV[key]
 
     if value.nil?
       ENV.delete(key)
@@ -11,6 +11,6 @@ def with_env(key, value)
 
     yield
   ensure
-    ENV[key] = old_env_id
+    ENV[key] = old_env_value
   end
 end

test/integration/runner_task_test.rb

@@ -116,23 +116,28 @@ def test_run_with_verify_result_success
   def test_run_with_verify_result_neither_misses_nor_duplicates_logs_across_pollings
     deploy_task_template
     task_runner = build_task_runner
-    result = task_runner.run(run_params(log_lines: 5_000, log_interval: 0.0001))
+    result = task_runner.run(run_params(log_lines: 5_000, log_interval: 0.0005))
     assert_task_run_success(result)
 
     logging_assertion do |all_logs|
       nums_printed = all_logs.scan(/Line (\d+)$/).flatten
 
-      missing_nums = nums_printed - (1..5_000).map(&:to_s)
-      refute missing_nums.present?, "Some lines were not streamed: #{missing_nums}"
+      first_num_printed = nums_printed[0].to_i
+      # The first time we fetch logs, we grab at most 250 lines, so we likely won't print the first few hundred
+      assert first_num_printed < 1500, "Unexpected number of initial logs skipped (started with #{first_num_printed})"
+
+      expected_nums = (first_num_printed..5_000).map(&:to_s)
+      missing_nums = expected_nums - nums_printed.uniq
+      assert missing_nums.empty?, "Some lines were not streamed: #{missing_nums}"
 
       num_lines_duplicated = nums_printed.length - nums_printed.uniq.length
       assert num_lines_duplicated.zero?, "#{num_lines_duplicated} lines were duplicated"
     end
   end
 
   def test_run_with_bad_restart_policy
-    deploy_task_template do |f|
-      f["template-runner.yml"]["PodTemplate"].first["template"]["spec"]["restartPolicy"] = "OnFailure"
+    deploy_task_template do |fixtures|
+      fixtures["template-runner.yml"]["PodTemplate"].first["template"]["spec"]["restartPolicy"] = "OnFailure"
     end
 
     task_runner = build_task_runner
@@ -166,7 +171,7 @@ def test_run_bang_raises_exceptions_as_well_as_printing_failure
     assert_equal 1, pods.length, "Expected 1 pod to exist, found #{pods.length}"
   end
 
-  def test_run_with_missing_namespace
+  def test_run_fails_if_namespace_is_missing
     task_runner = build_task_runner(ns: "missing")
     assert_task_run_failure(task_runner.run(run_params))
 
@@ -227,14 +232,15 @@ def deploy_task_template(subset = ["template-runner.yml", "configmap-data.yml"])
   end
 
   def deploy_unschedulable_task_template
-    deploy_task_template do |f|
+    deploy_task_template do |fixtures|
       way_too_fat = {
         "requests" => {
           "cpu" => 1000,
           "memory" => "100Gi"
         }
       }
-      f["template-runner.yml"]["PodTemplate"].first["template"]["spec"]["containers"].first["resources"] = way_too_fat
+      template = fixtures["template-runner.yml"]["PodTemplate"].first["template"]
+      template["spec"]["containers"].first["resources"] = way_too_fat
     end
   end
 

test/test_helper.rb

@@ -214,7 +214,7 @@ def obj.run(*)
     private
 
     def log_to_stderr?
-      ENV["PRINT_LOGS"].to_s == "1"
+      ENV["PRINT_LOGS"] == "1"
     end
 
     def logging_assertion