From 127e443f33f469efc6dface4affe0ab294d9d3ed Mon Sep 17 00:00:00 2001
From: akihiro17 <akihiro17@users.noreply.github.com>
Date: Tue, 6 Aug 2024 20:02:52 +0900
Subject: [PATCH] detach only detachable instances

---
 .../auto_scaler/auto_scaling_group_config.rb  |  7 +-
 .../auto_scaling_group_config_spec.rb         | 73 +++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/lib/ecs_deploy/auto_scaler/auto_scaling_group_config.rb b/lib/ecs_deploy/auto_scaler/auto_scaling_group_config.rb
index 15590ed..828e403 100644
--- a/lib/ecs_deploy/auto_scaler/auto_scaling_group_config.rb
+++ b/lib/ecs_deploy/auto_scaler/auto_scaling_group_config.rb
@@ -66,7 +66,12 @@ def cluster_resource_manager
       def detach_instances(instance_ids:, should_decrement_desired_capacity:)
         return if instance_ids.empty?
 
-        instance_ids.each_slice(MAX_DETACHABLE_INSTANCE_COUNT) do |ids|
+        auto_scaling_group_instances = describe_detachable_instances
+        detachable_instance_ids = instance_ids.select do |instance_id|
+          auto_scaling_group_instances.any? { |instance| instance.instance_id == instance_id }
+        end
+
+        detachable_instance_ids.each_slice(MAX_DETACHABLE_INSTANCE_COUNT) do |ids|
           client.detach_instances(
             auto_scaling_group_name: name,
             instance_ids: ids,
diff --git a/spec/ecs_deploy/auto_scaler/auto_scaling_group_config_spec.rb b/spec/ecs_deploy/auto_scaler/auto_scaling_group_config_spec.rb
index dca848b..3c8552e 100644
--- a/spec/ecs_deploy/auto_scaler/auto_scaling_group_config_spec.rb
+++ b/spec/ecs_deploy/auto_scaler/auto_scaling_group_config_spec.rb
@@ -308,4 +308,77 @@
       end
     end
   end
+
+  describe "#detach_instances" do
+    subject(:auto_scaling_group_config) do
+      described_class.new({
+        "name"   => asg_name,
+        "region" => "ap-northeast-1",
+        "buffer" => 0,
+        "services" => [],
+      }, Logger.new(nil))
+    end
+
+    let(:asg_name) { "asg_name" }
+    let(:auto_scaling_group_instances) do
+      [
+        Aws::AutoScaling::Types::Instance.new(
+          instance_id: "i-000000",
+          availability_zone: "ap-notrheast-1a",
+          lifecycle_state: "InService",
+          health_status: "Healthy",
+          launch_template: "launch_template",
+          protected_from_scale_in: true,
+        ),
+        Aws::AutoScaling::Types::Instance.new(
+          instance_id: "i-222222",
+          availability_zone: "ap-notrheast-1c",
+          lifecycle_state: "Standby",
+          health_status: "Healthy",
+          launch_template: "launch_template",
+          protected_from_scale_in: true,
+        ),
+        Aws::AutoScaling::Types::Instance.new(
+          instance_id: "i-333333",
+          availability_zone: "ap-notrheast-1c",
+          lifecycle_state: "Terminating",
+          health_status: "",
+          launch_template: "launch_template",
+          protected_from_scale_in: true,
+        ),
+      ]
+    end
+
+    before do
+      allow_any_instance_of(Aws::AutoScaling::Client).to receive(:describe_auto_scaling_groups).with(
+        auto_scaling_group_names: [asg_name],
+      ).and_return(
+        double(
+          auto_scaling_groups: [
+            double(
+              desired_capacity: auto_scaling_group_instances.size,
+              instances: auto_scaling_group_instances.map do |i|
+                double(
+                  availability_zone: i.availability_zone,
+                  instance_id: i.instance_id,
+                  lifecycle_state: i.lifecycle_state,
+                )
+              end,
+            )
+          ]
+        )
+      )
+    end
+
+    it "detaches only detachable instances" do
+      expect_any_instance_of(Aws::AutoScaling::Client).to receive(:detach_instances).with(
+        auto_scaling_group_name: asg_name,
+        instance_ids: ["i-000000", "i-222222"],
+        should_decrement_desired_capacity: false,
+      )
+
+      auto_scaling_group_config.detach_instances(instance_ids: ["i-000000", "i-222222", "i-333333"], should_decrement_desired_capacity: false)
+    end
+  end
+
 end