allow not setting replicas for deployments

zendesk · May 3, 2023 · 3301c31 · 3301c31
1 parent c3faa03
commit 3301c31
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 29 deletions.
diff --git a/plugins/kubernetes/README.md b/plugins/kubernetes/README.md
@@ -201,6 +201,14 @@ Samson sets namespaces to the deploygroups `kubernetes_namespace` if no `metadat
 
 For namespace-less resources, set `metadata.namespace:` (which will result in `nil`)
 
+### Deployments without replicas
+
+When using a `HorizontalPodAutoscaler` for your `Deployment` or `StatefulSet`, it is recommended to not set `spec.replicas`.
+
+on the `Deployment`
+- set `metadata.annotations.samson/NoReplicas: "true"`
+- do not set `spec.replicas`
+
 ### Using custom resource names
 
 When not using project namespaces samson overrides each resource name in a particular role with the resource and service name set in the UI to prevent

diff --git a/plugins/kubernetes/app/models/kubernetes/template_filler.rb b/plugins/kubernetes/app/models/kubernetes/template_filler.rb
@@ -44,7 +44,11 @@ def to_hash(verification: false)
           make_stateful_set_match_service if kind == 'StatefulSet'
           set_pre_stop if kind == 'Deployment'
           set_name
-          (set_replica_target || validate_replica_target_is_supported) if kind != 'PodTemplate'
+          if ['Deployment', 'StatefulSet'].include?(kind)
+            set_replica_target
+          else
+            validate_replica_target_is_supported
+          end
           set_spec_template_metadata
           set_docker_image unless verification
           set_resource_usage
@@ -385,17 +389,11 @@ def set_init_containers
     end
 
     def set_replica_target
-      key = [:spec, :replicas]
-      target =
-        if ['StatefulSet', 'Deployment'].include?(template[:kind])
-          template
-        else
-          # custom resource that has replicas set on itself or it's template
-          templates = [template] + (template[:spec] || {}).values_at(*RoleConfigFile.template_keys(template))
-          templates.detect { |c| c.dig(*key) }
-        end
-
-      target&.dig_set key, @doc.replica_target
+      if template.dig(:metadata, :annotations, :"samson/NoReplicas")
+        raise Samson::Hooks::UserError, "Do not set spec.replicas with NoReplicas" if template.dig(:spec, :replicas)
+      else
+        template.dig_set [:spec, :replicas], @doc.replica_target
+      end
     end
 
     def validate_replica_target_is_supported

diff --git a/plugins/kubernetes/test/models/kubernetes/template_filler_test.rb b/plugins/kubernetes/test/models/kubernetes/template_filler_test.rb
@@ -136,16 +136,6 @@ def with_init_container(container)
       result.dig(:spec, :template, :metadata, :annotations, :"samson/deploy_url").must_be :blank?
     end
 
-    it "sets replicas for templates" do
-      raw_template[:apiVersion] = "zendesk.com/v1alpha1"
-      raw_template[:kind] = "ShardedDeployment"
-      raw_template[:spec].delete :replicas
-      raw_template[:spec][:template][:spec][:replicas] = 1
-      result = template.to_hash
-      result[:spec][:replicas].must_be_nil
-      result[:spec][:template][:spec][:replicas].must_equal 2
-    end
-
     it "does not fail when env/secrets are missing during deletion" do
       raw_template[:spec][:template][:metadata][:annotations] = {"secret/FOO": "bar"}
       doc.replica_target = 0
@@ -169,13 +159,6 @@ def with_init_container(container)
     end
 
     describe "name" do
-      it "sets name for unknown non-primary kinds" do
-        raw_template[:apiVersion] = "zendesk.com/v1alpha1"
-        raw_template[:kind] = "ShardedDeployment"
-        raw_template[:spec][:template][:spec].delete(:containers)
-        template.to_hash[:metadata][:name].must_equal "test-app-server"
-      end
-
       it "keeps resource name when keep_name is set" do
         raw_template[:metadata][:name] = "foobar"
         raw_template[:metadata][:annotations] = {"samson/keep_name": 'true'}
@@ -225,12 +208,30 @@ def with_init_container(container)
       end
 
       it "can read CRDs from currently deploying role" do
+        doc.replica_target = 1
         raw_template[:kind] = "MyCustomResource"
         doc.created_cluster_resources = {"MyCustomResource" => {"namespaced" => true}}
         template.to_hash[:metadata][:namespace].must_equal "pod1"
       end
     end
 
+    describe "NoReplicas" do
+      before do
+        raw_template[:spec].delete :replicas
+        raw_template[:metadata][:annotations] = {"samson/NoReplicas": "true"}
+      end
+
+      it "can set no replicas" do
+        template.to_hash[:spec].keys.wont_include :replicas
+      end
+
+      it "warns user when they send bad config" do
+        raw_template[:spec][:replicas] = 2
+        e = assert_raises(Samson::Hooks::UserError) { template.to_hash }
+        e.message.must_include "NoReplicas"
+      end
+    end
+
     describe "unique deployments" do
       let(:labels) do
         hash = template.to_hash
@@ -804,6 +805,7 @@ def secret_annotations(hash)
       let(:image) { build.docker_repo_digest }
 
       before do
+        doc.replica_target = 1
         raw_template.replace(
           YAML.safe_load(
             read_kubernetes_sample_file('kubernetes_podtemplate.yml')
@@ -917,6 +919,7 @@ def lifecycle_defined?
         end
 
         it "does not add preStop to DaemonSet" do
+          doc.replica_target = 1
           raw_template[:kind] = 'DaemonSet'
           refute lifecycle_defined?
         end