Merge pull request #2948 from zendesk/grosser/jobs

PAAS-2142 do not allow setting 0/2+ replicas for resources that do not support …

plugins/kubernetes/app/models/kubernetes/resource.rb

@@ -85,7 +85,7 @@ def uid
       end
 
       def desired_pod_count
-        replica_source.dig_fetch(:spec, :replicas)
+        @delete_resource ? 0 : replica_source.dig(:spec, :replicas) || 1
       end
 
       private
@@ -463,10 +463,6 @@ def deploy
         delete
         create
       end
-
-      def desired_pod_count
-        @delete_resource ? 0 : 1
-      end
     end
 
     class PodDisruptionBudget < Base

plugins/kubernetes/app/models/kubernetes/template_filler.rb

@@ -34,7 +34,11 @@ def to_hash(verification: false)
             set_history_limit
           end
 
-          set_replica_target unless ['DaemonSet', 'Pod'].include?(kind)
+          if ['StatefulSet', 'Deployment'].include?(kind)
+            set_replica_target
+          else
+            validate_replica_target_is_supported
+          end
 
           make_stateful_set_match_service if kind == 'StatefulSet'
           set_pre_stop if kind == 'Deployment'
@@ -262,6 +266,14 @@ def set_replica_target
       template.dig_set [:spec, :replicas], @doc.replica_target
     end
 
+    def validate_replica_target_is_supported
+      return if @doc.replica_target == 1 || (@doc.replica_target == 0 && @doc.delete_resource)
+      raise(
+        Samson::Hooks::UserError,
+        "A #{template[:kind]} can either have 1 replica or be marked for deletion."
+      )
+    end
+
     def set_name
       name = @doc.kubernetes_role.resource_name
       name += "-#{blue_green_color}" if blue_green_color

plugins/kubernetes/test/models/kubernetes/resource_test.rb

@@ -282,6 +282,16 @@ def delete_resource!
         resource.desired_pod_count.must_equal 2
       end
     end
+
+    it "is 1 when not set" do
+      template[:spec].delete :replicas
+      resource.desired_pod_count.must_equal 1
+    end
+
+    it "is 0 when pod is deleted" do
+      delete_resource!
+      resource.desired_pod_count.must_equal 0
+    end
   end
 
   describe "#loop_sleep" do
@@ -848,17 +858,6 @@ def deployment_stub(replica_count)
         end
       end
     end
-
-    describe "#desired_pod_count" do
-      it "is 1" do
-        resource.desired_pod_count.must_equal 1
-      end
-
-      it "is 0 when pod is deleted" do
-        delete_resource!
-        resource.desired_pod_count.must_equal 0
-      end
-    end
   end
 
   describe Kubernetes::Resource::HorizontalPodAutoscaler do

plugins/kubernetes/test/models/kubernetes/template_filler_test.rb

@@ -692,6 +692,7 @@ def secret_annotations(hash)
       before do
         raw_template[:kind] = 'DaemonSet'
         raw_template[:spec].delete(:replicas)
+        doc.replica_target = 1
       end
 
       it "does not add replicas since they are not supported" do
@@ -707,6 +708,7 @@ def secret_annotations(hash)
         raw_template[:metadata].merge!(original_metadata)
         raw_template[:kind] = "Pod"
         raw_template[:spec].delete :replicas
+        doc.replica_target = 1
       end
 
       it "fills out everything" do
@@ -722,11 +724,26 @@ def secret_annotations(hash)
         result = template.to_hash
         refute result[:spec].key?(:replicas)
       end
+
+      it "complains on invalid replica settings" do
+        doc.replica_target = 0
+        assert_raises(Samson::Hooks::UserError) { template.to_hash }
+
+        doc.replica_target = 2
+        assert_raises(Samson::Hooks::UserError) { template.to_hash }
+      end
+
+      it "allows deletion" do
+        doc.replica_target = 0
+        doc.delete_resource = true
+        template.to_hash
+      end
     end
 
     describe "cronjob" do
       before do
         raw_template.replace(YAML.safe_load(read_kubernetes_sample_file('kubernetes_cron_job.yml')).deep_symbolize_keys)
+        doc.replica_target = 1
       end
 
       it "works" do