diff --git a/lib/kubernetes-deploy/kubernetes_resource/deployment.rb b/lib/kubernetes-deploy/kubernetes_resource/deployment.rb index a6028836b..c4dacdcec 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 invalid "\ + "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..e5977b012 --- /dev/null +++ b/test/fixtures/for_unit_tests/deployment_test.yml @@ -0,0 +1,61 @@ +--- +apiVersion: apps/v1beta1 +kind: Deployment +metadata: + name: web + uid: foobar + annotations: + "deployment.kubernetes.io/revision": "1" +spec: + replicas: 3 + 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..7a35e4869 100644 --- a/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb +++ b/test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb @@ -2,118 +2,199 @@ 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' } - } + 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) - + new_rs_status = { + "replicas" => 3, + "availableReplicas" => 0, + "readyReplicas" => 0 + } + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, rollout: 'none', max_unavail: 1) assert deploy.deploy_succeeded? end + def test_deploy_succeeded_is_false_with_none_annotation_before_new_rs_created + status = { + "replicas" => 3, + "updatedReplicas" => 3, + "unavailableReplicas" => 0, + "availableReplicas" => 3 + } + deploy = build_synced_deployment(status: status, new_rs_status: nil, rollout: 'none', max_unavail: 1) + refute deploy.deploy_succeeded? + end + def test_deploy_succeeded_with_max_unavailable - rollout = { - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'maxUnavailable' } - } + status = { + "replicas" => 3, # one terminating in old rs, one starting in new rs, one up in new rs + "updatedReplicas" => 2, + "unavailableReplicas" => 2, + "availableReplicas" => 1 } - 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) + new_rs_status = { + "replicas" => 2, + "availableReplicas" => 1, + "readyReplicas" => 1 + } + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: 3) + assert deploy.deploy_succeeded? + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: 2) assert deploy.deploy_succeeded? + + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: 1) + refute deploy.deploy_succeeded? + + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: 0) + 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 + status = { + "replicas" => 3, + "updatedReplicas" => 2, + "unavailableReplicas" => 2, + "availableReplicas" => 1 } - 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) + new_rs_status = { + "replicas" => 2, + "availableReplicas" => 1, + "readyReplicas" => 1 + } + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: "100%") + assert deploy.deploy_succeeded? + + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: "67%") # rounds up to two max + assert deploy.deploy_succeeded? + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: "66%") # rounds down to one max + refute deploy.deploy_succeeded? + + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, + rollout: 'maxUnavailable', max_unavail: "0%") 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(status: {}, new_rs_status: {}, rollout: 'bad', max_unavail: "33%") + m = "'kubernetes-deploy.shopify.io/required-rollout: bad' is invalid. Acceptable values: maxUnavailable, full, none" + assert_raises_message(KubernetesDeploy::FatalDeploymentError, m) do + deploy.deploy_succeeded? + end + 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_rollout_annotation + deploy = build_synced_deployment(status: {}, new_rs_status: {}, rollout: 'bad', max_unavail: false) + deploy.kubectl.expects(:run).with('create', '-f', anything, '--dry-run', '--output=name', anything).returns( + ["", "super failed", SystemExit.new(1)] + ) + refute deploy.validate_definition - refute deploy.deploy_succeeded? + expected = <<~STRING.strip + super failed + 'kubernetes-deploy.shopify.io/required-rollout: bad' is invalid. Acceptable values: maxUnavailable, full, none + STRING + assert_equal expected, deploy.validation_error_msg end - def test_deploy_succeeded_raises_with_invalid_annotation - rollout = { - 'metadata' => { - 'name' => 'fake', - 'annotations' => { KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION => 'invalid' } - } + def test_validation_fails_with_invalid_mix_of_annotation + deploy = build_synced_deployment(status: {}, new_rs_status: {}, rollout: 'maxUnavailable', max_unavail: false) + 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 + 'kubernetes-deploy.shopify.io/required-rollout: maxUnavailable' is invalid 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 + status = { + "replicas" => 3, + "updatedReplicas" => 3, # stale -- hasn't been updated since new RS was created + "unavailableReplicas" => 0, + "availableReplicas" => 3 } - deploy = KubernetesDeploy::Deployment.new(namespace: "foo", context: "", logger: logger, definition: rollout) - deploy.instance_variable_set(:@latest_rs, true) + new_rs_status = { + "replicas" => 1, + "availableReplicas" => 0, + "readyReplicas" => 0 + } + deploy = build_synced_deployment(status: status, new_rs_status: new_rs_status, max_unavail: 1) + refute deploy.deploy_succeeded? + end - assert_raises(RuntimeError) { deploy.deploy_succeeded? } + private + + def build_synced_deployment(status:, new_rs_status:, rollout: nil, max_unavail:) + spec = base_deployment_manifest.deep_merge( + "spec" => { "replicas" => status["replicas"] }, # note: this ignores the possibility of surging + "status" => status + ) + spec["metadata"]["annotations"][KubernetesDeploy::Deployment::REQUIRED_ROLLOUT_ANNOTATION] = rollout if rollout + if max_unavail + spec["spec"]["strategy"]["rollingUpdate"]["maxUnavailable"] = max_unavail + else + spec["spec"]["strategy"] = { "type" => "Recreate" } + end + + deploy = KubernetesDeploy::Deployment.new(namespace: "test", context: "nope", logger: logger, definition: spec) + deploy.kubectl.expects(:run).with("get", "Deployment", "web", "--output=json").returns( + [spec.to_json, "", SystemExit.new(0)] + ) + + replicasets = { "items" => [] } + if new_rs_status + replicasets["items"] << base_rs_manifest.deep_merge( + "spec" => { "replicas" => new_rs_status["replicas"] }, + "status" => new_rs_status + ) + deploy.kubectl.expects(:run).with("get", "pods", "-a", "--output=json", anything).returns( + ['{ "items": [] }', "", SystemExit.new(0)] + ) + KubernetesDeploy::ReplicaSet.any_instance.expects(:kubectl).returns(deploy.kubectl) + end + + deploy.kubectl.expects(:run).with("get", "replicasets", "--output=json", anything).returns( + [replicasets.to_json, "", SystemExit.new(0)] + ) + deploy.sync + deploy 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 base_rs_manifest + fixtures.find { |fixture| fixture["kind"] == "ReplicaSet" } + 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) + def base_deployment_manifest + fixtures.find { |fixture| fixture["kind"] == "Deployment" } + end - refute deploy.validate_definition + def fixtures + @fixtures ||= YAML.load_stream(File.read(File.join(fixture_path('for_unit_tests'), 'deployment_test.yml'))) end end