From 3301c31aa11e55b2572a019b06e3c783c28ba74b Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Tue, 2 May 2023 17:03:34 -0700 Subject: [PATCH] allow not setting replicas for deployments --- plugins/kubernetes/README.md | 8 ++++ .../app/models/kubernetes/template_filler.rb | 22 +++++------ .../models/kubernetes/template_filler_test.rb | 37 ++++++++++--------- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/plugins/kubernetes/README.md b/plugins/kubernetes/README.md index ad08926acf..1d84467db8 100644 --- 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 index d25ad9e79a..eb9a3bdf49 100644 --- 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 index e10661ae18..ac6a615d54 100644 --- 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