Fix bug where redeploy hangs forever because PDS condition is ignored

lib/kubernetes-deploy/kubernetes_resource/deployment.rb

@@ -23,6 +23,8 @@ def sync
         @progress_condition = conditions.find { |condition| condition['type'] == 'Progressing' }
         @progress_deadline = deployment_data['spec']['progressDeadlineSeconds']
         @max_unavailable = deployment_data.dig('spec', 'strategy', 'rollingUpdate', 'maxUnavailable')
+        @current_generation = deployment_data.dig('metadata', 'generation')
+        @observed_generation = deployment_data.dig('status', 'observedGeneration')
       else # reset
         @latest_rs = nil
         @rollout_data = { "replicas" => 0 }
@@ -31,6 +33,7 @@ def sync
         @progress_deadline = @definition['spec']['progressDeadlineSeconds']
         @desired_replicas = -1
         @max_unavailable = @definition.dig('spec', 'strategy', 'rollingUpdate', 'maxUnavailable')
+        @observed_generation = -1
       end
     end
 
@@ -128,12 +131,12 @@ def deploy_failing_to_progress?
         # Deployments were being updated prematurely with incorrect progress information
         # https://github.com/kubernetes/kubernetes/issues/49637
         return false unless Time.now.utc - @deploy_started_at >= @progress_deadline.to_i
-      else
-        return false unless deploy_started?
       end
 
+      deploy_started? &&
+      @current_generation == @observed_generation &&
       @progress_condition["status"] == 'False' &&
-      Time.parse(@progress_condition["lastUpdateTime"]).to_i >= (@deploy_started_at - 5.seconds).to_i
+      @progress_condition["reason"] == "ProgressDeadlineExceeded"
     end
 
     def all_rs_data(match_labels)

test/fixtures/for_unit_tests/deployment_test.yml

@@ -4,6 +4,7 @@ kind: Deployment
 metadata:
   name: web
   uid: foobar
+  generation: 2
   annotations:
     "deployment.kubernetes.io/revision": "1"
 spec:

test/unit/kubernetes-deploy/kubernetes_resource/deployment_test.rb

@@ -267,34 +267,38 @@ def test_deploy_timed_out_based_on_progress_deadline
     Timecop.freeze do
       deployment_status = {
         "replicas" => 3,
+        "observedGeneration" => 2,
         "conditions" => [{
           "type" => "Progressing",
           "status" => 'False',
           "lastUpdateTime" => Time.now.utc - 10.seconds,
-          "reason" => "Failed to progress"
+          "reason" => "ProgressDeadlineExceeded"
         }]
       }
       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"))
 
+      deploy.kubectl.expects(:server_version).at_least_once.returns(Gem::Version.new("1.8"))
+      refute deploy.deploy_timed_out?, "Deploy not started shouldn't have timed out"
+
+      deploy.deploy_started_at = Time.now.utc - 3.minutes
       assert deploy.deploy_timed_out?
-      assert_equal "Timeout reason: Failed to progress\nLatest ReplicaSet: web-1", deploy.timeout_message.strip
+      assert_equal "Timeout reason: ProgressDeadlineExceeded\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
+  def test_deploy_timed_out_based_on_progress_deadline_ignores_statuses_for_older_generations
     Timecop.freeze do
       deployment_status = {
         "replicas" => 3,
+        "observedGeneration" => 1, # current generation is 2
         "conditions" => [{
           "type" => "Progressing",
           "status" => 'False',
           "lastUpdateTime" => Time.now.utc - 10.seconds,
-          "reason" => "Failed to progress"
+          "reason" => "ProgressDeadlineExceeded"
         }]
       }
       deploy = build_synced_deployment(
@@ -303,14 +307,8 @@ def test_deploy_timed_out_based_on_progress_deadline_ignores_conditions_older_th
       )
       deploy.kubectl.expects(:server_version).returns(Gem::Version.new("1.8")).at_least_once
 
-      deploy.deploy_started_at = nil # not started yet
+      deploy.deploy_started_at = Time.now.utc - 20.seconds
       refute deploy.deploy_timed_out?
-
-      deploy.deploy_started_at = Time.now.utc - 4.seconds # 10s ago is before deploy started
-      refute deploy.deploy_timed_out?
-
-      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