Try to unit test without setting ivars

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,21 +103,25 @@ 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?
     end
 
     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?
 

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

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