From c90ddaf9f3514b4145a9b3f342ee6925161dfc84 Mon Sep 17 00:00:00 2001
From: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Date: Wed, 8 Jul 2020 21:59:02 +0100
Subject: [PATCH] CPI tags work for spot instances

The CPI creates instances using two separate AWS API calls:

1. #run_instances
2. #request_spot_instances

The #run_instances method takes :tag_specifications as an argument
whereas the #request_spot_instances method does not

If we receive :tag_specifications in our launch_specification we should

1. not pass it on to the #request_spot_instances method
2. create the desired tags after our instance is created

Source: https://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/EC2/Client.html#request_spot_instances-instance_method

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
---
 .../lib/cloud/aws/spot_manager.rb             | 21 ++++++++++++++--
 .../spec/unit/spot_manager_spec.rb            | 25 +++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/src/bosh_aws_cpi/lib/cloud/aws/spot_manager.rb b/src/bosh_aws_cpi/lib/cloud/aws/spot_manager.rb
index 30bb5a58..d90ee372 100644
--- a/src/bosh_aws_cpi/lib/cloud/aws/spot_manager.rb
+++ b/src/bosh_aws_cpi/lib/cloud/aws/spot_manager.rb
@@ -12,10 +12,12 @@ def initialize(ec2)
     end
 
     def create(launch_specification, spot_bid_price)
+      launch_spec_without_tag_specs = launch_specification.reject { |k,_| k == :tag_specifications }.to_h
+
       spot_request_spec = {
         spot_price: "#{spot_bid_price}",
         instance_count: 1,
-        launch_specification: launch_specification
+        launch_specification: launch_spec_without_tag_specs
       }
       unless launch_specification[:security_groups].nil?
         message = 'Cannot use security group names when creating spot instances'
@@ -36,7 +38,22 @@ def create(launch_specification, spot_bid_price)
         raise Bosh::Clouds::VMCreationFailed.new(false), message
       end
 
-      wait_for_spot_instance
+      instance = wait_for_spot_instance
+
+      if launch_specification.key? :tag_specifications
+        tags = launch_specification[:tag_specifications]
+          .flat_map { |ts| ts[:tags] }
+          .map { |t| [t[:key], t[:value]] }
+          .to_h
+
+        begin
+          TagManager.create_tags(instance, tags)
+        rescue Aws::EC2::Errors::TagLimitExceeded => e
+          logger.error("could not tag #{instance.id}: #{e.message}")
+        end
+      end
+
+      instance
     end
 
     private
diff --git a/src/bosh_aws_cpi/spec/unit/spot_manager_spec.rb b/src/bosh_aws_cpi/spec/unit/spot_manager_spec.rb
index 44e9ef37..dc7c2d85 100644
--- a/src/bosh_aws_cpi/spec/unit/spot_manager_spec.rb
+++ b/src/bosh_aws_cpi/spec/unit/spot_manager_spec.rb
@@ -162,5 +162,30 @@
         expect(error.ok_to_retry).to eq false
       }
     end
+
+    context 'and tag_specifications are present' do
+      let(:fake_instance_params_with_tags) do
+        {
+          tag_specifications: [{ tags: { key: 'tag', value: 'tag_value'} }]
+        }.merge(fake_instance_params)
+      end
+
+      let(:expected_tags) do
+        { 'tag' => 'tag_value' }
+      end
+
+      before do
+        expect(aws_client).to receive(:describe_spot_instance_requests).and_return(describe_spot_instance_requests_result)
+
+        expect(spot_instance_requests[0]).to receive(:state).and_return('active')
+        expect(spot_instance_requests[0]).to receive(:instance_id).and_return('i-12345678')
+
+        expect(Bosh::AwsCloud::TagManager).to receive(:create_tags).with(instance, expected_tags)
+      end
+
+      it 'should not pass tag specifications to the ec2 client' do
+        expect(spot_manager.create(fake_instance_params_with_tags, 0.24)).to be(instance)
+      end
+    end
   end
 end