Merge pull request #954 from Shopify/fix-statefulset-pod-selector

Fix statefulset pod selector

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## next
 
+## 3.5.2
+
+- Fixed an issue where deploying StatefulSets monitored the health of the previous revision's pods instead of the updated one.
+
 ## 3.5.1
 
 - Fixed successful deployment check for StatefulSets introduced in v3.3.0.

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

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

lib/krane/version.rb

@@ -1,4 +1,4 @@
 # frozen_string_literal: true
 module Krane
-  VERSION = "3.5.1"
+  VERSION = "3.5.2"
 end

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")