From 2ce664ec14a194c62803d777f5e16b3a2d3900b9 Mon Sep 17 00:00:00 2001
From: Albin Vass <albin.vass@volvocars.com>
Date: Thu, 12 Mar 2020 13:09:03 +0100
Subject: [PATCH] Enable setting label and instance name separately

At the moment nodepools aws driver uses the label to set the instance
name in aws and fails to launch the instance if "Name" is supplied
as a tag.
This makes it possible to supply name as a tag.

Change-Id: I9585db8fe4b4ad6f5b588fb67a7201296c2fc954
---
 doc/source/configuration.rst           |  2 ++
 nodepool/driver/aws/provider.py        |  8 ++++++--
 nodepool/tests/fixtures/aws.yaml       | 14 +++++++++++++-
 nodepool/tests/unit/test_driver_aws.py | 25 ++++++++++++++++---------
 4 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst
index 451f4bcd8..f39b23e2a 100644
--- a/doc/source/configuration.rst
+++ b/doc/source/configuration.rst
@@ -1789,6 +1789,8 @@ section of the configuration.
               :required:
 
               Identifier to refer this label.
+              Nodepool will use this to set the name of the instance unless
+              the name is specified as a tag.
 
            .. attr:: cloud-image
               :type: str
diff --git a/nodepool/driver/aws/provider.py b/nodepool/driver/aws/provider.py
index 170b01840..95a1c7aad 100644
--- a/nodepool/driver/aws/provider.py
+++ b/nodepool/driver/aws/provider.py
@@ -162,6 +162,11 @@ class AwsProvider(Provider):
 
     def createInstance(self, label):
         image_id = self.getImageId(label.cloud_image)
+        tags = label.tags
+        if not [tag for tag in label.tags if tag["Key"] == "Name"]:
+            tags.append(
+                {"Key": "Name", "Value": str(label.name)}
+            )
         args = dict(
             ImageId=image_id,
             MinCount=1,
@@ -174,8 +179,7 @@ class AwsProvider(Provider):
                 'DeviceIndex': 0}],
             TagSpecifications=[{
                 'ResourceType': 'instance',
-                'Tags': [{"Key": "Name",
-                          "Value": str(label.name)}] + label.tags
+                'Tags': tags
             }]
         )
 
diff --git a/nodepool/tests/fixtures/aws.yaml b/nodepool/tests/fixtures/aws.yaml
index 0e5a7ac9c..dbb19787d 100644
--- a/nodepool/tests/fixtures/aws.yaml
+++ b/nodepool/tests/fixtures/aws.yaml
@@ -14,6 +14,7 @@ labels:
   - name: ubuntu1404-private-ip
   - name: ubuntu1404-userdata
   - name: ubuntu1404-with-tags
+  - name: ubuntu1404-with-name-tag
 
 providers:
   - name: ec2-us-west-2
@@ -116,4 +117,15 @@ providers:
             instance-type: t3.medium
             key-name: zuul
             tags:
-              has-tags: true
\ No newline at end of file
+              has-tags: true
+      - name: name-tag
+        max-servers: 1
+        subnet-id: null
+        security-group-id: null
+        labels:
+          - name: ubuntu1404-with-name-tag
+            cloud-image: ubuntu1404
+            instance-type: t3.medium
+            key-name: zuul
+            tags:
+              Name: different-name
diff --git a/nodepool/tests/unit/test_driver_aws.py b/nodepool/tests/unit/test_driver_aws.py
index 5e51cb0a2..a15443011 100644
--- a/nodepool/tests/unit/test_driver_aws.py
+++ b/nodepool/tests/unit/test_driver_aws.py
@@ -49,7 +49,7 @@ class TestDriverAws(tests.DBTestCase):
                           host_key_checking=True,
                           userdata=None,
                           public_ip=True,
-                          tags=False):
+                          tags=[]):
         aws_id = 'AK000000000000000000'
         aws_key = '0123456789abcdef0123456789abcdef0123456789abcdef'
         self.useFixture(
@@ -90,6 +90,8 @@ class TestDriverAws(tests.DBTestCase):
         raw_config['providers'][0]['pools'][3]['security-group-id'] = sg_id
         raw_config['providers'][0]['pools'][4]['subnet-id'] = subnet_id
         raw_config['providers'][0]['pools'][4]['security-group-id'] = sg_id
+        raw_config['providers'][0]['pools'][5]['subnet-id'] = subnet_id
+        raw_config['providers'][0]['pools'][5]['security-group-id'] = sg_id
 
         with tempfile.NamedTemporaryFile() as tf:
             tf.write(yaml.safe_dump(
@@ -160,13 +162,8 @@ class TestDriverAws(tests.DBTestCase):
                 if tags:
                     instance = ec2_resource.Instance(node.external_id)
                     tag_list = instance.tags
-
-                    self.assertIn({"Key": "has-tags", "Value": "true"},
-                                  tag_list)
-                    self.assertIn({
-                        "Key": "Name",
-                        "Value": "ubuntu1404-with-tags"
-                    }, tag_list)
+                    for tag in tags:
+                        self.assertIn(tag, tag_list)
 
                 # A new request will be paused and for lack of quota
                 # until this one is deleted
@@ -228,4 +225,14 @@ class TestDriverAws(tests.DBTestCase):
 
     def test_ec2_machine_tags(self):
         self._test_ec2_machine('ubuntu1404-with-tags',
-                               tags=True)
+                               tags=[
+                                   {"Key": "has-tags", "Value": "true"},
+                                   {"Key": "Name",
+                                    "Value": "ubuntu1404-with-tags"}
+                               ])
+
+    def test_ec2_machine_name_tag(self):
+        self._test_ec2_machine('ubuntu1404-with-name-tag',
+                               tags=[
+                                   {"Key": "Name", "Value": "different-name"}
+                               ])