Consider updated pods in StatefulSet#deploy_failed?

Shopify · Apr 19, 2024 · 745423c · 745423c
1 parent f1eb262
commit 745423c
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 11 deletions.
diff --git a/Gemfile b/Gemfile
@@ -4,4 +4,4 @@ source 'https://rubygems.org'
 # Specify your gem's dependencies in krane.gemspec
 gemspec
 
-gem "ruby-lsp", "~> 0.2.0", :group => :development
+gem "ruby-lsp", "~> 0.2.0", group: :development
diff --git a/lib/krane/kubernetes_resource/stateful_set.rb b/lib/krane/kubernetes_resource/stateful_set.rb
@@ -65,7 +65,7 @@ def desired_replicas
     def parent_of_pod?(pod_data)
       return false unless pod_data.dig("metadata", "ownerReferences")
       pod_data["metadata"]["ownerReferences"].any? { |ref| ref["uid"] == @instance_data["metadata"]["uid"] } &&
-      @instance_data["status"]["currentRevision"] == pod_data["metadata"]["labels"]["controller-revision-hash"]
+      @instance_data["status"]["updateRevision"] == pod_data["metadata"]["labels"]["controller-revision-hash"]
     end
 
     def required_rollout

diff --git a/test/unit/krane/kubernetes_resource/stateful_set_test.rb b/test/unit/krane/kubernetes_resource/stateful_set_test.rb
@@ -41,28 +41,59 @@ def test_deploy_does_not_succeed_when_current_and_observed_generations_do_not_ma
     refute_predicate(ss, :deploy_succeeded?)
   end
 
-  def test_deploy_failed_not_fooled_by_stale_status_for_rollingupdate_strategy
+  def test_deploy_failed_not_fooled_by_stale_status
     status = {
       "observedGeneration": 1,
       "readyReplicas": 0,
     }
-    ss_template = build_ss_template(status: status, updateStrategy: "RollingUpdate")
+    ss_template = build_ss_template(status: status)
     ss = build_synced_ss(ss_template: ss_template)
     ss.stubs(:pods).returns([stub(deploy_failed?: true)])
     refute_predicate(ss, :deploy_failed?)
   end
 
-  def test_deploy_failed_not_fooled_by_stale_status_for_ondelete_strategy
-    status = {
-      "observedGeneration": 1,
-      "readyReplicas": 0,
+  def test_deploy_failed_ignores_current_pods
+    current_pod = pod_fixture.deep_merge(
+      "metadata" => {
+        "labels" => {
+          "controller-revision-hash" => "current",
+        },
+      },
+      "status" => {
+        "phase" => "Failed",
+      },
+    )
+
+    ss_status = {
+      "observedGeneration" => 2,
+      "currentRevision" => "current",
+      "updateRevision" => "updated",
     }
-    ss_template = build_ss_template(status: status, updateStrategy: "OnDelete")
-    ss = build_synced_ss(ss_template: ss_template)
-    ss.stubs(:pods).returns([stub(deploy_failed?: true)])
+    ss = build_synced_ss(ss_template: build_ss_template(status: ss_status), pod_template: current_pod)
     refute_predicate(ss, :deploy_failed?)
   end
 
+  def test_deploy_failed_considers_updated_pods
+    updated_pod = pod_fixture.deep_merge(
+      "metadata" => {
+        "labels" => {
+          "controller-revision-hash" => "updated",
+        },
+      },
+      "status" => {
+        "phase" => "Failed",
+      },
+    )
+
+    ss_status = {
+      "observedGeneration" => 2,
+      "currentRevision" => "current",
+      "updateRevision" => "updated",
+    }
+    ss = build_synced_ss(ss_template: build_ss_template(status: ss_status), pod_template: updated_pod)
+    assert_predicate(ss, :deploy_failed?)
+  end
+
   private
 
   def build_ss_template(status: {}, updateStrategy: "RollingUpdate", rollout: "full")