From 8f46ab3fdf41572f42d1a5113cccf3737eccf77d Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Thu, 16 Nov 2017 14:50:59 -0600 Subject: [PATCH 1/6] partial-rollout-successby using an annotation add a restart test and unit tests Docs are nice --- README.md | 11 +- .../kubernetes_resource/deployment.rb | 71 +++++++++-- .../kubernetes_resource/replica_set.rb | 8 +- test/fixtures/slow-cloud/web.yml.erb | 37 ++++++ test/integration/kubernetes_deploy_test.rb | 12 ++ test/integration/restart_task_test.rb | 24 ++++ .../kubernetes_resource/deployment_test.rb | 119 ++++++++++++++++++ 7 files changed, 271 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/slow-cloud/web.yml.erb create mode 100644 test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb diff --git a/README.md b/README.md index c8f0438f3..39438997a 100644 --- a/README.md +++ b/README.md @@ -122,10 +122,19 @@ You can add additional variables using the `--bindings=BINDINGS` option. For exa ### Customizing behaviour with annotations - - `kubernetes-deploy.shopify.io/timeout-override`: Override the tool's hard timeout for one specific resource. Both full ISO8601 durations and the time portion of ISO8601 durations are valid. Value must be between 1 second and 24 hours. - _Example values_: 45s / 3m / 1h / PT0.25H - _Compatibility_: all resource types (Note: `Deployment` timeouts are based on `spec.progressDeadlineSeconds` if present, and that field has a default value as of the `apps/v1beta1` group version. Using this annotation will have no effect on `Deployment`s that time out with "Timeout reason: ProgressDeadlineExceeded".) +- `kubernetes-deploy.shopify.io/required-rollout`: Modifies how much of the rollout needs to finish +before the deployment is considered successful. + - _Compatibility_: Deployment + - `full`: The deploy is successful when all pods in the new ReplicaSet are ready. + - `none`: The deploy is successful as soon as the new `replicaSet` is created for the deploy + - `maxUnavailable`: The deploy is successful when minimum availability is reached in the new ReplicaSet. + In other words, the number of new pods that must be ready is equal to `spec.replicas` - `strategy.RollingUpdate.maxUnavailable` + (converted from percentages by rounding up, if applicable). This option is only valid for deployments + that use the `RollingUpdate` strategy. + ### Running tasks at the beginning of a deploy diff --git a/lib/kubernetes-deploy/kubernetes_resource/deployment.rb b/lib/kubernetes-deploy/kubernetes_resource/deployment.rb index 54be13008..a6028836b 100644 --- a/lib/kubernetes-deploy/kubernetes_resource/deployment.rb +++ b/lib/kubernetes-deploy/kubernetes_resource/deployment.rb @@ -2,6 +2,9 @@ module KubernetesDeploy class Deployment < KubernetesResource TIMEOUT = 7.minutes + REQUIRED_ROLLOUT_ANNOTATION = 'kubernetes-deploy.shopify.io/required-rollout' + REQUIRED_ROLLOUT_TYPES = %w(maxUnavailable full none).freeze + DEFAULT_REQUIRED_ROLLOUT = 'full' def sync raw_json, _err, st = kubectl.run("get", type, @name, "--output=json") @@ -19,6 +22,7 @@ def sync conditions = deployment_data.fetch("status", {}).fetch("conditions", []) @progress_condition = conditions.find { |condition| condition['type'] == 'Progressing' } @progress_deadline = deployment_data['spec']['progressDeadlineSeconds'] + @max_unavailable = deployment_data.dig('spec', 'strategy', 'rollingUpdate', 'maxUnavailable') else # reset @latest_rs = nil @rollout_data = { "replicas" => 0 } @@ -26,6 +30,7 @@ def sync @progress_condition = nil @progress_deadline = @definition['spec']['progressDeadlineSeconds'] @desired_replicas = -1 + @max_unavailable = @definition.dig('spec', 'strategy', 'rollingUpdate', 'maxUnavailable') end end @@ -43,10 +48,24 @@ def fetch_logs def deploy_succeeded? return false unless @latest_rs.present? - @latest_rs.deploy_succeeded? && - @latest_rs.desired_replicas == @desired_replicas && # latest RS fully scaled up - @rollout_data["updatedReplicas"].to_i == @desired_replicas && - @rollout_data["updatedReplicas"].to_i == @rollout_data["availableReplicas"].to_i + case required_rollout + when 'full' + @latest_rs.deploy_succeeded? && + @latest_rs.desired_replicas == @desired_replicas && # latest RS fully scaled up + @rollout_data["updatedReplicas"].to_i == @desired_replicas && + @rollout_data["updatedReplicas"].to_i == @rollout_data["availableReplicas"].to_i + when 'none' + true + when 'maxUnavailable' + minimum_needed = min_available_replicas + + @latest_rs.desired_replicas >= minimum_needed && + @latest_rs.ready_replicas >= minimum_needed && + @latest_rs.available_replicas >= minimum_needed + else + raise "#{REQUIRED_ROLLOUT_ANNOTATION}:#{required_rollout} is invalid "\ + " Acceptable options: #{REQUIRED_ROLLOUT_TYPES.join(',')}" + end end def deploy_failed? @@ -81,6 +100,23 @@ def exists? @found end + def validate_definition + super + + unless REQUIRED_ROLLOUT_TYPES.include?(required_rollout) + @validation_errors << "#{REQUIRED_ROLLOUT_ANNOTATION}:#{required_rollout} is invalid "\ + "Acceptable options: #{REQUIRED_ROLLOUT_TYPES.join(',')}" + end + + if required_rollout.downcase == 'maxunavailable' && @definition.dig('spec', 'strategy').respond_to?(:downcase) && + @definition.dig('spec', 'strategy').downcase == 'recreate' + @validation_errors << "#{REQUIRED_ROLLOUT_ANNOTATION}:#{required_rollout} is invalid "\ + "with strategy 'rollingUpdate'" + end + + @validation_errors.empty? + end + private def deploy_failing_to_progress? @@ -98,18 +134,22 @@ def deploy_failing_to_progress? Time.parse(@progress_condition["lastUpdateTime"]).to_i >= (@deploy_started_at - 5.seconds).to_i end - def find_latest_rs(deployment_data) - label_string = deployment_data["spec"]["selector"]["matchLabels"].map { |k, v| "#{k}=#{v}" }.join(",") + def all_rs_data(match_labels) + label_string = match_labels.map { |k, v| "#{k}=#{v}" }.join(",") raw_json, _err, st = kubectl.run("get", "replicasets", "--output=json", "--selector=#{label_string}") - return unless st.success? + return {} unless st.success? - all_rs_data = JSON.parse(raw_json)["items"] + JSON.parse(raw_json)["items"] + end + + def find_latest_rs(deployment_data) current_revision = deployment_data["metadata"]["annotations"]["deployment.kubernetes.io/revision"] - latest_rs_data = all_rs_data.find do |rs| + latest_rs_data = all_rs_data(deployment_data["spec"]["selector"]["matchLabels"]).find do |rs| rs["metadata"]["ownerReferences"].any? { |ref| ref["uid"] == deployment_data["metadata"]["uid"] } && rs["metadata"]["annotations"]["deployment.kubernetes.io/revision"] == current_revision end + return unless latest_rs_data.present? rs = ReplicaSet.new( @@ -123,5 +163,18 @@ def find_latest_rs(deployment_data) rs.sync(latest_rs_data) rs end + + def min_available_replicas + if @max_unavailable =~ /%/ + (@desired_replicas * (100 - @max_unavailable.to_i) / 100.0).ceil + else + @desired_replicas - @max_unavailable.to_i + end + end + + def required_rollout + @definition.dig('metadata', 'annotations', REQUIRED_ROLLOUT_ANNOTATION).presence || + DEFAULT_REQUIRED_ROLLOUT + end end end diff --git a/lib/kubernetes-deploy/kubernetes_resource/replica_set.rb b/lib/kubernetes-deploy/kubernetes_resource/replica_set.rb index 25a091ce7..e4fbf6f0f 100644 --- a/lib/kubernetes-deploy/kubernetes_resource/replica_set.rb +++ b/lib/kubernetes-deploy/kubernetes_resource/replica_set.rb @@ -3,13 +3,15 @@ module KubernetesDeploy class ReplicaSet < PodSetBase TIMEOUT = 5.minutes - attr_reader :desired_replicas, :pods + attr_reader :desired_replicas, :ready_replicas, :available_replicas, :pods def initialize(namespace:, context:, definition:, logger:, parent: nil, deploy_started_at: nil) @parent = parent @deploy_started_at = deploy_started_at @rollout_data = { "replicas" => 0 } @desired_replicas = -1 + @ready_replicas = -1 + @available_replicas = -1 @pods = [] super(namespace: namespace, context: context, definition: definition, logger: logger) end @@ -26,6 +28,8 @@ def sync(rs_data = nil) @rollout_data = { "replicas" => 0 }.merge( rs_data["status"].slice("replicas", "availableReplicas", "readyReplicas") ) + @ready_replicas = @rollout_data['readyReplicas'].to_i + @available_replicas = @rollout_data["availableReplicas"].to_i @status = @rollout_data.map { |state_replicas, num| "#{num} #{state_replicas.chop.pluralize(num)}" }.join(", ") @pods = find_pods(rs_data) else # reset @@ -34,6 +38,8 @@ def sync(rs_data = nil) @status = nil @pods = [] @desired_replicas = -1 + @ready_replicas = -1 + @available_replicas = -1 end end diff --git a/test/fixtures/slow-cloud/web.yml.erb b/test/fixtures/slow-cloud/web.yml.erb new file mode 100644 index 000000000..12c233599 --- /dev/null +++ b/test/fixtures/slow-cloud/web.yml.erb @@ -0,0 +1,37 @@ +apiVersion: apps/v1beta1 +kind: Deployment +metadata: + name: web + annotations: + shipit.shopify.io/restart: "true" + kubernetes-deploy.shopify.io/required-rollout: maxUnavailable +spec: + replicas: 3 + strategy: + type: RollingUpdate + rollingUpdate: + maxSurge: 1 + maxUnavailable: 1 + template: + metadata: + labels: + name: web + app: slow-cloud + spec: + containers: + - name: app + readinessProbe: + exec: + command: + - sleep + - '7' + timeoutSeconds: 10 + image: busybox + imagePullPolicy: IfNotPresent + command: ["tail", "-f", "/dev/null"] + ports: + - containerPort: 80 + name: http + env: + - name: GITHUB_REV + value: <%= current_sha %> diff --git a/test/integration/kubernetes_deploy_test.rb b/test/integration/kubernetes_deploy_test.rb index e81056970..30ac70f7d 100644 --- a/test/integration/kubernetes_deploy_test.rb +++ b/test/integration/kubernetes_deploy_test.rb @@ -688,6 +688,18 @@ def test_can_deploy_deployment_with_zero_replicas ]) end + def test_deploy_successful_with_partial_availability + result = deploy_fixtures("slow-cloud") + assert_deploy_success(result) + + result = deploy_fixtures("slow-cloud") + assert_deploy_success(result) + + assert_logs_match_all( + [%r{Deployment\/web\s+[34] replicas, 3 updatedReplicas, 2 availableReplicas, [12] unavailableReplica}] + ) + end + def test_deploy_aborts_immediately_if_metadata_name_missing result = deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"]) do |fixtures| definition = fixtures["configmap-data.yml"]["ConfigMap"].first diff --git a/test/integration/restart_task_test.rb b/test/integration/restart_task_test.rb index 1eefa5439..4cb67dbb2 100644 --- a/test/integration/restart_task_test.rb +++ b/test/integration/restart_task_test.rb @@ -206,6 +206,30 @@ def test_restart_failure in_order: true) end + def test_restart_successful_with_partial_availability + result = deploy_fixtures("slow-cloud") do |fixtures| + web = fixtures["web.yml.erb"]["Deployment"].first + web["spec"]["strategy"]['rollingUpdate']['maxUnavailable'] = '34%' + end + assert_deploy_success(result) + + restart = build_restart_task + assert_restart_success(restart.perform(["web"])) + + assert_logs_match_all([ + "Configured to restart deployments by name: web", + "Triggered `web` restart", + "Waiting for rollout", + %r{Successfully restarted in \d+\.\d+s: Deployment/web}, + "Result: SUCCESS", + "Successfully restarted 1 resource", + %r{Deployment\/web\s+[34] replicas, 3 updatedReplicas, 2 availableReplicas, [12] unavailableReplica} + ], + in_order: true) + + assert fetch_restarted_at("web"), "RESTARTED_AT is present after the restart" + end + private def build_restart_task diff --git a/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb b/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb new file mode 100644 index 000000000..94164460e --- /dev/null +++ b/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true +require 'test_helper' + +class DeploymentTest < KubernetesDeploy::TestCase + def test_deploy_succeeded_with_none_annotation + rollout = { + 'metadata' => { + 'name' => 'fake', + 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'none' } + } + } + + deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) + deploy.instance_variable_set(:@latest_rs, true) + + assert deploy.deploy_succeeded? + end + + def test_deploy_succeeded_with_max_unavailable + rollout = { + 'metadata' => { + 'name' => 'fake', + 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } + } + } + + deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) + mock_rs = Minitest::Mock.new + needed = 2 + mock_rs.expect :present?, true + mock_rs.expect :desired_replicas, needed + mock_rs.expect :ready_replicas, needed + mock_rs.expect :available_replicas, needed + deploy.instance_variable_set(:@max_unavailable, 0) + deploy.instance_variable_set(:@latest_rs, mock_rs) + deploy.instance_variable_set(:@desired_replicas, needed) + + assert deploy.deploy_succeeded? + end + + def test_deploy_succeeded_fails_with_max_unavailable + rollout = { + 'metadata' => { + 'name' => 'fake', + 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } + } + } + + deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) + mock_rs = Minitest::Mock.new + needed = 2 + mock_rs.expect :present?, true + mock_rs.expect :desired_replicas, needed + mock_rs.expect :ready_replicas, needed - 1 + mock_rs.expect :available_replicas, needed - 1 + deploy.instance_variable_set(:@max_unavailable, 0) + deploy.instance_variable_set(:@latest_rs, mock_rs) + deploy.instance_variable_set(:@desired_replicas, needed) + + refute deploy.deploy_succeeded? + end + + def test_deploy_succeeded_fails_with_max_unavailable_as_a_percent + rollout = { + 'metadata' => { + 'name' => 'fake', + 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } + } + } + + deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) + mock_rs = Minitest::Mock.new + needed = 2 + mock_rs.expect :present?, true + mock_rs.expect :desired_replicas, needed + mock_rs.expect :ready_replicas, needed - 1 + mock_rs.expect :available_replicas, needed - 1 + deploy.instance_variable_set(:@max_unavailable, '49%') + deploy.instance_variable_set(:@latest_rs, mock_rs) + deploy.instance_variable_set(:@desired_replicas, needed) + + refute deploy.deploy_succeeded? + end + + def test_deploy_succeeded_raises_with_invalid_annotation + rollout = { + 'metadata' => { + 'name' => 'fake', + 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'invalid' } + } + } + + deploy = KubernetesDeploy::Deployment.new(namespace: "foo", context: "", logger: logger, definition: rollout) + deploy.instance_variable_set(:@latest_rs, true) + + assert_raises(RuntimeError) { deploy.deploy_succeeded? } + end + + def test_deploy_succeeded_raises_with_invalid_mix_of_annotation + rollout = { + 'spec' => { + 'strategy' => 'recreate' + }, + 'metadata' => { + 'name' => 'fake', + 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } + } + } + + kubectl_mock = Minitest::Mock.new + status_mock = Minitest::Mock.new + status_mock.expect :success?, true + kubectl_mock.expect(:run, [true, true, status_mock], [Object, Object, Object, Object, Object, Object]) + deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) + deploy.instance_variable_set(:@kubectl, kubectl_mock) + + refute deploy.validate_definition + end +end From d6004c19b45c242d31b42322690c8f7a44e814ce Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Tue, 9 Jan 2018 17:25:09 -0500 Subject: [PATCH 2/6] Unit testing with simulated sync for Deployment --- .../kubernetes_resource/deployment.rb | 19 +- .../for_unit_tests/deployment_test.yml | 62 +++ .../kubernetes_resource/deployment_test.rb | 376 ++++++++++++++---- 3 files changed, 368 insertions(+), 89 deletions(-) create mode 100644 test/fixtures/for_unit_tests/deployment_test.yml diff --git a/lib/kubernetes-deploy/kubernetes_resource/deployment.rb b/lib/kubernetes-deploy/kubernetes_resource/deployment.rb index a6028836b..6e67b7780 100644 --- a/lib/kubernetes-deploy/kubernetes_resource/deployment.rb +++ b/lib/kubernetes-deploy/kubernetes_resource/deployment.rb @@ -63,8 +63,7 @@ def deploy_succeeded? @latest_rs.ready_replicas >= minimum_needed && @latest_rs.available_replicas >= minimum_needed else - raise "#{REQUIRED_ROLLOUT_ANNOTATION}:#{required_rollout} is invalid "\ - " Acceptable options: #{REQUIRED_ROLLOUT_TYPES.join(',')}" + raise FatalDeploymentError, rollout_annotation_err_msg end end @@ -104,14 +103,13 @@ def validate_definition super unless REQUIRED_ROLLOUT_TYPES.include?(required_rollout) - @validation_errors << "#{REQUIRED_ROLLOUT_ANNOTATION}:#{required_rollout} is invalid "\ - "Acceptable options: #{REQUIRED_ROLLOUT_TYPES.join(',')}" + @validation_errors << rollout_annotation_err_msg end - if required_rollout.downcase == 'maxunavailable' && @definition.dig('spec', 'strategy').respond_to?(:downcase) && - @definition.dig('spec', 'strategy').downcase == 'recreate' - @validation_errors << "#{REQUIRED_ROLLOUT_ANNOTATION}:#{required_rollout} is invalid "\ - "with strategy 'rollingUpdate'" + strategy = @definition.dig('spec', 'strategy', 'type').to_s + if required_rollout.downcase == 'maxunavailable' && strategy.downcase != 'rollingupdate' + @validation_errors << "'#{REQUIRED_ROLLOUT_ANNOTATION}: #{required_rollout}' is incompatible "\ + "with strategy '#{strategy}'" end @validation_errors.empty? @@ -119,6 +117,11 @@ def validate_definition private + def rollout_annotation_err_msg + "'#{REQUIRED_ROLLOUT_ANNOTATION}: #{required_rollout}' is invalid. "\ + "Acceptable values: #{REQUIRED_ROLLOUT_TYPES.join(', ')}" + end + def deploy_failing_to_progress? return false unless @progress_condition.present? diff --git a/test/fixtures/for_unit_tests/deployment_test.yml b/test/fixtures/for_unit_tests/deployment_test.yml new file mode 100644 index 000000000..b262abdce --- /dev/null +++ b/test/fixtures/for_unit_tests/deployment_test.yml @@ -0,0 +1,62 @@ +--- +apiVersion: apps/v1beta1 +kind: Deployment +metadata: + name: web + uid: foobar + annotations: + "deployment.kubernetes.io/revision": "1" +spec: + replicas: 3 + progressDeadlineSeconds: 10 + strategy: + type: RollingUpdate + rollingUpdate: + maxSurge: 1 + maxUnavailable: 1 + selector: + matchLabels: + name: web + app: hello-cloud + template: + metadata: + labels: + name: web + app: hello-cloud + spec: + containers: + - name: app + image: busybox +status: + replicas: 3 + conditions: + - type: Progressing + status: True + lastUpdateTime: "2018-01-09 22:56:45 UTC" + +--- +apiVersion: apps/v1beta1 +kind: ReplicaSet +metadata: + name: web-1 + annotations: + "deployment.kubernetes.io/revision": "1" + ownerReferences: + - uid: foobar +spec: + replicas: 3 + selector: + matchLabels: + name: web + app: hello-cloud + template: + metadata: + labels: + name: web + app: hello-cloud + spec: + containers: + - name: app + image: busybox +status: + replicas: 3 diff --git a/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb b/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb index 94164460e..082a606ea 100644 --- a/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb +++ b/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb @@ -2,118 +2,332 @@ require 'test_helper' class DeploymentTest < KubernetesDeploy::TestCase + def setup + KubernetesDeploy::Kubectl.any_instance.expects(:run).never + super + end + def test_deploy_succeeded_with_none_annotation - rollout = { - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'none' } - } + deployment_status = { + "replicas" => 3, + "updatedReplicas" => 1, + "unavailableReplicas" => 1, + "availableReplicas" => 0 } - deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) - deploy.instance_variable_set(:@latest_rs, true) - + rs_status = { + "replicas" => 3, + "availableReplicas" => 0, + "readyReplicas" => 0 + } + dep_template = build_deployment_template(status: deployment_status, rollout: 'none', + strategy: 'RollingUpdate', max_unavailable: 1) + deploy = build_synced_deployment(template: dep_template, replica_sets: [build_rs_template(status: rs_status)]) assert deploy.deploy_succeeded? end + def test_deploy_succeeded_is_false_with_none_annotation_before_new_rs_created + deployment_status = { + "replicas" => 3, + "updatedReplicas" => 3, + "unavailableReplicas" => 0, + "availableReplicas" => 3 + } + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'none'), + replica_sets: [] + ) + refute deploy.deploy_succeeded? + end + def test_deploy_succeeded_with_max_unavailable - rollout = { - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } - } + deployment_status = { + "replicas" => 3, # one terminating in old rs, one starting in new rs, one up in new rs + "updatedReplicas" => 2, + "unavailableReplicas" => 2, + "availableReplicas" => 1 + } + + rs_status = { + "replicas" => 2, + "availableReplicas" => 1, + "readyReplicas" => 1 } + replica_sets = [build_rs_template(status: rs_status)] - deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) - mock_rs = Minitest::Mock.new - needed = 2 - mock_rs.expect :present?, true - mock_rs.expect :desired_replicas, needed - mock_rs.expect :ready_replicas, needed - mock_rs.expect :available_replicas, needed - deploy.instance_variable_set(:@max_unavailable, 0) - deploy.instance_variable_set(:@latest_rs, mock_rs) - deploy.instance_variable_set(:@desired_replicas, needed) + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'maxUnavailable', max_unavailable: 3), + replica_sets: replica_sets + ) + assert deploy.deploy_succeeded? + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'maxUnavailable', max_unavailable: 2), + replica_sets: replica_sets + ) assert deploy.deploy_succeeded? + + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'maxUnavailable', max_unavailable: 1), + replica_sets: replica_sets + ) + refute deploy.deploy_succeeded? + + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'maxUnavailable', max_unavailable: 0), + replica_sets: replica_sets + ) + refute deploy.deploy_succeeded? end - def test_deploy_succeeded_fails_with_max_unavailable - rollout = { - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } - } + def test_deploy_succeeded_with_max_unavailable_as_percent + deployment_status = { + "replicas" => 3, + "updatedReplicas" => 2, + "unavailableReplicas" => 2, + "availableReplicas" => 1 + } + + rs_status = { + "replicas" => 2, + "availableReplicas" => 1, + "readyReplicas" => 1 } + replica_sets = [build_rs_template(status: rs_status)] + + dep_template = build_deployment_template(status: deployment_status, + rollout: 'maxUnavailable', max_unavailable: '100%') + deploy = build_synced_deployment(template: dep_template, replica_sets: replica_sets) + assert deploy.deploy_succeeded? + + # rounds up to two max + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'maxUnavailable', max_unavailable: '67%'), + replica_sets: replica_sets + ) + assert deploy.deploy_succeeded? - deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) - mock_rs = Minitest::Mock.new - needed = 2 - mock_rs.expect :present?, true - mock_rs.expect :desired_replicas, needed - mock_rs.expect :ready_replicas, needed - 1 - mock_rs.expect :available_replicas, needed - 1 - deploy.instance_variable_set(:@max_unavailable, 0) - deploy.instance_variable_set(:@latest_rs, mock_rs) - deploy.instance_variable_set(:@desired_replicas, needed) + # rounds down to one max + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'maxUnavailable', max_unavailable: '66%'), + replica_sets: replica_sets + ) + refute deploy.deploy_succeeded? + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'maxUnavailable', max_unavailable: '0%'), + replica_sets: replica_sets + ) refute deploy.deploy_succeeded? end - def test_deploy_succeeded_fails_with_max_unavailable_as_a_percent - rollout = { - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } - } - } + def test_deploy_succeeded_raises_with_invalid_rollout_annotation + deploy = build_synced_deployment( + template: build_deployment_template(rollout: 'bad'), + replica_sets: [build_rs_template] + ) + msg = "'#{KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION}: bad' is "\ + "invalid. Acceptable values: #{KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_TYPES.join(', ')}" + assert_raises_message(KubernetesDeploy::FatalDeploymentError, msg) do + deploy.deploy_succeeded? + end + end + + def test_validation_fails_with_invalid_rollout_annotation + deploy = build_synced_deployment(template: build_deployment_template(rollout: 'bad'), replica_sets: []) + deploy.kubectl.expects(:run).with('create', '-f', anything, '--dry-run', '--output=name', anything).returns( + ["", "super failed", SystemExit.new(1)] + ) + refute deploy.validate_definition + + expected = <<~STRING.strip + super failed + '#{KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION}: bad' is invalid. Acceptable values: #{KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_TYPES.join(', ')} + STRING + assert_equal expected, deploy.validation_error_msg + end - deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) - mock_rs = Minitest::Mock.new - needed = 2 - mock_rs.expect :present?, true - mock_rs.expect :desired_replicas, needed - mock_rs.expect :ready_replicas, needed - 1 - mock_rs.expect :available_replicas, needed - 1 - deploy.instance_variable_set(:@max_unavailable, '49%') - deploy.instance_variable_set(:@latest_rs, mock_rs) - deploy.instance_variable_set(:@desired_replicas, needed) + def test_validation_fails_with_invalid_mix_of_annotation + deploy = build_synced_deployment( + template: build_deployment_template(rollout: 'maxUnavailable', strategy: 'Recreate'), + replica_sets: [build_rs_template] + ) + deploy.kubectl.expects(:run).with('create', '-f', anything, '--dry-run', '--output=name', anything).returns( + ["", "super failed", SystemExit.new(1)] + ) + refute deploy.validate_definition + expected = <<~STRING.strip + super failed + '#{KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION}: maxUnavailable' is incompatible with strategy 'Recreate' + STRING + assert_equal expected, deploy.validation_error_msg + end + + def test_deploy_succeeded_not_fooled_by_stale_rs_data_in_deploy_status + deployment_status = { + "replicas" => 3, + "updatedReplicas" => 3, # stale -- hasn't been updated since new RS was created + "unavailableReplicas" => 0, + "availableReplicas" => 3 + } + + rs_status = { + "replicas" => 1, + "availableReplicas" => 0, + "readyReplicas" => 0 + } + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status, rollout: 'full', max_unavailable: 1), + replica_sets: [build_rs_template(status: rs_status)] + ) refute deploy.deploy_succeeded? end - def test_deploy_succeeded_raises_with_invalid_annotation - rollout = { - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'invalid' } + def test_deploy_timed_out_with_hard_timeout + Timecop.freeze do + deploy = build_synced_deployment( + template: build_deployment_template(status: { "replicas" => 3, "conditions" => [] }), + replica_sets: [build_rs_template(status: { "replica" => 1 })] + ) + deploy.deploy_started_at = Time.now.utc - KubernetesDeploy::Deployment::TIMEOUT + refute deploy.deploy_timed_out? + + deploy.deploy_started_at = Time.now.utc - KubernetesDeploy::Deployment::TIMEOUT - 1 + assert deploy.deploy_timed_out? + assert_equal "Timeout reason: hard deadline for Deployment\nLatest ReplicaSet: web-1", + deploy.timeout_message.strip + end + end + + def test_deploy_timed_out_based_on_progress_deadline + Timecop.freeze do + deployment_status = { + "replicas" => 3, + "conditions" => [{ + "type" => "Progressing", + "status" => 'False', + "lastUpdateTime" => Time.now.utc - 10.seconds, + "reason" => "Failed to progress" + }] } - } + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status), + replica_sets: [build_rs_template(status: { "replica" => 1 })] + ) + deploy.deploy_started_at = Time.now.utc - 3.minutes + deploy.kubectl.expects(:server_version).returns(Gem::Version.new("1.8")) + + assert deploy.deploy_timed_out? + assert_equal "Timeout reason: Failed to progress\nLatest ReplicaSet: web-1", deploy.timeout_message.strip + end + end + + def test_deploy_timed_out_based_on_progress_deadline_ignores_conditions_older_than_the_deploy + Timecop.freeze do + deployment_status = { + "replicas" => 3, + "conditions" => [{ + "type" => "Progressing", + "status" => 'False', + "lastUpdateTime" => Time.now.utc - 10.seconds, + "reason" => "Failed to progress" + }] + } + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status), + replica_sets: [build_rs_template(status: { "replica" => 1 })] + ) + deploy.kubectl.expects(:server_version).returns(Gem::Version.new("1.8")).at_least_once + + deploy.deploy_started_at = nil # not started yet + refute deploy.deploy_timed_out? - deploy = KubernetesDeploy::Deployment.new(namespace: "foo", context: "", logger: logger, definition: rollout) - deploy.instance_variable_set(:@latest_rs, true) + deploy.deploy_started_at = Time.now.utc - 4.seconds # 10s ago is before deploy started + refute deploy.deploy_timed_out? - assert_raises(RuntimeError) { deploy.deploy_succeeded? } + deploy.deploy_started_at = Time.now.utc - 5.seconds # 10s ago is "equal" to deploy time (fudge for clock skew) + assert deploy.deploy_timed_out? + end end - def test_deploy_succeeded_raises_with_invalid_mix_of_annotation - rollout = { - 'spec' => { - 'strategy' => 'recreate' - }, - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } + def test_deploy_timed_out_based_on_progress_deadline_accommodates_stale_conditions_bug_in_k8s_176_and_lower + Timecop.freeze do + deployment_status = { + "replicas" => 3, + "conditions" => [{ + "type" => "Progressing", + "status" => 'False', + "lastUpdateTime" => Time.now.utc - 5.seconds, + "reason" => "Failed to progress" + }] } - } + deploy = build_synced_deployment( + template: build_deployment_template(status: deployment_status), + replica_sets: [build_rs_template(status: { "replica" => 1 })] + ) + deploy.deploy_started_at = Time.now.utc - 5.seconds # progress deadline of 10s has not elapsed + deploy.kubectl.expects(:server_version).returns(Gem::Version.new("1.7.6")) + + refute deploy.deploy_timed_out? + end + end - kubectl_mock = Minitest::Mock.new - status_mock = Minitest::Mock.new - status_mock.expect :success?, true - kubectl_mock.expect(:run, [true, true, status_mock], [Object, Object, Object, Object, Object, Object]) - deploy = KubernetesDeploy::Deployment.new(namespace: "", context: "", logger: logger, definition: rollout) - deploy.instance_variable_set(:@kubectl, kubectl_mock) + private - refute deploy.validate_definition + def build_deployment_template(status: { 'replicas' => 3 }, rollout: nil, + strategy: 'rollingUpdate', max_unavailable: nil) + + base_deployment_manifest = fixtures.find { |fixture| fixture["kind"] == "Deployment" } + result = base_deployment_manifest.deep_merge("status" => status) + result["metadata"]["annotations"][KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION] = rollout if rollout + + if spec_override = status["replicas"].presence # ignores possibility of surge; need a spec_replicas arg for that + result["spec"]["replicas"] = spec_override + end + + if strategy == "Recreate" + result["spec"]["strategy"] = { "type" => strategy } + end + + if max_unavailable + result["spec"]["strategy"]["rollingUpdate"] = { "maxUnavailable" => max_unavailable } + end + + result + end + + def build_rs_template(status: { 'replicas' => 3 }) + base_rs_manifest = fixtures.find { |fixture| fixture["kind"] == "ReplicaSet" } + result = base_rs_manifest.deep_merge("status" => status) + + if spec_override = status["replicas"].presence # ignores possibility of surge; need a spec_replicas arg for that + result["spec"]["replicas"] = spec_override + end + result + end + + def build_synced_deployment(template:, replica_sets:) + deploy = KubernetesDeploy::Deployment.new(namespace: "test", context: "nope", logger: logger, definition: template) + deploy.kubectl.expects(:run).with("get", "Deployment", "web", "--output=json").returns( + [template.to_json, "", SystemExit.new(0)] + ) + + if replica_sets.present? + KubernetesDeploy::ReplicaSet.any_instance.expects(:kubectl).returns(deploy.kubectl) + deploy.kubectl.expects(:run).with("get", "pods", "-a", "--output=json", anything).returns( + ['{ "items": [] }', "", SystemExit.new(0)] + ) + end + + deploy.kubectl.expects(:run).with("get", "replicasets", "--output=json", anything).returns( + [{ "items" => replica_sets }.to_json, "", SystemExit.new(0)] + ) + deploy.sync + deploy + end + + def fixtures + @fixtures ||= YAML.load_stream(File.read(File.join(fixture_path('for_unit_tests'), 'deployment_test.yml'))) end end From ba66e36c5efc03d08db7d8034e8b48c98f9970b4 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Thu, 11 Jan 2018 09:10:29 -0800 Subject: [PATCH 3/6] New tests --- test/fixtures/slow-cloud/web.yml.erb | 19 +++++++--------- test/integration/kubernetes_deploy_test.rb | 26 ++++++++++++++++------ test/integration/restart_task_test.rb | 9 ++++++-- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/test/fixtures/slow-cloud/web.yml.erb b/test/fixtures/slow-cloud/web.yml.erb index 12c233599..52f5ca8c9 100644 --- a/test/fixtures/slow-cloud/web.yml.erb +++ b/test/fixtures/slow-cloud/web.yml.erb @@ -6,32 +6,29 @@ metadata: shipit.shopify.io/restart: "true" kubernetes-deploy.shopify.io/required-rollout: maxUnavailable spec: - replicas: 3 + replicas: 2 + selector: + matchLabels: + name: web + app: slow-cloud strategy: type: RollingUpdate rollingUpdate: - maxSurge: 1 + maxSurge: 0 maxUnavailable: 1 template: metadata: labels: name: web app: slow-cloud + sha: "<%= current_sha %>" spec: + terminationGracePeriodSeconds: 0 containers: - name: app - readinessProbe: - exec: - command: - - sleep - - '7' - timeoutSeconds: 10 image: busybox imagePullPolicy: IfNotPresent command: ["tail", "-f", "/dev/null"] ports: - containerPort: 80 name: http - env: - - name: GITHUB_REV - value: <%= current_sha %> diff --git a/test/integration/kubernetes_deploy_test.rb b/test/integration/kubernetes_deploy_test.rb index 30ac70f7d..0f31d634d 100644 --- a/test/integration/kubernetes_deploy_test.rb +++ b/test/integration/kubernetes_deploy_test.rb @@ -689,15 +689,27 @@ def test_can_deploy_deployment_with_zero_replicas end def test_deploy_successful_with_partial_availability - result = deploy_fixtures("slow-cloud") - assert_deploy_success(result) + result = deploy_fixtures("slow-cloud", sha: "deploy1") + assert_deploy_success(result) - result = deploy_fixtures("slow-cloud") - assert_deploy_success(result) + result = deploy_fixtures("slow-cloud", sha: "deploy2") do |fixtures| + dep = fixtures["web.yml.erb"]["Deployment"].first + container = dep["spec"]["template"]["spec"]["containers"].first + container["readinessProbe"] = { + "exec" => { "command" => %w(sleep 5) }, + "timeoutSeconds" => 6 + } + end + assert_deploy_success(result) - assert_logs_match_all( - [%r{Deployment\/web\s+[34] replicas, 3 updatedReplicas, 2 availableReplicas, [12] unavailableReplica}] - ) + new_pods = kubeclient.get_pods(namespace: @namespace, label_selector: 'name=web,app=slow-cloud,sha=deploy2') + assert new_pods.length >= 1, "Expected at least one new pod, saw #{new_pods.length}" + + new_ready_pods = new_pods.select do |pod| + pod.status.phase == "Running" && + pod.status.conditions.any? { |condition| condition["type"] == "Ready" && condition["status"] == "True" } + end + assert_equal 1, new_ready_pods.length, "Expected exactly one new pod to be ready, saw #{new_ready_pods.length}" end def test_deploy_aborts_immediately_if_metadata_name_missing diff --git a/test/integration/restart_task_test.rb b/test/integration/restart_task_test.rb index 4cb67dbb2..55b3d899b 100644 --- a/test/integration/restart_task_test.rb +++ b/test/integration/restart_task_test.rb @@ -209,7 +209,12 @@ def test_restart_failure def test_restart_successful_with_partial_availability result = deploy_fixtures("slow-cloud") do |fixtures| web = fixtures["web.yml.erb"]["Deployment"].first - web["spec"]["strategy"]['rollingUpdate']['maxUnavailable'] = '34%' + web["spec"]["strategy"]['rollingUpdate']['maxUnavailable'] = '50%' + container = web["spec"]["template"]["spec"]["containers"].first + container["readinessProbe"] = { + "exec" => { "command" => %w(sleep 5) }, + "timeoutSeconds" => 6 + } end assert_deploy_success(result) @@ -223,7 +228,7 @@ def test_restart_successful_with_partial_availability %r{Successfully restarted in \d+\.\d+s: Deployment/web}, "Result: SUCCESS", "Successfully restarted 1 resource", - %r{Deployment\/web\s+[34] replicas, 3 updatedReplicas, 2 availableReplicas, [12] unavailableReplica} + %r{Deployment\/web\s+2 replicas, [12] updatedReplicas?, [12] availableReplicas?, 1 unavailableReplica} ], in_order: true) From 480d41b10adf37aff8deead364251bcd8a7c35fc Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Thu, 11 Jan 2018 11:11:52 -0800 Subject: [PATCH 4/6] Rework restart test --- test/integration/kubernetes_deploy_test.rb | 36 +++++++++++----------- test/integration/restart_task_test.rb | 20 ++++++------ 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/test/integration/kubernetes_deploy_test.rb b/test/integration/kubernetes_deploy_test.rb index 0f31d634d..3239fb8b8 100644 --- a/test/integration/kubernetes_deploy_test.rb +++ b/test/integration/kubernetes_deploy_test.rb @@ -689,27 +689,27 @@ def test_can_deploy_deployment_with_zero_replicas end def test_deploy_successful_with_partial_availability - result = deploy_fixtures("slow-cloud", sha: "deploy1") - assert_deploy_success(result) + result = deploy_fixtures("slow-cloud", sha: "deploy1") + assert_deploy_success(result) - result = deploy_fixtures("slow-cloud", sha: "deploy2") do |fixtures| - dep = fixtures["web.yml.erb"]["Deployment"].first - container = dep["spec"]["template"]["spec"]["containers"].first - container["readinessProbe"] = { - "exec" => { "command" => %w(sleep 5) }, - "timeoutSeconds" => 6 - } - end - assert_deploy_success(result) + result = deploy_fixtures("slow-cloud", sha: "deploy2") do |fixtures| + dep = fixtures["web.yml.erb"]["Deployment"].first + container = dep["spec"]["template"]["spec"]["containers"].first + container["readinessProbe"] = { + "exec" => { "command" => %w(sleep 5) }, + "timeoutSeconds" => 6 + } + end + assert_deploy_success(result) - new_pods = kubeclient.get_pods(namespace: @namespace, label_selector: 'name=web,app=slow-cloud,sha=deploy2') - assert new_pods.length >= 1, "Expected at least one new pod, saw #{new_pods.length}" + new_pods = kubeclient.get_pods(namespace: @namespace, label_selector: 'name=web,app=slow-cloud,sha=deploy2') + assert new_pods.length >= 1, "Expected at least one new pod, saw #{new_pods.length}" - new_ready_pods = new_pods.select do |pod| - pod.status.phase == "Running" && - pod.status.conditions.any? { |condition| condition["type"] == "Ready" && condition["status"] == "True" } - end - assert_equal 1, new_ready_pods.length, "Expected exactly one new pod to be ready, saw #{new_ready_pods.length}" + new_ready_pods = new_pods.select do |pod| + pod.status.phase == "Running" && + pod.status.conditions.any? { |condition| condition["type"] == "Ready" && condition["status"] == "True" } + end + assert_equal 1, new_ready_pods.length, "Expected exactly one new pod to be ready, saw #{new_ready_pods.length}" end def test_deploy_aborts_immediately_if_metadata_name_missing diff --git a/test/integration/restart_task_test.rb b/test/integration/restart_task_test.rb index 55b3d899b..9626a17b3 100644 --- a/test/integration/restart_task_test.rb +++ b/test/integration/restart_task_test.rb @@ -221,16 +221,16 @@ def test_restart_successful_with_partial_availability restart = build_restart_task assert_restart_success(restart.perform(["web"])) - assert_logs_match_all([ - "Configured to restart deployments by name: web", - "Triggered `web` restart", - "Waiting for rollout", - %r{Successfully restarted in \d+\.\d+s: Deployment/web}, - "Result: SUCCESS", - "Successfully restarted 1 resource", - %r{Deployment\/web\s+2 replicas, [12] updatedReplicas?, [12] availableReplicas?, 1 unavailableReplica} - ], - in_order: true) + pods = kubeclient.get_pods(namespace: @namespace, label_selector: 'name=web,app=slow-cloud') + new_pods = pods.select { |pod| pod.spec.containers.select { |c| c["name"] == "app" && c.env&.find { |n| n.name == "RESTARTED_AT" } } } + assert new_pods.length >= 1, "Expected at least one new pod, saw #{new_pods.length}" + + new_ready_pods = new_pods.select do |pod| + pod.status.phase == "Running" && + pod.status.conditions.any? { |condition| condition["type"] == "Ready" && condition["status"] == "True" } && + pod.spec.containers.detect { |c| c["name"] == "app" && c.env&.find { |n| n.name == "RESTARTED_AT" } } + end + assert_equal 1, new_ready_pods.length, "Expected exactly one new pod to be ready, saw #{new_ready_pods.length}" assert fetch_restarted_at("web"), "RESTARTED_AT is present after the restart" end From 5447bc67016478b5c6898da9ea625f3f9e18926a Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Thu, 11 Jan 2018 11:15:00 -0800 Subject: [PATCH 5/6] line length --- test/integration/restart_task_test.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integration/restart_task_test.rb b/test/integration/restart_task_test.rb index 9626a17b3..bae924ddd 100644 --- a/test/integration/restart_task_test.rb +++ b/test/integration/restart_task_test.rb @@ -222,7 +222,9 @@ def test_restart_successful_with_partial_availability assert_restart_success(restart.perform(["web"])) pods = kubeclient.get_pods(namespace: @namespace, label_selector: 'name=web,app=slow-cloud') - new_pods = pods.select { |pod| pod.spec.containers.select { |c| c["name"] == "app" && c.env&.find { |n| n.name == "RESTARTED_AT" } } } + new_pods = pods.select do |pod| + pod.spec.containers.select { |c| c["name"] == "app" && c.env&.find { |n| n.name == "RESTARTED_AT" } } + ebd assert new_pods.length >= 1, "Expected at least one new pod, saw #{new_pods.length}" new_ready_pods = new_pods.select do |pod| From ee5e9cba71fa725f9c3ca217011e5239ce692005 Mon Sep 17 00:00:00 2001 From: Daniel Turner Date: Thu, 11 Jan 2018 13:54:26 -0800 Subject: [PATCH 6/6] Fix typos --- README.md | 6 +++--- test/integration/restart_task_test.rb | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 39438997a..16353dcee 100644 --- a/README.md +++ b/README.md @@ -128,9 +128,9 @@ You can add additional variables using the `--bindings=BINDINGS` option. For exa - `kubernetes-deploy.shopify.io/required-rollout`: Modifies how much of the rollout needs to finish before the deployment is considered successful. - _Compatibility_: Deployment - - `full`: The deploy is successful when all pods in the new ReplicaSet are ready. - - `none`: The deploy is successful as soon as the new `replicaSet` is created for the deploy - - `maxUnavailable`: The deploy is successful when minimum availability is reached in the new ReplicaSet. + - `full`: The deployment is successful when all pods in the new `replicaSet` are ready. + - `none`: The deployment is successful as soon as the new `replicaSet` is created for the deployment. + - `maxUnavailable`: The deploy is successful when minimum availability is reached in the new `replicaSet`. In other words, the number of new pods that must be ready is equal to `spec.replicas` - `strategy.RollingUpdate.maxUnavailable` (converted from percentages by rounding up, if applicable). This option is only valid for deployments that use the `RollingUpdate` strategy. diff --git a/test/integration/restart_task_test.rb b/test/integration/restart_task_test.rb index bae924ddd..6c9cddd3c 100644 --- a/test/integration/restart_task_test.rb +++ b/test/integration/restart_task_test.rb @@ -223,14 +223,13 @@ def test_restart_successful_with_partial_availability pods = kubeclient.get_pods(namespace: @namespace, label_selector: 'name=web,app=slow-cloud') new_pods = pods.select do |pod| - pod.spec.containers.select { |c| c["name"] == "app" && c.env&.find { |n| n.name == "RESTARTED_AT" } } - ebd + pod.spec.containers.any? { |c| c["name"] == "app" && c.env&.find { |n| n.name == "RESTARTED_AT" } } + end assert new_pods.length >= 1, "Expected at least one new pod, saw #{new_pods.length}" new_ready_pods = new_pods.select do |pod| pod.status.phase == "Running" && - pod.status.conditions.any? { |condition| condition["type"] == "Ready" && condition["status"] == "True" } && - pod.spec.containers.detect { |c| c["name"] == "app" && c.env&.find { |n| n.name == "RESTARTED_AT" } } + pod.status.conditions.any? { |condition| condition["type"] == "Ready" && condition["status"] == "True" } end assert_equal 1, new_ready_pods.length, "Expected exactly one new pod to be ready, saw #{new_ready_pods.length}"